Patchwork Fix PR55521 by switching libsanitizer from mach_override to mac interpose functions on darwin

login
register
mail settings
Submitter Jack Howarth
Date Dec. 2, 2012, 2:43 a.m.
Message ID <20121202024303.GA32709@bromo.med.uc.edu>
Download mbox | patch
Permalink /patch/203177/
State New
Headers show

Comments

Jack Howarth - Dec. 2, 2012, 2:43 a.m.
The attached patch eliminates PR 55521/sanitizer by switching libasan on darwin
from using mach_override to mac function interposition via the importation of the
asan/dynamic/asan_interceptors_dynamic.cc file from llvm.org's compiler-rt svn.
The changes involve defining USING_MAC_INTERPOSE in configure.ac rather than
rather than USING_MACH_OVERRIDE, introduction of the use of USING_MAC_INTERPOSE
in Makefile.am to avoid building the interception subdirectory, the passage of
-DMAC_INTERPOSE_FUNCTIONS in asan/Makefile.am when USING_MAC_INTERPOSE as well as
the introduction of a -DMISSING_BLOCKS_SUPPORT flag to disable code that requires
blocks support which FSF gcc lacks. The depreciated usage of USING_MACH_OVERRIDE 
is also removed from interception/Makefile.am. Bootstrapped on x86_64-apple-darwin10,
x86_64-apple-darwin11 and x86_64-apple-darwin12. Passes...

make -k check RUNTESTFLAGS="asan.exp --target_board=unix'{-m32,-m64}'"

and fixes the previously failing cond1.C test case from PR55521 on all three targets. 
Okay for gcc trunk?
              Jack
2012-12-01  Kostya Serebryany kcc@google.com
	    Jack Howarth <howarth@bromo.med.uc.edu>

/libsanitizer

	PR 55521/sanitizer
        * configure.ac: Define USING_MAC_INTERPOSE when on darwin.
        * Makefile.am: Don't build interception subdir when
	USING_MAC_INTERPOSE defined.
        * asan/Makefile.am: Pass -DMAC_INTERPOSE_FUNCTIONS and 
	-DMISSING_BLOCKS_SUPPORT when USING_MAC_INTERPOSE defined.
	Compile asan_interceptors_dynamic.cc but not libinterception
	when USING_MAC_INTERPOSE defined.
	* interception/Makefile.am: Remove usage of USING_MACH_OVERRIDE.
	* configure: Regenerated.
	* Makefile.in: Likewise.
	* asan/Makefile.in: Likewise.
	* interception/Makefile.in: Likewise.
        * asan/asan_intercepted_functions.h: Use MISSING_BLOCKS_SUPPORT.
        * asan/asan_mac.cc: Likewise.
        * asan/dynamic/asan_interceptors_dynamic.cc: Migrate from llvm
	and use MISSING_BLOCKS_SUPPORT.
Alexander Potapenko - Dec. 2, 2012, 6:21 a.m.
Hi Jack,

IIUC the wrappers for dispatch_async_f, dispatch_sync_f and other
dispatch_smth_f do not need blocks support in the compiler, since
regular functions are passed into them. So you may want to add the
dynamic interceptors for those back.
The remaining problem is that dispach_async and other functions using
blocks won't be intercepted. This may lead to assertion failures in
big projects (e.g. we needed those for Chrome).
Overall, the change looks good. Do you want me to backport
MISSING_BLOCKS_SUPPORT into the LLVM version of the runtime?

Alex

