| Message ID | 20121114144356.GA29142@bromo.med.uc.edu |
|---|---|
| State | New |
| Headers | show |
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 <howarth@bromo.med.uc.edu> 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 <howarth@bromo.med.uc.edu> > > * 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 > ;;
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
On Wed, Nov 14, 2012 at 07:00:14PM +0400, Alexander Potapenko wrote: > 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 Alex, I have alway done some of this... http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55289#c11 It seems to be related to the comments found in mach_override.c... #elif defined(__x86_64__) // TODO(glider): disassembling the "0x48, 0x89" sequences is trickier than it's done below. // If it stops working, refer to http://ref.x86asm.net/geek.html#modrm_byte_32_64 to do it // more accurately. // Note: 0x48 is in fact the REX.W prefix, but it might be wrong to treat it as a separate // instruction. It is interesting the the same code for mach_override.h/mach_override.c from llvm-3.2 used under clang 3.2 doesn't trigger this issue. Jack > > On Wed, Nov 14, 2012 at 6:43 PM, Jack Howarth <howarth@bromo.med.uc.edu> 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 <howarth@bromo.med.uc.edu> > > > > * 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
On Wed, Nov 14, 2012 at 04:08:06PM +0100, 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 I am confused on the strategy here. Will the FSF gcc developers be prohibiting the addition of darwin support for libsanitizer until all issues in its operation are resolved? It seems like a chicken and egg situation. I think this should be considered an experimental feature and exposed to as many developers as possible so we can get some help on resolving these issues in mach_override.c. Making it unnecessarily difficult to build this feature doesn't help with that project. Jack > > -- > ----------------------------------------------------------------------------- > Rainer Orth, Center for Biotechnology, Bielefeld University
Jack Howarth <howarth@bromo.med.uc.edu> writes: > I am confused on the strategy here. Will the FSF gcc developers be prohibiting > the addition of darwin support for libsanitizer until all issues in its operation > are resolved? It seems like a chicken and egg situation. I think this should be > considered an experimental feature and exposed to as many developers as possible > so we can get some help on resolving these issues in mach_override.c. Making it > unnecessarily difficult to build this feature doesn't help with that project. Where do you get this idea? OTOH, I think it makes sense to only install asan patches and enable it once a reasonable subset of its testsuite (once that's in place) works on a target. Until that point, the necessary patches can be kept in mailing list postings for developers working on that stuff (if more than one). Who is served by installing patches for micro steps that maybe allow libasan to compile, but not do anything useful? Rainer
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 <ro@cebitec.uni-bielefeld.de> 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
Hi Alex, > 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 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.). > 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. 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. > 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. This hopefully won't be an issue on Solaris where dlsym should work the same as on Linux. Thanks. Rainer
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 > <ro@cebitec.uni-bielefeld.de> 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
On Wed, Nov 14, 2012 at 8:09 PM, Rainer Orth <ro@cebitec.uni-bielefeld.de> 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.
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 <howarth@bromo.med.uc.edu> 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 >> <ro@cebitec.uni-bielefeld.de> 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
On Nov 14, 2012, at 6:43 AM, Jack Howarth <howarth@bromo.med.uc.edu> 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. So, the patches are a nice start. Since we are in stage3, they need to go in, in a way that is suitable for release. If the feature is expected to work (I think that's true) and if these patches don't yet work well enough (I don't have a take on wether this is the case or not), then as the patches go in, they need to go in with the feature off or disabled. So, I'd like a person that understand s libsanitizer and what we need (what is suitable) for release to approve the patches. If I do, I'd need to understand more than I do. What we don't want, a half implementation that is worse than saying, unsupported. I don't mind if the support isn't complete, yet, what is there works fine.
On Wed, Nov 14, 2012 at 10:54:18AM -0800, Mike Stump wrote: > On Nov 14, 2012, at 6:43 AM, Jack Howarth <howarth@bromo.med.uc.edu> 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. > > So, the patches are a nice start. Since we are in stage3, they need to go in, in a way that is suitable for release. If the feature is expected to work (I think that's true) and if these patches don't yet work well enough (I don't have a take on wether this is the case or not), then as the patches go in, they need to go in with the feature off or disabled. So, I'd like a person that understand s libsanitizer and what we need (what is suitable) for release to approve the patches. If I do, I'd need to understand more than I do. What we don't want, a half implementation that is worse than saying, unsupported. I don't mind if the support isn't complete, yet, what is there works fine. Mike, With Alexander Potapenko's proposed patch for interception/mach_override/mach_override.c... http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55289#c29 the use-after-free test case from http://code.google.com/p/address-sanitizer/wiki/AddressSanitizer now passes without errors on both x86_64-apple-darwin12 and i386-apple-darwin10. So at the moment we don't have any known issues. Hopefully we can get the missing interception/mach_override/mach_override.c and interception/mach_override/mach_override.h files added soon along with the build patch so we can start monitoring libsanitizer for other issues in mach_override.c. Jack
Maybe Konstantin could Help with the review, as this touches libsanitizer? Cheers. Mike Stump <mikestump@comcast.net> writes: > On Nov 14, 2012, at 6:43 AM, Jack Howarth <howarth@bromo.med.uc.edu> 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. > > So, the patches are a nice start. Since we are in stage3, they need > to go in, in a way that is suitable for release. If the feature is > expected to work (I think that's true) and if these patches don't yet > work well enough (I don't have a take on wether this is the case or > not), then as the patches go in, they need to go in with the feature > off or disabled. So, I'd like a person that understand s libsanitizer > and what we need (what is suitable) for release to approve the > patches. If I do, I'd need to understand more than I do. What we > don't want, a half implementation that is worse than saying, > unsupported. I don't mind if the support isn't complete, yet, what is > there works fine.
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'm talking about the internal_* wrappers e.g. in sanitizer_linux.cc. For Linux, they use syscall directly, for Mac OS, they call the underlying function. >> 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. That's hard to tell: all of Linux, FreeBSD and Solaris are Unix/ELF systems, but apart from the resulting similarities there will be many differences like details how to implement stuff like GetEnv, ReExec or MemoryMappingLayout which rely on /proc details on Linux. I guess this will have to be checked on a case-by-case basis to determine areas that can be handled identically across several different Unix OSes and those that are highly system-specific. Rainer
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 ;;
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 <howarth@bromo.med.uc.edu> * 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.