Message ID | 20210408093629.GX1179226@tucnak |
---|---|
State | New |
Headers | show |
Series | c++: Don't cache constexpr functions which are passed pointers to heap or static vars being constructed [PR99859] | expand |
On 4/8/21 5:36 AM, Jakub Jelinek wrote: > 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. > 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). > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2021-04-08 Jakub Jelinek <jakub@redhat.com> > > 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.C: New test. > * g++.dg/cpp2a/constexpr-new18.C: New test. > * g++.dg/cpp2a/constexpr-new19.C: New test. > > --- gcc/cp/constexpr.c.jj 2021-04-07 15:50:32.274927848 +0200 > +++ gcc/cp/constexpr.c 2021-04-07 18:21:20.845684670 +0200 > @@ -1555,6 +1555,32 @@ free_constructor (tree t) > } > } > > +/* Helper function of cxx_bind_parameters_in_call. Return non-NULL > + if *TP is address of a static variable (or part of it) currently being > + constructed or of a heap artificial variable. */ > +static tree > +addr_of_non_const_var (tree *tp, int *walk_subtrees, void *data) > +{ > + if (TREE_CODE (*tp) == ADDR_EXPR) > + if (tree var = get_base_address (TREE_OPERAND (*tp, 0))) > + if (VAR_P (var) && TREE_STATIC (var)) > + { > + if (DECL_NAME (var) == heap_uninit_identifier > + || DECL_NAME (var) == heap_identifier > + || DECL_NAME (var) == heap_vec_uninit_identifier > + || DECL_NAME (var) == heap_vec_identifier) > + return var; > + > + constexpr_global_ctx *global = (constexpr_global_ctx *) data; > + if (global->values.get (var)) > + return var; > + } > + if (TYPE_P (*tp)) > + *walk_subtrees = false; > + return NULL_TREE; > +} > + > /* Subroutine of cxx_eval_call_expression. > We are processing a call expression (either CALL_EXPR or > AGGR_INIT_EXPR) in the context of CTX. Evaluate > @@ -1616,6 +1642,14 @@ cxx_bind_parameters_in_call (const const > /* The destructor needs to see any modifications the callee makes > to the argument. */ > *non_constant_args = true; > + /* If arg is or contains address of a heap artificial variable or > + of a static variable being constructed, avoid caching the > + function call, as those variables might be modified by the > + function. */ We don't want to cache even if the call only reads from the variable, because the result might depend on a value that can be changed in the caller: struct A { int i; constexpr int f() { return i; } constexpr A() : i(0) { f(); i = 1; i = f(); } }; constexpr A a = A(); static_assert (a.i == 1, ""); OK with this comment adjustment, and adding this case to the tests. > + else if (!*non_constant_args > + && cp_walk_tree (&arg, addr_of_non_const_var, ctx->global, > + NULL)) > + *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 > --- gcc/testsuite/g++.dg/cpp1y/constexpr-99859.C.jj 2021-04-07 19:15:12.488877570 +0200 > +++ gcc/testsuite/g++.dg/cpp1y/constexpr-99859.C 2021-04-07 19:15:33.796641707 +0200 > @@ -0,0 +1,24 @@ > +// PR c++/99859 > +// { dg-do compile { target c++14 } } > + > +constexpr int > +foo (int *x) > +{ > + return ++*x; > +} > + > +struct S { constexpr S () : a(0) { foo (&a); foo (&a); } int a; }; > +constexpr S s = S (); > +static_assert (s.a == 2, ""); > + > +struct R { int *p; }; > + > +constexpr int > +bar (R x) > +{ > + return ++*x.p; > +} > + > +struct T { int a = 0; constexpr T () { bar (R{&a}); bar (R{&a}); } }; > +constexpr T t = T (); > +static_assert (t.a == 2, ""); > --- gcc/testsuite/g++.dg/cpp2a/constexpr-new18.C.jj 2021-04-07 17:35:36.272075995 +0200 > +++ gcc/testsuite/g++.dg/cpp2a/constexpr-new18.C 2021-04-07 17:35:36.272075995 +0200 > @@ -0,0 +1,45 @@ > +// PR c++/99859 > +// { dg-do compile { target c++20 } } > + > +template <class T> > +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(&f); > + return true; > +} > +constexpr bool x = baz(); > + > +constexpr void bar(intrusive_ptr<Foo> a) > +{ > + if (a.ptr->count_ != 2) throw 1; > +} > + > +constexpr bool foo() { > + intrusive_ptr a(new Foo()); > + bar(a); > + return true; > +} > + > +static_assert(foo()); > --- gcc/testsuite/g++.dg/cpp2a/constexpr-new19.C.jj 2021-04-07 19:03:14.800821556 +0200 > +++ gcc/testsuite/g++.dg/cpp2a/constexpr-new19.C 2021-04-07 19:02:48.626111273 +0200 > @@ -0,0 +1,43 @@ > +// PR c++/99859 > +// { dg-do compile { target c++20 } } > + > +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); > + > +struct R { int a, *b; }; > + > +constexpr void > +baz (R x) > +{ > + ++*x.b; > +} > + > +constexpr int > +qux () > +{ > + int *x = new int (0); > + R r{1, x}; > + baz (r); > + baz (r); > + int y = *x; > + delete x; > + return y; > +} > + > +static_assert (qux () == 2); > > Jakub >
--- gcc/cp/constexpr.c.jj 2021-04-07 15:50:32.274927848 +0200 +++ gcc/cp/constexpr.c 2021-04-07 18:21:20.845684670 +0200 @@ -1555,6 +1555,32 @@ free_constructor (tree t) } } +/* Helper function of cxx_bind_parameters_in_call. Return non-NULL + if *TP is address of a static variable (or part of it) currently being + constructed or of a heap artificial variable. */ + +static tree +addr_of_non_const_var (tree *tp, int *walk_subtrees, void *data) +{ + if (TREE_CODE (*tp) == ADDR_EXPR) + if (tree var = get_base_address (TREE_OPERAND (*tp, 0))) + if (VAR_P (var) && TREE_STATIC (var)) + { + if (DECL_NAME (var) == heap_uninit_identifier + || DECL_NAME (var) == heap_identifier + || DECL_NAME (var) == heap_vec_uninit_identifier + || DECL_NAME (var) == heap_vec_identifier) + return var; + + constexpr_global_ctx *global = (constexpr_global_ctx *) data; + if (global->values.get (var)) + return var; + } + if (TYPE_P (*tp)) + *walk_subtrees = false; + return NULL_TREE; +} + /* Subroutine of cxx_eval_call_expression. We are processing a call expression (either CALL_EXPR or AGGR_INIT_EXPR) in the context of CTX. Evaluate @@ -1616,6 +1642,14 @@ cxx_bind_parameters_in_call (const const /* The destructor needs to see any modifications the callee makes to the argument. */ *non_constant_args = true; + /* If arg is or contains address of a heap artificial variable or + of a static variable being constructed, avoid caching the + function call, as those variables might be modified by the + function. */ + else if (!*non_constant_args + && cp_walk_tree (&arg, addr_of_non_const_var, ctx->global, + NULL)) + *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 --- gcc/testsuite/g++.dg/cpp1y/constexpr-99859.C.jj 2021-04-07 19:15:12.488877570 +0200 +++ gcc/testsuite/g++.dg/cpp1y/constexpr-99859.C 2021-04-07 19:15:33.796641707 +0200 @@ -0,0 +1,24 @@ +// PR c++/99859 +// { dg-do compile { target c++14 } } + +constexpr int +foo (int *x) +{ + return ++*x; +} + +struct S { constexpr S () : a(0) { foo (&a); foo (&a); } int a; }; +constexpr S s = S (); +static_assert (s.a == 2, ""); + +struct R { int *p; }; + +constexpr int +bar (R x) +{ + return ++*x.p; +} + +struct T { int a = 0; constexpr T () { bar (R{&a}); bar (R{&a}); } }; +constexpr T t = T (); +static_assert (t.a == 2, ""); --- gcc/testsuite/g++.dg/cpp2a/constexpr-new18.C.jj 2021-04-07 17:35:36.272075995 +0200 +++ gcc/testsuite/g++.dg/cpp2a/constexpr-new18.C 2021-04-07 17:35:36.272075995 +0200 @@ -0,0 +1,45 @@ +// PR c++/99859 +// { dg-do compile { target c++20 } } + +template <class T> +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(&f); + return true; +} +constexpr bool x = baz(); + +constexpr void bar(intrusive_ptr<Foo> a) +{ + if (a.ptr->count_ != 2) throw 1; +} + +constexpr bool foo() { + intrusive_ptr a(new Foo()); + bar(a); + return true; +} + +static_assert(foo()); --- gcc/testsuite/g++.dg/cpp2a/constexpr-new19.C.jj 2021-04-07 19:03:14.800821556 +0200 +++ gcc/testsuite/g++.dg/cpp2a/constexpr-new19.C 2021-04-07 19:02:48.626111273 +0200 @@ -0,0 +1,43 @@ +// PR c++/99859 +// { dg-do compile { target c++20 } } + +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); + +struct R { int a, *b; }; + +constexpr void +baz (R x) +{ + ++*x.b; +} + +constexpr int +qux () +{ + int *x = new int (0); + R r{1, x}; + baz (r); + baz (r); + int y = *x; + delete x; + return y; +} + +static_assert (qux () == 2);