* [PATCH] c++: Fix ICE with non-constant satisfaction [PR98644] @ 2021-01-13 17:05 Patrick Palka 2021-01-19 20:40 ` Jason Merrill 0 siblings, 1 reply; 6+ messages in thread From: Patrick Palka @ 2021-01-13 17:05 UTC (permalink / raw) To: gcc-patches In the below testcase, the expression of the atomic constraint after substitution is (int *) NON_LVALUE_EXPR <1> != 0B which is not a C++ constant expression, but its TREE_CONSTANT flag is set (from build2), so satisfy_atom fails to notice that it's non-constant (and we end up tripping over the assert in satisfaction_value). Since TREE_CONSTANT doesn't necessarily correspond to C++ constantness, this patch makes satisfy_atom instead check is_rvalue_constant_expression. Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk/10? gcc/cp/ChangeLog: PR c++/98644 * constraint.cc (satisfy_atom): Check is_rvalue_constant_expression instead of TREE_CONSTANT. gcc/testsuite/ChangeLog: PR c++/98644 * g++.dg/cpp2a/concepts-pr98644.C: New test. --- gcc/cp/constraint.cc | 2 +- gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index 9049d087859..f99a25dc8a4 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -2969,7 +2969,7 @@ satisfy_atom (tree t, tree args, sat_info info) { result = maybe_constant_value (result, NULL_TREE, /*manifestly_const_eval=*/true); - if (!TREE_CONSTANT (result)) + if (!is_rvalue_constant_expression (result)) result = error_mark_node; } result = satisfaction_value (result); diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C new file mode 100644 index 00000000000..6772f72a3ce --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C @@ -0,0 +1,7 @@ +// PR c++/98644 +// { dg-do compile { target c++20 } } + +template<class T> concept Signed = bool(T(1)); // { dg-error "reinterpret_cast" } +static_assert(Signed<int*>); // { dg-error "non-constant" } + +constexpr bool B = requires { requires bool((char *)1); }; // { dg-error "reinterpret_cast" } -- 2.30.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Fix ICE with non-constant satisfaction [PR98644] 2021-01-13 17:05 [PATCH] c++: Fix ICE with non-constant satisfaction [PR98644] Patrick Palka @ 2021-01-19 20:40 ` Jason Merrill 2022-03-01 4:10 ` Patrick Palka 0 siblings, 1 reply; 6+ messages in thread From: Jason Merrill @ 2021-01-19 20:40 UTC (permalink / raw) To: Patrick Palka, gcc-patches On 1/13/21 12:05 PM, Patrick Palka wrote: > In the below testcase, the expression of the atomic constraint after > substitution is (int *) NON_LVALUE_EXPR <1> != 0B which is not a C++ > constant expression, but its TREE_CONSTANT flag is set (from build2), > so satisfy_atom fails to notice that it's non-constant (and we end > up tripping over the assert in satisfaction_value). > > Since TREE_CONSTANT doesn't necessarily correspond to C++ constantness, > this patch makes satisfy_atom instead check is_rvalue_constant_expression. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk/10? > > gcc/cp/ChangeLog: > > PR c++/98644 > * constraint.cc (satisfy_atom): Check is_rvalue_constant_expression > instead of TREE_CONSTANT. > > gcc/testsuite/ChangeLog: > > PR c++/98644 > * g++.dg/cpp2a/concepts-pr98644.C: New test. > --- > gcc/cp/constraint.cc | 2 +- > gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C | 7 +++++++ > 2 files changed, 8 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > index 9049d087859..f99a25dc8a4 100644 > --- a/gcc/cp/constraint.cc > +++ b/gcc/cp/constraint.cc > @@ -2969,7 +2969,7 @@ satisfy_atom (tree t, tree args, sat_info info) > { > result = maybe_constant_value (result, NULL_TREE, > /*manifestly_const_eval=*/true); > - if (!TREE_CONSTANT (result)) This should be sufficient. If the result isn't constant, maybe_constant_value shouldn't return it with TREE_CONSTANT set. See > /* This isn't actually constant, so unset TREE_CONSTANT. in cxx_eval_outermost_constant_expr. Jason ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Fix ICE with non-constant satisfaction [PR98644] 2021-01-19 20:40 ` Jason Merrill @ 2022-03-01 4:10 ` Patrick Palka 2022-03-10 20:05 ` Jason Merrill 0 siblings, 1 reply; 6+ messages in thread From: Patrick Palka @ 2022-03-01 4:10 UTC (permalink / raw) To: Jason Merrill; +Cc: Patrick Palka, gcc-patches On Tue, 19 Jan 2021, Jason Merrill wrote: > On 1/13/21 12:05 PM, Patrick Palka wrote: > > In the below testcase, the expression of the atomic constraint after > > substitution is (int *) NON_LVALUE_EXPR <1> != 0B which is not a C++ > > constant expression, but its TREE_CONSTANT flag is set (from build2), > > so satisfy_atom fails to notice that it's non-constant (and we end > > up tripping over the assert in satisfaction_value). > > > > Since TREE_CONSTANT doesn't necessarily correspond to C++ constantness, > > this patch makes satisfy_atom instead check is_rvalue_constant_expression. > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > > trunk/10? > > > > gcc/cp/ChangeLog: > > > > PR c++/98644 > > * constraint.cc (satisfy_atom): Check is_rvalue_constant_expression > > instead of TREE_CONSTANT. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/98644 > > * g++.dg/cpp2a/concepts-pr98644.C: New test. > > --- > > gcc/cp/constraint.cc | 2 +- > > gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C | 7 +++++++ > > 2 files changed, 8 insertions(+), 1 deletion(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > > index 9049d087859..f99a25dc8a4 100644 > > --- a/gcc/cp/constraint.cc > > +++ b/gcc/cp/constraint.cc > > @@ -2969,7 +2969,7 @@ satisfy_atom (tree t, tree args, sat_info info) > > { > > result = maybe_constant_value (result, NULL_TREE, > > /*manifestly_const_eval=*/true); > > - if (!TREE_CONSTANT (result)) > > This should be sufficient. If the result isn't constant, maybe_constant_value > shouldn't return it with TREE_CONSTANT set. See > > > /* This isn't actually constant, so unset TREE_CONSTANT. > > in cxx_eval_outermost_constant_expr. I see, so the problem seems to be that the fail-fast path of maybe_constant_value isn't clearing TREE_CONSTANT sufficiently. Would it make sense to fix this like so? -- >8 -- Subject: [PATCH] c++: ICE with non-constant satisfaction value [PR98644] Here during satisfaction the expression of the atomic constraint after substitution is (int *) NON_LVALUE_EXPR <1> != 0B, which is not a C++ constant expression due to the reinterpret_cast, but TREE_CONSTANT is set since its value is otherwise effectively constant. We then call maybe_constant_value on it, which proceeds via its fail-fast path to exit early without clearing TREE_CONSTANT. But satisfy_atom relies on checking TREE_CONSTANT of the result of maybe_constant_value in order to detect non-constant satisfaction. This patch fixes this by making the fail-fast path of maybe_constant_value clear TREE_CONSTANT in this case, like cxx_eval_outermost_constant_expr in the normal path would have done. Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk? PR c++/98644 gcc/cp/ChangeLog: * constexpr.cc (maybe_constant_value): In the fail-fast path, clear TREE_CONSTANT on the result if it's set on the input. gcc/testsuite/ChangeLog: * g++.dg/cpp2a/concepts-pr98644.C: New test. * g++.dg/parse/array-size2.C: Remove expected diagnostic about a narrowing conversion. --- gcc/cp/constexpr.cc | 4 +++- gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C | 7 +++++++ gcc/testsuite/g++.dg/parse/array-size2.C | 2 -- 3 files changed, 10 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index 4716694cb71..234cf0acc26 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -7965,8 +7965,10 @@ maybe_constant_value (tree t, tree decl, bool manifestly_const_eval) if (!is_nondependent_constant_expression (t)) { - if (TREE_OVERFLOW_P (t)) + if (TREE_OVERFLOW_P (t) + || (!processing_template_decl && TREE_CONSTANT (t))) { + /* This isn't actually constant, so unset TREE_CONSTANT. */ t = build_nop (TREE_TYPE (t), t); TREE_CONSTANT (t) = false; } diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C new file mode 100644 index 00000000000..6772f72a3ce --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C @@ -0,0 +1,7 @@ +// PR c++/98644 +// { dg-do compile { target c++20 } } + +template<class T> concept Signed = bool(T(1)); // { dg-error "reinterpret_cast" } +static_assert(Signed<int*>); // { dg-error "non-constant" } + +constexpr bool B = requires { requires bool((char *)1); }; // { dg-error "reinterpret_cast" } diff --git a/gcc/testsuite/g++.dg/parse/array-size2.C b/gcc/testsuite/g++.dg/parse/array-size2.C index c4a69df3b01..e58fe266e77 100644 --- a/gcc/testsuite/g++.dg/parse/array-size2.C +++ b/gcc/testsuite/g++.dg/parse/array-size2.C @@ -15,8 +15,6 @@ void foo (void) { char g[(char *) &((struct S *) 0)->b - (char *) 0]; // { dg-error "40:size of array .g. is not an integral constant-expression" } - // { dg-error "narrowing conversion" "" { target c++11 } .-1 } - // { dg-message "expression has a constant value but is not a C.. constant-expression" "" { target c++11 } .-2 } char h[(__SIZE_TYPE__) &((struct S *) 8)->b]; // { dg-error "10:size of array .h. is not an integral constant-expression" } bar (g, h); } -- 2.35.1.354.g715d08a9e5 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Fix ICE with non-constant satisfaction [PR98644] 2022-03-01 4:10 ` Patrick Palka @ 2022-03-10 20:05 ` Jason Merrill 2022-03-11 15:46 ` Patrick Palka 0 siblings, 1 reply; 6+ messages in thread From: Jason Merrill @ 2022-03-10 20:05 UTC (permalink / raw) To: Patrick Palka; +Cc: gcc-patches On 3/1/22 00:10, Patrick Palka wrote: > On Tue, 19 Jan 2021, Jason Merrill wrote: > >> On 1/13/21 12:05 PM, Patrick Palka wrote: >>> In the below testcase, the expression of the atomic constraint after >>> substitution is (int *) NON_LVALUE_EXPR <1> != 0B which is not a C++ >>> constant expression, but its TREE_CONSTANT flag is set (from build2), >>> so satisfy_atom fails to notice that it's non-constant (and we end >>> up tripping over the assert in satisfaction_value). >>> >>> Since TREE_CONSTANT doesn't necessarily correspond to C++ constantness, >>> this patch makes satisfy_atom instead check is_rvalue_constant_expression. >>> >>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for >>> trunk/10? >>> >>> gcc/cp/ChangeLog: >>> >>> PR c++/98644 >>> * constraint.cc (satisfy_atom): Check is_rvalue_constant_expression >>> instead of TREE_CONSTANT. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR c++/98644 >>> * g++.dg/cpp2a/concepts-pr98644.C: New test. >>> --- >>> gcc/cp/constraint.cc | 2 +- >>> gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C | 7 +++++++ >>> 2 files changed, 8 insertions(+), 1 deletion(-) >>> create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C >>> >>> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc >>> index 9049d087859..f99a25dc8a4 100644 >>> --- a/gcc/cp/constraint.cc >>> +++ b/gcc/cp/constraint.cc >>> @@ -2969,7 +2969,7 @@ satisfy_atom (tree t, tree args, sat_info info) >>> { >>> result = maybe_constant_value (result, NULL_TREE, >>> /*manifestly_const_eval=*/true); >>> - if (!TREE_CONSTANT (result)) >> >> This should be sufficient. If the result isn't constant, maybe_constant_value >> shouldn't return it with TREE_CONSTANT set. See >> >>> /* This isn't actually constant, so unset TREE_CONSTANT. >> >> in cxx_eval_outermost_constant_expr. > > I see, so the problem seems to be that the fail-fast path of > maybe_constant_value isn't clearing TREE_CONSTANT sufficiently. Would > it make sense to fix this like so? > > -- >8 -- > > Subject: [PATCH] c++: ICE with non-constant satisfaction value [PR98644] > > Here during satisfaction the expression of the atomic constraint after > substitution is (int *) NON_LVALUE_EXPR <1> != 0B, which is not a C++ > constant expression due to the reinterpret_cast, but TREE_CONSTANT is > set since its value is otherwise effectively constant. We then call > maybe_constant_value on it, which proceeds via its fail-fast path to > exit early without clearing TREE_CONSTANT. But satisfy_atom relies > on checking TREE_CONSTANT of the result of maybe_constant_value in order > to detect non-constant satisfaction. > > This patch fixes this by making the fail-fast path of maybe_constant_value > clear TREE_CONSTANT in this case, like cxx_eval_outermost_constant_expr > in the normal path would have done. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk? > > PR c++/98644 > > gcc/cp/ChangeLog: > > * constexpr.cc (maybe_constant_value): In the fail-fast path, > clear TREE_CONSTANT on the result if it's set on the input. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp2a/concepts-pr98644.C: New test. > * g++.dg/parse/array-size2.C: Remove expected diagnostic about a > narrowing conversion. > --- > gcc/cp/constexpr.cc | 4 +++- > gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C | 7 +++++++ > gcc/testsuite/g++.dg/parse/array-size2.C | 2 -- > 3 files changed, 10 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > index 4716694cb71..234cf0acc26 100644 > --- a/gcc/cp/constexpr.cc > +++ b/gcc/cp/constexpr.cc > @@ -7965,8 +7965,10 @@ maybe_constant_value (tree t, tree decl, bool manifestly_const_eval) > > if (!is_nondependent_constant_expression (t)) > { > - if (TREE_OVERFLOW_P (t)) > + if (TREE_OVERFLOW_P (t) > + || (!processing_template_decl && TREE_CONSTANT (t))) > { > + /* This isn't actually constant, so unset TREE_CONSTANT. */ > t = build_nop (TREE_TYPE (t), t); build_nop isn't appropriate for arbitrary expressions (classes, in particular). We probably want to factor out the code in cxx_eval_outermost_constant_expr under the "this isn't actually constant" comment. > TREE_CONSTANT (t) = false; > } > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C > new file mode 100644 > index 00000000000..6772f72a3ce > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C > @@ -0,0 +1,7 @@ > +// PR c++/98644 > +// { dg-do compile { target c++20 } } > + > +template<class T> concept Signed = bool(T(1)); // { dg-error "reinterpret_cast" } > +static_assert(Signed<int*>); // { dg-error "non-constant" } > + > +constexpr bool B = requires { requires bool((char *)1); }; // { dg-error "reinterpret_cast" } > diff --git a/gcc/testsuite/g++.dg/parse/array-size2.C b/gcc/testsuite/g++.dg/parse/array-size2.C > index c4a69df3b01..e58fe266e77 100644 > --- a/gcc/testsuite/g++.dg/parse/array-size2.C > +++ b/gcc/testsuite/g++.dg/parse/array-size2.C > @@ -15,8 +15,6 @@ void > foo (void) > { > char g[(char *) &((struct S *) 0)->b - (char *) 0]; // { dg-error "40:size of array .g. is not an integral constant-expression" } > - // { dg-error "narrowing conversion" "" { target c++11 } .-1 } > - // { dg-message "expression has a constant value but is not a C.. constant-expression" "" { target c++11 } .-2 } > char h[(__SIZE_TYPE__) &((struct S *) 8)->b]; // { dg-error "10:size of array .h. is not an integral constant-expression" } > bar (g, h); > } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Fix ICE with non-constant satisfaction [PR98644] 2022-03-10 20:05 ` Jason Merrill @ 2022-03-11 15:46 ` Patrick Palka 2022-03-11 22:16 ` Jason Merrill 0 siblings, 1 reply; 6+ messages in thread From: Patrick Palka @ 2022-03-11 15:46 UTC (permalink / raw) To: Jason Merrill; +Cc: Patrick Palka, gcc-patches On Thu, 10 Mar 2022, Jason Merrill wrote: > On 3/1/22 00:10, Patrick Palka wrote: > > On Tue, 19 Jan 2021, Jason Merrill wrote: > > > > > On 1/13/21 12:05 PM, Patrick Palka wrote: > > > > In the below testcase, the expression of the atomic constraint after > > > > substitution is (int *) NON_LVALUE_EXPR <1> != 0B which is not a C++ > > > > constant expression, but its TREE_CONSTANT flag is set (from build2), > > > > so satisfy_atom fails to notice that it's non-constant (and we end > > > > up tripping over the assert in satisfaction_value). > > > > > > > > Since TREE_CONSTANT doesn't necessarily correspond to C++ constantness, > > > > this patch makes satisfy_atom instead check > > > > is_rvalue_constant_expression. > > > > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > > > > trunk/10? > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > PR c++/98644 > > > > * constraint.cc (satisfy_atom): Check is_rvalue_constant_expression > > > > instead of TREE_CONSTANT. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > PR c++/98644 > > > > * g++.dg/cpp2a/concepts-pr98644.C: New test. > > > > --- > > > > gcc/cp/constraint.cc | 2 +- > > > > gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C | 7 +++++++ > > > > 2 files changed, 8 insertions(+), 1 deletion(-) > > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C > > > > > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > > > > index 9049d087859..f99a25dc8a4 100644 > > > > --- a/gcc/cp/constraint.cc > > > > +++ b/gcc/cp/constraint.cc > > > > @@ -2969,7 +2969,7 @@ satisfy_atom (tree t, tree args, sat_info info) > > > > { > > > > result = maybe_constant_value (result, NULL_TREE, > > > > /*manifestly_const_eval=*/true); > > > > - if (!TREE_CONSTANT (result)) > > > > > > This should be sufficient. If the result isn't constant, > > > maybe_constant_value > > > shouldn't return it with TREE_CONSTANT set. See > > > > > > > /* This isn't actually constant, so unset TREE_CONSTANT. > > > > > > in cxx_eval_outermost_constant_expr. > > > > I see, so the problem seems to be that the fail-fast path of > > maybe_constant_value isn't clearing TREE_CONSTANT sufficiently. Would > > it make sense to fix this like so? > > > > -- >8 -- > > > > Subject: [PATCH] c++: ICE with non-constant satisfaction value [PR98644] > > > > Here during satisfaction the expression of the atomic constraint after > > substitution is (int *) NON_LVALUE_EXPR <1> != 0B, which is not a C++ > > constant expression due to the reinterpret_cast, but TREE_CONSTANT is > > set since its value is otherwise effectively constant. We then call > > maybe_constant_value on it, which proceeds via its fail-fast path to > > exit early without clearing TREE_CONSTANT. But satisfy_atom relies > > on checking TREE_CONSTANT of the result of maybe_constant_value in order > > to detect non-constant satisfaction. > > > > This patch fixes this by making the fail-fast path of maybe_constant_value > > clear TREE_CONSTANT in this case, like cxx_eval_outermost_constant_expr > > in the normal path would have done. > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > > trunk? > > > > PR c++/98644 > > > > gcc/cp/ChangeLog: > > > > * constexpr.cc (maybe_constant_value): In the fail-fast path, > > clear TREE_CONSTANT on the result if it's set on the input. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/cpp2a/concepts-pr98644.C: New test. > > * g++.dg/parse/array-size2.C: Remove expected diagnostic about a > > narrowing conversion. > > --- > > gcc/cp/constexpr.cc | 4 +++- > > gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C | 7 +++++++ > > gcc/testsuite/g++.dg/parse/array-size2.C | 2 -- > > 3 files changed, 10 insertions(+), 3 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C > > > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > > index 4716694cb71..234cf0acc26 100644 > > --- a/gcc/cp/constexpr.cc > > +++ b/gcc/cp/constexpr.cc > > @@ -7965,8 +7965,10 @@ maybe_constant_value (tree t, tree decl, bool > > manifestly_const_eval) > > if (!is_nondependent_constant_expression (t)) > > { > > - if (TREE_OVERFLOW_P (t)) > > + if (TREE_OVERFLOW_P (t) > > + || (!processing_template_decl && TREE_CONSTANT (t))) > > { > > + /* This isn't actually constant, so unset TREE_CONSTANT. */ > > t = build_nop (TREE_TYPE (t), t); > > build_nop isn't appropriate for arbitrary expressions (classes, in > particular). We probably want to factor out the code in > cxx_eval_outermost_constant_expr under the "this isn't actually constant" > comment. Gotcha, makes sense. Like the following? Bootstrapped and regtested on x86_64-pc-linux-gnu. -- >8 -- Subject: [PATCH] c++: ICE with non-constant satisfaction value [PR98644] Here during satisfaction, the expression of the atomic constraint after substitution is (int *) NON_LVALUE_EXPR <1> != 0B, which is not a C++ constant expression due to the reinterpret_cast, but TREE_CONSTANT is set since its value is otherwise effectively constant. We then call maybe_constant_value on it, which proceeds via its fail-fast path to exit early without clearing TREE_CONSTANT. But satisfy_atom relies on checking TREE_CONSTANT of the result of maybe_constant_value in order to detect non-constant satisfaction. This patch fixes this by making the fail-fast path of maybe_constant_value clear TREE_CONSTANT in this case, like cxx_eval_outermost_constant_expr in the normal path would have done. PR c++/98644 gcc/cp/ChangeLog: * constexpr.cc (mark_non_constant): Define, split out from ... (cxx_eval_outermost_constant_expr): ... here. (maybe_constant_value): Use it. gcc/testsuite/ChangeLog: * g++.dg/cpp2a/concepts-pr98644.C: New test. * g++.dg/parse/array-size2.C: Remove expected diagnostic about a narrowing conversion. Co-authored-by: Jason Merrill <jason@redhat.com> --- gcc/cp/constexpr.cc | 46 +++++++++++-------- gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C | 7 +++ gcc/testsuite/g++.dg/parse/array-size2.C | 2 - 3 files changed, 34 insertions(+), 21 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index 4716694cb71..22d86f023d0 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -7579,6 +7579,29 @@ find_immediate_fndecl (tree *tp, int */*walk_subtrees*/, void */*data*/) return NULL_TREE; } +/* T has TREE_CONSTANT set but has been deemed not a valid C++ constant + expression. Return a version of T that has TREE_CONSTANT cleared. */ + +static tree +mark_non_constant (tree t) +{ + gcc_checking_assert (TREE_CONSTANT (t)); + + /* This isn't actually constant, so unset TREE_CONSTANT. + Don't clear TREE_CONSTANT on ADDR_EXPR, as the middle-end requires + it to be set if it is invariant address, even when it is not + a valid C++ constant expression. Wrap it with a NOP_EXPR + instead. */ + if (EXPR_P (t) && TREE_CODE (t) != ADDR_EXPR) + t = copy_node (t); + else if (TREE_CODE (t) == CONSTRUCTOR) + t = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (t), t); + else + t = build_nop (TREE_TYPE (t), t); + TREE_CONSTANT (t) = false; + return t; +} + /* ALLOW_NON_CONSTANT is false if T is required to be a constant expression. STRICT has the same sense as for constant_value_1: true if we only allow conforming C++ constant expressions, or false if we want a constant value @@ -7801,20 +7824,7 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant, else if (constexpr_dtor) return r; else if (non_constant_p && TREE_CONSTANT (r)) - { - /* This isn't actually constant, so unset TREE_CONSTANT. - Don't clear TREE_CONSTANT on ADDR_EXPR, as the middle-end requires - it to be set if it is invariant address, even when it is not - a valid C++ constant expression. Wrap it with a NOP_EXPR - instead. */ - if (EXPR_P (r) && TREE_CODE (r) != ADDR_EXPR) - r = copy_node (r); - else if (TREE_CODE (r) == CONSTRUCTOR) - r = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (r), r); - else - r = build_nop (TREE_TYPE (r), r); - TREE_CONSTANT (r) = false; - } + r = mark_non_constant (r); else if (non_constant_p) return t; @@ -7965,11 +7975,9 @@ maybe_constant_value (tree t, tree decl, bool manifestly_const_eval) if (!is_nondependent_constant_expression (t)) { - if (TREE_OVERFLOW_P (t)) - { - t = build_nop (TREE_TYPE (t), t); - TREE_CONSTANT (t) = false; - } + if (TREE_OVERFLOW_P (t) + || (!processing_template_decl && TREE_CONSTANT (t))) + t = mark_non_constant (t); return t; } else if (CONSTANT_CLASS_P (t)) diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C new file mode 100644 index 00000000000..6772f72a3ce --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C @@ -0,0 +1,7 @@ +// PR c++/98644 +// { dg-do compile { target c++20 } } + +template<class T> concept Signed = bool(T(1)); // { dg-error "reinterpret_cast" } +static_assert(Signed<int*>); // { dg-error "non-constant" } + +constexpr bool B = requires { requires bool((char *)1); }; // { dg-error "reinterpret_cast" } diff --git a/gcc/testsuite/g++.dg/parse/array-size2.C b/gcc/testsuite/g++.dg/parse/array-size2.C index c4a69df3b01..e58fe266e77 100644 --- a/gcc/testsuite/g++.dg/parse/array-size2.C +++ b/gcc/testsuite/g++.dg/parse/array-size2.C @@ -15,8 +15,6 @@ void foo (void) { char g[(char *) &((struct S *) 0)->b - (char *) 0]; // { dg-error "40:size of array .g. is not an integral constant-expression" } - // { dg-error "narrowing conversion" "" { target c++11 } .-1 } - // { dg-message "expression has a constant value but is not a C.. constant-expression" "" { target c++11 } .-2 } char h[(__SIZE_TYPE__) &((struct S *) 8)->b]; // { dg-error "10:size of array .h. is not an integral constant-expression" } bar (g, h); } -- 2.35.1.455.g1a4874565f ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Fix ICE with non-constant satisfaction [PR98644] 2022-03-11 15:46 ` Patrick Palka @ 2022-03-11 22:16 ` Jason Merrill 0 siblings, 0 replies; 6+ messages in thread From: Jason Merrill @ 2022-03-11 22:16 UTC (permalink / raw) To: Patrick Palka; +Cc: gcc-patches On 3/11/22 11:46, Patrick Palka wrote: > On Thu, 10 Mar 2022, Jason Merrill wrote: > >> On 3/1/22 00:10, Patrick Palka wrote: >>> On Tue, 19 Jan 2021, Jason Merrill wrote: >>> >>>> On 1/13/21 12:05 PM, Patrick Palka wrote: >>>>> In the below testcase, the expression of the atomic constraint after >>>>> substitution is (int *) NON_LVALUE_EXPR <1> != 0B which is not a C++ >>>>> constant expression, but its TREE_CONSTANT flag is set (from build2), >>>>> so satisfy_atom fails to notice that it's non-constant (and we end >>>>> up tripping over the assert in satisfaction_value). >>>>> >>>>> Since TREE_CONSTANT doesn't necessarily correspond to C++ constantness, >>>>> this patch makes satisfy_atom instead check >>>>> is_rvalue_constant_expression. >>>>> >>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for >>>>> trunk/10? >>>>> >>>>> gcc/cp/ChangeLog: >>>>> >>>>> PR c++/98644 >>>>> * constraint.cc (satisfy_atom): Check is_rvalue_constant_expression >>>>> instead of TREE_CONSTANT. >>>>> >>>>> gcc/testsuite/ChangeLog: >>>>> >>>>> PR c++/98644 >>>>> * g++.dg/cpp2a/concepts-pr98644.C: New test. >>>>> --- >>>>> gcc/cp/constraint.cc | 2 +- >>>>> gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C | 7 +++++++ >>>>> 2 files changed, 8 insertions(+), 1 deletion(-) >>>>> create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C >>>>> >>>>> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc >>>>> index 9049d087859..f99a25dc8a4 100644 >>>>> --- a/gcc/cp/constraint.cc >>>>> +++ b/gcc/cp/constraint.cc >>>>> @@ -2969,7 +2969,7 @@ satisfy_atom (tree t, tree args, sat_info info) >>>>> { >>>>> result = maybe_constant_value (result, NULL_TREE, >>>>> /*manifestly_const_eval=*/true); >>>>> - if (!TREE_CONSTANT (result)) >>>> >>>> This should be sufficient. If the result isn't constant, >>>> maybe_constant_value >>>> shouldn't return it with TREE_CONSTANT set. See >>>> >>>>> /* This isn't actually constant, so unset TREE_CONSTANT. >>>> >>>> in cxx_eval_outermost_constant_expr. >>> >>> I see, so the problem seems to be that the fail-fast path of >>> maybe_constant_value isn't clearing TREE_CONSTANT sufficiently. Would >>> it make sense to fix this like so? >>> >>> -- >8 -- >>> >>> Subject: [PATCH] c++: ICE with non-constant satisfaction value [PR98644] >>> >>> Here during satisfaction the expression of the atomic constraint after >>> substitution is (int *) NON_LVALUE_EXPR <1> != 0B, which is not a C++ >>> constant expression due to the reinterpret_cast, but TREE_CONSTANT is >>> set since its value is otherwise effectively constant. We then call >>> maybe_constant_value on it, which proceeds via its fail-fast path to >>> exit early without clearing TREE_CONSTANT. But satisfy_atom relies >>> on checking TREE_CONSTANT of the result of maybe_constant_value in order >>> to detect non-constant satisfaction. >>> >>> This patch fixes this by making the fail-fast path of maybe_constant_value >>> clear TREE_CONSTANT in this case, like cxx_eval_outermost_constant_expr >>> in the normal path would have done. >>> >>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for >>> trunk? >>> >>> PR c++/98644 >>> >>> gcc/cp/ChangeLog: >>> >>> * constexpr.cc (maybe_constant_value): In the fail-fast path, >>> clear TREE_CONSTANT on the result if it's set on the input. >>> >>> gcc/testsuite/ChangeLog: >>> >>> * g++.dg/cpp2a/concepts-pr98644.C: New test. >>> * g++.dg/parse/array-size2.C: Remove expected diagnostic about a >>> narrowing conversion. >>> --- >>> gcc/cp/constexpr.cc | 4 +++- >>> gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C | 7 +++++++ >>> gcc/testsuite/g++.dg/parse/array-size2.C | 2 -- >>> 3 files changed, 10 insertions(+), 3 deletions(-) >>> create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C >>> >>> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc >>> index 4716694cb71..234cf0acc26 100644 >>> --- a/gcc/cp/constexpr.cc >>> +++ b/gcc/cp/constexpr.cc >>> @@ -7965,8 +7965,10 @@ maybe_constant_value (tree t, tree decl, bool >>> manifestly_const_eval) >>> if (!is_nondependent_constant_expression (t)) >>> { >>> - if (TREE_OVERFLOW_P (t)) >>> + if (TREE_OVERFLOW_P (t) >>> + || (!processing_template_decl && TREE_CONSTANT (t))) >>> { >>> + /* This isn't actually constant, so unset TREE_CONSTANT. */ >>> t = build_nop (TREE_TYPE (t), t); >> >> build_nop isn't appropriate for arbitrary expressions (classes, in >> particular). We probably want to factor out the code in >> cxx_eval_outermost_constant_expr under the "this isn't actually constant" >> comment. > > Gotcha, makes sense. Like the following? Bootstrapped and regtested on > x86_64-pc-linux-gnu. OK. > -- >8 -- > > Subject: [PATCH] c++: ICE with non-constant satisfaction value [PR98644] > > Here during satisfaction, the expression of the atomic constraint after > substitution is (int *) NON_LVALUE_EXPR <1> != 0B, which is not a C++ > constant expression due to the reinterpret_cast, but TREE_CONSTANT is > set since its value is otherwise effectively constant. We then call > maybe_constant_value on it, which proceeds via its fail-fast path to > exit early without clearing TREE_CONSTANT. But satisfy_atom relies > on checking TREE_CONSTANT of the result of maybe_constant_value in order > to detect non-constant satisfaction. > > This patch fixes this by making the fail-fast path of maybe_constant_value > clear TREE_CONSTANT in this case, like cxx_eval_outermost_constant_expr > in the normal path would have done. > > PR c++/98644 > > gcc/cp/ChangeLog: > > * constexpr.cc (mark_non_constant): Define, split out from ... > (cxx_eval_outermost_constant_expr): ... here. > (maybe_constant_value): Use it. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp2a/concepts-pr98644.C: New test. > * g++.dg/parse/array-size2.C: Remove expected diagnostic about a > narrowing conversion. > > Co-authored-by: Jason Merrill <jason@redhat.com> > --- > gcc/cp/constexpr.cc | 46 +++++++++++-------- > gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C | 7 +++ > gcc/testsuite/g++.dg/parse/array-size2.C | 2 - > 3 files changed, 34 insertions(+), 21 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > index 4716694cb71..22d86f023d0 100644 > --- a/gcc/cp/constexpr.cc > +++ b/gcc/cp/constexpr.cc > @@ -7579,6 +7579,29 @@ find_immediate_fndecl (tree *tp, int */*walk_subtrees*/, void */*data*/) > return NULL_TREE; > } > > +/* T has TREE_CONSTANT set but has been deemed not a valid C++ constant > + expression. Return a version of T that has TREE_CONSTANT cleared. */ > + > +static tree > +mark_non_constant (tree t) > +{ > + gcc_checking_assert (TREE_CONSTANT (t)); > + > + /* This isn't actually constant, so unset TREE_CONSTANT. > + Don't clear TREE_CONSTANT on ADDR_EXPR, as the middle-end requires > + it to be set if it is invariant address, even when it is not > + a valid C++ constant expression. Wrap it with a NOP_EXPR > + instead. */ > + if (EXPR_P (t) && TREE_CODE (t) != ADDR_EXPR) > + t = copy_node (t); > + else if (TREE_CODE (t) == CONSTRUCTOR) > + t = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (t), t); > + else > + t = build_nop (TREE_TYPE (t), t); > + TREE_CONSTANT (t) = false; > + return t; > +} > + > /* ALLOW_NON_CONSTANT is false if T is required to be a constant expression. > STRICT has the same sense as for constant_value_1: true if we only allow > conforming C++ constant expressions, or false if we want a constant value > @@ -7801,20 +7824,7 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant, > else if (constexpr_dtor) > return r; > else if (non_constant_p && TREE_CONSTANT (r)) > - { > - /* This isn't actually constant, so unset TREE_CONSTANT. > - Don't clear TREE_CONSTANT on ADDR_EXPR, as the middle-end requires > - it to be set if it is invariant address, even when it is not > - a valid C++ constant expression. Wrap it with a NOP_EXPR > - instead. */ > - if (EXPR_P (r) && TREE_CODE (r) != ADDR_EXPR) > - r = copy_node (r); > - else if (TREE_CODE (r) == CONSTRUCTOR) > - r = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (r), r); > - else > - r = build_nop (TREE_TYPE (r), r); > - TREE_CONSTANT (r) = false; > - } > + r = mark_non_constant (r); > else if (non_constant_p) > return t; > > @@ -7965,11 +7975,9 @@ maybe_constant_value (tree t, tree decl, bool manifestly_const_eval) > > if (!is_nondependent_constant_expression (t)) > { > - if (TREE_OVERFLOW_P (t)) > - { > - t = build_nop (TREE_TYPE (t), t); > - TREE_CONSTANT (t) = false; > - } > + if (TREE_OVERFLOW_P (t) > + || (!processing_template_decl && TREE_CONSTANT (t))) > + t = mark_non_constant (t); > return t; > } > else if (CONSTANT_CLASS_P (t)) > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C b/gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C > new file mode 100644 > index 00000000000..6772f72a3ce > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-pr98644.C > @@ -0,0 +1,7 @@ > +// PR c++/98644 > +// { dg-do compile { target c++20 } } > + > +template<class T> concept Signed = bool(T(1)); // { dg-error "reinterpret_cast" } > +static_assert(Signed<int*>); // { dg-error "non-constant" } > + > +constexpr bool B = requires { requires bool((char *)1); }; // { dg-error "reinterpret_cast" } > diff --git a/gcc/testsuite/g++.dg/parse/array-size2.C b/gcc/testsuite/g++.dg/parse/array-size2.C > index c4a69df3b01..e58fe266e77 100644 > --- a/gcc/testsuite/g++.dg/parse/array-size2.C > +++ b/gcc/testsuite/g++.dg/parse/array-size2.C > @@ -15,8 +15,6 @@ void > foo (void) > { > char g[(char *) &((struct S *) 0)->b - (char *) 0]; // { dg-error "40:size of array .g. is not an integral constant-expression" } > - // { dg-error "narrowing conversion" "" { target c++11 } .-1 } > - // { dg-message "expression has a constant value but is not a C.. constant-expression" "" { target c++11 } .-2 } > char h[(__SIZE_TYPE__) &((struct S *) 8)->b]; // { dg-error "10:size of array .h. is not an integral constant-expression" } > bar (g, h); > } ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-11 22:16 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-13 17:05 [PATCH] c++: Fix ICE with non-constant satisfaction [PR98644] Patrick Palka 2021-01-19 20:40 ` Jason Merrill 2022-03-01 4:10 ` Patrick Palka 2022-03-10 20:05 ` Jason Merrill 2022-03-11 15:46 ` Patrick Palka 2022-03-11 22:16 ` 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).