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

[Jack Howarth <howarth at bromo dot med dot uc dot edu>]

On Fri, Nov 16, 2012 at 09:27:26AM +0100, Dodji Seketeli wrote:
> Jack Howarth <howarth@bromo.med.uc.edu> writes:
> 
> >     The Google branch is missing the required
> > interception/mach_override/mach_override.h and
> > interception/mach_override/mach_override.c files from compiler-rt svn
> > for darwin. I have posted what I believe to be the final patch which
> > eanbles libsanitizer on darwin...
> >
> > http://gcc.gnu.org/ml/gcc-patches/2012-11/msg01285.html
> 
> I see in that thread that Mike Stump has approves the patch if no
> asan{-darwin} people disagrees.  I'll abide by principle, FWIW.  :-)
> 
> > which has been tested with the existing asan testsuite, the
> > use-after-free.c testcase as well as the Polyhedron 2005 benchmarks
> > for -O1 -g -fno-omit-frame-pointer -faddress-sanitizer and -O3
> > -funroll-loops -ffast-math -g -fno-omit-frame-pointer
> > -faddress-sanitizer to prove that the current mach_override from
> > upstream is sufficient for darwin to use.
> 
> I see.   Thanks.
> 
> > Due to the large number of maintainers for libsanitizer, it is unclear
> > who is the person responsible for upstream merges to lobby for these
> > files to be ported into gcc trunk.  With Alexander Potapenko's commit
> > of the bug fix to mach_override/mach_override.c required for FSF
> > gcc...
> >
> > http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20121112/155989.html
> >
> > ...there really is no reason to continue to delay (as the interpose code simply won't
> > be completed in time for gcc 4.8.0).
> 
> It makes sense to me.
> 
> > Can we please get some movement on importing these missing files from
> > upstream?
> 
> Well, given that ....
> 
> Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes:
> 
> > I see no problems with committing mach_override to gcc.
> > The code should be verbatim copy from
> > llvm/projects/compiler-rt/lib/interception/mach_override
> > Note that this code comes with an MIT license and was not developed by
> > Google (we did add quite a few patches).
> 
> ... Konstantin who is one of the libsanitizer maintainers agrees, I see
> no reason to delay this either.
> 
> So, Jack, as you are on top of this topic and has the platform to test
> at hand, I guess you could just import the missing files from the llvm
> repository and commit them to GCC, unless a GCC maintainers disagrees,
> of course.

Can one of the libsanitizer maintainers handle the importation? The only
requirements are that they use a mach_override/mach_override.c and
mach_override/mach_override.h from on or after llvm r168032...

Author: glider
Date: Thu Nov 15 02:32:16 2012
New Revision: 168032

URL: http://llvm.org/viewvc/llvm-project?rev=168032&view=rev
Log:
[ASan] Add the "lea $imm(%rip),%rax" instruction to mach_override.c
The need for this has been reported by Jack Howarth (howarth at bromo.med.uc.edu) who's porting ASan-Darwin to GCC

Modified:
    compiler-rt/trunk/lib/interception/mach_override/mach_override.c

Modified: compiler-rt/trunk/lib/interception/mach_override/mach_override.c
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/interception/mach_override/mach_override.c?rev=168032&r1=168031&r2=168032&view=diff
==============================================================================
--- compiler-rt/trunk/lib/interception/mach_override/mach_override.c (original)
+++ compiler-rt/trunk/lib/interception/mach_override/mach_override.c Thu Nov 15 02:32:16 2012
@@ -725,6 +725,8 @@
         { 0x2, {0xFF, 0x00}, {0x89, 0x00} },                               // mov r/m32,r32 or r/m16,r16
         { 0x3, {0xFF, 0xFF, 0xFF}, {0x49, 0x89, 0xF8} },                   // mov %rdi,%r8
         { 0x4, {0xFF, 0xFF, 0xFF, 0xFF}, {0x40, 0x0F, 0xBE, 0xCE} },       // movsbl %sil,%ecx
+        { 0x7, {0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00},
+               {0x48, 0x8D, 0x05, 0x00, 0x00, 0x00, 0x00} },  // lea $imm(%rip),%rax
         { 0x3, {0xFF, 0xFF, 0xFF}, {0x0F, 0xBE, 0xCE} },  // movsbl, %dh, %ecx
         { 0x3, {0xFF, 0xFF, 0x00}, {0xFF, 0x77, 0x00} },  // pushq $imm(%rdi)
         { 0x2, {0xFF, 0xFF}, {0xDB, 0xE3} }, // fninit


and place these two files in libsanitizer/interception/mach_override. I am unclear on what adjustments
you are doing to the licensing comments in these files. The imported files could just be tacked onto
my posted patch (reattached to this message) and done as a single commit. Thanks in advance.
        Jack
ps I assume that these changes should also be committed to the gcc asan branch as well.


> 
> Thus, you could maybe just send the patch of the file you are about to
> commit as a reply to this thread, so that Konstantin and Alexander can
> officially ACK it?  I am mentioning Alexander because of what Konstantin
> is saying ...
> 
> > 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.
> 
> -- 
> 		Dodji

Attachment: asan_v5.diff
Description: Text document