Bug 93786 - [11/12/13/14 Regression] gimplifier ICE with statement expression since r8-5526
Summary: [11/12/13/14 Regression] gimplifier ICE with statement expression since r8-5526
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 9.2.1
: P2 normal
Target Milestone: 11.5
Assignee: Not yet assigned to anyone
URL:
Keywords: ice-on-valid-code, stmt-expr
Depends on:
Blocks:
 
Reported: 2020-02-17 16:39 UTC by Jakub Jelinek
Modified: 2024-02-21 04:12 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2024-02-20 00:00:00


Attachments
gcc10-pr93786.patch (781 bytes, patch)
2020-03-27 16:18 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelinek 2020-02-17 16:39:57 UTC
struct S {virtual void v();};
void f(S * s) {
  ({ s; })->v();
}
ICEs with -O -g since r8-5526-gcb6332333854a698cbfc2ea4b1e52ce9bd59f664:
gimplification failed:
 <statement_list 0x7feef52fe820
    type <void_type 0x7feef51aef18 void VOID
        align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7feef51aef18
        pointer_to_this <pointer_type 0x7feef51b60a8>>
    head (nil) tail (nil) stmts
>
rh1803909.C: In function ‘void f(S*)’:
rh1803909.C:3:14: internal compiler error: gimplification failed
   ({ s; })->v();
   ~~~~~~~~~~~^~
0xe9db35 gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*), int)
	../../gcc/gimplify.c:12393
0xe9bffb gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*), int)
	../../gcc/gimplify.c:11845
0xe771ed gimplify_call_expr
	../../gcc/gimplify.c:3308

Doesn't ICE without -g.
Comment 1 Jakub Jelinek 2020-02-17 20:57:29 UTC
The change makes the STATEMENT_LIST non-TREE_SIDE_EFFECTS, so that it doesn't cause -fcompare-debug failures.  Unfortunately, for the OBJ_TYPE_REF the tree is used twice, once in OBJ_TYPE_REF's second operand and once as the first argument of the call; because of the missing TREE_SIDE_EFFECTs save_expr isn't called on it (that would cause -fcompare-debug too), unshare_body doesn't unshare STATEMENT_EXPRs and as gimplification is destructive, this means that when we gimplify it once, we voidify it and destroy and the next time we gimplify it we ICE because it has void type.
I'm afraid I don't really have a good solution here.
Comment 2 Richard Biener 2020-02-18 09:26:24 UTC
Hmm, but this destructive modification of something we don't unshare doesn't work.  Either we have to always wrap statement lists but then we're back to
the compare-debug issue or we somehow fix the destructiveness on the
stmt list node itself - like not setting it's type to void or marking it
as having side-effects (why do we do this anyways, in voidify_wrapper_expr?
is that for debugging?), we'd also need to leave the final value in the
stmt list for a possible second gimplification round...

Maybe the best thing is to kill off the debug stmts when they would cause
a stmt list to appear and revert the previous fix.
Comment 3 Jakub Jelinek 2020-02-18 09:33:28 UTC
(In reply to Richard Biener from comment #2)
> Hmm, but this destructive modification of something we don't unshare doesn't
> work.  Either we have to always wrap statement lists but then we're back to
> the compare-debug issue or we somehow fix the destructiveness on the
> stmt list node itself - like not setting it's type to void or marking it
> as having side-effects (why do we do this anyways, in voidify_wrapper_expr?
> is that for debugging?), we'd also need to leave the final value in the
> stmt list for a possible second gimplification round...
> 
> Maybe the best thing is to kill off the debug stmts when they would cause
> a stmt list to appear and revert the previous fix.

We might kill them only in one of the two places (say keep in the argument and remove from the OBJ_TYPE_REF operand or vice versa), but we'd need some function that walks the non-TREE_SIDE_EFFECTS tree and looks for such STATEMENT_LISTs, as they might be wrapped in casts/whatever else that doesn't have side-effects.
Another possibility would be do that in copy_if_shared_r or so (only handle the !TREE_SIDE_EFFECTS STATEMENT_EXPR that contain only DEBUG_STMTs + one non-debug one), but we wouldn't have control over which of the two or more copies gets to keep the STATEMENT_LIST and which don't (well, it would be whatever we walk_tree first).
Comment 4 rguenther@suse.de 2020-02-18 09:46:25 UTC
On Tue, 18 Feb 2020, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93786
> 
> --- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #2)
> > Hmm, but this destructive modification of something we don't unshare doesn't
> > work.  Either we have to always wrap statement lists but then we're back to
> > the compare-debug issue or we somehow fix the destructiveness on the
> > stmt list node itself - like not setting it's type to void or marking it
> > as having side-effects (why do we do this anyways, in voidify_wrapper_expr?
> > is that for debugging?), we'd also need to leave the final value in the
> > stmt list for a possible second gimplification round...
> > 
> > Maybe the best thing is to kill off the debug stmts when they would cause
> > a stmt list to appear and revert the previous fix.
> 
> We might kill them only in one of the two places (say keep in the argument and
> remove from the OBJ_TYPE_REF operand or vice versa), but we'd need some
> function that walks the non-TREE_SIDE_EFFECTS tree and looks for such
> STATEMENT_LISTs, as they might be wrapped in casts/whatever else that doesn't
> have side-effects.
> Another possibility would be do that in copy_if_shared_r or so (only handle the
> !TREE_SIDE_EFFECTS STATEMENT_EXPR that contain only DEBUG_STMTs + one non-debug
> one), but we wouldn't have control over which of the two or more copies gets to
> keep the STATEMENT_LIST and which don't (well, it would be whatever we
> walk_tree first).

