From patchwork Mon Dec 5 22:55:09 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "sarah@hederstierna.com" X-Patchwork-Id: 129463 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 5DC6F1007D4 for ; Tue, 6 Dec 2011 10:00:20 +1100 (EST) Received: (qmail 3142 invoked by alias); 5 Dec 2011 23:00:16 -0000 Received: (qmail 3123 invoked by uid 22791); 5 Dec 2011 23:00:13 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, TW_CF, TW_TM X-Spam-Check-By: sourceware.org Received: from hubext03.fsdata.se (HELO smtp04.fsdata.se) (195.35.82.72) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 05 Dec 2011 22:59:58 +0000 Received: from MBXVS01.HMC.local ([192.168.46.111]) by hubext03.HMC.local ([192.168.46.132]) with mapi; Mon, 5 Dec 2011 23:59:56 +0100 From: "sarah@hederstierna.com" To: "gcc-patches@gcc.gnu.org" Date: Mon, 5 Dec 2011 23:55:09 +0100 Subject: Warning for unreachable code patch Message-ID: MIME-Version: 1.0 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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 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; }