Re: [PATCH][Revised] Enable libsanitizer on darwin

2012-11-14 Thread Alexander Potapenko
Hi Jack,

most certainly the functionality of asan is not intact.
The error messages denote that mach_override couldn't parse some of
the function prologues, which means some of ASan interceptors just
won't work.
In order to fix this you need to change the DEBUG definition in
mach_override.c, look at the bytes being parsed and fix the
instruction table in mach_override.c
Please also send a patch to LLVM containing the fix (sending the patch
to the original mach_override repo makes little sense, because we've
forked it long time ago).

HTH,
Alex

On Wed, Nov 14, 2012 at 6:43 PM, Jack Howarth  wrote:
>The attached patch assumes that mach_override/mach_override.h
> and mach_override/mach_override.c has been imported by the libsanitizer
> maintainers for use by darwin. The patch adds darwin to the supported
> target list in configure.tgt and defines USING_MACH_OVERRIDE for darwin
> in configure.ac. The definition of USING_MACH_OVERRIDE is used in
> Makefile.am as the test for appending mach_override/mach_override.c
> to libinterception_la_SOURCES. Tested on x86_64-apple-darwin12 against
> the mach_override/mach_override.h and mach_override/mach_override.c
> from llvm compiler-rt 3.2 branch. While there is some noise on the
> output of asan...
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55289#c14
>
> the functionality of asan appears to be intact. Okay for gcc trunk
> after the libsanitizer maintainers import the missing 
> mach_override/mach_override.h
> and mach_override/mach_override.c files?
>   Jack
> ps Note that this patch assumes that both mach_override.h and mach_override.c
> reside in a mach_override subdirectory in interception as is the case in the
> llvm's compiler-rt.
> pps Patch to configure.tgt revised to use a distinct instance for darwin in
> the case statement and to limit libsanitizer to i?86 and x86_64 on darwin.
>
> libsanitizer/
>
> 2012-11-14  Jack Howarth 
>
> * configure.tgt: Add darwin to supported targets.
> * configure.ac: Define USING_MACH_OVERRIDE when on darwin.
> * interception/Makefile.am: Compile mach_override.c when
> USING_MACH_OVERRIDE defined.
> * configure: Regenerated.
> * interception/Makefile.in: Likewise.
>
> Index: libsanitizer/interception/Makefile.am
> ===
> --- libsanitizer/interception/Makefile.am   (revision 193500)
> +++ libsanitizer/interception/Makefile.am   (working copy)
> @@ -11,7 +11,11 @@ interception_files = \
>  interception_mac.cc \
>  interception_win.cc
>
> -libinterception_la_SOURCES = $(interception_files)
> +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 193500)
> +++ libsanitizer/configure.ac   (working copy)
> @@ -17,6 +17,12 @@ AM_PROG_LIBTOOL
>  AC_SUBST(enable_shared)
>  AC_SUBST(enable_static)
>
> +case "$host" in
> +  *-*-darwin*) MACH_OVERRIDE=true ;;
> +  *) MACH_OVERRIDE=false ;;
> +esac
> +AM_CONDITIONAL(USING_MACH_OVERRIDE, $MACH_OVERRIDE)
> +
>  #AM_ENABLE_MULTILIB(, ..)
>  target_alias=${target_alias-$host_alias}
>  AC_SUBST(target_alias)
> Index: libsanitizer/configure.tgt
> ===
> --- libsanitizer/configure.tgt  (revision 193500)
> +++ libsanitizer/configure.tgt  (working copy)
> @@ -22,6 +22,8 @@
>  case "${target}" in
>x86_64-*-linux* | i?86-*-linux*)
> ;;
> +  x86_64-*-darwin* | i?86-*-darwin*)
> +   ;;
>*)
> UNSUPPORTED=1
> ;;



-- 
Alexander Potapenko
Software Engineer
Google Moscow


Re: [PATCH][Revised] Enable libsanitizer on darwin

2012-11-14 Thread Alexander Potapenko
Hi Rainer,