Could we make those sepcifc DEBUG_STMTS appear only later (during
gimplification maybe?) and represent them as expression flags in the
FEs?  That would avoid the stmt-list wrapping dependent on -g
Comment 5 Jakub Jelinek 2020-02-18 09:50:33 UTC
(In reply to rguenther@suse.de from comment #4)
> Could we make those sepcifc DEBUG_STMTS appear only later (during
> gimplification maybe?) and represent them as expression flags in the
> FEs?  That would avoid the stmt-list wrapping dependent on -g

A question for Alex I guess.
Comment 6 Jakub Jelinek 2020-02-25 12:53:10 UTC
Tried:
--- gcc/gimplify.c.jj	2020-02-09 08:16:19.399581468 +0100
+++ gcc/gimplify.c	2020-02-25 13:46:51.166409528 +0100
@@ -886,7 +886,29 @@ mostly_copy_tree_r (tree *tp, int *walk_
 
   /* Cope with the statement expression extension.  */
   else if (code == STATEMENT_LIST)
-    ;
+    {
+      if (!TREE_SIDE_EFFECTS (t))
+	{
+	  tree nt = NULL_TREE;
+          tree_stmt_iterator i;
+	  /* With -g we can get STATEMENT_LISTs without side-effects
+	     that contain some debug stmts and exactly one other stmt.
+	     "unshare" those by extracting the single other stmt in the
+	     copy.  */
+	  for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))
+	    if (TREE_CODE (tsi_stmt (i)) == DEBUG_BEGIN_STMT)
+	      ;
+	    else if (nt)
+	      break;
+	    else
+	      nt = tsi_stmt (i);
+	  if (nt && tsi_end_p (i) && TREE_TYPE (t) == TREE_TYPE (nt))
+	    {
+	      *tp = nt;
+	      mostly_copy_tree_r (tp, walk_subtrees, data);
+	    }
+	}
+    }
 
   /* Leave the bulk of the work to copy_tree_r itself.  */
   else
and while we don't ICE anymore on the testcase, it FAILs with -fcompare-debug:
-  _1 = s->_vptr.S;
+  struct S * retval.0;
+
+  # DEBUG BEGIN_STMT
+  # DEBUG BEGIN_STMT
+  retval.0 = s;
+  _1 = retval.0->_vptr.S;
gimple difference.
Comment 7 Jakub Jelinek 2020-03-04 09:38:31 UTC
GCC 8.4.0 has been released, adjusting target milestone.
Comment 8 Jakub Jelinek 2020-03-27 16:18:38 UTC
Created attachment 48133 [details]
gcc10-pr93786.patch

Untested fix.
Comment 9 Jakub Jelinek 2020-04-21 10:28:55 UTC
Tried now also:
--- gcc/gimplify.c.jj	2020-04-19 12:10:35.700627184 +0200
+++ gcc/gimplify.c	2020-04-21 12:24:41.444307978 +0200
@@ -886,7 +886,11 @@ mostly_copy_tree_r (tree *tp, int *walk_
 
   /* Cope with the statement expression extension.  */
   else if (code == STATEMENT_LIST)
-    ;
+    {
+      if (!TREE_SIDE_EFFECTS (t))
+	if (tree nt = expr_single (t))
+	  *tp = nt;
+    }
 
   /* Leave the bulk of the work to copy_tree_r itself.  */
   else
--- gcc/testsuite/g++.dg/other/pr94340.C.jj	2020-04-21 12:15:35.855765641 +0200
+++ gcc/testsuite/g++.dg/other/pr94340.C	2020-04-21 12:15:35.855765641 +0200
@@ -0,0 +1,16 @@
+// PR debug/94340
+// { dg-do compile }
+// { dg-options "-O -fcompare-debug" }
+
+struct D { int i; D (); ~D (); };
+D foo ();
+
+void
+bar (void)
+{
+  if (
+      ({
+         foo ();
+       }).i)
+    return;
+}
--- gcc/testsuite/g++.dg/other/pr93786.C.jj	2020-04-21 12:15:35.855765641 +0200
+++ gcc/testsuite/g++.dg/other/pr93786.C	2020-04-21 12:15:35.855765641 +0200
@@ -0,0 +1,11 @@
+// PR middle-end/93786
+// { dg-do compile }
+// { dg-options "-O2 -fcompare-debug" }
+
+struct S { virtual void bar (); };
+
+void
+foo (S *s)
+{
+  ({ s; })->bar ();
+}

and while it doesn't ICE, it fails -fcompare-debug on both tests.
So, I'm afraid I have no further ideas what to do here, everything I've tried didn't work.
Alex, do you think you could have a look at this for GCC11?
Thanks.
Comment 10 Jakub Jelinek 2021-05-14 09:52:54 UTC
GCC 8 branch is being closed.
Comment 11 Richard Biener 2021-06-01 08:16:29 UTC
GCC 9.4 is being released, retargeting bugs to GCC 9.5.
Comment 12 Richard Biener 2022-05-27 09:42:10 UTC
GCC 9 branch is being closed
Comment 13 Jakub Jelinek 2022-06-28 10:39:48 UTC
GCC 10.4 is being released, retargeting bugs to GCC 10.5.
Comment 14 Richard Biener 2023-07-07 10:36:50 UTC
GCC 10 branch is being closed.