* [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).