The quick answer is no, although the expansion into GCC world may
require such a guideline.
We've initially implemented ASan on Linux and then ported it to
Android (which is very similar to Linux) and Mac (which is very
different), so we have little experience with porting yet.
I've summarized the differences between Linux and Mac here:
https://code.google.com/p/address-sanitizer/wiki/AddressSanitizerForMacImplementationDetails

The core things to pay attention at are function wrapping (e.g. dlsym
on Linux, mach_override on Mac), hooking the allocations/deallocations
in the program, stack unwinding (already done for x86 and ARM), thread
creation, early initialization, debug info and symbolization. Maybe
something else. In fact if any of these points work on your platform
differently than they do on Linux/Mac, you'll have to re-implement
those.

Regarding mach_override, it's a Mac OS-specific thing that we use
because dlsym doesn't reliably override functions (keywords: two-level
namespace)
This only works on x86/x86_64 and is just a bunch of ad-hoc hacks to
hot-patch the library code.
Extending the instruction table in mach_override.c is irrelevant to
porting, it's just a mean of making it work with some newer libraries
provided by Apple.
It won't work on any other OS (well, without some refactoring; in fact
it's possible to use it on Linux) or any other arch (one could rewrite
mach_override.c for ARM, but iOS doesn't allow code patching).

I'm working on a replacement for mach_override, which will compile
libasan as a dynamic library on Mac OS and preload it before running
the program.
It's not ready yet, so the easiest thing for GCC will be to add some
more instructions to mach_override and stick to it for now.

Alex

On Wed, Nov 14, 2012 at 7:08 PM, Rainer Orth
 wrote:
> Hi Alex,
>
>> most certainly the functionality of asan is not intact.
>> The error messages denote that mach_override couldn't parse some of
>> the function prologues, which means some of ASan interceptors just
>> won't work.
>> In order to fix this you need to change the DEBUG definition in
>> mach_override.c, look at the bytes being parsed and fix the
>> instruction table in mach_override.c
>
> is there some guideline how to port asan to a new OS or CPU?  That would
> certainly be easier than figuring things out on your own one by one.  I
> guess several target and os port maintainers would want to do so in GCC.
>
> Thanks.
> Rainer
>
> --
> -----
> Rainer Orth, Center for Biotechnology, Bielefeld University



-- 
Alexander Potapenko
Software Engineer
Google Moscow


Re: [PATCH][Revised] Enable libsanitizer on darwin

2012-11-14 Thread Alexander Potapenko
On Wed, Nov 14, 2012 at 8:09 PM, Rainer Orth
 wrote:
> Hi Alex,
>

> thanks, that's certainly helpful.  I'm primarily interested in porting
> to Solaris, both SPARC and x86.  Several things should be similar to
> Linux (both being ELF systems), while other areas are certainly
> different (syscalls implementation etc.).
We don't wrap the syscalls in ASan, if you're speaking about that.

