public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c: Fix ICE with cast to VLA [93576]
@ 2020-02-13 17:57 Jakub Jelinek
  2020-02-13 19:35 ` Jeff Law
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2020-02-13 17:57 UTC (permalink / raw)
  To: Joseph S. Myers, Marek Polacek; +Cc: gcc-patches

Hi!

The following testcase ICEs, because the PR84305 changes try to evaluate
the size earlier.  If size has side-effects, that is desirable, and the
side-effects will actually be wrapped in a SAVE_EXPR.  The problem on this
testcase is that there are no side-effects, and c_fully_fold doesn't fold
those COMPOUND_EXPRs to constant, and while before gimplification we unshare
trees found in the expressions, the unsharing doesn't involve TYPE_SIZE etc.
of used types.  Gimplification is destructive though, so when we gimplify
the two nested COMPOUND_EXPRs and then try to gimplify it the second time
for the TYPE_SIZEs, we ICE.
Now, we could use unshare_expr in what we push to *expr, SAVE_EXPRs and
their operands in there aren't unshared, but I really don't see a point of
evaluating expressions that don't have side-effects before, so instead
this just pushes there expressions that do have side-effects.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
release branches?

2020-02-13  Jakub Jelinek  <jakub@redhat.com>

	PR c/93576
	* c-decl.c (grokdeclarator): If this_size_varies, only push size into
	*expr if it has side effects.

	* gcc.dg/pr93576.c: New test.

--- gcc/c/c-decl.c.jj	2020-01-15 12:34:15.079253187 +0100
+++ gcc/c/c-decl.c	2020-02-13 13:49:06.427304070 +0100
@@ -6512,11 +6512,14 @@ grokdeclarator (const struct c_declarato
 		  }
 		if (this_size_varies)
 		  {
-		    if (*expr)
-		      *expr = build2 (COMPOUND_EXPR, TREE_TYPE (size),
-				      *expr, size);
-		    else
-		      *expr = size;
+		    if (TREE_SIDE_EFFECTS (size))
+		      {
+			if (*expr)
+			  *expr = build2 (COMPOUND_EXPR, TREE_TYPE (size),
+					  *expr, size);
+			else
+			  *expr = size;
+		      }
 		    *expr_const_operands &= size_maybe_const;
 		  }
 	      }
--- gcc/testsuite/gcc.dg/pr93576.c.jj	2020-02-13 13:53:06.089708612 +0100
+++ gcc/testsuite/gcc.dg/pr93576.c	2020-02-13 13:52:40.041099237 +0100
@@ -0,0 +1,10 @@
+/* PR c/93576 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+void
+foo (void)
+{
+  int b[] = { 0 };
+  (char (*)[(1, 7, 2)]) 0;
+}

	Jakub

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] c: Fix ICE with cast to VLA [93576]
  2020-02-13 17:57 [PATCH] c: Fix ICE with cast to VLA [93576] Jakub Jelinek
@ 2020-02-13 19:35 ` Jeff Law
  0 siblings, 0 replies; 2+ messages in thread
From: Jeff Law @ 2020-02-13 19:35 UTC (permalink / raw)
  To: Jakub Jelinek, Joseph S. Myers, Marek Polacek; +Cc: gcc-patches

On Thu, 2020-02-13 at 18:57 +0100, Jakub Jelinek wrote:
> Hi!
> 
> The following testcase ICEs, because the PR84305 changes try to evaluate
> the size earlier.  If size has side-effects, that is desirable, and the
> side-effects will actually be wrapped in a SAVE_EXPR.  The problem on this
> testcase is that there are no side-effects, and c_fully_fold doesn't fold
> those COMPOUND_EXPRs to constant, and while before gimplification we unshare
> trees found in the expressions, the unsharing doesn't involve TYPE_SIZE etc.
> of used types.  Gimplification is destructive though, so when we gimplify
> the two nested COMPOUND_EXPRs and then try to gimplify it the second time
> for the TYPE_SIZEs, we ICE.
> Now, we could use unshare_expr in what we push to *expr, SAVE_EXPRs and
> their operands in there aren't unshared, but I really don't see a point of
> evaluating expressions that don't have side-effects before, so instead
> this just pushes there expressions that do have side-effects.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
> release branches?
> 
> 2020-02-13  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c/93576
> 	* c-decl.c (grokdeclarator): If this_size_varies, only push size into
> 	*expr if it has side effects.
> 
> 	* gcc.dg/pr93576.c: New test.
OK
jeff
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-02-13 19:35 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-13 17:57 [PATCH] c: Fix ICE with cast to VLA [93576] Jakub Jelinek
2020-02-13 19:35 ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).