diff mbox

Handle C++ x ? y : throw 1 COND_EXPRs in shortcut_cond_r (PR c++/49165)

Message ID 20110526103241.GU17079@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek May 26, 2011, 10:32 a.m. UTC
Hi!

The C++ FE creates COND_EXPRs which have non-void type
and the same type on one of the arms, but the other arm
is THROW_EXPR (which has void type).
gimplify_cond_expr knows how to gimplify this, but shortcut_cond_r
tried to optimize
if (a ? b : throw 1) goto yes; else goto no;
into
if (a)
  if (b) goto yes; else goto no;
else
  if (throw 1) goto yes; else goto no;
which ICEs or errors out.  Fixed by not trying to optimize
such COND_EXPRs.  Bootstrapped/regtested on x86_64-linux and
i686-linux, committed to trunk/4.6 so far.

2011-05-26  Jakub Jelinek  <jakub@redhat.com>

	PR c++/49165
	* gimplify.c (shortcut_cond_r): Don't special case
	COND_EXPRs if they have void type on one of their arms.

	* g++.dg/eh/cond5.C: New test.


	Jakub

Comments

Jason Merrill May 26, 2011, 1:26 p.m. UTC | #1
On 05/26/2011 06:32 AM, Jakub Jelinek wrote:
> gimplify_cond_expr knows how to gimplify this, but shortcut_cond_r
> tried to optimize
> if (a ? b : throw 1) goto yes; else goto no;
> into
> if (a)
>    if (b) goto yes; else goto no;
> else
>    if (throw 1) goto yes; else goto no;
> which ICEs or errors out.  Fixed by not trying to optimize
> such COND_EXPRs.

Why not optimize them to

if (!a)
   throw 1;
if (b) goto yes; else goto no;

?

Jason
Jakub Jelinek May 26, 2011, 1:41 p.m. UTC | #2
On Thu, May 26, 2011 at 09:26:58AM -0400, Jason Merrill wrote:
> On 05/26/2011 06:32 AM, Jakub Jelinek wrote:
> >gimplify_cond_expr knows how to gimplify this, but shortcut_cond_r
> >tried to optimize
> >if (a ? b : throw 1) goto yes; else goto no;
> >into
> >if (a)
> >   if (b) goto yes; else goto no;
> >else
> >   if (throw 1) goto yes; else goto no;
> >which ICEs or errors out.  Fixed by not trying to optimize
> >such COND_EXPRs.
> 
> Why not optimize them to
> 
> if (!a)
>   throw 1;
> if (b) goto yes; else goto no;
> 
> ?

That is how it ends up being optimized later on, I just think
given how long the bug has been in this is so rare that
we don't need to try to optimize it already at the gimplifier level.

	Jakub
Jason Merrill May 26, 2011, 1:41 p.m. UTC | #3
On 05/26/2011 09:41 AM, Jakub Jelinek wrote:
> That is how it ends up being optimized later on, I just think
> given how long the bug has been in this is so rare that
> we don't need to try to optimize it already at the gimplifier level.

Makes sense.

Jason
diff mbox

Patch

--- gcc/gimplify.c.jj	2011-05-20 08:14:08.000000000 +0200
+++ gcc/gimplify.c	2011-05-26 09:12:54.000000000 +0200
@@ -2573,7 +2573,9 @@  shortcut_cond_r (tree pred, tree *true_l
 			   new_locus);
       append_to_statement_list (t, &expr);
     }
-  else if (TREE_CODE (pred) == COND_EXPR)
+  else if (TREE_CODE (pred) == COND_EXPR
+	   && !VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (pred, 1)))
+	   && !VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (pred, 2))))
     {
       location_t new_locus;
 
@@ -2581,7 +2583,10 @@  shortcut_cond_r (tree pred, tree *true_l
 	 if (a)
 	   if (b) goto yes; else goto no;
 	 else
-	   if (c) goto yes; else goto no;  */
+	   if (c) goto yes; else goto no;
+
+	 Don't do this if one of the arms has void type, which can happen
+	 in C++ when the arm is throw.  */
 
       /* Keep the original source location on the first 'if'.  Set the source
 	 location of the ? on the second 'if'.  */
--- gcc/testsuite/g++.dg/eh/cond5.C.jj	2011-05-26 09:15:24.000000000 +0200
+++ gcc/testsuite/g++.dg/eh/cond5.C	2011-05-26 09:07:16.000000000 +0200
@@ -0,0 +1,43 @@ 
+// PR c++/49165
+// { dg-do run }
+
+extern "C" void abort ();
+
+int
+foo (bool x, int y)
+{
+  if (y < 10 && (x ? true : throw 1))
+    y++;
+  if (y > 20 || (x ? true : throw 2))
+    y++;
+  return y;
+}
+
+int
+main ()
+{
+  if (foo (true, 0) != 2
+      || foo (true, 10) != 11
+      || foo (false, 30) != 31)
+    abort ();
+  try
+    {
+      foo (false, 0);
+      abort ();
+    }
+  catch (int i)
+    {
+      if (i != 1)
+	abort ();
+    }
+  try
+    {
+      foo (false, 10);
+      abort ();
+    }
+  catch (int i)
+    {
+      if (i != 2)
+	abort ();
+    }
+}