> I'll certainly be looking around.  One problem I see with the current
> code organization is duplication between different platforms:
> e.g. considerable parts of sanitizer_mac.cc and a new
> sanitizer_solaris.cc are identical.  Perhaps we need more granularity in
> the future, but that can be decided once an initial port (with
> duplication to avoid treading on other port's feet) is done.  At the
> same time, we should add infrastructure (like libffi) to only compile
> target-specific code on the relevant platforms, instead of wrapping
> every source file in #ifdef __linux__ or similar and extending the
> conditionals once a new platform is added.
Yes, this might be a problem. I also wonder how much Solaris is like
FreeBSD, which users are also interested in ASan.

-- 
Alexander Potapenko
Software Engineer
Google Moscow


Re: [PATCH][Revised] Enable libsanitizer on darwin

2012-11-14 Thread Alexander Potapenko
I've responded to the bug.
Sorry for offtopics unrelated to your original patch.

On Wed, Nov 14, 2012 at 8:11 PM, Jack Howarth  wrote:
> On Wed, Nov 14, 2012 at 07:56:18PM +0400, Alexander Potapenko wrote:
>> Hi Rainer,
>>
>> The quick answer is no, although the expansion into GCC world may
>> require such a guideline.
>> We've initially implemented ASan on Linux and then ported it to
>> Android (which is very similar to Linux) and Mac (which is very
>> different), so we have little experience with porting yet.
>> I've summarized the differences between Linux and Mac here:
>> https://code.google.com/p/address-sanitizer/wiki/AddressSanitizerForMacImplementationDetails
>>
>> The core things to pay attention at are function wrapping (e.g. dlsym
>> on Linux, mach_override on Mac), hooking the allocations/deallocations
>> in the program, stack unwinding (already done for x86 and ARM), thread
>> creation, early initialization, debug info and symbolization. Maybe
>> something else. In fact if any of these points work on your platform
>> differently than they do on Linux/Mac, you'll have to re-implement
>> those.
>>
>> Regarding mach_override, it's a Mac OS-specific thing that we use
>> because dlsym doesn't reliably override functions (keywords: two-level
>> namespace)
>> This only works on x86/x86_64 and is just a bunch of ad-hoc hacks to
>> hot-patch the library code.
>> Extending the instruction table in mach_override.c is irrelevant to
>> porting, it's just a mean of making it work with some newer libraries
>> provided by Apple.
>> It won't work on any other OS (well, without some refactoring; in fact
>> it's possible to use it on Linux) or any other arch (one could rewrite
>> mach_override.c for ARM, but iOS doesn't allow code patching).
>>
>> I'm working on a replacement for mach_override, which will compile
>> libasan as a dynamic library on Mac OS and preload it before running
>> the program.
>> It's not ready yet, so the easiest thing for GCC will be to add some
>> more instructions to mach_override and stick to it for now.
>
> Alex,
>Please see...
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55289#c27
>
> So far for all the test cases that I have looked at, the problem
> seems to be when mach_overrride.c hits the opcodes...
>
> 48 8d 5
>
> Can you suggest a patch for that one? It may be the only blocker to
> asan support on darwin (for x86_64 anyway).
>Jack
>
>>
>> Alex
>>
>> On Wed, Nov 14, 2012 at 7:08 PM, Rainer Orth
>>  wrote:
>> > Hi Alex,
>> >
>> >> most certainly the functionality of asan is not intact.
>> >> The error messages denote that mach_override couldn't parse some of
>> >> the function prologues, which means some of ASan interceptors just
>> >> won't work.
>> >> In order to fix this you need to change the DEBUG definition in
>> >> mach_override.c, look at the bytes being parsed and fix the
>> >> instruction table in mach_override.c
>> >
>> > is there some guideline how to port asan to a new OS or CPU?  That would
>> > certainly be easier than figuring things out on your own one by one.  I
>> > guess several target and os port maintainers would want to do so in GCC.
>> >
>> > Thanks.
>> > Rainer
>> >
>> > --
>> > -
>> > Rainer Orth, Center for Biotechnology, Bielefeld University
>>
>>
>>
>> --
>> Alexander Potapenko
>> Software Engineer
>> Google Moscow



-- 
Alexander Potapenko
Software Engineer
Google Moscow


Re: [PATCH 00/13] Request to merge Address Sanitizer in

2012-11-16 Thread Alexander Potapenko
>> Also, Alexander Potapenko is the best person to ask about asan-darwin.
>
>  here.
>
>> Maybe we can add him to the list of sanitizer maintainers?
>
> Seconded.  At least for libsanitier/Darwin.
>
> Cheers.

I can take this, but I'll be busy within the several upcoming days
(till mid-next-week), so I won't be able to merge any patches from
LLVM into GCC.
Also, I'm not a GCC committer yet, and I guess someone will need to nominate me.


Re: Sparc ASAN

