diff mbox series

C++ PATCH for c++/88983 - ICE with switch in constexpr function

Message ID 20190201000154.GJ26714@redhat.com
State New
Headers show
Series C++ PATCH for c++/88983 - ICE with switch in constexpr function | expand

Commit Message

Marek Polacek Feb. 1, 2019, 12:01 a.m. UTC
Here we have switch (1) with body

  if (1)
    {
      case 1:;
      return <retval> = 1;
    }
  else
    {
      default:;
    }

so we're looking for "case 1:", which we found in the then branch while
evaluating the COND_EXPR, so *jump_target is cleared.  But it's followed
by a jumping statement so *jump_target is set to a RETURN_EXPR, which as
the new comment describes, confuses the code in <case COND_EXPR>.

label_matches then crashes if it gets a RETURN_EXPR as a jump_target.

I've expanded the original testcase so as to cover break/continue after
the label.

I imagine this could also be fixed differently -- don't call label_matches
in cxx_eval_constant_expression if jump_target isn't a label/switch case.

Bootstrapped/regtested on x86_64-linux, ok for trunk?  I'd like to commit
to 8 too.

2019-01-31  Marek Polacek  <polacek@redhat.com>

	PR c++/88983 - ICE with switch in constexpr function.
	* constexpr.c (cxx_eval_switch_expr): Use SWITCH_COND and SWITCH_BODY.
	(cxx_eval_constant_expression) <case COND_EXPR>: Don't look for the
	label in the else branch if we found it in the then branch.

	* g++.dg/cpp1y/constexpr-88983.C: New test.

Comments

Jason Merrill Feb. 1, 2019, 12:11 a.m. UTC | #1
On 1/31/19 7:01 PM, Marek Polacek wrote:
> +	  if (*jump_target && *jump_target == orig_jump)

You shouldn't need to check that it's non-null, since the enclosing if 
established that orig_jump will be non-null.  OK with that tweak.

Jason
Marek Polacek Feb. 1, 2019, 12:29 a.m. UTC | #2
On Thu, Jan 31, 2019 at 07:11:59PM -0500, Jason Merrill wrote:
> On 1/31/19 7:01 PM, Marek Polacek wrote:
> > +	  if (*jump_target && *jump_target == orig_jump)
> 
> You shouldn't need to check that it's non-null, since the enclosing if
> established that orig_jump will be non-null.  OK with that tweak.

Duh, of course.  Committed with that fixed.

Marek
diff mbox series

Patch

diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 19eb44fc0c0..fc12912676d 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -4140,13 +4140,13 @@  cxx_eval_switch_expr (const constexpr_ctx *ctx, tree t,
 		      bool *non_constant_p, bool *overflow_p,
 		      tree *jump_target)
 {
-  tree cond = TREE_OPERAND (t, 0);
+  tree cond = SWITCH_COND (t);
   cond = cxx_eval_constant_expression (ctx, cond, false,
 				       non_constant_p, overflow_p);
   VERIFY_CONSTANT (cond);
   *jump_target = cond;
 
-  tree body = TREE_OPERAND (t, 1);
+  tree body = SWITCH_BODY (t);
   constexpr_ctx new_ctx = *ctx;
   constexpr_switch_state css = css_default_not_seen;
   new_ctx.css_state = &css;
@@ -4681,6 +4681,7 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
     case COND_EXPR:
       if (jump_target && *jump_target)
 	{
+	  tree orig_jump = *jump_target;
 	  /* When jumping to a label, the label might be either in the
 	     then or else blocks, so process then block first in skipping
 	     mode first, and if we are still in the skipping mode at its end,
@@ -4688,7 +4689,19 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	  r = cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 1),
 					    lval, non_constant_p, overflow_p,
 					    jump_target);
-	  if (*jump_target)
+	  /* It's possible that we found the label in the then block.  But
+	     it could have been followed by another jumping statement, e.g.
+	     say we're looking for case 1:
+	      if (cond)
+		{
+		  // skipped statements
+		  case 1:; // clears up *jump_target
+		  return 1; // and sets it to a RETURN_EXPR
+		}
+	      else { ... }
+	     in which case we need not go looking to the else block.
+	     (goto is not allowed in a constexpr function.)  */
+	  if (*jump_target && *jump_target == orig_jump)
 	    r = cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 2),
 					      lval, non_constant_p, overflow_p,
 					      jump_target);
diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-88983.C gcc/testsuite/g++.dg/cpp1y/constexpr-88983.C
new file mode 100644
index 00000000000..9d70601d427
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-88983.C
@@ -0,0 +1,71 @@ 
+// PR c++/88983
+// { dg-do compile { target c++14 } }
+
+constexpr int
+fn1 (int ay)
+{
+  switch (ay)
+    {
+      if (1)
+        {
+          case 1:
+            return 1;
+        }
+      else
+        {
+          default:;
+        }
+    }
+
+  return 0;
+}
+
+constexpr int
+fn2 (int ay)
+{
+  switch (ay)
+    {
+      if (1)
+        {
+          case 1:
+	    break;
+        }
+      else
+        {
+          default:;
+        }
+    }
+
+  return 0;
+}
+
+constexpr int
+fn3 (int ay)
+{
+  int i = 0;
+  while (i++ < 100)
+    {
+      if (i == 1)
+	return 1;
+      switch (ay)
+	{
+	  if (1)
+	    {
+	      case 1:
+		continue;
+	    }
+	  else
+	    {
+	      default:;
+	      return -1;
+	    }
+	}
+      return -1;
+    }
+
+  return -1;
+}
+
+static_assert (fn1 (1) == 1, "");
+static_assert (fn2 (1) == 0, "");
+static_assert (fn3 (1) == 1, "");