* [C++ PATCH] Fix ICE with return in statement expression in constexpr.c (PR c++/84192)
@ 2018-02-16 9:49 Jakub Jelinek
2018-02-16 13:52 ` Jason Merrill
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2018-02-16 9:49 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
Hi!
pop_stmt_list, if there is just a single stmt inside statement expression
moves the stmt out of the STATEMENT_LIST (and I think cp_fold does too).
We only initialize jump_target to non-NULL in cxx_eval_statement_list
or for calls, so before we have a chance to diagnose the error of using
an expression with void type, we ICE trying to dereference NULL jump_target.
This can't happen with BREAK_STMT nor CONTINUE_STMT, because they are not
potential constant expressions, and I think can only happen when ctx->quiet
is true, otherwise it should have been diagnosed already before.
If a RETURN_EXPR (or in theory break/continue) appears in a (potential) constant
expression we want to evaluate, not doing anything with jump_target if we
aren't inside a statement list makes sense to me, there is no following
statement to bypass.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2018-02-16 Marek Polacek <polacek@redhat.com>
Jakub Jelinek <jakub@redhat.com>
PR c++/84192
* constexpr.c (cxx_eval_constant_expression) <case RETURN_EXPR>: Don't
set *jump_target to anything if jump_target is NULL.
* g++.dg/cpp1y/constexpr-84192.C: New test.
--- gcc/cp/constexpr.c.jj 2018-02-12 19:17:37.937216029 +0100
+++ gcc/cp/constexpr.c 2018-02-15 16:10:56.630572360 +0100
@@ -4254,7 +4254,13 @@ cxx_eval_constant_expression (const cons
r = cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0),
lval,
non_constant_p, overflow_p);
- *jump_target = t;
+ if (jump_target)
+ *jump_target = t;
+ else
+ /* Can happen with ({ return true; }) && false; passed to
+ maybe_constant_value. There is nothing to jump over in this
+ case, and the bug will be diagnosed later. */
+ gcc_assert (ctx->quiet);
break;
case SAVE_EXPR:
--- gcc/testsuite/g++.dg/cpp1y/constexpr-84192.C.jj 2018-02-15 16:00:58.242588914 +0100
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-84192.C 2018-02-15 16:01:30.219585291 +0100
@@ -0,0 +1,41 @@
+// PR c++/84192
+// { dg-do compile { target c++14 } }
+// { dg-options "" }
+
+bool
+f1 ()
+{
+ return ({ return true; }) && false; // { dg-error "could not convert" }
+}
+
+void
+f2 ()
+{
+ for (;;)
+ constexpr bool b = ({ break; false; }) && false; // { dg-error "statement is not a constant expression" }
+}
+
+constexpr bool
+f3 (int n)
+{
+ bool b = false;
+ for (int i = 0; i < n; i++)
+ b = ({ break; }); // { dg-error "void value not ignored as it ought to be" }
+ return b;
+}
+
+constexpr bool b = f3 (4);
+
+bool
+f4 ()
+{
+ constexpr bool b = ({ return true; }) && false; // { dg-error "could not convert" }
+ return false;
+}
+
+constexpr bool
+f5 (int x)
+{
+ constexpr bool b = ({ switch (x) case 0: true; }) && false; // { dg-error "could not convert" }
+ return false;
+}
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [C++ PATCH] Fix ICE with return in statement expression in constexpr.c (PR c++/84192)
2018-02-16 9:49 [C++ PATCH] Fix ICE with return in statement expression in constexpr.c (PR c++/84192) Jakub Jelinek
@ 2018-02-16 13:52 ` Jason Merrill
2018-02-16 18:49 ` Jakub Jelinek
0 siblings, 1 reply; 4+ messages in thread
From: Jason Merrill @ 2018-02-16 13:52 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches List
On Fri, Feb 16, 2018 at 3:28 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> pop_stmt_list, if there is just a single stmt inside statement expression
> moves the stmt out of the STATEMENT_LIST (and I think cp_fold does too).
> We only initialize jump_target to non-NULL in cxx_eval_statement_list
> or for calls, so before we have a chance to diagnose the error of using
> an expression with void type, we ICE trying to dereference NULL jump_target.
>
> This can't happen with BREAK_STMT nor CONTINUE_STMT, because they are not
> potential constant expressions, and I think can only happen when ctx->quiet
> is true, otherwise it should have been diagnosed already before.
> If a RETURN_EXPR (or in theory break/continue) appears in a (potential) constant
> expression we want to evaluate, not doing anything with jump_target if we
> aren't inside a statement list makes sense to me, there is no following
> statement to bypass.
I think we should also set *non_constant_p.
Jason
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [C++ PATCH] Fix ICE with return in statement expression in constexpr.c (PR c++/84192)
2018-02-16 13:52 ` Jason Merrill
@ 2018-02-16 18:49 ` Jakub Jelinek
2018-02-16 15:20 ` Jason Merrill
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2018-02-16 18:49 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches List
On Fri, Feb 16, 2018 at 08:52:10AM -0500, Jason Merrill wrote:
> On Fri, Feb 16, 2018 at 3:28 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> > pop_stmt_list, if there is just a single stmt inside statement expression
> > moves the stmt out of the STATEMENT_LIST (and I think cp_fold does too).
> > We only initialize jump_target to non-NULL in cxx_eval_statement_list
> > or for calls, so before we have a chance to diagnose the error of using
> > an expression with void type, we ICE trying to dereference NULL jump_target.
> >
> > This can't happen with BREAK_STMT nor CONTINUE_STMT, because they are not
> > potential constant expressions, and I think can only happen when ctx->quiet
> > is true, otherwise it should have been diagnosed already before.
> > If a RETURN_EXPR (or in theory break/continue) appears in a (potential) constant
> > expression we want to evaluate, not doing anything with jump_target if we
> > aren't inside a statement list makes sense to me, there is no following
> > statement to bypass.
>
> I think we should also set *non_constant_p.
Just like this? Tested so far just on the testcase, but given that we'd ICE
on the *jump_target before, it can't really regress anything else (though of
course I'll bootstrap/regtest it normally).
2018-02-16 Marek Polacek <polacek@redhat.com>
Jakub Jelinek <jakub@redhat.com>
PR c++/84192
* constexpr.c (cxx_eval_constant_expression) <case RETURN_EXPR>: Don't
set *jump_target to anything if jump_target is NULL.
* g++.dg/cpp1y/constexpr-84192.C: New test.
--- gcc/cp/constexpr.c.jj 2018-02-12 19:17:37.937216029 +0100
+++ gcc/cp/constexpr.c 2018-02-15 16:10:56.630572360 +0100
@@ -4254,7 +4254,16 @@ cxx_eval_constant_expression (const cons
r = cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0),
lval,
non_constant_p, overflow_p);
- *jump_target = t;
+ if (jump_target)
+ *jump_target = t;
+ else
+ {
+ /* Can happen with ({ return true; }) && false; passed to
+ maybe_constant_value. There is nothing to jump over in this
+ case, and the bug will be diagnosed later. */
+ gcc_assert (ctx->quiet);
+ *non_constant_p = true;
+ }
break;
case SAVE_EXPR:
--- gcc/testsuite/g++.dg/cpp1y/constexpr-84192.C.jj 2018-02-15 16:00:58.242588914 +0100
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-84192.C 2018-02-15 16:01:30.219585291 +0100
@@ -0,0 +1,41 @@
+// PR c++/84192
+// { dg-do compile { target c++14 } }
+// { dg-options "" }
+
+bool
+f1 ()
+{
+ return ({ return true; }) && false; // { dg-error "could not convert" }
+}
+
+void
+f2 ()
+{
+ for (;;)
+ constexpr bool b = ({ break; false; }) && false; // { dg-error "statement is not a constant expression" }
+}
+
+constexpr bool
+f3 (int n)
+{
+ bool b = false;
+ for (int i = 0; i < n; i++)
+ b = ({ break; }); // { dg-error "void value not ignored as it ought to be" }
+ return b;
+}
+
+constexpr bool b = f3 (4);
+
+bool
+f4 ()
+{
+ constexpr bool b = ({ return true; }) && false; // { dg-error "could not convert" }
+ return false;
+}
+
+constexpr bool
+f5 (int x)
+{
+ constexpr bool b = ({ switch (x) case 0: true; }) && false; // { dg-error "could not convert" }
+ return false;
+}
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [C++ PATCH] Fix ICE with return in statement expression in constexpr.c (PR c++/84192)
2018-02-16 18:49 ` Jakub Jelinek
@ 2018-02-16 15:20 ` Jason Merrill
0 siblings, 0 replies; 4+ messages in thread
From: Jason Merrill @ 2018-02-16 15:20 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches List
OK.
On Fri, Feb 16, 2018 at 10:19 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Feb 16, 2018 at 08:52:10AM -0500, Jason Merrill wrote:
>> On Fri, Feb 16, 2018 at 3:28 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > pop_stmt_list, if there is just a single stmt inside statement expression
>> > moves the stmt out of the STATEMENT_LIST (and I think cp_fold does too).
>> > We only initialize jump_target to non-NULL in cxx_eval_statement_list
>> > or for calls, so before we have a chance to diagnose the error of using
>> > an expression with void type, we ICE trying to dereference NULL jump_target.
>> >
>> > This can't happen with BREAK_STMT nor CONTINUE_STMT, because they are not
>> > potential constant expressions, and I think can only happen when ctx->quiet
>> > is true, otherwise it should have been diagnosed already before.
>> > If a RETURN_EXPR (or in theory break/continue) appears in a (potential) constant
>> > expression we want to evaluate, not doing anything with jump_target if we
>> > aren't inside a statement list makes sense to me, there is no following
>> > statement to bypass.
>>
>> I think we should also set *non_constant_p.
>
> Just like this? Tested so far just on the testcase, but given that we'd ICE
> on the *jump_target before, it can't really regress anything else (though of
> course I'll bootstrap/regtest it normally).
>
> 2018-02-16 Marek Polacek <polacek@redhat.com>
> Jakub Jelinek <jakub@redhat.com>
>
> PR c++/84192
> * constexpr.c (cxx_eval_constant_expression) <case RETURN_EXPR>: Don't
> set *jump_target to anything if jump_target is NULL.
>
> * g++.dg/cpp1y/constexpr-84192.C: New test.
>
> --- gcc/cp/constexpr.c.jj 2018-02-12 19:17:37.937216029 +0100
> +++ gcc/cp/constexpr.c 2018-02-15 16:10:56.630572360 +0100
> @@ -4254,7 +4254,16 @@ cxx_eval_constant_expression (const cons
> r = cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0),
> lval,
> non_constant_p, overflow_p);
> - *jump_target = t;
> + if (jump_target)
> + *jump_target = t;
> + else
> + {
> + /* Can happen with ({ return true; }) && false; passed to
> + maybe_constant_value. There is nothing to jump over in this
> + case, and the bug will be diagnosed later. */
> + gcc_assert (ctx->quiet);
> + *non_constant_p = true;
> + }
> break;
>
> case SAVE_EXPR:
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-84192.C.jj 2018-02-15 16:00:58.242588914 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-84192.C 2018-02-15 16:01:30.219585291 +0100
> @@ -0,0 +1,41 @@
> +// PR c++/84192
> +// { dg-do compile { target c++14 } }
> +// { dg-options "" }
> +
> +bool
> +f1 ()
> +{
> + return ({ return true; }) && false; // { dg-error "could not convert" }
> +}
> +
> +void
> +f2 ()
> +{
> + for (;;)
> + constexpr bool b = ({ break; false; }) && false; // { dg-error "statement is not a constant expression" }
> +}
> +
> +constexpr bool
> +f3 (int n)
> +{
> + bool b = false;
> + for (int i = 0; i < n; i++)
> + b = ({ break; }); // { dg-error "void value not ignored as it ought to be" }
> + return b;
> +}
> +
> +constexpr bool b = f3 (4);
> +
> +bool
> +f4 ()
> +{
> + constexpr bool b = ({ return true; }) && false; // { dg-error "could not convert" }
> + return false;
> +}
> +
> +constexpr bool
> +f5 (int x)
> +{
> + constexpr bool b = ({ switch (x) case 0: true; }) && false; // { dg-error "could not convert" }
> + return false;
> +}
>
>
> Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-02-16 18:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-16 9:49 [C++ PATCH] Fix ICE with return in statement expression in constexpr.c (PR c++/84192) Jakub Jelinek
2018-02-16 13:52 ` Jason Merrill
2018-02-16 18:49 ` Jakub Jelinek
2018-02-16 15:20 ` Jason Merrill
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).