Patchwork Warning for unreachable code patch

login
register
mail settings
Submitter sarah@hederstierna.com
Date Dec. 5, 2011, 10:55 p.m.
Message ID <CE36BD26828FA5408B9F87E4DD2ACB0B012BB3676D48@MBXVS01.HMC.local>
Download mbox | patch
Permalink /patch/129463/
State New
Headers show

Comments

sarah@hederstierna.com - Dec. 5, 2011, 10:55 p.m.
Hi

Recently I discovered that the warning for unreachable code (-Wunreachable-code) was removed from GCC two years ago.

    http://gcc.gnu.org/ml/gcc-patches/2009-11/msg00251.html

I think this is unfortunate, as this warning is very valuable for static code analysis checking.
Maybe the warning was removed too easy, and limited effort was put into trying to repair it?

I did a minor tweak for the warning checker, and I propose it should be added again.
It works much better now I think. Maybe it's not perfect, but I think its better than nothing.
I've runned it on my 300k+ source code base and it gave no false alarms, actually it found one real bug!

The problem I assume previously with alot of false warnings, was that it was used on all optimization passes,
eg. loop-unrolling and inlining seems to 'remove' alot of source lines just by replacement, and not caused by code actually being unreachble.
If running the checker for just the cfg-tree pass it gives alot less false warnings (I haven't seen any false alarms yet).

Possibly it could be added a parameter to remove_bb(enum delete_reason) with some kind of information submitted in parameter why this line was deleted.
In case of actual unreachable code reasons, the warning could be triggered.

But I think though the my proposed tweak will handle alot of cases,
please also see attached 10 test cases (most of them old ones) that now works much better I think.

Thanks and Best Regards!

Fredrik Hederstierna
Securitas Direct AB
Malmoe SWEDEN

Patch

Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 181788)
+++ doc/invoke.texi	(working copy)
@@ -266,7 +266,7 @@  Objective-C and Objective-C++ Dialects}.
 -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{]} @gol
 -Wswitch  -Wswitch-default  -Wswitch-enum -Wsync-nand @gol
 -Wsystem-headers  -Wtrampolines  -Wtrigraphs  -Wtype-limits  -Wundef @gol
--Wuninitialized  -Wunknown-pragmas  -Wno-pragmas @gol
+-Wuninitialized  -Wunknown-pragmas  -Wno-pragmas  -Wunreachable-code @gol
 -Wunsuffixed-float-constants  -Wunused  -Wunused-function @gol
 -Wunused-label  -Wunused-local-typedefs -Wunused-parameter @gol
 -Wno-unused-result -Wunused-value @gol -Wunused-variable @gol
@@ -4511,6 +4511,29 @@  cases where multiple declaration is vali
 @opindex Wno-nested-externs
 Warn if an @code{extern} declaration is encountered within a function.
 
+@item -Wunreachable-code
+@opindex Wunreachable-code
+@opindex Wno-unreachable-code
+Warn if the compiler detects that code will never be executed.
+
+This option is intended to warn when the compiler detects that at
+least a whole line of source code will never be executed, because
+some condition is never satisfied or because it is after a
+procedure that never returns.
+
+It is possible for this option to produce a warning even though there
+are circumstances under which part of the affected line can be executed,
+so care should be taken when removing apparently-unreachable code.
+
+For instance, when a function is inlined, a warning may mean that the
+line is unreachable in only one inlined copy of the function.
+
+This option is not made part of @option{-Wall} because in a debugging
+version of a program there is often substantial code which checks
+correct functioning of the program and is, hopefully, unreachable
+because the program does work.  Another common use of unreachable
+code is to provide behavior which is selectable at compile-time.
+
 @item -Winline
 @opindex Winline
 @opindex Wno-inline
