diff mbox series

[v4,1/3] c++: Track lifetimes in constant evaluation [PR70331,PR96630,PR98675]

Message ID ZLj/54VvX7Xz7wRk@Thaum.localdomain
State New
Headers show
Series c++: Track lifetimes in constant evaluation [PR70331, ...] | expand

Commit Message

Nathaniel Shead July 20, 2023, 9:35 a.m. UTC
This adds rudimentary lifetime tracking in C++ constexpr contexts,
allowing the compiler to report errors with using values after their
backing has gone out of scope. We don't yet handle other ways of
accessing values outside their lifetime (e.g. following explicit
destructor calls).

	PR c++/96630
	PR c++/98675
	PR c++/70331

gcc/cp/ChangeLog:

	* constexpr.cc (constexpr_global_ctx::is_outside_lifetime): New
	function.
	(constexpr_global_ctx::get_value): Don't return expired values.
	(constexpr_global_ctx::get_value_ptr): Likewise.
	(constexpr_global_ctx::remove_value): Mark value outside
	lifetime.
	(outside_lifetime_error): New function.
	(cxx_eval_call_expression): No longer track save_exprs.
	(cxx_eval_loop_expr): Likewise.
	(cxx_eval_constant_expression): Add checks for outside lifetime
	values. Remove local variables at end of bind exprs, and
	temporaries after cleanup points.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp1y/constexpr-lifetime1.C: New test.
	* g++.dg/cpp1y/constexpr-lifetime2.C: New test.
	* g++.dg/cpp1y/constexpr-lifetime3.C: New test.
	* g++.dg/cpp1y/constexpr-lifetime4.C: New test.
	* g++.dg/cpp1y/constexpr-lifetime5.C: New test.
	* g++.dg/cpp1y/constexpr-lifetime6.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/constexpr.cc                           | 132 ++++++++++++------
 .../g++.dg/cpp1y/constexpr-lifetime1.C        |  13 ++
 .../g++.dg/cpp1y/constexpr-lifetime2.C        |  20 +++
 .../g++.dg/cpp1y/constexpr-lifetime3.C        |  13 ++
 .../g++.dg/cpp1y/constexpr-lifetime4.C        |  11 ++
 .../g++.dg/cpp1y/constexpr-lifetime5.C        |  11 ++
 .../g++.dg/cpp1y/constexpr-lifetime6.C        |  15 ++
 7 files changed, 170 insertions(+), 45 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime5.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime6.C

Comments

Jason Merrill July 20, 2023, 2:42 p.m. UTC | #1
On 7/20/23 05:35, Nathaniel Shead wrote:
> This adds rudimentary lifetime tracking in C++ constexpr contexts,
> allowing the compiler to report errors with using values after their
> backing has gone out of scope. We don't yet handle other ways of
> accessing values outside their lifetime (e.g. following explicit
> destructor calls).

Incidentally, much of that should be straightforward to handle by no 
longer ignoring clobbers here:

>     case MODIFY_EXPR:
>       if (cxx_dialect < cxx14)
>         goto fail;
>       if (!RECUR (TREE_OPERAND (t, 0), any))
>         return false;
>       /* Just ignore clobbers.  */
>       if (TREE_CLOBBER_P (TREE_OPERAND (t, 1)))
>         return true;

Assignment from a clobber represents end of lifetime to the middle-end. 
This can be a follow-up patch.

> @@ -7051,10 +7065,17 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
>   	return ctx->ctor;
>         if (VAR_P (t))
>   	if (tree v = ctx->global->get_value (t))
> -	    {
> -	      r = v;
> -	      break;
> -	    }
> +	  {
> +	    r = v;
> +	    break;
> +	  }
> +      if (ctx->global->is_outside_lifetime (t))
> +	{
> +	  if (!ctx->quiet)
> +	    outside_lifetime_error (loc, t);
> +	  *non_constant_p = true;
> +	  break;
> +	}

Shouldn't this new check also be under the if (VAR_P (t))?  A CONST_DECL 
can't go out of scope.

