Patchwork Ubsan merged into trunk

login
register
mail settings
Submitter Iain Sandoe
Date Aug. 31, 2013, 3:04 p.m.
Message ID <02CCE197-2D37-4BFD-B165-B14EE602FB85@codesourcery.com>
Download mbox | patch
Permalink /patch/271537/
State New
Headers show

Comments

Iain Sandoe - Aug. 31, 2013, 3:04 p.m.
Hi,

On 30 Aug 2013, at 20:43, Jakub Jelinek wrote:

> On Fri, Aug 30, 2013 at 09:38:01PM +0200, Dominique Dhumieres wrote:
>>> I've just merged ubsan into trunk.  Please send complaints my way.
>> 
>> Bootstrap is broken on x86_64-apple-darwin10:

> (wonder why not
> libasan_la_LIBADD = $(top_builddir)/sanitizer_common/libsanitizer_common.la
> if !USING_MAC_INTERPOSE
> libasan_la_LIBADD += $(top_builddir)/interception/libinterception.la
> endif
> libasan_la_LIBADD += $(LIBSTDCXX_RAW_CXX_LDFLAGS)
> ).

… indeed, that's what I did for ubsan.

> perhaps tsan/Makefile.am too (though, tsan isn't supported on darwin, so
> it doesn't matter that much).

tsan isn't relevant (yet): although, when we get some time to work on it, native thread support should be feasible for Darwin >= 11.
Marek Polacek - Aug. 31, 2013, 3:09 p.m.
On Sat, Aug 31, 2013 at 04:04:03PM +0100, Iain Sandoe wrote:
> Hi,
> 
> On 30 Aug 2013, at 20:43, Jakub Jelinek wrote:
> 
> > On Fri, Aug 30, 2013 at 09:38:01PM +0200, Dominique Dhumieres wrote:
> >>> I've just merged ubsan into trunk.  Please send complaints my way.
> >> 
> >> Bootstrap is broken on x86_64-apple-darwin10:
> 
> > (wonder why not
> > libasan_la_LIBADD = $(top_builddir)/sanitizer_common/libsanitizer_common.la
> > if !USING_MAC_INTERPOSE
> > libasan_la_LIBADD += $(top_builddir)/interception/libinterception.la
> > endif
> > libasan_la_LIBADD += $(LIBSTDCXX_RAW_CXX_LDFLAGS)
> > ).
> 
> … indeed, that's what I did for ubsan.
> 
> > perhaps tsan/Makefile.am too (though, tsan isn't supported on darwin, so
> > it doesn't matter that much).
> 
> tsan isn't relevant (yet): although, when we get some time to work on it, native thread support should be feasible for Darwin >= 11.
> 
> ===
> 
> the patch below fixes bootstrap - along the lines of your observation;
> it also fixes the specs to actually use the library (so that the tests pass too).
> 
> bootstrapped x86_64-darwin12 for c,c++ and fortran (objc and ada bootstraps are broken from other causes).
> [also bootstrapped on x86_64-linux, and checked RUNTESTFLAGS="asan.exp ubsan.exp"]
> 
> OK for trunk?
> Iain

I was just about to post something similar; thanks for the patch and sorry
for the breakage.  Can't give you formal approval though.

> gcc:
> 	* config/darwin.h (LINK_COMMAND_SPEC_A): Revise sanitiser specs to
> 	include sanitise(undefined).

s/sanitise/sanitize/ ;)

	Marek
Dominique Dhumieres - Aug. 31, 2013, 8:48 p.m.
> bootstrapped x86_64-darwin12 for c,c++ and fortran ...

Bootstrapped x86_64-apple-darwin10 for c,c++,fortran,java and
also tested asan.exp and ubsan.exp for gcc and g++.

Thanks for the patch,

Dominique
Jakub Jelinek - Sept. 1, 2013, 11:35 a.m.
On Sat, Aug 31, 2013 at 04:04:03PM +0100, Iain Sandoe wrote:
> OK for trunk?

Ok with the suggested s/sanitise/sanitize/g change.

> gcc:
> 	* config/darwin.h (LINK_COMMAND_SPEC_A): Revise sanitiser specs to
> 	include sanitise(undefined).
> 
> libsanitiser:
> 
> 	* ubsan/Makefile.am (libubsan_la_LIBADD): Revise to omit libinterception.la
> 	for Darwin.
> 	* ubsan/Makefile.in: Regenerate.
> 
> Index: gcc/config/darwin.h
> ===================================================================
> --- gcc/config/darwin.h	(revision 202118)
> +++ gcc/config/darwin.h	(working copy)
> @@ -178,10 +178,11 @@ 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 } } \
> -    %{%:sanitize(address): -lasan } \
>      %{fgnu-tm: \
>        %{static|static-libgcc|static-libstdc++|static-libgfortran: libitm.a%s; : -litm } } \
>      %{!nostdlib:%{!nodefaultlibs:\
> +      %{%:sanitize(address): -lasan } \
> +      %{%:sanitize(undefined): -lubsan } \
>        %(link_ssp) %(link_gcc_c_sequence)\
>      }}\
>      %{!nostdlib:%{!nostartfiles:%E}} %{T*} %{F*} }}}}}}}"
> 
> Index: libsanitizer/ubsan/Makefile.am
> ===================================================================
> --- libsanitizer/ubsan/Makefile.am	(revision 202118)
> +++ libsanitizer/ubsan/Makefile.am	(working copy)
> @@ -18,7 +18,11 @@ ubsan_files = \
>  	ubsan_value.cc
>  
>  libubsan_la_SOURCES = $(ubsan_files) 
> -libubsan_la_LIBADD = $(top_builddir)/sanitizer_common/libsanitizer_common.la $(top_builddir)/interception/libinterception.la $(LIBSTDCXX_RAW_CXX_LDFLAGS)
> +libubsan_la_LIBADD = $(top_builddir)/sanitizer_common/libsanitizer_common.la 
> +if !USING_MAC_INTERPOSE
> +libubsan_la_LIBADD += $(top_builddir)/interception/libinterception.la
> +endif
> +libubsan_la_LIBADD += $(LIBSTDCXX_RAW_CXX_LDFLAGS)
>  libubsan_la_LDFLAGS = -version-info `grep -v '^\#' $(srcdir)/libtool-version` -lpthread -ldl
>  
>  # Work around what appears to be a GNU make bug handling MAKEFLAGS

	Jakub

