[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476 --- Comment #30 from Andrew Pinski --- *** Bug 106456 has been marked as a duplicate of this bug. ***
[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476 --- Comment #29 from Richard Biener --- Created attachment 51895 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51895=edit -Wunreachable-code-ctrl at GIMPLE lowering time This is the -Wunreachable-code-ctrl (not enabled by -Wextra) patch diagnosing unreachable stmts after a break, continue, goto (or loops without exit via the backedge goto). Note that unlike clang which seems to model the option names after what kind of stmt is detected as unreachable these patches model the option names after what kind of stmt makes other stmts unreachable. Not sure what is more useful in practice [to avoid coding-style issues].
[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476 Richard Biener changed: What|Removed |Added Attachment #51878|0 |1 is obsolete|| --- Comment #28 from Richard Biener --- Created attachment 51894 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51894=edit -Wunreachable-code-return at GIMPLE lowering time Updated patch
[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476 Bug 46476 depends on bug 103439, which changed state. Bug 103439 Summary: genemit emits dead code https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103439 What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |WONTFIX
[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476 --- Comment #27 from Richard Biener --- (In reply to Richard Biener from comment #25) > Created attachment 51878 [details] > -Wunreachable-code-return at GIMPLE lowering time ... > At least this patch passes bootstrap and would have found one real issue > but not the problematic dead "looping" stmts. I was mistaken. The patch runs into some of the same stray return/gcc_unreachable stmts as the other patch, even some more.
[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476
--- Comment #26 from Richard Biener ---
diff --git a/gcc/gimple-low.c b/gcc/gimple-low.c
index 18e66450977..dc56e14b605 100644
--- a/gcc/gimple-low.c
+++ b/gcc/gimple-low.c
@@ -60,7 +60,7 @@ typedef struct return_statements_t return_statements_t;
/* Helper tracking the reason a previous stmt cannot fallthru. */
struct cft_reason
{
- enum reason { CAN_FALLTHRU = false, UNKNOWN = true, RETURN };
+ enum reason { CAN_FALLTHRU = false, UNKNOWN = true, RETURN, CTRL };
cft_reason () : m_reason (CAN_FALLTHRU) {}
cft_reason (bool b) : m_reason (b ? UNKNOWN : CAN_FALLTHRU) {}
cft_reason (reason r) : m_reason (r) {}
@@ -272,6 +304,12 @@ lower_stmt (gimple_stmt_iterator *gsi, struct lower_data
*data)
warning_at (gimple_location (stmt), OPT_Wunreachable_code_return,
"statement after return is not reachable");
+ if (data->cannot_fallthru.m_reason == cft_reason::CTRL
+ && gimple_code (stmt) != GIMPLE_LABEL
+ && LOCATION_LOCUS (gimple_location (stmt)) > BUILTINS_LOCATION)
+warning_at (gimple_location (stmt), OPT_Wunreachable_code,
+ "statement after control statement is not reachable");
+
switch (gimple_code (stmt))
{
case GIMPLE_BIND:
@@ -282,7 +320,7 @@ lower_stmt (gimple_stmt_iterator *gsi, struct lower_data
*data)
case GIMPLE_COND:
case GIMPLE_GOTO:
case GIMPLE_SWITCH:
- data->cannot_fallthru = true;
+ data->cannot_fallthru = cft_reason::CTRL;
gsi_next (gsi);
return;
would then warn about things like the following (via GIMPLE_GOTO handling),
also stmts after continue.
void baz();
void foo (int b)
{
switch (b)
{
case 1:
break;
baz ();
}
}
Looks like there's no GIMPLE stmt for throw but we have calls to __cxa_throw
so we can handle noreturn & throw here covering all throwing but not
fall thru stmts or we can match the exact ABI function being called.
As said the main issue will be premature IL eliding.
[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476
--- Comment #25 from Richard Biener ---
Created attachment 51878
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51878=edit
-Wunreachable-code-return at GIMPLE lowering time
This is an alternative change only implementing -Wunreachable-code-return
(-Wunreachable-code-throw should be doable within this framework as well).
It does so during GIMPLE lowering where we still have return stmts using
the can_fallthru logic already present.
The approach has the same issues with premature optimization by the C++
frontend
eliding if (0) and if (1) as shown during bootstrap so the relevant hunk is
included here, too, likewise the double return in main().
It also warns for
void baz();
void foo (int b)
{
if (b)
__builtin_abort ();
else
return;
baz ();
}
but not for
void baz();
void foo (int b)
{
if (b)
return;
else
__builtin_abort ();
baz ();
}
as the previous stmt here is not the return but the abort() but in both
cases baz () is not really "after return" but after an if, but that part
of the IL structure is not easily visible. For the same reason
implementing -Wunreachable-code-break as supported by clang is difficult
(break is just a goto in GIMPLE).
At least this patch passes bootstrap and would have found one real issue
but not the problematic dead "looping" stmts.
[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476 Thomas Schwinge changed: What|Removed |Added See Also||http://gcc.gnu.org/bugzilla ||/show_bug.cgi?id=50847 --- Comment #24 from Thomas Schwinge --- See PR50847 re dead code after C++ 'throw'.
[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476 --- Comment #23 from Richard Biener --- Created attachment 51877 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51877=edit some fallout in GCC This fixes some fallout appearant when bootstrapping with the patch, mostly style, so not pushed to trunk. There are more unresolved -Werror cases because of the change so I'm not sure that warning at CFG construction is good enough for a narrow scope warning. Instead the summary requested stmt after return could likely be implemented at parsing time without too much hassle (but obviously repeated in every frontend).
[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476 --- Comment #22 from Richard Biener --- Created attachment 51876 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51876=edit -Wunreachable-code at CFG construction time
[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476 Bug 46476 depends on bug 80701, which changed state. Bug 80701 Summary: Option for generating link symbol for functions removed by DCE https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80701 What|Removed |Added Status|NEW |RESOLVED Resolution|--- |DUPLICATE
[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476 Richard Biener changed: What|Removed |Added CC||gustavo.hime at mpimet dot mpg.de --- Comment #21 from Richard Biener --- *** Bug 80701 has been marked as a duplicate of this bug. ***
[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476
Thomas Schwinge changed:
What|Removed |Added
CC||tschwinge at gcc dot gnu.org
--- Comment #20 from Thomas Schwinge ---
(In reply to Richard Biener from comment #18)
> /home/rguenther/src/trunk/libgomp/oacc-plugin.c: In function
> 'GOMP_PLUGIN_acc_default_dim':
> /home/rguenther/src/trunk/libgomp/oacc-plugin.c:65:7: error: statement is
> not reachable [-Werror]
>65 | return -1;
> | ^~
(That's correct, and you do address that in the patch posted.)
It feels strange to not have a 'return' in a non-'void' function, but that's
fine, given 'gomp_fatal' being 'noreturn'.
For posterity (only; these "bad" cases have not made it into the patch posted):
> /home/rguenther/src/trunk/libgomp/oacc-profiling.c: In function
> 'acc_prof_register':
> /home/rguenther/src/trunk/libgomp/oacc-profiling.c:354:7: error: statement
> is not reachable [-Werror]
> 354 | __builtin_unreachable ();
> | ^
> /home/rguenther/src/trunk/libgomp/oacc-profiling.c: In function
> 'acc_prof_unregister':
> /home/rguenther/src/trunk/libgomp/oacc-profiling.c:475:7: error: statement
> is not reachable [-Werror]
> 475 | __builtin_unreachable ();
> | ^
>
> the latter two are an issue with inital CFG construction I think, where
> group_case_labels turns
>
> void bar (foo x)
> {
>:
> switch (x) , case 0: , case 1: >
>
>:
> :
> goto ;
>
>:
> :
> __builtin_unreachable ();
>
>:
> :
> return;
>
> into the following with BB 4 now unreachable.
>
> void bar (foo x)
> {
>:
> switch (x) , case 0: >
>
>:
> :
> goto ;
>
>:
> :
> __builtin_unreachable ();
>
>:
> :
> return;
The source-level situation here is:
[...]
256/* Special cases. */
257if (reg == acc_toggle)
[...]
274else if (reg == acc_toggle_per_thread)
275 {
[...]
284/* Silently ignore. */
285gomp_debug (0, " ignoring bogus request\n");
286return;
287 }
[...]
302switch (reg)
303 {
[...]
353 case acc_toggle_per_thread:
354__builtin_unreachable ();
355 }
[...]
..., and similar for the other instance.
Here, the point is to (a) enumerate all possible 'enum' values in the 'switch
(reg)', but (b) make it clear ('__builtin_unreachable') that we're not
expecting 'acc_toggle_per_thread' here, as it has already been handled (plus
early 'return') above. In my opinion, we shouldn't diagnose these cases (and
you don't, per the patch posted).
[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476
Richard Biener changed:
What|Removed |Added
Status|NEW |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |rguenth at gcc dot
gnu.org
URL||https://gcc.gnu.org/piperma
||il/gcc-patches/2021-Novembe
||r/585352.html
--- Comment #19 from Richard Biener ---
Posted RFC patch. The C++ frontend poses some more issues in GCC code for
statically true evaluated conditions like
if (! GATHER_STATISTICS)
{
fprintf (stderr, "No RTX statistics\n");
return;
}
where it elides the if (). It doesn't do that for if (0) though.
[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476
--- Comment #18 from Richard Biener ---
We can warn at CFG construction time. Note the ??? though, we'd want to improve
here to avoid duplicate diagnostics. Tricky cases:
/* Unreachable region entry has a predecessor (backedge). */
void foo()
{
return;
for (int i = 0; i < 5; ++i)
;
}
/* Unreachable region not backwards reachable from exit. */
void bar1()
{
return;
__builtin_abort ();
}
void bar2()
{
return ;
for (;;);
}
/* Unreachable code in if (0) block. */
void baz(int *p)
{
if (0)
{
return;
*p = 0;
}
}
bootstrap with the prototype currently fails in libgomp:
/home/rguenther/src/trunk/libgomp/oacc-plugin.c: In function
'GOMP_PLUGIN_acc_default_dim':
/home/rguenther/src/trunk/libgomp/oacc-plugin.c:65:7: error: statement is not
reachable [-Werror]
65 | return -1;
| ^~
/home/rguenther/src/trunk/libgomp/oacc-profiling.c: In function
'acc_prof_register':
/home/rguenther/src/trunk/libgomp/oacc-profiling.c:354:7: error: statement is
not reachable [-Werror]
354 | __builtin_unreachable ();
| ^
/home/rguenther/src/trunk/libgomp/oacc-profiling.c: In function
'acc_prof_unregister':
/home/rguenther/src/trunk/libgomp/oacc-profiling.c:475:7: error: statement is
not reachable [-Werror]
475 | __builtin_unreachable ();
| ^
the latter two are an issue with inital CFG construction I think, where
group_case_labels turns
void bar (foo x)
{
:
switch (x) , case 0: , case 1: >
:
:
goto ;
:
:
__builtin_unreachable ();
:
:
return;
into the following with BB 4 now unreachable.
void bar (foo x)
{
:
switch (x) , case 0: >
:
:
goto ;
:
:
__builtin_unreachable ();
:
:
return;
The C++ FE also warns about the implicit return in main when there's a
preceeding one (the C frontend "appropriately" assigns an internal
location which supresses the warning).
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index b3a27bcd17c..64ab2607c56 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -242,7 +242,8 @@ build_gimple_cfg (gimple_seq seq)
/* Group case nodes to reduce the number of edges.
We do this after cleaning up dead labels because otherwise we miss
a lot of obvious case merging opportunities. */
- group_case_labels ();
+ /* ??? This interferes with unreachable code diagnostics. */
+ //group_case_labels ();
/* Create the edges of the flowgraph. */
discriminator_per_locus = new hash_table (13);
@@ -374,6 +375,24 @@ execute_build_cfg (void)
fprintf (dump_file, "Scope blocks:\n");
dump_scope_blocks (dump_file, dump_flags);
}
+
+ find_unreachable_blocks ();
+ basic_block bb;
+ FOR_EACH_BB_FN (bb, cfun)
+if (!(bb->flags & BB_REACHABLE))
+ for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
+ gsi_next ())
+ {
+ if ((LOCATION_LOCUS (gimple_location (gsi_stmt (gsi)))
+ > BUILTINS_LOCATION)
+ && !gimple_no_warning_p (gsi_stmt (gsi)))
+ warning_at (gimple_location (gsi_stmt (gsi)), 0,
+ "statement is not reachable");
+ /* ??? Mark blocks reachable from here. And even better make
+sure to process entries to unreachable regions first. */
+ break;
+ }
+
cleanup_tree_cfg ();
bb_to_omp_idx.release ();
[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476 Eric Gallager changed: What|Removed |Added Blocks||89863 --- Comment #17 from Eric Gallager --- (In reply to David Binderman from comment #16) > (In reply to Manuel López-Ibáñez from comment #14) > > But the main barrier for this is not technical or acceptance, it is > > leadership and human resources. > > And the usual time and money. There are plenty of static analysers out there. > Unless it is substantially better, why write another one ? > > My favourite static analyser, cppcheck, says this for the original code: > > $ ~/cppcheck/trunk/cppcheck --enable=all bug46476.cc > [bug46476.cc:5]: (style) Statements following return, break, continue, goto > or throw will never be executed. > [bug46476.cc:11]: (style) Statements following return, break, continue, goto > or throw will never be executed. > [bug46476.cc:8]: (style) The function 'bar' is never used. > [bug46476.cc:2]: (style) The function 'foo' is never used. > $ > > which pretty much does the job. > > Running the same static analyser over the source code of a recent gcc > found 22 occurrences of this kind of problem. > > Here they are: > > $ fgrep "Statements following" cppcheck.20170617.out > [trunk/gcc/c/c-decl.c:3211]: (style) Statements following return, break, > continue, goto or throw will never be executed. > [trunk/gcc/fortran/arith.c:2009]: (style) Statements following return, > break, continue, goto or throw will never be executed. > [trunk/libbacktrace/dwarf.c:2709]: (style) Statements following return, > break, continue, goto or throw will never be executed. > [trunk/libbacktrace/dwarf.c:2758]: (style) Statements following return, > break, continue, goto or throw will never be executed. > [trunk/libbacktrace/dwarf.c:2892]: (style) Statements following return, > break, continue, goto or throw will never be executed. > [trunk/libbacktrace/dwarf.c:3025]: (style) Statements following return, > break, continue, goto or throw will never be executed. > [trunk/libbacktrace/elf.c:448]: (style) Statements following return, break, > continue, goto or throw will never be executed. > [trunk/libbacktrace/elf.c:493]: (style) Statements following return, break, > continue, goto or throw will never be executed. > [trunk/libbacktrace/elf.c:967]: (style) Statements following return, break, > continue, goto or throw will never be executed. > [trunk/libbacktrace/fileline.c:64]: (style) Statements following return, > break, continue, goto or throw will never be executed. > [trunk/libbacktrace/fileline.c:75]: (style) Statements following return, > break, continue, goto or throw will never be executed. > [trunk/libbacktrace/pecoff.c:499]: (style) Statements following return, > break, continue, goto or throw will never be executed. > [trunk/libbacktrace/pecoff.c:564]: (style) Statements following return, > break, continue, goto or throw will never be executed. > [trunk/libbacktrace/pecoff.c:931]: (style) Statements following return, > break, continue, goto or throw will never be executed. > [trunk/libcilkrts/runtime/cilk_fiber.cpp:968]: (style) Statements following > return, break, continue, goto or throw will never be executed. > [trunk/libcilkrts/runtime/scheduler.c:2468]: (style) Statements following > return, break, continue, goto or throw will never be executed. > [trunk/libcilkrts/runtime/scheduler.c:2550]: (style) Statements following > return, break, continue, goto or throw will never be executed. > [trunk/libcilkrts/runtime/scheduler.c:2439]: (style) Statements following > return, break, continue, goto or throw will never be executed. > [trunk/libffi/src/dlmalloc.c:3877]: (style) Statements following return, > break, continue, goto or throw will never be executed. > [trunk/libgomp/error.c:90]: (style) Statements following return, break, > continue, goto or throw will never be executed. > [trunk/libgomp/libgomp-plugin.c:79]: (style) Statements following return, > break, continue, goto or throw will never be executed. > [trunk/libobjc/error.c:41]: (style) Statements following return, break, > continue, goto or throw will never be executed. > $ > > Most of them seem to be in libbacktrace. I could look deeper into these > and generate some bug reports. That's the usual way to provoke gcc developers > into developing a new warning: show that the gcc source code would benefit > from it. Dunno how I missed this when I created the new cppcheck meta-bug; adding this as a dependency for it now Referenced Bugs: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89863 [Bug 89863] [meta-bug] Issues that cppcheck finds that gcc misses
