[Bug c++/46476] New: Missing Warning about unreachable code after return

[Bug c++/46476] New: Missing Warning about unreachable code after return

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

           Summary: Missing Warning about unreachable code after return
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: paolo.carlini@oracle.com


It seems impossible to me that this issue has not been discussed already, but
in the library I fixed only two weeks ago a *very* stupid bug which went
unnoticed for *many* months exactly because of this. Couldn't believe that GCC
didn't warn even at -Wall -Wextra (and -O2). Thus:

int foo(int& num)
{
  return num;
  ++num;      // Warn!
}

int bar(int num)
{
  return num;
  ++num;      // Warn!
  return num;
}

For the record, ICC *does* warn, with -Wall.

[Bug c++/46476] Missing Warning about unreachable code after return

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2010.11.14 17:38:28
     Ever Confirmed|0                           |1

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2010-11-14 17:38:28 UTC ---
Confirmed.

[Bug c++/46476] Missing Warning about unreachable code after return

[manu at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |manu at gcc dot gnu.org

--- Comment #2 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-05-12 12:35:20 UTC ---
(In reply to comment #1)
> Confirmed.

Hi Richard, how do you think this could be implemented? Wasn't
-Wunreachable-code removed just a few releases ago?

[Bug c++/46476] Missing Warning about unreachable code after return

[stefan at schweter dot it]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

Stefan <stefan at schweter dot it> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |stefan at schweter dot it

--- Comment #3 from Stefan <stefan at schweter dot it> ---
As the *-Wunreachable-code* was removed, why isn't there any *deprecated*
message or *warning* shown to the user, when using this option? That leads to
the illusion *-Wunreachable-code* is "working" - which is obviously not the
case.

[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

--- Comment #18 from Richard Biener <rguenth at gcc dot gnu.org> ---
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)
{
  <bb 2> :
  switch (x) <default: <L2>, case 0: <L0>, case 1: <L1>>

  <bb 3> :
<L0>:
  goto <L2>;

  <bb 4> :
<L1>:
  __builtin_unreachable ();

  <bb 5> :
<L2>:
  return;

into the following with BB 4 now unreachable.

void bar (foo x)
{
  <bb 2> :
  switch (x) <default: <L2>, case 0: <L0>>

  <bb 3> :
<L0>:
  goto <L2>;

  <bb 4> :
<L1>:
  __builtin_unreachable ();

  <bb 5> :
<L2>:
  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<locus_discrim_hasher> (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 (&gsi))
+       {
+         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]

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

Richard Biener <rguenth at gcc dot gnu.org> 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 <rguenth at gcc dot gnu.org> ---
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]

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

Thomas Schwinge <tschwinge at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tschwinge at gcc dot gnu.org

--- Comment #20 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
(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)
> {
>   <bb 2> :
>   switch (x) <default: <L2>, case 0: <L0>, case 1: <L1>>
> 
>   <bb 3> :
> <L0>:
>   goto <L2>;
> 
>   <bb 4> :
> <L1>:
>   __builtin_unreachable ();
> 
>   <bb 5> :
> <L2>:
>   return;
> 
> into the following with BB 4 now unreachable.
> 
> void bar (foo x)
> {
>   <bb 2> :
>   switch (x) <default: <L2>, case 0: <L0>>
> 
>   <bb 3> :
> <L0>:
>   goto <L2>;
> 
>   <bb 4> :
> <L1>:
>   __builtin_unreachable ();
> 
>   <bb 5> :
> <L2>:
>   return;

The source-level situation here is:

    [...]
       256        /* Special cases.  */
       257        if (reg == acc_toggle)
    [...]
       274        else if (reg == acc_toggle_per_thread)
       275          {
    [...]
       284            /* Silently ignore.  */
       285            gomp_debug (0, "  ignoring bogus request\n");
       286            return;
       287          }
    [...]
       302        switch (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]

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gustavo.hime at mpimet dot mpg.de

--- Comment #21 from Richard Biener <rguenth at gcc dot gnu.org> ---
*** Bug 80701 has been marked as a duplicate of this bug. ***

[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]

[rguenth at gcc dot gnu.org]

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]

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

--- Comment #22 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 51876
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51876&action=edit
-Wunreachable-code at CFG construction time

[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

--- Comment #23 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 51877
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51877&action=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]

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

Thomas Schwinge <tschwinge at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |http://gcc.gnu.org/bugzilla
                   |                            |/show_bug.cgi?id=50847

--- Comment #24 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
See PR50847 re dead code after C++ 'throw'.

[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

--- Comment #25 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 51878
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51878&action=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]

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

--- Comment #26 from Richard Biener <rguenth at gcc dot gnu.org> ---
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]

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

--- Comment #27 from Richard Biener <rguenth at gcc dot gnu.org> ---
(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]

[rguenth at gcc dot gnu.org]

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]

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #51878|0                           |1
        is obsolete|                            |

--- Comment #28 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 51894
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51894&action=edit
-Wunreachable-code-return at GIMPLE lowering time

Updated patch

[Bug c++/46476] Missing Warning about unreachable code after return [-Wunreachable-code-return]

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

--- Comment #29 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 51895
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51895&action=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]

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46476

--- Comment #30 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 106456 has been marked as a duplicate of this bug. ***