diff mbox series

Swap ubsan and early uninit warning passes (PR middle-end/89284)

Message ID 20190213231054.GE2135@tucnak
State New
Headers show
Series Swap ubsan and early uninit warning passes (PR middle-end/89284) | expand

Commit Message

Jakub Jelinek Feb. 13, 2019, 11:10 p.m. UTC
Hi!

The ubsan pass makes early uninit warning harder (and often doesn't warn),
because e.g. for enum/bool loads we add instrumentation and the reads from
the fields isn't easily visible to the uninit pass, we have
  _7 = &a.a;
  _8 = MEM[(_Bool *)_7];
instead etc.  As the early uninit warning pass shouldn't change the IL
(except for nowarning flags), I think it should be ok to swap the passes,
first warn and only then do ubsan instrumentation.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk (or
just for GCC10)?

2019-02-13  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/89284
	* passes.def: Swap pass_ubsan and pass_early_warn_uninitialized.

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


	Jakub

Comments

Richard Biener Feb. 14, 2019, 5:56 a.m. UTC | #1
On February 14, 2019 12:10:54 AM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>The ubsan pass makes early uninit warning harder (and often doesn't
>warn),
>because e.g. for enum/bool loads we add instrumentation and the reads
>from
>the fields isn't easily visible to the uninit pass, we have
>  _7 = &a.a;
>  _8 = MEM[(_Bool *)_7];
>instead etc.  As the early uninit warning pass shouldn't change the IL
>(except for nowarning flags), I think it should be ok to swap the
>passes,
>first warn and only then do ubsan instrumentation.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk (or
>just for GCC10)?

OK for trunk. 

Richard. 

>2019-02-13  Jakub Jelinek  <jakub@redhat.com>
>
>	PR middle-end/89284
>	* passes.def: Swap pass_ubsan and pass_early_warn_uninitialized.
>
>	* gcc.dg/ubsan/pr89284.c: New test.
>
>--- gcc/passes.def.jj	2019-01-01 12:37:18.041960447 +0100
>+++ gcc/passes.def	2019-02-13 14:45:20.205700845 +0100
>@@ -56,8 +56,8 @@ along with GCC; see the file COPYING3.
>       NEXT_PASS (pass_fixup_cfg);
>       NEXT_PASS (pass_build_ssa);
>       NEXT_PASS (pass_warn_nonnull_compare);
>-      NEXT_PASS (pass_ubsan);
>       NEXT_PASS (pass_early_warn_uninitialized);
>+      NEXT_PASS (pass_ubsan);
>       NEXT_PASS (pass_nothrow);
>       NEXT_PASS (pass_rebuild_cgraph_edges);
>   POP_INSERT_PASSES ()
>--- gcc/testsuite/gcc.dg/ubsan/pr89284.c.jj	2019-02-13
>15:01:25.666165920 +0100
>+++ gcc/testsuite/gcc.dg/ubsan/pr89284.c	2019-02-13 15:01:09.987414305
>+0100
>@@ -0,0 +1,23 @@
>+/* PR middle-end/89284 */
>+/* { dg-do compile } */
>+/* { dg-options "-fsanitize=undefined -O0 -Wuninitialized" } */
>+
>+struct A { _Bool a; int i; };
>+
>+int
>+foo (void)
>+{
>+  struct A a;
>+  if (a.i)	/* { dg-warning "'a.i' is used uninitialized in this
>function" } */
>+    return 1;
>+  return 0;
>+}
>+
>+int
>+bar (void)
>+{
>+  struct A a;
>+  if (a.a)	/* { dg-warning "'a.a' is used uninitialized in this
>function" } */
>+    return 1;
>+  return 0;
>+}
>
>	Jakub
diff mbox series

Patch

--- gcc/passes.def.jj	2019-01-01 12:37:18.041960447 +0100
+++ gcc/passes.def	2019-02-13 14:45:20.205700845 +0100
@@ -56,8 +56,8 @@  along with GCC; see the file COPYING3.
       NEXT_PASS (pass_fixup_cfg);
       NEXT_PASS (pass_build_ssa);
       NEXT_PASS (pass_warn_nonnull_compare);
-      NEXT_PASS (pass_ubsan);
       NEXT_PASS (pass_early_warn_uninitialized);
+      NEXT_PASS (pass_ubsan);
       NEXT_PASS (pass_nothrow);
       NEXT_PASS (pass_rebuild_cgraph_edges);
   POP_INSERT_PASSES ()
--- gcc/testsuite/gcc.dg/ubsan/pr89284.c.jj	2019-02-13 15:01:25.666165920 +0100
+++ gcc/testsuite/gcc.dg/ubsan/pr89284.c	2019-02-13 15:01:09.987414305 +0100
@@ -0,0 +1,23 @@ 
+/* PR middle-end/89284 */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=undefined -O0 -Wuninitialized" } */
+
+struct A { _Bool a; int i; };
+
+int
+foo (void)
+{
+  struct A a;
+  if (a.i)	/* { dg-warning "'a.i' is used uninitialized in this function" } */
+    return 1;
+  return 0;
+}
+
+int
+bar (void)
+{
+  struct A a;
+  if (a.a)	/* { dg-warning "'a.a' is used uninitialized in this function" } */
+    return 1;
+  return 0;
+}