[gcc r11-8056] c++: Don't cache constexpr functions which are passed pointers to heap or static vars being construc

[gcc r11-8056] c++: Don't cache constexpr functions which are passed pointers to heap or static vars being construc

[Jakub Jelinek]

https://gcc.gnu.org/g:559d2f1e0eafd96c19dc5324db1a466286c0e7fc

commit r11-8056-g559d2f1e0eafd96c19dc5324db1a466286c0e7fc
Author: Jakub Jelinek <jakub@redhat.com>
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  <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-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.

Diff:
---
 gcc/cp/constexpr.c                             | 35 ++++++++++++++++++++
 gcc/testsuite/g++.dg/cpp1y/constexpr-99859-1.C | 24 ++++++++++++++
 gcc/testsuite/g++.dg/cpp1y/constexpr-99859-2.C | 12 +++++++
 gcc/testsuite/g++.dg/cpp2a/constexpr-new18.C   | 45 ++++++++++++++++++++++++++
 gcc/testsuite/g++.dg/cpp2a/constexpr-new19.C   | 43 ++++++++++++++++++++++++
 5 files changed, 159 insertions(+)

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 42d00ec8d31..c8d9dae36fb 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -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,15 @@ cxx_bind_parameters_in_call (const constexpr_ctx *ctx, tree t,
 	    /* 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, or might be modified by the callers in between
+	       the cached function and just read 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
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-99859-1.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-99859-1.C
new file mode 100644
index 00000000000..dea5a5b56f8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-99859-1.C
@@ -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, "");
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-99859-2.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-99859-2.C
new file mode 100644
index 00000000000..a249f474666
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-99859-2.C
@@ -0,0 +1,12 @@
+// PR c++/99859
+// { dg-do compile { target c++14 } }
+
+struct A
+{
+  int i;
+  constexpr int f() { return i; }
+  constexpr A() : i(0) { i = f(); i = 1; i = f(); }
+};
+
+constexpr A a = A();
+static_assert (a.i == 1, "");
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-new18.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-new18.C
new file mode 100644
index 00000000000..24b298aafd4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-new18.C
@@ -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());
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-new19.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-new19.C
new file mode 100644
index 00000000000..7a73deed693
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-new19.C
@@ -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);