On Sun, Dec 2, 2012 at 6:43 AM, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
>    The attached patch eliminates PR 55521/sanitizer by switching libasan on darwin
> from using mach_override to mac function interposition via the importation of the
> asan/dynamic/asan_interceptors_dynamic.cc file from llvm.org's compiler-rt svn.
> The changes involve defining USING_MAC_INTERPOSE in configure.ac rather than
> rather than USING_MACH_OVERRIDE, introduction of the use of USING_MAC_INTERPOSE
> in Makefile.am to avoid building the interception subdirectory, the passage of
> -DMAC_INTERPOSE_FUNCTIONS in asan/Makefile.am when USING_MAC_INTERPOSE as well as
> the introduction of a -DMISSING_BLOCKS_SUPPORT flag to disable code that requires
> blocks support which FSF gcc lacks. The depreciated usage of USING_MACH_OVERRIDE
> is also removed from interception/Makefile.am. Bootstrapped on x86_64-apple-darwin10,
> x86_64-apple-darwin11 and x86_64-apple-darwin12. Passes...
>
> make -k check RUNTESTFLAGS="asan.exp --target_board=unix'{-m32,-m64}'"
>
> and fixes the previously failing cond1.C test case from PR55521 on all three targets.
> Okay for gcc trunk?
>               Jack
>
Konstantin Serebryany - Dec. 2, 2012, 10:32 a.m.
On Sun, Dec 2, 2012 at 10:21 AM, Alexander Potapenko <glider@google.com> wrote:
> Hi Jack,
>
> IIUC the wrappers for dispatch_async_f, dispatch_sync_f and other
> dispatch_smth_f do not need blocks support in the compiler, since
> regular functions are passed into them. So you may want to add the
> dynamic interceptors for those back.
> The remaining problem is that dispach_async and other functions using
> blocks won't be intercepted. This may lead to assertion failures in
> big projects (e.g. we needed those for Chrome).
> Overall, the change looks good. Do you want me to backport
> MISSING_BLOCKS_SUPPORT into the LLVM version of the runtime?

Please remember that all non-trivial patches to files that exist in
the upstream (llvm) repository should go there first.
Otherwise we'll be drown in merges.

--kcc

>
> Alex
>
> On Sun, Dec 2, 2012 at 6:43 AM, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
>>    The attached patch eliminates PR 55521/sanitizer by switching libasan on darwin
>> from using mach_override to mac function interposition via the importation of the
>> asan/dynamic/asan_interceptors_dynamic.cc file from llvm.org's compiler-rt svn.
>> The changes involve defining USING_MAC_INTERPOSE in configure.ac rather than
>> rather than USING_MACH_OVERRIDE, introduction of the use of USING_MAC_INTERPOSE
>> in Makefile.am to avoid building the interception subdirectory, the passage of
>> -DMAC_INTERPOSE_FUNCTIONS in asan/Makefile.am when USING_MAC_INTERPOSE as well as
>> the introduction of a -DMISSING_BLOCKS_SUPPORT flag to disable code that requires
>> blocks support which FSF gcc lacks. The depreciated usage of USING_MACH_OVERRIDE
>> is also removed from interception/Makefile.am. Bootstrapped on x86_64-apple-darwin10,
>> x86_64-apple-darwin11 and x86_64-apple-darwin12. Passes...
>>
>> make -k check RUNTESTFLAGS="asan.exp --target_board=unix'{-m32,-m64}'"
>>
>> and fixes the previously failing cond1.C test case from PR55521 on all three targets.
>> Okay for gcc trunk?
>>               Jack
>>
>
>
>
> --
> Alexander Potapenko
> Software Engineer
> Google Moscow
Jack Howarth - Dec. 2, 2012, 6:15 p.m.
On Sun, Dec 02, 2012 at 10:21:02AM +0400, Alexander Potapenko wrote:
> Hi Jack,
> 
> IIUC the wrappers for dispatch_async_f, dispatch_sync_f and other
> dispatch_smth_f do not need blocks support in the compiler, since
> regular functions are passed into them. So you may want to add the
> dynamic interceptors for those back.

Alex,
  This seems to only require the readjustment of the preprocessor
statements in libsanitizer/asan/dynamic/asan_interceptors_dynamic.cc.
I'll post the revised patch as soon as regression testing is finished.

> The remaining problem is that dispach_async and other functions using
> blocks won't be intercepted. This may lead to assertion failures in
> big projects (e.g. we needed those for Chrome).

In theory that is a problem, however my understanding was that the asan
dispatch support was added for use by objc/obj-c++ code. The objc/obj-c++
in FSF gcc is very fragile, on later darwin, since it runs against the
system Objective-C runtime but the compiler is only based on a merge
of the remaining bits of code from the old FSF gcc Apple branch circa
2006. IMHO, it is insane to try to do any production work with our
objc/obj-c++ compiler and current Mac OS X releases.

