| Message ID | 20121202024303.GA32709@bromo.med.uc.edu |
|---|---|
| State | New |
| Headers | show |
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 >
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
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
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
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__
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.