public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix bootstrap-ubsan (PR bootstrap/82670)
@ 2017-11-08  7:56 Jakub Jelinek
  2017-11-08  8:37 ` Richard Biener
  2017-11-08  9:05 ` Marek Polacek
  0 siblings, 2 replies; 3+ messages in thread
From: Jakub Jelinek @ 2017-11-08  7:56 UTC (permalink / raw)
  To: Richard Biener, Marek Polacek, Martin Liška, Maxim Ostapenko
  Cc: gcc-patches

Hi!

The upstream libubsan in the name of behaving more similarly between
all the other sanitizers turned the library from a lightweight set of a few
helper routines that print errors if something goes wrong into yet another
library that overrides various functions.

In particular, it now overrides signal and sigaction functions and therefore
requires to be initialized before anything else (otherwise, if somebody
calls signal or sigaction before the library is fully initialized, it just
crashes).  But this overriding isn't really needed for what the library is
doing, just so that there is some pretty printing of fatal signals.

I really don't like that, IMHO libubsan has no business in doing that, so
instead of adding .preinit_array initializers for the library, this patch
just removes that useless initialization and overriding from the library.
If people really need the pretty printing, they can use -fsanitize=address,
or leak or thread which all do need the heavy early initialization.

With this change, the ubsan initialization is performed on the first call
that actually wants to print something (and the initialization is only the
minimal set of stuff, parsing env vars, handling suppressions etc.).

Bootstrapped/regtested on x86_64-linux and i686-linux, additionally tested
with x86_64-linux --with-build-config=bootstrap-ubsan --enable-checking=release
bootstrap/regtest.

Ok for trunk?

2017-11-08  Jakub Jelinek  <jakub@redhat.com>

	PR bootstrap/82670
	* ubsan/Makefile.am (ubsan_files): Remove ubsan_init_standalone.cc
	and ubsan_signals_standalone.cc.
	* ubsan/Makefile.in: Regenerated.

--- libsanitizer/ubsan/Makefile.am.jj	2017-10-19 13:20:58.000000000 +0200
+++ libsanitizer/ubsan/Makefile.am	2017-11-07 17:40:23.589026215 +0100
@@ -22,10 +22,7 @@ ubsan_plugin_files = \
 	ubsan_type_hash_win.cc \
 	ubsan_value.cc
 
-ubsan_files = \
-	$(ubsan_plugin_files) \
-	ubsan_init_standalone.cc \
-	ubsan_signals_standalone.cc
+ubsan_files = $(ubsan_plugin_files)
 
 libubsan_la_SOURCES = $(ubsan_files) 
 libubsan_la_LIBADD = $(top_builddir)/sanitizer_common/libsanitizer_common.la 
--- libsanitizer/ubsan/Makefile.in.jj	2017-10-19 15:14:17.000000000 +0200
+++ libsanitizer/ubsan/Makefile.in	2017-11-07 17:40:47.378733040 +0100
@@ -111,8 +111,7 @@ am__objects_1 = ubsan_diag.lo ubsan_flag
 	ubsan_handlers_cxx.lo ubsan_init.lo ubsan_type_hash.lo \
 	ubsan_type_hash_itanium.lo ubsan_type_hash_win.lo \
 	ubsan_value.lo
-am__objects_2 = $(am__objects_1) ubsan_init_standalone.lo \
-	ubsan_signals_standalone.lo
+am__objects_2 = $(am__objects_1)
 am_libubsan_la_OBJECTS = $(am__objects_2)
 libubsan_la_OBJECTS = $(am_libubsan_la_OBJECTS)
 libubsan_la_LINK = $(LIBTOOL) --tag=CXX $(AM_LIBTOOLFLAGS) \
@@ -306,11 +305,7 @@ ubsan_plugin_files = \
 	ubsan_type_hash_win.cc \
 	ubsan_value.cc
 
-ubsan_files = \
-	$(ubsan_plugin_files) \
-	ubsan_init_standalone.cc \
-	ubsan_signals_standalone.cc
-
+ubsan_files = $(ubsan_plugin_files)
 libubsan_la_SOURCES = $(ubsan_files) 
 libubsan_la_LIBADD =  \
 	$(top_builddir)/sanitizer_common/libsanitizer_common.la \
@@ -436,8 +431,6 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ubsan_handlers.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ubsan_handlers_cxx.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ubsan_init.Plo@am__quote@
-@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ubsan_init_standalone.Plo@am__quote@
-@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ubsan_signals_standalone.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ubsan_type_hash.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ubsan_type_hash_itanium.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ubsan_type_hash_win.Plo@am__quote@

	Jakub

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Fix bootstrap-ubsan (PR bootstrap/82670)
  2017-11-08  7:56 [PATCH] Fix bootstrap-ubsan (PR bootstrap/82670) Jakub Jelinek
@ 2017-11-08  8:37 ` Richard Biener
  2017-11-08  9:05 ` Marek Polacek
  1 sibling, 0 replies; 3+ messages in thread
From: Richard Biener @ 2017-11-08  8:37 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Marek Polacek, Martin Liška, Maxim Ostapenko, gcc-patches

On Wed, 8 Nov 2017, Jakub Jelinek wrote:

> Hi!
> 
> The upstream libubsan in the name of behaving more similarly between
> all the other sanitizers turned the library from a lightweight set of a few
> helper routines that print errors if something goes wrong into yet another
> library that overrides various functions.
> 
> In particular, it now overrides signal and sigaction functions and therefore
> requires to be initialized before anything else (otherwise, if somebody
> calls signal or sigaction before the library is fully initialized, it just
> crashes).  But this overriding isn't really needed for what the library is
> doing, just so that there is some pretty printing of fatal signals.
> 
> I really don't like that, IMHO libubsan has no business in doing that, so
> instead of adding .preinit_array initializers for the library, this patch
> just removes that useless initialization and overriding from the library.
> If people really need the pretty printing, they can use -fsanitize=address,
> or leak or thread which all do need the heavy early initialization.
> 
> With this change, the ubsan initialization is performed on the first call
> that actually wants to print something (and the initialization is only the
> minimal set of stuff, parsing env vars, handling suppressions etc.).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, additionally tested
> with x86_64-linux --with-build-config=bootstrap-ubsan --enable-checking=release
> bootstrap/regtest.
> 
> Ok for trunk?

Ok.

Richard.

> 
> 2017-11-08  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR bootstrap/82670
> 	* ubsan/Makefile.am (ubsan_files): Remove ubsan_init_standalone.cc
> 	and ubsan_signals_standalone.cc.
> 	* ubsan/Makefile.in: Regenerated.
> 
> --- libsanitizer/ubsan/Makefile.am.jj	2017-10-19 13:20:58.000000000 +0200
> +++ libsanitizer/ubsan/Makefile.am	2017-11-07 17:40:23.589026215 +0100
> @@ -22,10 +22,7 @@ ubsan_plugin_files = \
>  	ubsan_type_hash_win.cc \
>  	ubsan_value.cc
>  
> -ubsan_files = \
> -	$(ubsan_plugin_files) \
> -	ubsan_init_standalone.cc \
> -	ubsan_signals_standalone.cc
> +ubsan_files = $(ubsan_plugin_files)
>  
>  libubsan_la_SOURCES = $(ubsan_files) 
>  libubsan_la_LIBADD = $(top_builddir)/sanitizer_common/libsanitizer_common.la 
> --- libsanitizer/ubsan/Makefile.in.jj	2017-10-19 15:14:17.000000000 +0200
> +++ libsanitizer/ubsan/Makefile.in	2017-11-07 17:40:47.378733040 +0100
> @@ -111,8 +111,7 @@ am__objects_1 = ubsan_diag.lo ubsan_flag
>  	ubsan_handlers_cxx.lo ubsan_init.lo ubsan_type_hash.lo \
>  	ubsan_type_hash_itanium.lo ubsan_type_hash_win.lo \
>  	ubsan_value.lo
> -am__objects_2 = $(am__objects_1) ubsan_init_standalone.lo \
> -	ubsan_signals_standalone.lo
> +am__objects_2 = $(am__objects_1)
>  am_libubsan_la_OBJECTS = $(am__objects_2)
>  libubsan_la_OBJECTS = $(am_libubsan_la_OBJECTS)
>  libubsan_la_LINK = $(LIBTOOL) --tag=CXX $(AM_LIBTOOLFLAGS) \
> @@ -306,11 +305,7 @@ ubsan_plugin_files = \
>  	ubsan_type_hash_win.cc \
>  	ubsan_value.cc
>  
> -ubsan_files = \
> -	$(ubsan_plugin_files) \
> -	ubsan_init_standalone.cc \
> -	ubsan_signals_standalone.cc
> -
> +ubsan_files = $(ubsan_plugin_files)
>  libubsan_la_SOURCES = $(ubsan_files) 
>  libubsan_la_LIBADD =  \
>  	$(top_builddir)/sanitizer_common/libsanitizer_common.la \
> @@ -436,8 +431,6 @@ distclean-compile:
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ubsan_handlers.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ubsan_handlers_cxx.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ubsan_init.Plo@am__quote@
> -@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ubsan_init_standalone.Plo@am__quote@
> -@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ubsan_signals_standalone.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ubsan_type_hash.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ubsan_type_hash_itanium.Plo@am__quote@
>  @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ubsan_type_hash_win.Plo@am__quote@
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Fix bootstrap-ubsan (PR bootstrap/82670)
  2017-11-08  7:56 [PATCH] Fix bootstrap-ubsan (PR bootstrap/82670) Jakub Jelinek
  2017-11-08  8:37 ` Richard Biener
@ 2017-11-08  9:05 ` Marek Polacek
  1 sibling, 0 replies; 3+ messages in thread
From: Marek Polacek @ 2017-11-08  9:05 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Richard Biener, Martin Liška, Maxim Ostapenko, gcc-patches

On Wed, Nov 08, 2017 at 08:43:43AM +0100, Jakub Jelinek wrote:
> Hi!
> 
> The upstream libubsan in the name of behaving more similarly between
> all the other sanitizers turned the library from a lightweight set of a few
> helper routines that print errors if something goes wrong into yet another
> library that overrides various functions.
> 
> In particular, it now overrides signal and sigaction functions and therefore
> requires to be initialized before anything else (otherwise, if somebody
> calls signal or sigaction before the library is fully initialized, it just
> crashes).  But this overriding isn't really needed for what the library is
> doing, just so that there is some pretty printing of fatal signals.
> 
> I really don't like that, IMHO libubsan has no business in doing that, so
> instead of adding .preinit_array initializers for the library, this patch
> just removes that useless initialization and overriding from the library.
> If people really need the pretty printing, they can use -fsanitize=address,
> or leak or thread which all do need the heavy early initialization.
> 
> With this change, the ubsan initialization is performed on the first call
> that actually wants to print something (and the initialization is only the
> minimal set of stuff, parsing env vars, handling suppressions etc.).

I agree, I think this lazy initialization is much better.

	Marek

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-11-08  8:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-08  7:56 [PATCH] Fix bootstrap-ubsan (PR bootstrap/82670) Jakub Jelinek
2017-11-08  8:37 ` Richard Biener
2017-11-08  9:05 ` Marek Polacek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).