> Overall, the change looks good. Do you want me to backport
> MISSING_BLOCKS_SUPPORT into the LLVM version of the runtime?

Yes, as it should be transparent to llvm's builds.
        Jack
> 
> Alex
> 
> On Sun, Dec 2, 2012 at 6:43 AM, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> >    The attached patch eliminates PR 55521/sanitizer by switching libasan on darwin
> > from using mach_override to mac function interposition via the importation of the
> > asan/dynamic/asan_interceptors_dynamic.cc file from llvm.org's compiler-rt svn.
> > The changes involve defining USING_MAC_INTERPOSE in configure.ac rather than
> > rather than USING_MACH_OVERRIDE, introduction of the use of USING_MAC_INTERPOSE
> > in Makefile.am to avoid building the interception subdirectory, the passage of
> > -DMAC_INTERPOSE_FUNCTIONS in asan/Makefile.am when USING_MAC_INTERPOSE as well as
> > the introduction of a -DMISSING_BLOCKS_SUPPORT flag to disable code that requires
> > blocks support which FSF gcc lacks. The depreciated usage of USING_MACH_OVERRIDE
> > is also removed from interception/Makefile.am. Bootstrapped on x86_64-apple-darwin10,
> > x86_64-apple-darwin11 and x86_64-apple-darwin12. Passes...
> >
> > make -k check RUNTESTFLAGS="asan.exp --target_board=unix'{-m32,-m64}'"
> >
> > and fixes the previously failing cond1.C test case from PR55521 on all three targets.
> > Okay for gcc trunk?
> >               Jack
> >
> 
> 
> 
> -- 
> Alexander Potapenko
> Software Engineer
> Google Moscow
Alexander Potapenko - Dec. 2, 2012, 6:45 p.m.
On Sun, Dec 2, 2012 at 10:15 PM, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
> On Sun, Dec 02, 2012 at 10:21:02AM +0400, Alexander Potapenko wrote:
>> Hi Jack,
>>
>> IIUC the wrappers for dispatch_async_f, dispatch_sync_f and other
>> dispatch_smth_f do not need blocks support in the compiler, since
>> regular functions are passed into them. So you may want to add the
>> dynamic interceptors for those back.
>
> Alex,
>   This seems to only require the readjustment of the preprocessor
> statements in libsanitizer/asan/dynamic/asan_interceptors_dynamic.cc.
> I'll post the revised patch as soon as regression testing is finished.
>
>> The remaining problem is that dispach_async and other functions using
>> blocks won't be intercepted. This may lead to assertion failures in
>> big projects (e.g. we needed those for Chrome).
>
> In theory that is a problem, however my understanding was that the asan
> dispatch support was added for use by objc/obj-c++ code. The objc/obj-c++
> in FSF gcc is very fragile, on later darwin, since it runs against the
> system Objective-C runtime but the compiler is only based on a merge
> of the remaining bits of code from the old FSF gcc Apple branch circa
> 2006. IMHO, it is insane to try to do any production work with our
> objc/obj-c++ compiler and current Mac OS X releases.
Ok, this makes sense.
>> Overall, the change looks good. Do you want me to backport
>> MISSING_BLOCKS_SUPPORT into the LLVM version of the runtime?
>
> Yes, as it should be transparent to llvm's builds.
I'll do that tomorrow morning then.
>>
>> Alex
>>
>> On Sun, Dec 2, 2012 at 6:43 AM, Jack Howarth <howarth@bromo.med.uc.edu> wrote:
>> >    The attached patch eliminates PR 55521/sanitizer by switching libasan on darwin
>> > from using mach_override to mac function interposition via the importation of the
>> > asan/dynamic/asan_interceptors_dynamic.cc file from llvm.org's compiler-rt svn.
>> > The changes involve defining USING_MAC_INTERPOSE in configure.ac rather than
>> > rather than USING_MACH_OVERRIDE, introduction of the use of USING_MAC_INTERPOSE
>> > in Makefile.am to avoid building the interception subdirectory, the passage of
>> > -DMAC_INTERPOSE_FUNCTIONS in asan/Makefile.am when USING_MAC_INTERPOSE as well as
>> > the introduction of a -DMISSING_BLOCKS_SUPPORT flag to disable code that requires
>> > blocks support which FSF gcc lacks. The depreciated usage of USING_MACH_OVERRIDE
>> > is also removed from interception/Makefile.am. Bootstrapped on x86_64-apple-darwin10,
>> > x86_64-apple-darwin11 and x86_64-apple-darwin12. Passes...
>> >
>> > make -k check RUNTESTFLAGS="asan.exp --target_board=unix'{-m32,-m64}'"
>> >
>> > and fixes the previously failing cond1.C test case from PR55521 on all three targets.
>> > Okay for gcc trunk?
>> >               Jack
>> >
>>
>>
>>
>> --
>> Alexander Potapenko
>> Software Engineer
>> Google Moscow