2012-11-22 Thread Alexander Potapenko
>> While trying to add support for ARM (AArch32 GNU / Linux) implementation for
>> GCC after-hours but still keep seeing failures on my chromebook running an
>> ubuntu fs on a ChrOS kernel, because the shadow memory apparently overlaps
>> with normal memory. Has anyone else hit this while porting ?
>
> I've heard someone complain about this recently.
> glider?
>
> Running asan on *huge* binaries in 32-bit address space is a challenge indeed.
> My suggestion was to link with -pie.
>
> --kcc
Yes, there were complaints about that recently: http://crbug.com/159816
-pie MAY help, but not if the program occupies too much address space.


Re: [PATCH][Revisedx5] Enable libsanitizer on darwin

2012-11-23 Thread Alexander Potapenko
The mach_override path looks good to me. I don't have enough knowledge
of GCC buildsystem yet to review the rest.

On Fri, Nov 23, 2012 at 8:17 PM, Jack Howarth  wrote:
>The attached patch imports the missing mach_override/mach_override.h and
> mach_override/mach_override.c files from llvm.org's compiler-rt at
>
> r168032 | glider | 2012-11-15 03:32:16 -0500 (Thu, 15 Nov 2012) | 3 lines
>
> [ASan] Add the "lea $imm(%rip),%rax" instruction to mach_override.c
> The need for this has been reported by Jack Howarth 
> (howa...@bromo.med.uc.edu) who's porting ASan-Darwin to GCC
>
> The patch adds darwin to the supported target list in configure.tgt and
> defines USING_MACH_OVERRIDE for darwin in configure.ac. The definition of
> USING_MACH_OVERRIDE is used in Makefile.am as the test for appending
> mach_override/mach_override.c to libinterception_la_SOURCES. 
> LINK_COMMAND_SPEC_A
> in gcc/config/darwin.h is modified to add an entry to handle fsanitize=address
> so that the required linkages are used for libasan. The static linkage of 
> libasan.a
> in LINK_COMMAND_SPEC_A is handle separately for -static-libstdc++ (which 
> requires
> libstdc++.a) and the -static, -static-gcc and -static-gfortran cases. Tested 
> at rr193756
> on x86_64-apple-darwin12 for both -m32 and -m64 with the both 
> use-after-free.c testcase
> and...
>
>  make -k check RUNTESTFLAGS="asan.exp --target_board=unix'{-m32,-m64}'"
>
> without regressions.
>   Jack
> ps This version adds the requested importation of the LICENSE.txt file from 
> llvm.org's
> compiler-rt in interception/mach_override. It also sets TSAN_SUPPORTED=no in 
> configure.tgt
> for the 'x86_64-*-darwin* | i?86-*-darwin*' case to avoid building tsan 
> support.



-- 
Alexander Potapenko
Software Engineer
Google Moscow


Re: [PATCH] Fix PR55521 by switching libsanitizer from mach_override to mac interpose functions on darwin

2012-12-01 Thread Alexander Potapenko
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  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


Re: [PATCH] Fix PR55521 by switching libsanitizer from mach_override to mac interpose functions on darwin

2012-12-02 Thread Alexander Potapenko
On Sun, Dec 2, 2012 at 10:15 PM, Jack Howarth  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  
>> 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


Re: [PATCh][Revisedx2] Fix PR55521 by switching libsanitizer from mach_overrideto mac interpose functions on darwin

2012-12-03 Thread Alexander Potapenko
Jack,

Note that MAC_INTERPOSE_FUNCTIONS is always defined in interception.h
to either 0 or 1.
I'm going to keep "#if MAC_INTERPOSE_FUNCTIONS" (adding "&&
!defined(MISSING_BLOCKS_SUPPORT)
" where appropriate) in libsanitizer.

On Mon, Dec 3, 2012 at 11:17 AM, Mike Stump  wrote:
> On Dec 3, 2012, at 8:02 AM, Jack Howarth  wrote:
>>   The attached patch eliminates PR 55521/sanitizer by switching libasan on 
>> darwin
>> from using mach_override to mac function interposition
>
> So, I'm thinking the sanitizer people will just review and approve it, even 
> though it says darwin and is heavily darwin specific…  It's ok by me.