Jason
Nathaniel Shead July 22, 2023, 5:28 a.m. UTC | #2
On Thu, Jul 20, 2023 at 10:42:29AM -0400, Jason Merrill wrote:
> On 7/20/23 05:35, Nathaniel Shead wrote:
> > This adds rudimentary lifetime tracking in C++ constexpr contexts,
> > allowing the compiler to report errors with using values after their
> > backing has gone out of scope. We don't yet handle other ways of
> > accessing values outside their lifetime (e.g. following explicit
> > destructor calls).
> 
> Incidentally, much of that should be straightforward to handle by no longer
> ignoring clobbers here:
> 
> >     case MODIFY_EXPR:
> >       if (cxx_dialect < cxx14)
> >         goto fail;
> >       if (!RECUR (TREE_OPERAND (t, 0), any))
> >         return false;
> >       /* Just ignore clobbers.  */
> >       if (TREE_CLOBBER_P (TREE_OPERAND (t, 1)))
> >         return true;
> 
> Assignment from a clobber represents end of lifetime to the middle-end. This
> can be a follow-up patch.

Thanks, this is very helpful to know. I'll keep this in mind.

> > @@ -7051,10 +7065,17 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
> >   	return ctx->ctor;
> >         if (VAR_P (t))
> >   	if (tree v = ctx->global->get_value (t))
> > -	    {
> > -	      r = v;
> > -	      break;
> > -	    }
> > +	  {
> > +	    r = v;
> > +	    break;
> > +	  }
> > +      if (ctx->global->is_outside_lifetime (t))
> > +	{
> > +	  if (!ctx->quiet)
> > +	    outside_lifetime_error (loc, t);
> > +	  *non_constant_p = true;
> > +	  break;
> > +	}
> 
> Shouldn't this new check also be under the if (VAR_P (t))?  A CONST_DECL
> can't go out of scope.
> 
> Jason
> 

Yup you're right; I didn't properly read the documentation on what a
CONST_DECL was and misunderstood. I'll fix this up for the next version.
diff mbox series