--
Alexander Potapenko
Software Engineer
Google Moscow

Patch

Index: libsanitizer/asan/asan_mac.cc
===================================================================
--- libsanitizer/asan/asan_mac.cc	(revision 194037)
+++ libsanitizer/asan/asan_mac.cc	(working copy)
@@ -383,7 +383,7 @@  INTERCEPTOR(void, dispatch_group_async_f
                                asan_dispatch_call_block_and_release);
 }
 
-#if MAC_INTERPOSE_FUNCTIONS
+#if defined(MAC_INTERPOSE_FUNCTIONS) && !defined(MISSING_BLOCKS_SUPPORT)
 // dispatch_async, dispatch_group_async and others tailcall the corresponding
 // dispatch_*_f functions. When wrapping functions with mach_override, those
 // dispatch_*_f are intercepted automatically. But with dylib interposition
Index: libsanitizer/asan/asan_intercepted_functions.h
===================================================================
--- libsanitizer/asan/asan_intercepted_functions.h	(revision 194037)
+++ libsanitizer/asan/asan_intercepted_functions.h	(working copy)
@@ -203,7 +203,7 @@  DECLARE_FUNCTION_AND_WRAPPER(void, __CFI
 DECLARE_FUNCTION_AND_WRAPPER(CFStringRef, CFStringCreateCopy,
                              CFAllocatorRef alloc, CFStringRef str);
 DECLARE_FUNCTION_AND_WRAPPER(void, free, void* ptr);
-#if MAC_INTERPOSE_FUNCTIONS
+#if defined(MAC_INTERPOSE_FUNCTIONS) && !defined(MISSING_BLOCKS_SUPPORT)
 DECLARE_FUNCTION_AND_WRAPPER(void, dispatch_group_async,
                              dispatch_group_t dg,
                              dispatch_queue_t dq, void (^work)(void));
Index: libsanitizer/asan/Makefile.am
===================================================================
--- libsanitizer/asan/Makefile.am	(revision 194037)
+++ libsanitizer/asan/Makefile.am	(working copy)
@@ -4,6 +4,9 @@  AM_CPPFLAGS = -I $(top_srcdir)/include -
 gcc_version := $(shell cat $(top_srcdir)/../gcc/BASE-VER)
 
 DEFS = -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DASAN_HAS_EXCEPTIONS=1 -DASAN_FLEXIBLE_MAPPING_AND_OFFSET=0 -DASAN_NEEDS_SEGV=1
+if USING_MAC_INTERPOSE
+DEFS += -DMAC_INTERPOSE_FUNCTIONS -DMISSING_BLOCKS_SUPPORT
+endif
 AM_CXXFLAGS = -Wall -W -Wno-unused-parameter -Wwrite-strings -pedantic -Wno-long-long  -fPIC -fno-builtin -fno-exceptions -fomit-frame-pointer -funwind-tables -fvisibility=hidden -Wno-variadic-macros -Wno-c99-extensions 
 ACLOCAL_AMFLAGS = -I $(top_srcdir) -I $(top_srcdir)/config
 
@@ -29,8 +32,14 @@  asan_files = \
 	asan_thread.cc \
 	asan_win.cc
 
-libasan_la_SOURCES = $(asan_files) 
+libasan_la_SOURCES = $(asan_files)
+if USING_MAC_INTERPOSE
+libasan_la_SOURCES += dynamic/asan_interceptors_dynamic.cc
+libasan_la_LIBADD = $(top_builddir)/sanitizer_common/libsanitizer_common.la $(top_builddir)/../libstdc++-v3/src/libstdc++.la
+else
 libasan_la_LIBADD = $(top_builddir)/sanitizer_common/libsanitizer_common.la $(top_builddir)/interception/libinterception.la $(top_builddir)/../libstdc++-v3/src/libstdc++.la
+endif
+
 libasan_la_LDFLAGS = -version-info `grep -v '^\#' $(srcdir)/libtool-version` -lpthread -ldl
 
 # Work around what appears to be a GNU make bug handling MAKEFLAGS
Index: libsanitizer/interception/Makefile.am
===================================================================
--- libsanitizer/interception/Makefile.am	(revision 194037)
+++ libsanitizer/interception/Makefile.am	(working copy)
@@ -14,11 +14,7 @@  interception_files = \
         interception_mac.cc \
         interception_win.cc
 
-if USING_MACH_OVERRIDE
-libinterception_la_SOURCES = $(interception_files) mach_override/mach_override.c
-else
 libinterception_la_SOURCES = $(interception_files)
-endif
 
 # Work around what appears to be a GNU make bug handling MAKEFLAGS
 # values defined in terms of make variables, as is the case for CC and
Index: libsanitizer/configure.ac
===================================================================
--- libsanitizer/configure.ac	(revision 194037)
+++ libsanitizer/configure.ac	(working copy)
@@ -81,10 +81,10 @@  unset TSAN_SUPPORTED
 AM_CONDITIONAL(TSAN_SUPPORTED, [test "x$TSAN_SUPPORTED" = "xyes"])
 
 case "$host" in
-  *-*-darwin*) MACH_OVERRIDE=true ;;
-  *) MACH_OVERRIDE=false ;;
+  *-*-darwin*) MAC_INTERPOSE=true ;;
+  *) MAC_INTERPOSE=false ;;
 esac