-- 
Alexander Potapenko
Software Engineer
Google Moscow


Re: [PATCh][Revisedx2] Fix PR55521 by switching libsanitizer from mach_overrideto mac interpose functions on darwin

2012-12-03 Thread Alexander Potapenko
I've added MISSING_BLOCKS_SUPPORT to LLVM compiler-rt in r169206.
The rest of your change looks good to me as well.

On Mon, Dec 3, 2012 at 6:33 PM, Alexander Potapenko  wrote:
> Jack,
>
> Note that MAC_INTERPOSE_FUNCTIONS is always defined in interception.h
> to either 0 or 1.
> I'm going to keep "#if MAC_INTERPOSE_FUNCTIONS" (adding "&&
> !defined(MISSING_BLOCKS_SUPPORT)
> " where appropriate) in libsanitizer.
>
> On Mon, Dec 3, 2012 at 11:17 AM, Mike Stump  wrote:
>> On Dec 3, 2012, at 8:02 AM, Jack Howarth  wrote:
>>>   The attached patch eliminates PR 55521/sanitizer by switching libasan on 
>>> darwin
>>> from using mach_override to mac function interposition
>>
>> So, I'm thinking the sanitizer people will just review and approve it, even 
>> though it says darwin and is heavily darwin specific…  It's ok by me.
>
>
>
> --
> Alexander Potapenko
> Software Engineer
> Google Moscow



-- 
Alexander Potapenko
Software Engineer
Google Moscow


Re: [PATCh][Revisedx2] Fix PR55521 by switching libsanitizer from mach_overrideto mac interpose functions on darwin

2012-12-04 Thread Alexander Potapenko
Clang hasn't fully switched to dylib interposition yet.
The -fsanitize=address still links the static version of the runtime.

On Tue, Dec 4, 2012 at 7:24 PM, Jack Howarth  wrote:
> On Tue, Dec 04, 2012 at 07:10:27PM +0400, Konstantin Serebryany wrote:
>> On Tue, Dec 4, 2012 at 7:03 PM, Jack Howarth  
>> wrote:
>> > On Tue, Dec 04, 2012 at 11:02:01AM +0400, Konstantin Serebryany wrote:
>> >> r194120.
>> >> I've tested it on Linux, but not on Mac.
>> >
>> > kcc,
>> >Tested at r194135 on x86_64-apple-darwin12. Thanks for the commit.
>> >Jack
>> > ps Since clang in svn trunk has already switched to the mac function 
>> > imposition version of asan,
>>
>> did it?
>
> Thats what Nick Kledzik said...
>
> "The clang version of asan has already moved to using interposing and 
> DYLD_INSERT_LIBRARIES."
>
> I'll reconfirm that with llvm/clang trunk svn tonight (as the bootstrap on 
> darwin there is
> currently broken).
>   Jack
>
>>
>> > it might be time to deprecate mach_override out of llvm's compiler-rt 
>> > trunk so
>> > lib/interception/mach_override/, lib/interception/interception_mac.cc and
>> > lib/interception/interception_mac.h could be removed.
>> >>
>> >> On Tue, Dec 4, 2012 at 6:44 AM, Alexander Potapenko  
>> >> wrote:
>> >> > I've added MISSING_BLOCKS_SUPPORT to LLVM compiler-rt in r169206.
>> >> > The rest of your change looks good to me as well.
>> >> >
>> >> > On Mon, Dec 3, 2012 at 6:33 PM, Alexander Potapenko  
>> >> > wrote:
>> >> >> Jack,
>> >> >>
>> >> >> Note that MAC_INTERPOSE_FUNCTIONS is always defined in interception.h
>> >> >> to either 0 or 1.
>> >> >> I'm going to keep "#if MAC_INTERPOSE_FUNCTIONS" (adding "&&
>> >> >> !defined(MISSING_BLOCKS_SUPPORT)
>> >> >> " where appropriate) in libsanitizer.
>> >> >>
>> >> >> On Mon, Dec 3, 2012 at 11:17 AM, Mike Stump  
>> >> >> wrote:
>> >> >>> On Dec 3, 2012, at 8:02 AM, Jack Howarth  
>> >> >>> wrote:
>> >> >>>>   The attached patch eliminates PR 55521/sanitizer by switching 
>> >> >>>> libasan on darwin
>> >> >>>> from using mach_override to mac function interposition
>> >> >>>
>> >> >>> So, I'm thinking the sanitizer people will just review and approve 
>> >> >>> it, even though it says darwin and is heavily darwin specific…  It's 
>> >> >>> ok by me.
>> >> >>
>> >> >>
>> >> >>
>> >> >> --
>> >> >> Alexander Potapenko
>> >> >> Software Engineer
>> >> >> Google Moscow
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Alexander Potapenko
>> >> > Software Engineer
>> >> > Google Moscow



