[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 Jonathan Wakely changed: What|Removed |Added CC||asorenji at gmail dot com --- Comment #24 from Jonathan Wakely --- *** Bug 104348 has been marked as a duplicate of this bug. ***
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 Andrew Pinski changed: What|Removed |Added Status|ASSIGNED|RESOLVED Target Milestone|--- |10.4 Resolution|--- |FIXED --- Comment #23 from Andrew Pinski --- Fixed.
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 Andrew Pinski changed: What|Removed |Added CC||johelegp at gmail dot com --- Comment #22 from Andrew Pinski --- *** Bug 95806 has been marked as a duplicate of this bug. ***
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 --- Comment #21 from Andrew Pinski --- Looks to be fixed in GCC 11. It still fails for me with GCC 10.3 though.
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 --- Comment #20 from CVS Commits --- The releases/gcc-10 branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:e961da38630c892dfc0723e0726b6a0b0833e951 commit r10-9722-ge961da38630c892dfc0723e0726b6a0b0833e951 Author: Jakub Jelinek Date: Thu Apr 8 17:15:39 2021 +0200 c++: Don't cache constexpr functions which are passed pointers to heap or static vars being constructed [PR99859] When cxx_bind_parameters_in_call is called e.g. on a method on an automatic variable, we evaluate the argument and because ADDR_EXPR of an automatic decl is not TREE_CONSTANT, we set *non_constant_args and don't cache it. But when it is called on an object located on the heap (allocated using C++20 constexpr new) where we represent it as TREE_STATIC artificial var, or when it is called on a static var that is currently being constructed, such ADDR_EXPRs are TREE_CONSTANT and we happily cache such calls, but they can in those cases have side-effects in the heap or static var objects and so caching them means such side-effects will happen only once and not as many times as that method or function is called. Furthermore, as Patrick mentioned in the PR, the argument doesn't need to be just ADDR_EXPR of the heap or static var or its components, but it could be a CONSTRUCTOR that has the ADDR_EXPR embedded anywhere. And the incorrectly cached function doesn't need to modify the pointed vars or their components, but some caller could be changing them in between the call that was cached and the call that used the cached result. The following patch fixes it by setting *non_constant_args also when the argument contains somewhere such an ADDR_EXPR, either of a heap artificial var or component thereof, or of a static var currently being constructed (where for that it uses the same check as cxx_eval_store_expression, ctx->global->values.get (...); addresses of other static variables would be rejected by cxx_eval_store_expression and therefore it is ok to cache such calls). 2021-04-08 Jakub Jelinek PR c++/99859 * constexpr.c (addr_of_non_const_var): New function. (cxx_bind_parameters_in_call): Set *non_constant_args to true even if cp_walk_tree on arg with addr_of_non_const_var callback returns true. * g++.dg/cpp1y/constexpr-99859-1.C: New test. * g++.dg/cpp1y/constexpr-99859-2.C: New test. * g++.dg/cpp2a/constexpr-new18.C: New test. * g++.dg/cpp2a/constexpr-new19.C: New test. (cherry picked from commit 559d2f1e0eafd96c19dc5324db1a466286c0e7fc)
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 Patrick Palka changed: What|Removed |Added CC||pkeir at outlook dot com --- Comment #19 from Patrick Palka --- *** Bug 96414 has been marked as a duplicate of this bug. ***
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 Jakub Jelinek changed: What|Removed |Added CC||vittorio.romeo at outlook dot com --- Comment #18 from Jakub Jelinek --- *** Bug 80039 has been marked as a duplicate of this bug. ***
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 --- Comment #17 from Jakub Jelinek --- Fixed on the trunk, I think we want to backport this though eventually.
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 --- Comment #16 from CVS Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:559d2f1e0eafd96c19dc5324db1a466286c0e7fc commit r11-8056-g559d2f1e0eafd96c19dc5324db1a466286c0e7fc Author: Jakub Jelinek Date: Thu Apr 8 17:15:39 2021 +0200 c++: Don't cache constexpr functions which are passed pointers to heap or static vars being constructed [PR99859] When cxx_bind_parameters_in_call is called e.g. on a method on an automatic variable, we evaluate the argument and because ADDR_EXPR of an automatic decl is not TREE_CONSTANT, we set *non_constant_args and don't cache it. But when it is called on an object located on the heap (allocated using C++20 constexpr new) where we represent it as TREE_STATIC artificial var, or when it is called on a static var that is currently being constructed, such ADDR_EXPRs are TREE_CONSTANT and we happily cache such calls, but they can in those cases have side-effects in the heap or static var objects and so caching them means such side-effects will happen only once and not as many times as that method or function is called. Furthermore, as Patrick mentioned in the PR, the argument doesn't need to be just ADDR_EXPR of the heap or static var or its components, but it could be a CONSTRUCTOR that has the ADDR_EXPR embedded anywhere. And the incorrectly cached function doesn't need to modify the pointed vars or their components, but some caller could be changing them in between the call that was cached and the call that used the cached result. The following patch fixes it by setting *non_constant_args also when the argument contains somewhere such an ADDR_EXPR, either of a heap artificial var or component thereof, or of a static var currently being constructed (where for that it uses the same check as cxx_eval_store_expression, ctx->global->values.get (...); addresses of other static variables would be rejected by cxx_eval_store_expression and therefore it is ok to cache such calls). 2021-04-08 Jakub Jelinek PR c++/99859 * constexpr.c (addr_of_non_const_var): New function. (cxx_bind_parameters_in_call): Set *non_constant_args to true even if cp_walk_tree on arg with addr_of_non_const_var callback returns true. * g++.dg/cpp1y/constexpr-99859-1.C: New test. * g++.dg/cpp1y/constexpr-99859-2.C: New test. * g++.dg/cpp2a/constexpr-new18.C: New test. * g++.dg/cpp2a/constexpr-new19.C: New test.
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 Jakub Jelinek changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org --- Comment #15 from Jakub Jelinek --- Created attachment 50525 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50525=edit gcc11-pr99859.patch Full untested patch. As for PR80039, I for some reason can't reproduce the #c0 in there and #c1 looks too close to the testcases from this PR that I think it isn't useful to add it to the testsuite.
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 --- Comment #14 from Patrick Palka --- (In reply to Jakub Jelinek from comment #11) > For the global vars (so PR80039 too), can the problem be anything but when > cxx_eval_outermost_constant_expr is called on such an object (or part > thereof)? > Unfortunately, ctx->object might be NULL, perhaps we could stick a copy of > that into ctx->global->object (get_base_address of the outermost object it > is called on)? Sounds good to me FWIW (In reply to Jakub Jelinek from comment #13) > (In reply to Jakub Jelinek from comment #12) > > And in #c9 you're right that it could be embedded in CONSTRUCTORs too. > > Wonder if cp_walk_tree to find the ADDR_EXPR of heap var addresses and > ctx->global->object address would be enough. Sounds good to me too. Using cp_walk_tree means we'd also detect the case where the argument is 'heap_ptr + CST'.
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859
--- Comment #13 from Jakub Jelinek ---
(In reply to Jakub Jelinek from comment #12)
> And in #c9 you're right that it could be embedded in CONSTRUCTORs too.
Wonder if cp_walk_tree to find the ADDR_EXPR of heap var addresses and
ctx->global->object address would be enough.
Counter example I was thinking about would be:
constexpr int foo(auto x) { return ++*x->p; }
struct S {
int a = 0;
constexpr S() {
struct R { int* p; };
R b{};
R c{};
foo();
foo();
}
};
constexpr S s = S();
static_assert (s.a == 2);
but in that case we handle it correctly already as the address of the automatic
vars is already non-constant.
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 --- Comment #12 from Jakub Jelinek --- And in #c9 you're right that it could be embedded in CONSTRUCTORs too.
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 --- Comment #11 from Jakub Jelinek --- For the global vars (so PR80039 too), can the problem be anything but when cxx_eval_outermost_constant_expr is called on such an object (or part thereof)? Unfortunately, ctx->object might be NULL, perhaps we could stick a copy of that into ctx->global->object (get_base_address of the outermost object it is called on)?
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859
--- Comment #10 from Jakub Jelinek ---
So perhaps
--- gcc/cp/constexpr.c.jj 2021-03-19 18:36:49.165304923 +0100
+++ gcc/cp/constexpr.c 2021-04-07 15:33:31.993242067 +0200
@@ -1616,6 +1616,22 @@ cxx_bind_parameters_in_call (const const
/* The destructor needs to see any modifications the callee makes
to the argument. */
*non_constant_args = true;
+ else
+ {
+ tree addr = arg;
+ STRIP_NOPS (addr);
+ /* When a method or function is called with address of a heap
+allocated variable, don't cache it so that any modifications
+in it are performed. */
+ if (TREE_CODE (addr) == ADDR_EXPR)
+ if (tree var = get_base_address (TREE_OPERAND (addr, 0)))
+ if (VAR_P (var)
+ && (DECL_NAME (var) == heap_uninit_identifier
+ || DECL_NAME (var) == heap_identifier
+ || DECL_NAME (var) == heap_vec_uninit_identifier
+ || DECL_NAME (var) == heap_vec_identifier))
+ *non_constant_args = true;
+ }
/* For virtual calls, adjust the this argument, so that it is
the object on which the method is called, rather than
for the heap vars and something similar for the global var ctors?
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859
--- Comment #9 from Patrick Palka ---
(In reply to Jakub Jelinek from comment #6)
> The argument is a pointer.
> Now, I bet a pointer to an automatic variable will be seen as non-constant
> and so in that case we might be ok.
> If the argument is a pointer to some global constexpr variable, dunno.
FWIW I think this is PR80039. I think we might have to worry about this case
only when evaluating the constructor of the global constexpr variable.
> Why does it work for:
> constexpr int foo(int* x) { return ++*x; }
> struct S { constexpr S() : a(0) { foo(); foo(); } int a; };
> constexpr S s;
> static_assert (s.a == 2);
> though? The argument to foo after constexpr processing it is which is
> TREE_CONSTANT too, yet we don't cache the calls.
Hmm, seems because ctx->strict is not set when the initialization of s gets
evaluated in this case. But it breaks if we instead do "constexpr S s = S();"
> And, on this testcase the argument is a heap pointer which during constexpr
> evaluation we pretend it is constant.
> So maybe cxx_bind_parameters_in_call should set *non_constant_args also when
> arg is ADDR_EXPR of something with heap_*identifier* VAR_DECL as base?
I think we'd also have to look inside TREE_CONSTANT CONSTRUCTORs to see if
they're storing a pointer to such a variable, so that we also suppress caching
for cases like:
constexpr int foo(auto x) { return ++*x.p; }
struct S {
int a = 0;
constexpr S() {
struct R { int* p; };
foo(R{});
foo(R{});
}
};
constexpr S s = S();
static_assert (s.a == 2);
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859
--- Comment #8 from Jakub Jelinek ---
Why does it work for:
constexpr int foo(int* x) { return ++*x; }
struct S { constexpr S() : a(0) { foo(); foo(); } int a; };
constexpr S s;
static_assert (s.a == 2);
though? The argument to foo after constexpr processing it is which is
TREE_CONSTANT too, yet we don't cache the calls.
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859
--- Comment #7 from Patrick Palka ---
constexpr void foo(int* x) { ++*x; }
constexpr int bar() {
int* x = new int(0);
foo(x);
foo(x);
int y = *x;
delete x;
return y;
}
static_assert(bar() == 2);
We reject the above testcase for seemingly the same reason -- we cache the call
to foo(x) when we shouldn't have, because cxx_bind_parameters_in_call considers
the call's argument to be TREE_CONSTANT.
We accept the analogous testcase that doesn't use constexpr new/delete:
constexpr void foo(int* x) { ++*x; }
constexpr int bar() {
int x = 0;
foo();
foo();
return x;
}
static_assert(bar() == 2);
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 --- Comment #6 from Jakub Jelinek --- The argument is a pointer. Now, I bet a pointer to an automatic variable will be seen as non-constant and so in that case we might be ok. If the argument is a pointer to some global constexpr variable, dunno. And, on this testcase the argument is a heap pointer which during constexpr evaluation we pretend it is constant. So maybe cxx_bind_parameters_in_call should set *non_constant_args also when arg is ADDR_EXPR of something with heap_*identifier* VAR_DECL as base?
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 --- Comment #5 from Patrick Palka --- (In reply to Jakub Jelinek from comment #4) > So, on the #c3 testcase, if I put a breakpoint before and after > fold_nondependent_expr in finish_static_assert and temporarily in between > those two breakpoints put a breakpoint on cxx_eval_call_expression and > cxx_eval_increment_expression, it is clear that while > cxx_eval_call_expression is called on > ... > intrusive_ptr::~intrusive_ptr (); > Foo::dec (NON_LVALUE_EXPR <((struct intrusive_ptr *) this)->ptr>); > intrusive_ptr::~intrusive_ptr (); > Foo::dec (NON_LVALUE_EXPR <((struct intrusive_ptr *) this)->ptr>); Shouldn't cxx_bind_parameters_in_call have noticed that the argument to these Foo::dec calls is non-constant, which would inhibit their caching?
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 Jakub Jelinek changed: What|Removed |Added CC||jason at gcc dot gnu.org, ||mpolacek at gcc dot gnu.org, ||ppalka at gcc dot gnu.org --- Comment #4 from Jakub Jelinek --- So, on the #c3 testcase, if I put a breakpoint before and after fold_nondependent_expr in finish_static_assert and temporarily in between those two breakpoints put a breakpoint on cxx_eval_call_expression and cxx_eval_increment_expression, it is clear that while cxx_eval_call_expression is called on ... intrusive_ptr::~intrusive_ptr (); Foo::dec (NON_LVALUE_EXPR <((struct intrusive_ptr *) this)->ptr>); intrusive_ptr::~intrusive_ptr (); Foo::dec (NON_LVALUE_EXPR <((struct intrusive_ptr *) this)->ptr>); where during the first Foo::dec call evaluation cxx_eval_increment_expression is called on --((struct Foo *) this)->count_ during the second Foo::dec call we use cached result (1) and don't evaluate the body at all. But the body of the constexpr method has side-effects on the class it is called on (and the result is incorrect too), on the testcase when not evaluating at compile time but runtime the first Foo::dec shall predecrement count_ from 2 to 1 (thus return 1) and the second one should predecrement count_ from 1 to 0 (thus return 0). For the constexpr new I had to add the /* If the call allocated some heap object that hasn't been deallocated during the call, or if it deallocated some heap object it has not allocated, the call isn't really stateless for the constexpr evaluation and should not be cached. It is fine if the call allocates something and deallocates it too. */ setting of cacheable to false in some cases, but this PR let's me wonder if it is ever ok to cache constexpr results and what the standard actually says (if constexpr/consteval functions can have side-effects besides computing a return value (I believe they can't just change some unrelated constexpr variable, right?) and ditto for constexpr/consteval methods. So, e.g. shall we remember during the constexpr evaluation if the currently evaluated constexpr function has side-effects to something other than its automatic variables or return value and if so, make it effectively non-cacheable?
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859
--- Comment #3 from Jakub Jelinek ---
The instantiation isn't the problem,
template
struct intrusive_ptr
{
T *ptr = nullptr;
constexpr explicit intrusive_ptr(T* p) : ptr(p) {
++ptr->count_;
}
constexpr ~intrusive_ptr() {
if (ptr->dec() == 0)
delete ptr;
}
constexpr intrusive_ptr(intrusive_ptr const& a) : ptr(a.ptr) {
++ptr->count_;
}
};
struct Foo {
int count_ = 0;
constexpr int dec() {
return --count_;
}
};
constexpr bool baz() {
Foo f { 4 };
intrusive_ptr a();
return true;
}
constexpr bool x = baz();
constexpr void bar(intrusive_ptr a)
{
}
constexpr bool foo() {
intrusive_ptr a(new Foo());
bar(a);
return true;
}
static_assert(foo());
fails too, and during the finish_static_assert fold_nondependent_expr
evaluation only one cxx_eval_outermost_expr is called.
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859
Jakub Jelinek changed:
What|Removed |Added
CC||jakub at gcc dot gnu.org
Last reconfirmed||2021-04-01
Ever confirmed|0 |1
Status|UNCONFIRMED |NEW
--- Comment #2 from Jakub Jelinek ---
We want testcases in our bugzilla rather than external links.
No need for any headers,
template
struct intrusive_ptr
{
T *ptr = nullptr;
constexpr explicit intrusive_ptr(T* p) : ptr(p) {
++ptr->count_;
}
constexpr ~intrusive_ptr() {
if (ptr->dec() == 0)
//if (--ptr->count_ == 0)
delete ptr;
}
constexpr intrusive_ptr(intrusive_ptr const& a) : ptr(a.ptr) {
++ptr->count_;
}
};
struct Foo {
int count_ = 0;
constexpr int dec() {
return --count_;
}
};
template class intrusive_ptr;
constexpr void bar(intrusive_ptr a)
{
// if (a.ptr->count_ != 2) throw 1;
}
constexpr bool foo() {
intrusive_ptr a(new Foo());
bar(a);
return true;
}
static_assert(foo());
reproduces too. Wonder if it isn't related to the instantiation of
intrusive_ptr::~intrusive_ptr() while constexpr evaluating the static
assert.
[Bug c++/99859] constexpr evaluation with member function is incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99859 --- Comment #1 from Luke Dalessandro --- It was pointed out that it _also_ works if I change > static_assert(foo()); to > constexpr bool b = foo(); > static_assert(b); static_assert(foo());