Index: testsuite/gcc.dg/Wunreachable-1.c
===================================================================
--- testsuite/gcc.dg/Wunreachable-1.c	(revision 0)
+++ testsuite/gcc.dg/Wunreachable-1.c	(revision 0)
@@ -0,0 +1,24 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wunreachable-code" } */
+
+extern void foo (void);
+extern void baz (void);
+
+void bar (int i)
+{
+  if (i < 2)
+    {
+      baz ();
+      return;
+    }
+  else
+    {
+      if (i >= 4 && i <= 5)
+        foo ();
+      return;
+    }
+
+  baz ();	/* { dg-warning "will never be executed" "" } */
+  baz ();
+  baz ();
+}
Index: testsuite/gcc.dg/Wunreachable-5.c
===================================================================
--- testsuite/gcc.dg/Wunreachable-5.c	(revision 0)
+++ testsuite/gcc.dg/Wunreachable-5.c	(revision 0)
@@ -0,0 +1,16 @@ 
+/* PR c/10175 */
+
+/* { dg-do compile } */
+/* { dg-options "-Wunreachable-code" } */
+
+int value;
+
+int main(void)
+{
+    if (0)
+        value = 0;  /* { dg-warning "will never be executed" "" } */
+    else
+        value = 1;
+
+    return 0;
+}
Index: testsuite/gcc.dg/Wunreachable-9.c
===================================================================
--- testsuite/gcc.dg/Wunreachable-9.c	(revision 0)
+++ testsuite/gcc.dg/Wunreachable-9.c	(revision 0)
@@ -0,0 +1,24 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Wunreachable-code" } */
+
+extern void foo (void);
+
+int main(void)
+{
+    goto a;
+    goto b; /* { dg-warning "will never be executed" "" } */
+
+ a:
+    foo ();
+ b:
+
+    goto x;
+    foo (); /* { dg-warning "will never be executed" "" } */
+    goto y;
+
+ x:
+    foo ();
+ y:
+
+    return 0;
+}
Index: testsuite/gcc.dg/Wunreachable-2.c
===================================================================
--- testsuite/gcc.dg/Wunreachable-2.c	(revision 0)
+++ testsuite/gcc.dg/Wunreachable-2.c	(revision 0)
@@ -0,0 +1,21 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wunreachable-code" } */
+
+extern int foo (const char *);
+extern void baz (void);
+const char *a[] = { "one", "two" };
+
+void bar (void)
+{
+  int i;
+
+  for (i = 0; i < 2; i++)  /* { dg-bogus "will never be executed" ""
+{ xfail *-*-* } } */
+    if (! foo (a[i]))  /* { dg-bogus "will never be executed" "" {
+xfail *-*-* } } */
+      return;
+
+  baz ();	/* { dg-bogus "will never be executed" } */
+  baz ();
+  baz ();
+}
Index: testsuite/gcc.dg/Wunreachable-6.c
===================================================================
--- testsuite/gcc.dg/Wunreachable-6.c	(revision 0)
+++ testsuite/gcc.dg/Wunreachable-6.c	(revision 0)
@@ -0,0 +1,21 @@ 
+/* PR c/11370  */
+/* { dg-do compile } */
+/* { dg-options "-Wunreachable-code" } */
+
+extern int printf (const char *, ...);
+extern void exit (int);
+
+int main(int argc, char *argv[])
+{
+  if (argc != 1)
+    exit(1);
+
+  {
+    int ix;  /* { dg-bogus "will never be executed" } */
+    ix = printf("hello\n");
+    printf("%d\n", ix);
+  }
+
+  return 0;
+}
+
Index: testsuite/gcc.dg/Wunreachable-10.c
===================================================================
--- testsuite/gcc.dg/Wunreachable-10.c	(revision 0)
+++ testsuite/gcc.dg/Wunreachable-10.c	(revision 0)
@@ -0,0 +1,14 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Wunreachable-code" } */
+
+int i;
+int main(void)
+{
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunreachable-code"
+  if (0) {
+    i = 0;
+  }
+#pragma GCC diagnostic pop
+  return 0;
+}
Index: testsuite/gcc.dg/Wunreachable-3.c
===================================================================
--- testsuite/gcc.dg/Wunreachable-3.c	(revision 0)
+++ testsuite/gcc.dg/Wunreachable-3.c	(revision 0)
@@ -0,0 +1,17 @@ 
+/* PR c/10175 */
+/* { dg-do compile } */
+/* { dg-options "-Wunreachable-code" } */
+
+int i,j;
+int main(void)
+{
+  if (0) {
+    i = 0;		   /* { dg-warning "will never be executed" "" } */
+    j = 0;
+  } else {
+    i = 1;
+    j = 1;
+  }
+
+  return 0;
+}
Index: testsuite/gcc.dg/Wunreachable-7.c
===================================================================
--- testsuite/gcc.dg/Wunreachable-7.c	(revision 0)
+++ testsuite/gcc.dg/Wunreachable-7.c	(revision 0)
@@ -0,0 +1,20 @@ 
+/* PR c/11370  */
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wunreachable-code" } */
+
+extern int printf (const char *, ...);
+extern void exit (int);
+
+int main(int argc, char *argv[])
+{
+  if (argc != 1)
+    exit(1);
+
+  {
+    int ix;  /* { dg-bogus "will never be executed" } */
+    ix = printf("hello\n");
+    printf("%d\n", ix);
+  }
+
+  return 0;
+}
Index: testsuite/gcc.dg/Wunreachable-4.c
===================================================================
--- testsuite/gcc.dg/Wunreachable-4.c	(revision 0)
+++ testsuite/gcc.dg/Wunreachable-4.c	(revision 0)
@@ -0,0 +1,12 @@ 
+/* PR middle-end/10336 */
+/* { dg-options "-Wunreachable-code" } */
+
+void foo(int i)
+{
+  switch(i) {
+    case 0:
+      break;
+    case 1:
+      break;
+  }
+}
Index: testsuite/gcc.dg/Wunreachable-8.c
===================================================================
--- testsuite/gcc.dg/Wunreachable-8.c	(revision 0)
+++ testsuite/gcc.dg/Wunreachable-8.c	(revision 0)
@@ -0,0 +1,21 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wunreachable-code" } */
+float Factorial(float X)
+{
+  float val = 1.0;
+  int k,j;
+  for (k=1; k < 5; k++) /* { dg-bogus "will never be executed" "" {
+xfail *-*-* } } */
+    {
+      val += 1.0; /* { dg-bogus "will never be executed" "" { xfail
+*-*-* } } */
+    }
+  return (val); /* { dg-bogus "will never be executed" } */
+}
+
+int main (void)
+{
+  float result;
+  result=Factorial(2.1);
+  return (0);
+}
Index: common.opt
===================================================================
--- common.opt	(revision 181788)
+++ common.opt	(working copy)
@@ -659,8 +659,8 @@  Common Var(warn_maybe_uninitialized) War
 Warn about maybe uninitialized automatic variables
 
 Wunreachable-code