Patch

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 6e8f1c2b61e..cd4424bcb44 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -1148,7 +1148,8 @@  enum constexpr_switch_state {
 
 class constexpr_global_ctx {
   /* Values for any temporaries or local variables within the
-     constant-expression. */
+     constant-expression. Objects outside their lifetime have
+     value 'void_node'.  */
   hash_map<tree,tree> values;
 public:
   /* Number of cxx_eval_constant_expression calls (except skipped ones,
@@ -1170,17 +1171,28 @@  public:
     : constexpr_ops_count (0), cleanups (NULL), modifiable (nullptr),
       heap_dealloc_count (0) {}
 
+  bool is_outside_lifetime (tree t)
+  {
+    if (tree *p = values.get(t))
+      if (*p == void_node)
+	return true;
+    return false;
+  }
  tree get_value (tree t)
   {
     if (tree *p = values.get (t))
-      return *p;
+      if (*p != void_node)
+	return *p;
     return NULL_TREE;
   }
   tree *get_value_ptr (tree t)
   {
     if (modifiable && !modifiable->contains (t))
       return nullptr;
-    return values.get (t);
+    if (tree *p = values.get (t))
+      if (*p != void_node)
+	return p;
+    return nullptr;
   }
   void put_value (tree t, tree v)
   {
@@ -1188,7 +1200,13 @@  public:
     if (!already_in_map && modifiable)
       modifiable->add (t);
   }
-  void remove_value (tree t) { values.remove (t); }
+  void remove_value (tree t)
+  {
+    if (DECL_P (t))
+      values.put (t, void_node);
+    else
+      values.remove (t);
+  }
 };
 
 /* Helper class for constexpr_global_ctx.  In some cases we want to avoid
@@ -3085,12 +3103,9 @@  cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 	  gcc_assert (!DECL_BY_REFERENCE (res));
 	  ctx->global->put_value (res, NULL_TREE);
 
-	  /* Track the callee's evaluated SAVE_EXPRs and TARGET_EXPRs so that
-	     we can forget their values after the call.  */
-	  constexpr_ctx ctx_with_save_exprs = *ctx;
-	  auto_vec<tree, 10> save_exprs;
-	  ctx_with_save_exprs.save_exprs = &save_exprs;
-	  ctx_with_save_exprs.call = &new_call;
+	  /* Remember the current call we're evaluating.  */
+	  constexpr_ctx call_ctx = *ctx;
+	  call_ctx.call = &new_call;
 	  unsigned save_heap_alloc_count = ctx->global->heap_vars.length ();
 	  unsigned save_heap_dealloc_count = ctx->global->heap_dealloc_count;
 
@@ -3101,7 +3116,7 @@  cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 				      non_constant_p, overflow_p);
 
 	  tree jump_target = NULL_TREE;
-	  cxx_eval_constant_expression (&ctx_with_save_exprs, body,
+	  cxx_eval_constant_expression (&call_ctx, body,
 					vc_discard, non_constant_p, overflow_p,
 					&jump_target);
 
@@ -3157,15 +3172,7 @@  cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 	    cxx_set_object_constness (ctx, new_obj, /*readonly_p=*/true,
 				      non_constant_p, overflow_p);
 
-	  /* Forget the saved values of the callee's SAVE_EXPRs and
-	     TARGET_EXPRs.  */
-	  for (tree save_expr : save_exprs)
-	    ctx->global->remove_value (save_expr);
-
-	  /* Remove the parms/result from the values map.  Is it worth
-	     bothering to do this when the map itself is only live for
-	     one constexpr evaluation?  If so, maybe also clear out
-	     other vars from call, maybe in BIND_EXPR handling?  */
+	  /* Remove the parms/result from the values map.  */
 	  ctx->global->remove_value (res);
 	  for (tree parm = parms; parm; parm = TREE_CHAIN (parm))
 	    ctx->global->remove_value (parm);
@@ -5697,6 +5704,25 @@  cxx_eval_indirect_ref (const constexpr_ctx *ctx, tree t,
   return r;
 }
 
+/* Complain about R, a DECL that is accessed outside its lifetime.  */
+
+static void
+outside_lifetime_error (location_t loc, tree r)
+{
+  if (DECL_NAME (r) == heap_deleted_identifier)
+    {
+      /* Provide a more accurate message for deleted variables.  */
+      error_at (loc, "use of allocated storage after deallocation "
+		"in a constant expression");
+      inform (DECL_SOURCE_LOCATION (r), "allocated here");
+    }
+  else
+    {
+      error_at (loc, "accessing object outside its lifetime");
+      inform (DECL_SOURCE_LOCATION (r), "declared here");
+    }
+}
+
 /* Complain about R, a VAR_DECL, not being usable in a constant expression.
    FUNDEF_P is true if we're checking a constexpr function body.
    Shared between potential_constant_expression and
@@ -6602,7 +6628,6 @@  cxx_eval_loop_expr (const constexpr_ctx *ctx, tree t,
 		    bool *non_constant_p, bool *overflow_p,
 		    tree *jump_target)
 {
-  constexpr_ctx new_ctx = *ctx;
   tree local_target;
   if (!jump_target)
     {
@@ -6640,14 +6665,12 @@  cxx_eval_loop_expr (const constexpr_ctx *ctx, tree t,
     default:
       gcc_unreachable ();
     }
-  auto_vec<tree, 10> save_exprs;
-  new_ctx.save_exprs = &save_exprs;
   do
     {
       if (count != -1)
 	{
 	  if (body)
-	    cxx_eval_constant_expression (&new_ctx, body, vc_discard,
+	    cxx_eval_constant_expression (ctx, body, vc_discard,
 					  non_constant_p, overflow_p,
 					  jump_target);
 	  if (breaks (jump_target))
@@ -6660,7 +6683,7 @@  cxx_eval_loop_expr (const constexpr_ctx *ctx, tree t,
 	    *jump_target = NULL_TREE;
 
 	  if (expr)
-	    cxx_eval_constant_expression (&new_ctx, expr, vc_prvalue,
+	    cxx_eval_constant_expression (ctx, expr, vc_prvalue,
 					  non_constant_p, overflow_p,
 					  jump_target);
 	}
@@ -6668,7 +6691,7 @@  cxx_eval_loop_expr (const constexpr_ctx *ctx, tree t,
       if (cond)
 	{
 	  tree res
-	    = cxx_eval_constant_expression (&new_ctx, cond, vc_prvalue,
+	    = cxx_eval_constant_expression (ctx, cond, vc_prvalue,
 					    non_constant_p, overflow_p,
 					    jump_target);
 	  if (res)
@@ -6683,11 +6706,6 @@  cxx_eval_loop_expr (const constexpr_ctx *ctx, tree t,
 	    gcc_assert (*jump_target);
 	}
 
-      /* Forget saved values of SAVE_EXPRs and TARGET_EXPRs.  */
-      for (tree save_expr : save_exprs)
-	ctx->global->remove_value (save_expr);
-      save_exprs.truncate (0);
-
       if (++count >= constexpr_loop_limit)
 	{
 	  if (!ctx->quiet)
@@ -6705,10 +6723,6 @@  cxx_eval_loop_expr (const constexpr_ctx *ctx, tree t,
 	 && (!switches (jump_target) || count == 0)
 	 && !*non_constant_p);
 
-  /* Forget saved values of SAVE_EXPRs and TARGET_EXPRs.  */
-  for (tree save_expr : save_exprs)
-    ctx->global->remove_value (save_expr);
-
   return NULL_TREE;
 }
 
@@ -7051,10 +7065,17 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	return ctx->ctor;
       if (VAR_P (t))
 	if (tree v = ctx->global->get_value (t))
-	    {
-	      r = v;
-	      break;
-	    }
+	  {
+	    r = v;
+	    break;
+	  }
+      if (ctx->global->is_outside_lifetime (t))
+	{
+	  if (!ctx->quiet)
+	    outside_lifetime_error (loc, t);
+	  *non_constant_p = true;
+	  break;
+	}
       if (ctx->manifestly_const_eval == mce_true)
 	maybe_warn_about_constant_value (loc, t);
       if (COMPLETE_TYPE_P (TREE_TYPE (t))
@@ -7099,6 +7120,13 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	r = v;
       else if (lval)
 	/* Defer in case this is only used for its type.  */;
+      else if (ctx->global->is_outside_lifetime (t))
+	{
+	  if (!ctx->quiet)
+	    outside_lifetime_error (loc, t);
+	  *non_constant_p = true;
+	  break;
+	}
       else if (COMPLETE_TYPE_P (TREE_TYPE (t))
 	       && is_really_empty_class (TREE_TYPE (t), /*ignore_vptr*/false))
 	{
@@ -7338,17 +7366,28 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	auto_vec<tree, 2> cleanups;
 	vec<tree> *prev_cleanups = ctx->global->cleanups;
 	ctx->global->cleanups = &cleanups;
-	r = cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0),
+
+	auto_vec<tree, 10> save_exprs;
+	constexpr_ctx new_ctx = *ctx;
+	new_ctx.save_exprs = &save_exprs;
+
+	r = cxx_eval_constant_expression (&new_ctx, TREE_OPERAND (t, 0),
 					  lval,
 					  non_constant_p, overflow_p,
 					  jump_target);
+
 	ctx->global->cleanups = prev_cleanups;
 	unsigned int i;
 	tree cleanup;
 	/* Evaluate the cleanups.  */
 	FOR_EACH_VEC_ELT_REVERSE (cleanups, i, cleanup)
-	  cxx_eval_constant_expression (ctx, cleanup, vc_discard,
+	  cxx_eval_constant_expression (&new_ctx, cleanup, vc_discard,
 					non_constant_p, overflow_p);
+
+	/* Forget SAVE_EXPRs and TARGET_EXPRs created by this
+	   full-expression.  */
+	for (tree save_expr : save_exprs)
+	  ctx->global->remove_value (save_expr);
       }
       break;
 
@@ -7864,10 +7903,13 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 				      non_constant_p, overflow_p, jump_target);
 
     case BIND_EXPR:
-      return cxx_eval_constant_expression (ctx, BIND_EXPR_BODY (t),
-					   lval,
-					   non_constant_p, overflow_p,
-					   jump_target);
+      r = cxx_eval_constant_expression (ctx, BIND_EXPR_BODY (t),
+					lval,
+					non_constant_p, overflow_p,
+					jump_target);
+      for (tree decl = BIND_EXPR_VARS (t); decl; decl = DECL_CHAIN (decl))
+	ctx->global->remove_value (decl);
+      return r;
 
     case PREINCREMENT_EXPR:
     case POSTINCREMENT_EXPR:
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C
new file mode 100644
index 00000000000..43aa7c974c1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime1.C
@@ -0,0 +1,13 @@ 
+// PR c++/96630
+// { dg-do compile { target c++14 } }
+
+struct S {
+  int x = 0;
+  constexpr const int& get() const { return x; }
+};
+
+constexpr const int& test() {
+  auto local = S{};  // { dg-message "note: declared here" }
+  return local.get();
+}
+constexpr int x = test();  // { dg-error "accessing object outside its lifetime" }
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C
new file mode 100644
index 00000000000..22cd919fcda
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime2.C
@@ -0,0 +1,20 @@ 
+// PR c++/98675
+// { dg-do compile { target c++14 } }
+
+struct S {
+  int x = 0;
+  constexpr const int& get() const { return x; }
+};
+
+constexpr int error() {
+  const auto& local = S{}.get();  // { dg-message "note: declared here" }
+  return local;
+}
+constexpr int x = error();  // { dg-error "accessing object outside its lifetime" }
+
+constexpr int ok() {
+  // temporary should only be destroyed after end of full-expression
+  auto local = S{}.get();
+  return local;
+}
+constexpr int y = ok();
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C
new file mode 100644
index 00000000000..6329f8cf6c6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime3.C
@@ -0,0 +1,13 @@ 
+// PR c++/70331
+// { dg-do compile { target c++14 } }
+
+constexpr int f(int i) {
+  int *p = &i;
+  if (i == 0) {
+    int j = 123;  // { dg-message "note: declared here" }
+    p = &j;
+  }
+  return *p;
+}
+
+constexpr int i = f(0);  // { dg-error "accessing object outside its lifetime" }
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C
new file mode 100644
index 00000000000..181a1201663
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime4.C
@@ -0,0 +1,11 @@ 
+// { dg-do compile { target c++14 } }
+
+constexpr const double& test() {
+  const double& local = 3.0;  // { dg-message "note: declared here" }
+  return local;
+}
+
+static_assert(test() == 3.0, "");  // { dg-error "constant|accessing object outside its lifetime" }
+
+// no deference, shouldn't error
+static_assert((test(), true), "");
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime5.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime5.C
new file mode 100644
index 00000000000..a4bc71d890a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime5.C
@@ -0,0 +1,11 @@ 
+// { dg-do compile { target c++14 } }
+// { dg-options "-Wno-return-local-addr" }
+
+constexpr const int& id(int x) { return x; }
+
+constexpr bool test() {
+  const int& y = id(3);
+  return y == 3;
+}
+
+constexpr bool x = test();  // { dg-error "" }
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime6.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime6.C
new file mode 100644
index 00000000000..f358aff4490
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime6.C
@@ -0,0 +1,15 @@ 
+// { dg-do compile { target c++14 } }
+// { dg-options "-Wno-return-local-addr" }
+
+struct Empty {};
+
+constexpr const Empty& empty() {
+  return Empty{};
+}
+
+constexpr const Empty& empty_parm(Empty e) {
+  return e;
+}
+
+constexpr Empty a = empty();  // { dg-error "" }
+constexpr Empty b = empty_parm({});  // { dg-error "" }