diff mbox

[00/13] Request to merge Address Sanitizer in

Message ID 20121116140319.GA23548@bromo.med.uc.edu
State New
Headers show

Commit Message

Jack Howarth Nov. 16, 2012, 2:03 p.m. UTC 
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


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
The attached patch assumes that the current mach_override/mach_override.h and
mach_override/mach_override.c files have been imported by the libsanitizer maintainers
from llvm compiler-rt svn 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.
LINK_COMMAND_SPEC_A in gcc/config/darwin.h is modified to add an entry to handle
faddress-sanitizer 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 on x86_64-apple-darwin12 against the mach_override/mach_override.h and
mach_override/mach_override.c from llvm compiler-rt svn 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 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.

gcc/

2012-11-15  Jack Howarth <howarth@bromo.med.uc.edu>

	* config/darwin.h (LINK_COMMAND_SPEC_A): Deal with -faddress-sanitizer.

libsanitizer/

2012-11-15  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 193537)
+++ libsanitizer/interception/Makefile.am	(working copy)
@@ -14,7 +14,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 193537)
+++ libsanitizer/configure.ac	(working copy)
@@ -22,6 +22,12 @@ AC_CANONICAL_SYSTEM
 target_alias=${target_alias-$host_alias}
 AC_SUBST(target_alias)
 
+case "$host" in
+  *-*-darwin*) MACH_OVERRIDE=true ;;
+  *) MACH_OVERRIDE=false ;;
+esac
+AM_CONDITIONAL(USING_MACH_OVERRIDE, $MACH_OVERRIDE)
+
 AM_INIT_AUTOMAKE(foreign)
 AM_ENABLE_MULTILIB(, ..)
 
Index: libsanitizer/configure.tgt
===================================================================
--- libsanitizer/configure.tgt	(revision 193537)
+++ 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
 	;;
Index: gcc/config/darwin.h
===================================================================
--- gcc/config/darwin.h	(revision 193537)
+++ gcc/config/darwin.h	(working copy)
@@ -180,6 +180,9 @@ extern GTY(()) int darwin_ms_struct;
     %{L*} %(link_libgcc) %o %{fprofile-arcs|fprofile-generate*|coverage:-lgcov} \
     %{fopenmp|ftree-parallelize-loops=*: \
       %{static|static-libgcc|static-libstdc++|static-libgfortran: libgomp.a%s; : -lgomp } } \
+    %{faddress-sanitizer: \
+      %{static|static-libgcc|static-libgfortran: -framework CoreFoundation -lstdc++ libasan.a%s; \
+      static-libstdc++: -framework CoreFoundation libstdc++.a%s libasan.a%s; : -framework CoreFoundation -lasan } } \
     %{fgnu-tm: \
       %{static|static-libgcc|static-libstdc++|static-libgfortran: libitm.a%s; : -litm } } \
     %{!nostdlib:%{!nodefaultlibs:\
diff mbox

Patch

==============================================================================
--- 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