-Common Ignore
-Does nothing. Preserved for backward compatibility.
+Common Var(warn_notreached) Warning
+Warn about code that will never be executed
 
 Wunused
 Common Var(warn_unused) Init(0) Warning
Index: tree-cfg.c
===================================================================
--- tree-cfg.c	(revision 181788)
+++ tree-cfg.c	(working copy)
@@ -1836,6 +1836,7 @@  static void
 remove_bb (basic_block bb)
 {
   gimple_stmt_iterator i;
+  source_location loc = UNKNOWN_LOCATION;
 
   if (dump_file)
     {
@@ -1905,9 +1906,29 @@  remove_bb (basic_block bb)
 	    i = gsi_last_bb (bb);
 	  else
 	    gsi_prev (&i);
+
+          /* Store first location to warn for unreachable code. */
+          if (gimple_has_location (stmt))
+            {
+              loc = gimple_location (stmt);
+            }
 	}
     }
 
+  /* If requested, give a warning that the first statement in the
+     block is unreachable.  We walk statements backwards in the
+     loop above, so the last statement we process is the first statement
+     in the block.  */
+  if (warn_notreached &&
+      (loc > BUILTINS_LOCATION) && (LOCATION_LINE (loc) > 0))
+    {
+      /* Only warn if running cfg pass, if code is removed due
+         to eg. inline or loop unroll passes, dont give warning
+         since then its probably a false warning. */
+      if (current_pass->tv_id == TV_TREE_CFG)
+        warning_at (loc, OPT_Wunreachable_code, "will never be executed");
+    }
+
   remove_phi_nodes_and_edges_for_unreachable_block (bb);
   bb->il.gimple = NULL;
 }