-- 
Alexander Potapenko
Software Engineer
Google Moscow


Re: [PATCH] Fix PR55599/sanitizer by disabling static libasan on darwin

2012-12-05 Thread Alexander Potapenko
In fact nothing prevents GCC from having a linkable static ASan runtime.
The reason for the static library being unusable is not in the dynamic
runtime itself, but in mach_override not working well with other GCC
libraries (it is unable to parse some function prologues). Because
ASan is transitioning to the dynamic runtime in LLVM, it's unlikely
we'll put any effort into fixing mach_override.

On Thu, Dec 6, 2012 at 1:23 AM, Mike Stump  wrote:
> On Dec 5, 2012, at 10:43 AM, Jack Howarth  wrote:
>>   The transition of libasan on darwin from using mach_override to the 
>> replacement mac function
>> imposition code results in a non-functional static libasan (PR55599).
>
> Ok unless the asan people have a better idea...



-- 
Alexander Potapenko
Software Engineer
Google Moscow


Re: [PATCH] fix PR sanitizer/55617

2013-02-04 Thread Alexander Potapenko
Constructor priorities on Darwin aren't supposed to work across
translation units, see http://llvm.org/bugs/show_bug.cgi?id=12556:

"""
I was told (by Apple folks) that darwin does not support cross-unit constructor
priorities, sorry. This is true for both gcc and llvm-gcc / clang. Within unit
priorities are emulated on darwin (via emission order).

Also, according to Nick Kledzik, constructors are executed on darwin strictly
in link order. So, when you link foo.o and bar.o, then first ctors from foo.o
are executed, then - from bar.o. Maybe this is even documented in some MachO
docs.
"""

Yet it should be possible to delay the constructor emission for a
single TU until all the compiler passes do their job, and then sort
those constructors according to their priorities.

On Mon, Feb 4, 2013 at 1:22 PM, Richard Biener
 wrote:
> On Sun, Feb 3, 2013 at 5:57 AM, Jack Howarth  wrote:
>>Currently darwin is unable to utilize libasan with constructors due to 
>> the lack of
>> constructor priority support on that target. The asan_finish_file routine 
>> inserts an
>> essential __asan_init into the array of constructors (via the 
>> __mod_init_func section).
>> However the insertion occurs at the end, and due to the lack of priority 
>> support for
>> constructors, these are executed from the start of the array of constructors 
>> on program
>> startup. This causes code any instrumented code that executes before the 
>> __asan_init
>> call to crash.
>>Since darwin sets...
>>
>> #undef SUPPORTS_INIT_PRIORITY
>> #define SUPPORTS_INIT_PRIORITY 0
>>
>> in gcc/config/darwin.h, all constructors are automatically set to
>>
>> #define DEFAULT_INIT_PRIORITY 65535
>>
>> in gcc/collect2.c. Any code that attempts to set the constructor/destructor 
>> priority
>> on darwin results in a compile time error of "constructor priorities are not 
>> supported".
>> So asan alone should be unique in emitting priorities different from 65535 
>> on darwin.
>> The attached patch uses a va_gc vector of constructor symbol/priority 
>> records to queue
>> this data as it is generated in calls to machopic_asm_out_constructor. Any 
>> instances of
>> the static constructor with priority 99 emitted by asan are inserted safely 
>> in the front
>> of the vector queue which retains the original order of the remaining 
>> constructors in
>> the queue. The contents of the vector queue are later processed in a new 
>> finalize_ctors
>> routine called from darwin_file_end if necessary. The patch also adds a 
>> g++.dg/asan/pr55617.C
>> test case which is targeted to i?86-*-darwin* and x86_64-*-darwin*.
>> The patch reduces the failures observed when running
>>
>> make -k check-g++ RUNTESTFLAGS="--target_board=unix'{-fsanitize=address}'"
>>
>> from 323 to only 85 on darwin (similar to the results on linux). The cov.C 
>> testcase also
>> fails on gcc trunk with -fsanitize=address when recrafted into a dynamic 
>> shared library
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55617#c28. This patch eliminates 
>> those
>> crashes. This problem doesn't extend to when the shared library or module is 
>> dlopen'd
>> (which works in stock gcc trunk and with this patch as well).
>> The patch has been bootstrap and regression tested on 
>> x86_64-apple-darwin12.
>> Okay for gcc trunk?
>
> But that does not work across translation units, no?  ISTR collect2 has 
> support
> to handle constructor priorities all by itself (at link time,
> considering all inputs).
> I wonder why darwin cannot use that mechanism to support init priorities?
>
> Richard.
>
>>  Jack
>> ps Unfortunately the flag_sort variable is unavailable inside of 
>> machopic_asm_out_constructor
>> so we have to unconditionally test for priority == 99.
>>



-- 
Alexander Potapenko
Software Engineer
Google Moscow


Re: [PATCH] fix PR sanitizer/55617

2013-02-04 Thread Alexander Potapenko
> But that does not work across translation units, no?  ISTR collect2 has 
> support
> to handle constructor priorities all by itself (at link time,
> considering all inputs).
> I wonder why darwin cannot use that mechanism to support init priorities?
>
> Richard.

(resending, sorry for top-posting)
Constructor priorities on Darwin aren't supposed to work across
translation units, see http://llvm.org/bugs/show_bug.cgi?id=12556:

"""
I was told (by Apple folks) that darwin does not support cross-unit constructor
priorities, sorry. This is true for both gcc and llvm-gcc / clang. Within unit
priorities are emulated on darwin (via emission order).

Also, according to Nick Kledzik, constructors are executed on darwin strictly
in link order. So, when you link foo.o and bar.o, then first ctors from foo.o
are executed, then - from bar.o. Maybe this is even documented in some MachO
docs.
"""

Yet it should be possible to delay the constructor emission for a
single TU until all the compiler passes do their job, and then sort
those constructors according to their priorities.


Re: [PATCH] fix PR sanitizer/55617

2013-02-04 Thread Alexander Potapenko
On Mon, Feb 4, 2013 at 1:38 PM, Jakub Jelinek  wrote:
> On Mon, Feb 04, 2013 at 10:22:48AM +0100, Richard Biener wrote:
>> > Okay for gcc trunk?
>>
>> But that does not work across translation units, no?  ISTR collect2 has 
>> support
>> to handle constructor priorities all by itself (at link time,
>> considering all inputs).
>
> I wonder why the patch turned from initially at least supporting intra-CU
> support for ctor priorities into an ugly hack for asan.  I guess asan
> doesn't care too much about inter-CU ctor priorities, it just needs its
> ctors to run before anything in the same CU is called (mainly the
> __asan_init call), other CUs either won't be asan instrumented, then it
> doesn't matter, or will be, but they will have their own __asan_init call.

Yes, I was going to ask the same question.
Since other compile-time instrumentation tools (like ThreadSanitizer)
will benefit from this as well, it's better to provide the intra-CU
support by sorting the list of constructors.