Patch

===

the patch below fixes bootstrap - along the lines of your observation;
it also fixes the specs to actually use the library (so that the tests pass too).

bootstrapped x86_64-darwin12 for c,c++ and fortran (objc and ada bootstraps are broken from other causes).
[also bootstrapped on x86_64-linux, and checked RUNTESTFLAGS="asan.exp ubsan.exp"]

OK for trunk?
Iain

gcc:
	* config/darwin.h (LINK_COMMAND_SPEC_A): Revise sanitiser specs to
	include sanitise(undefined).

libsanitiser:

	* ubsan/Makefile.am (libubsan_la_LIBADD): Revise to omit libinterception.la
	for Darwin.
	* ubsan/Makefile.in: Regenerate.

Index: gcc/config/darwin.h
===================================================================
--- gcc/config/darwin.h	(revision 202118)
+++ gcc/config/darwin.h	(working copy)
@@ -178,10 +178,11 @@  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 } } \
-    %{%:sanitize(address): -lasan } \
     %{fgnu-tm: \
       %{static|static-libgcc|static-libstdc++|static-libgfortran: libitm.a%s; : -litm } } \
     %{!nostdlib:%{!nodefaultlibs:\
+      %{%:sanitize(address): -lasan } \
+      %{%:sanitize(undefined): -lubsan } \
       %(link_ssp) %(link_gcc_c_sequence)\
     }}\
     %{!nostdlib:%{!nostartfiles:%E}} %{T*} %{F*} }}}}}}}"

Index: libsanitizer/ubsan/Makefile.am
===================================================================
--- libsanitizer/ubsan/Makefile.am	(revision 202118)
+++ libsanitizer/ubsan/Makefile.am	(working copy)
@@ -18,7 +18,11 @@  ubsan_files = \
 	ubsan_value.cc
 
 libubsan_la_SOURCES = $(ubsan_files) 
-libubsan_la_LIBADD = $(top_builddir)/sanitizer_common/libsanitizer_common.la $(top_builddir)/interception/libinterception.la $(LIBSTDCXX_RAW_CXX_LDFLAGS)
+libubsan_la_LIBADD = $(top_builddir)/sanitizer_common/libsanitizer_common.la 
+if !USING_MAC_INTERPOSE
+libubsan_la_LIBADD += $(top_builddir)/interception/libinterception.la
+endif
+libubsan_la_LIBADD += $(LIBSTDCXX_RAW_CXX_LDFLAGS)
 libubsan_la_LDFLAGS = -version-info `grep -v '^\#' $(srcdir)/libtool-version` -lpthread -ldl
 
 # Work around what appears to be a GNU make bug handling MAKEFLAGS