Bug 95857 - [8/9/10 Regression] Silencing an unused label warning with (void)&&label; can make gcc segfault
Summary: [8/9/10 Regression] Silencing an unused label warning with (void)&&label; can...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 8.4.0
: P2 normal
Target Milestone: 8.5
Assignee: Jakub Jelinek
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2020-06-24 07:17 UTC by Petr Skocik
Modified: 2020-09-17 17:52 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work: 7.4.0
Known to fail:
Last reconfirmed: 2020-06-29 00:00:00


Attachments
preprocessed reproducer that crashes gcc >= 8.1 at -O2/-O3/-Os (243 bytes, text/plain)
2020-06-24 07:17 UTC, Petr Skocik
Details
gcc11-pr95857.patch (1.54 KB, patch)
2020-06-30 11:05 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Petr Skocik 2020-06-24 07:17:26 UTC
Created attachment 48777 [details]
preprocessed reproducer that crashes gcc >= 8.1 at -O2/-O3/-Os

In certain more complex contexts and with optimization on (>= -O2), silencing -Wunused-label warnings  with
(void)&&label; will make gcc segfault.

The attached example ( https://gcc.godbolt.org/z/iEhgL2 ) obtained with creduce  crashes gcc >= 8.1 when compiled at -O2/-O3/-Os.

I haven't observed the bug in older versions of gcc.
Comment 1 Jakub Jelinek 2020-06-29 10:04:37 UTC
Started with r8-546-gca4d28516878755a01ab8c2ba48d083100aba3fb
Comment 2 Jakub Jelinek 2020-06-29 19:40:37 UTC
Slightly simplified:
struct E { int e; };
int bar (void), baz (void);

void
foo (void)
{
  struct E a = { 0 };
  struct E i = { 0 };
  if (baz ())
    i.e = 1;
  else
    a.e = -2;
  switch (a.e)
    {
    case -2:
    lab1:
      switch (i.e)
	{
	case -3:
	case 2:
	  if (i.e-- != 2)
	    __builtin_unreachable ();
	  (void) &&lab2;
	lab2:
	  baz ();
	  goto lab1;
	case 0:
	  bar ();
	}
    }
}
Comment 3 Jakub Jelinek 2020-06-30 11:05:55 UTC
Created attachment 48813 [details]
gcc11-pr95857.patch

Untested fix.  The problem is that while normal labels on bb removal are removed and their label_to_block becomes NULL, that is not what is done with forced/non-local labels, those are instead moved to some neightbouring block,
and if some later case label refers to them, they will see that random other block; now if they are kept around because of this rather than thrown away, that means we might not have an edge to them etc.
Comment 4 GCC Commits 2020-07-02 09:39:39 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:00f24f56732861d09a9716fa5b6b8a96c2289143

commit r11-1783-g00f24f56732861d09a9716fa5b6b8a96c2289143
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Jul 2 11:38:20 2020 +0200

    tree-cfg: Fix ICE with switch stmt to unreachable opt and forced labels [PR95857]
    
    The following testcase ICEs, because during the cfg cleanup, we see:
      switch (i$e_11) <default: <L12> [33.33%], case -3: <lab2> [33.33%], case 0: <L10> [33.33%], case 2: <lab2> [33.33%]>
    ...
    lab2:
      __builtin_unreachable ();
    where lab2 is FORCED_LABEL.  The way it works, we go through the case labels
    and when we reach the first one that points to gimple_seq_unreachable*
    basic block, we remove the edge (if any) from the switch bb to the bb
    containing the label and bbs reachable only through that edge we've just
    removed.  Once we do that, we must throw away all other cases that use
    the same label (or some other labels from the same bb we've removed the edge
    to and the bb).  To avoid quadratic behavior, this is not done by walking
    all remaining cases immediately before removing, but only when processing
    them later.
    For normal labels this works, fine, if the label is in a deleted bb, it will
    have NULL label_to_block and we handle that case, or, if the unreachable bb
    has some other edge to it, only the edge will be removed and not the bb,
    and again, find_edge will not find the edge and we only remove the case.
    And if a label would be to some other block, that other block wouldn't have
    been removed earlier because there would be still an edge from the switch
    block.
    Now, FORCED_LABEL (and I think DECL_NONLOCAL too) break this, because
    those labels aren't removed, but instead moved to some surrounding basic
    block.  So, when we later process those, when their gimple_seq_unreachable*
    basic block is removed, label_to_block will return some unrelated block
    (in the testcase the switch bb), so we decide to keep the case which doesn't
    seem to be unreachable, but we don't really have an edge from the switch
    block to the block the label got moved to.
    
    I thought first about punting in gimple_seq_unreachable* on
    FORCED_LABEL/DECL_NONLOCAL labels, but that might penalize even code that
    doesn't care, so this instead just makes sure that for
    FORCED_LABEL/DECL_NONLOCAL labels that are being removed (and thus moved
    randomly) we remember in a hash_set the fact that those labels should be
    treated as removed for the purpose of the optimization, and later on
    handle those labels that way.
    
    2020-07-02  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/95857
            * tree-cfg.c (group_case_labels_stmt): When removing an unreachable
            base_bb, remember all forced and non-local labels on it and later
            treat those as if they have NULL label_to_block.  Formatting fix.
            Fix a comment typo.
    
            * gcc.dg/pr95857.c: New test.
Comment 5 Jakub Jelinek 2020-07-02 09:40:24 UTC
Fixed on the trunk so far.
Comment 6 GCC Commits 2020-07-12 06:49:25 UTC
The releases/gcc-10 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:1ba192b5b7d2509b833f288000f21d6294420ace

commit r10-8466-g1ba192b5b7d2509b833f288000f21d6294420ace
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Jul 2 11:38:20 2020 +0200

    tree-cfg: Fix ICE with switch stmt to unreachable opt and forced labels [PR95857]
    
    The following testcase ICEs, because during the cfg cleanup, we see:
      switch (i$e_11) <default: <L12> [33.33%], case -3: <lab2> [33.33%], case 0: <L10> [33.33%], case 2: <lab2> [33.33%]>
    ...
    lab2:
      __builtin_unreachable ();
    where lab2 is FORCED_LABEL.  The way it works, we go through the case labels
    and when we reach the first one that points to gimple_seq_unreachable*
    basic block, we remove the edge (if any) from the switch bb to the bb
    containing the label and bbs reachable only through that edge we've just
    removed.  Once we do that, we must throw away all other cases that use
    the same label (or some other labels from the same bb we've removed the edge
    to and the bb).  To avoid quadratic behavior, this is not done by walking
    all remaining cases immediately before removing, but only when processing
    them later.
    For normal labels this works, fine, if the label is in a deleted bb, it will
    have NULL label_to_block and we handle that case, or, if the unreachable bb
    has some other edge to it, only the edge will be removed and not the bb,
    and again, find_edge will not find the edge and we only remove the case.
    And if a label would be to some other block, that other block wouldn't have
    been removed earlier because there would be still an edge from the switch
    block.
    Now, FORCED_LABEL (and I think DECL_NONLOCAL too) break this, because
    those labels aren't removed, but instead moved to some surrounding basic
    block.  So, when we later process those, when their gimple_seq_unreachable*
    basic block is removed, label_to_block will return some unrelated block
    (in the testcase the switch bb), so we decide to keep the case which doesn't
    seem to be unreachable, but we don't really have an edge from the switch
    block to the block the label got moved to.
    
    I thought first about punting in gimple_seq_unreachable* on
    FORCED_LABEL/DECL_NONLOCAL labels, but that might penalize even code that
    doesn't care, so this instead just makes sure that for
    FORCED_LABEL/DECL_NONLOCAL labels that are being removed (and thus moved
    randomly) we remember in a hash_set the fact that those labels should be
    treated as removed for the purpose of the optimization, and later on
    handle those labels that way.
    
    2020-07-02  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/95857
            * tree-cfg.c (group_case_labels_stmt): When removing an unreachable
            base_bb, remember all forced and non-local labels on it and later
            treat those as if they have NULL label_to_block.  Formatting fix.
            Fix a comment typo.
    
            * gcc.dg/pr95857.c: New test.
    
    (cherry picked from commit 00f24f56732861d09a9716fa5b6b8a96c2289143)
Comment 7 GCC Commits 2020-09-16 19:22:33 UTC
The releases/gcc-9 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:62714a106493d0f1f22ff98c2dff2204f09cfcc0

commit r9-8903-g62714a106493d0f1f22ff98c2dff2204f09cfcc0
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Jul 2 11:38:20 2020 +0200

    tree-cfg: Fix ICE with switch stmt to unreachable opt and forced labels [PR95857]
    
    The following testcase ICEs, because during the cfg cleanup, we see:
      switch (i$e_11) <default: <L12> [33.33%], case -3: <lab2> [33.33%], case 0: <L10> [33.33%], case 2: <lab2> [33.33%]>
    ...
    lab2:
      __builtin_unreachable ();
    where lab2 is FORCED_LABEL.  The way it works, we go through the case labels
    and when we reach the first one that points to gimple_seq_unreachable*
    basic block, we remove the edge (if any) from the switch bb to the bb
    containing the label and bbs reachable only through that edge we've just
    removed.  Once we do that, we must throw away all other cases that use
    the same label (or some other labels from the same bb we've removed the edge
    to and the bb).  To avoid quadratic behavior, this is not done by walking
    all remaining cases immediately before removing, but only when processing
    them later.
    For normal labels this works, fine, if the label is in a deleted bb, it will
    have NULL label_to_block and we handle that case, or, if the unreachable bb
    has some other edge to it, only the edge will be removed and not the bb,
    and again, find_edge will not find the edge and we only remove the case.
    And if a label would be to some other block, that other block wouldn't have
    been removed earlier because there would be still an edge from the switch
    block.
    Now, FORCED_LABEL (and I think DECL_NONLOCAL too) break this, because
    those labels aren't removed, but instead moved to some surrounding basic
    block.  So, when we later process those, when their gimple_seq_unreachable*
    basic block is removed, label_to_block will return some unrelated block
    (in the testcase the switch bb), so we decide to keep the case which doesn't
    seem to be unreachable, but we don't really have an edge from the switch
    block to the block the label got moved to.
    
    I thought first about punting in gimple_seq_unreachable* on
    FORCED_LABEL/DECL_NONLOCAL labels, but that might penalize even code that
    doesn't care, so this instead just makes sure that for
    FORCED_LABEL/DECL_NONLOCAL labels that are being removed (and thus moved
    randomly) we remember in a hash_set the fact that those labels should be
    treated as removed for the purpose of the optimization, and later on
    handle those labels that way.
    
    2020-07-02  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/95857
            * tree-cfg.c (group_case_labels_stmt): When removing an unreachable
            base_bb, remember all forced and non-local labels on it and later
            treat those as if they have NULL label_to_block.  Formatting fix.
            Fix a comment typo.
    
            * gcc.dg/pr95857.c: New test.
    
    (cherry picked from commit 00f24f56732861d09a9716fa5b6b8a96c2289143)
Comment 8 Jakub Jelinek 2020-09-17 17:52:40 UTC
Fixed for 8.5 in r8-10508-gcac9ff3a809f90236dc737a51eb8ff0e9088783c and by the above commit for 9.4+ too.