-AM_CONDITIONAL(USING_MACH_OVERRIDE, $MACH_OVERRIDE)
+AM_CONDITIONAL(USING_MAC_INTERPOSE, $MAC_INTERPOSE)
 
 AC_CONFIG_FILES([Makefile])
 
Index: libsanitizer/Makefile.am
===================================================================
--- libsanitizer/Makefile.am	(revision 194037)
+++ libsanitizer/Makefile.am	(working copy)
@@ -6,6 +6,10 @@  else
 SUBDIRS = interception sanitizer_common asan 
 endif
 
+if USING_MAC_INTERPOSE
+SUBDIRS = sanitizer_common asan
+endif
+
 # Work around what appears to be a GNU make bug handling MAKEFLAGS
 # values defined in terms of make variables, as is the case for CC and
 # friends when we are called from the top level Makefile.
--- /dev/null	2012-12-01 18:41:54.000000000 -0500
+++ libsanitizer/asan/dynamic/asan_interceptors_dynamic.cc	2012-12-01 18:37:57.000000000 -0500
@@ -0,0 +1,112 @@ 
+//===-- asan_interceptors_dynamic.cc --------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file is a part of AddressSanitizer, an address sanity checker.
+//
+// __DATA,__interpose section of the dynamic runtime library for Mac OS.
+//===----------------------------------------------------------------------===//
+
+#if defined(__APPLE__)
+
+#include "../asan_interceptors.h"
+#include "../asan_intercepted_functions.h"
+
+namespace __asan {
+
+#if !MAC_INTERPOSE_FUNCTIONS
+# error \
+  Dynamic interposing library should be built with -DMAC_INTERPOSE_FUNCTIONS
+#endif
+
+#define INTERPOSE_FUNCTION(function) \
+    { reinterpret_cast<const uptr>(WRAP(function)), \
+      reinterpret_cast<const uptr>(function) }
+
+#define INTERPOSE_FUNCTION_2(function, wrapper) \
+    { reinterpret_cast<const uptr>(wrapper), \
+      reinterpret_cast<const uptr>(function) }
+
+struct interpose_substitution {
+  const uptr replacement;
+  const uptr original;
+};
+
+__attribute__((used))
+const interpose_substitution substitutions[]
+    __attribute__((section("__DATA, __interpose"))) = {
+  INTERPOSE_FUNCTION(strlen),
+  INTERPOSE_FUNCTION(memcmp),
+  INTERPOSE_FUNCTION(memcpy),
+  INTERPOSE_FUNCTION(memmove),
+  INTERPOSE_FUNCTION(memset),
+  INTERPOSE_FUNCTION(strchr),
+  INTERPOSE_FUNCTION(strcat),
+  INTERPOSE_FUNCTION(strncat),
+  INTERPOSE_FUNCTION(strcpy),
+  INTERPOSE_FUNCTION(strncpy),
+  INTERPOSE_FUNCTION(pthread_create),
+  INTERPOSE_FUNCTION(longjmp),
+#if ASAN_INTERCEPT__LONGJMP
+  INTERPOSE_FUNCTION(_longjmp),
+#endif
+#if ASAN_INTERCEPT_SIGLONGJMP
+  INTERPOSE_FUNCTION(siglongjmp),
+#endif
+#if ASAN_INTERCEPT_STRDUP
+  INTERPOSE_FUNCTION(strdup),
+#endif
+#if ASAN_INTERCEPT_STRNLEN
+  INTERPOSE_FUNCTION(strnlen),
+#endif
+#if ASAN_INTERCEPT_INDEX
+  INTERPOSE_FUNCTION_2(index, WRAP(strchr)),
+#endif
+  INTERPOSE_FUNCTION(strcmp),
+  INTERPOSE_FUNCTION(strncmp),
+#if ASAN_INTERCEPT_STRCASECMP_AND_STRNCASECMP
+  INTERPOSE_FUNCTION(strcasecmp),
+  INTERPOSE_FUNCTION(strncasecmp),
+#endif
+  INTERPOSE_FUNCTION(atoi),
+  INTERPOSE_FUNCTION(atol),
+  INTERPOSE_FUNCTION(strtol),
+#if ASAN_INTERCEPT_ATOLL_AND_STRTOLL
+  INTERPOSE_FUNCTION(atoll),
+  INTERPOSE_FUNCTION(strtoll),
+#endif
+#if ASAN_INTERCEPT_MLOCKX
+  INTERPOSE_FUNCTION(mlock),
+  INTERPOSE_FUNCTION(munlock),
+  INTERPOSE_FUNCTION(mlockall),
+  INTERPOSE_FUNCTION(munlockall),
+#endif
+#if defined(MAC_INTERPOSE_FUNCTIONS) && !defined(MISSING_BLOCKS_SUPPORT)
+  INTERPOSE_FUNCTION(dispatch_async_f),
+  INTERPOSE_FUNCTION(dispatch_sync_f),
+  INTERPOSE_FUNCTION(dispatch_after_f),
+  INTERPOSE_FUNCTION(dispatch_barrier_async_f),
+  INTERPOSE_FUNCTION(dispatch_group_async_f),
+
+  INTERPOSE_FUNCTION(dispatch_group_async),
+  INTERPOSE_FUNCTION(dispatch_async),
+  INTERPOSE_FUNCTION(dispatch_after),
+  INTERPOSE_FUNCTION(dispatch_source_set_event_handler),
+  INTERPOSE_FUNCTION(dispatch_source_set_cancel_handler),
+#endif
+  INTERPOSE_FUNCTION(signal),
+  INTERPOSE_FUNCTION(sigaction),
+
+  INTERPOSE_FUNCTION(__CFInitialize),
+  INTERPOSE_FUNCTION(CFStringCreateCopy),
+  INTERPOSE_FUNCTION(free),
+};
+
+}  // namespace __asan
+
+#endif  // __APPLE__