public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [ubsan PATCH] Don't instrument static initializers (PR sanitizer/67279)
@ 2015-09-04 11:29 Marek Polacek
  2015-09-04 11:48 ` Jakub Jelinek
  0 siblings, 1 reply; 3+ messages in thread
From: Marek Polacek @ 2015-09-04 11:29 UTC (permalink / raw)
  To: GCC Patches, Jakub Jelinek

I think it really doesn't make sense to instrument static initializers; we
wouldn't be able to error at run-time anyway.  Besides, it causes a compile-time
error.  Generally, I think *compiling* with -fsanitize=undefined shouldn't add
any new compile-time errors.  Yes, I know it does in some other cases; this is
just an incremental improvement.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2015-09-04  Marek Polacek  <polacek@redhat.com>

	PR sanitizer/67279
	* c-typeck.c (build_binary_op): Don't instrument static initializers.

	* gcc.dg/ubsan/pr67279.c: New test.

diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index c622a90..dc22396 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -11292,7 +11292,8 @@ build_binary_op (location_t location, enum tree_code code,
   if ((flag_sanitize & (SANITIZE_SHIFT | SANITIZE_DIVIDE
 			| SANITIZE_FLOAT_DIVIDE))
       && do_ubsan_in_current_function ()
-      && (doing_div_or_mod || doing_shift))
+      && (doing_div_or_mod || doing_shift)
+      && !require_constant_value)
     {
       /* OP0 and/or OP1 might have side-effects.  */
       op0 = c_save_expr (op0);
diff --git gcc/testsuite/gcc.dg/ubsan/pr67279.c gcc/testsuite/gcc.dg/ubsan/pr67279.c
index e69de29..5b5db42 100644
--- gcc/testsuite/gcc.dg/ubsan/pr67279.c
+++ gcc/testsuite/gcc.dg/ubsan/pr67279.c
@@ -0,0 +1,14 @@
+/* PR sanitizer/67279 */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=undefined -w" } */
+
+#define INT_MIN (-__INT_MAX__ - 1)
+
+void
+foo (void)
+{
+  static int a1 = 1 << 31;
+  static int a2 = 10 << 30;
+  static int a3 = 100 << 28;
+  static int a4 = INT_MIN / -1;
+}

	Marek

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

* Re: [ubsan PATCH] Don't instrument static initializers (PR sanitizer/67279)
  2015-09-04 11:29 [ubsan PATCH] Don't instrument static initializers (PR sanitizer/67279) Marek Polacek
@ 2015-09-04 11:48 ` Jakub Jelinek
  2015-09-04 13:15   ` Marek Polacek
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2015-09-04 11:48 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches

On Fri, Sep 04, 2015 at 01:28:38PM +0200, Marek Polacek wrote:
> I think it really doesn't make sense to instrument static initializers; we
> wouldn't be able to error at run-time anyway.  Besides, it causes a compile-time
> error.  Generally, I think *compiling* with -fsanitize=undefined shouldn't add
> any new compile-time errors.  Yes, I know it does in some other cases; this is
> just an incremental improvement.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> 2015-09-04  Marek Polacek  <polacek@redhat.com>
> 
> 	PR sanitizer/67279
> 	* c-typeck.c (build_binary_op): Don't instrument static initializers.
> 
> 	* gcc.dg/ubsan/pr67279.c: New test.

Ok, but please make sure it is handled similarly in the C++ FE too (perhaps
incrementally).

	Jakub

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

* Re: [ubsan PATCH] Don't instrument static initializers (PR sanitizer/67279)
  2015-09-04 11:48 ` Jakub Jelinek
@ 2015-09-04 13:15   ` Marek Polacek
  0 siblings, 0 replies; 3+ messages in thread
From: Marek Polacek @ 2015-09-04 13:15 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: GCC Patches

On Fri, Sep 04, 2015 at 01:31:09PM +0200, Jakub Jelinek wrote:
> On Fri, Sep 04, 2015 at 01:28:38PM +0200, Marek Polacek wrote:
> > I think it really doesn't make sense to instrument static initializers; we
> > wouldn't be able to error at run-time anyway.  Besides, it causes a compile-time
> > error.  Generally, I think *compiling* with -fsanitize=undefined shouldn't add
> > any new compile-time errors.  Yes, I know it does in some other cases; this is
> > just an incremental improvement.
> > 
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > 
> > 2015-09-04  Marek Polacek  <polacek@redhat.com>
> > 
> > 	PR sanitizer/67279
> > 	* c-typeck.c (build_binary_op): Don't instrument static initializers.
> > 
> > 	* gcc.dg/ubsan/pr67279.c: New test.
> 
> Ok, but please make sure it is handled similarly in the C++ FE too (perhaps
> incrementally).

cc1plus doesn't reject this particular testcase, so I didn't touch the C++ FE
this time.

Thanks.

	Marek

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

end of thread, other threads:[~2015-09-04 12:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-04 11:29 [ubsan PATCH] Don't instrument static initializers (PR sanitizer/67279) Marek Polacek
2015-09-04 11:48 ` Jakub Jelinek
2015-09-04 13:15   ` 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).