Re: [PATCH] fix PR sanitizer/55617

2013-02-06 Thread Alexander Potapenko
I can't see how full init_priority support can work without proper aid
from ld and/or the dynamic linker. According to the Apple people,
those don't treat the cross-module priorities properly, so there's
little that can be done on the compiler side.

On Mon, Feb 4, 2013 at 11:39 PM, Mike Stump  wrote:
> On Feb 4, 2013, at 1:38 AM, Jakub Jelinek  wrote:
>> On Mon, Feb 04, 2013 at 10:22:48AM +0100, Richard Biener wrote:
>>>> Okay for gcc trunk?
>>>
>>> But that does not work across translation units, no?  ISTR collect2 has 
>>> support
>>> to handle constructor priorities all by itself (at link time,
>>> considering all inputs).
>>
>> I wonder why the patch turned from initially at least supporting intra-CU
>> support for ctor priorities into an ugly hack for asan.  I guess asan
>> doesn't care too much about inter-CU ctor priorities, it just needs its
>> ctors to run before anything in the same CU is called (mainly the
>> __asan_init call), other CUs either won't be asan instrumented, then it
>> doesn't matter, or will be, but they will have their own __asan_init call.
>>
>>> I wonder why darwin cannot use that mechanism to support init priorities?
>>
>> But sure, if collect2 can be used for full init prio support, the better.
>
> It would be nice if someone contributed full init_priority support…  I'd be 
> happy to review that. A good patch for that would add it to clang for darwin, 
> and have gcc use that same mechanism so that we can interoperate nicely.  
> Absent interoperability…  I think it would be annoying, as then you have to 
> have a binary incompatibility to fix the one that is wrong.



--
Alexander Potapenko
Software Engineer
Google Moscow


Re: [PATCH] fix PR sanitizer/55617

2013-02-06 Thread Alexander Potapenko
> Alexander,
>I never claimed full init priority support however FSF gcc on darwin 
> currently
> has no init priority support at all. Since Mike wanted to sort the 
> destructors as
> well as the constructors and this achieves usable intra-module init priority 
> support
> for FSF gcc darwin, I don't see why we don't take advantage of it. Especially
> considering that the constructors and destructors will now always be sorted 
> anyway.
> Jack
> ps We will have one advantage over clang's init priority support as we can 
> use -flto
> to combine all of the code modules (outside of libraries) into a single one 
> for the
> sorting of constructors/destructors. This allows the g++.dg/special/conpr-3.C 
> execution
> test case to operate properly on darwin with -flto. Again, remember that 
> clang currently
> at least supports init priority on a intra-module level. I am just trying to 
> leverage
> the sorting of constructors/destructors that we added for asan to achive the 
> same
> level of functionality in FSF gcc on darwin.
>

Jack,
I understand and fully support your desire for intra-module ctor/dtor priority.
My comment was meant to reply to Mike (sorry for top-posting it,
again), who, as far as I understood him, wanted to see full
init_priority support on Darwin, which IIUC can't be implemented
without the proper linker support. LTO may help as a bandaid, but I
don't think this solution scales well enough yet.

Alex


Re: [PATCH] Disable libsanitizer before darwin10

2013-02-11 Thread Alexander Potapenko
For the record, ASan never claimed darwin9 support and isn't going to
support darwin9 in the future, so it should be fine to disable
building ASan on this platform.

On Mon, Feb 11, 2013 at 7:55 PM, Jack Howarth  wrote:
>   Iain Sandoe discovered that on intel darwin9, the asan testsuite suffers 
> hundreds of
> failures due to the absence of dispatch calls (Grand Central Dispatch) prior 
> to darwin10.
> The attached patch disables building libsanitizer on darwin8 and darwin9 
> until upstream
> decides to support the earlier darwin releases. Bootstrap and regression 
> tested on
> x86_64-apple-darwin12. Okay for gcc trunk?
>  Jack



--
Alexander Potapenko
Software Engineer
Google Moscow