public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Nathaniel Shead <nathanieloshead@gmail.com>
To: gcc-patches@gcc.gnu.org
Cc: Jason Merrill <jason@redhat.com>, Patrick Palka <ppalka@redhat.com>
Subject: [PATCH v4 1/3] c++: Track lifetimes in constant evaluation [PR70331,PR96630,PR98675]
Date: Thu, 20 Jul 2023 19:35:35 +1000	[thread overview]
Message-ID: <ZLj/54VvX7Xz7wRk@Thaum.localdomain> (raw)
In-Reply-To: <ZLj/dOJSQA37eKUE@Thaum.localdomain>

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

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 "" }
-- 
2.41.0


  reply	other threads:[~2023-07-20  9:35 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-20  9:33 [PATCH v4 0/3] c++: Track lifetimes in constant evaluation [PR70331, ...] Nathaniel Shead
2023-07-20  9:35 ` Nathaniel Shead [this message]
2023-07-20 14:42   ` [PATCH v4 1/3] c++: Track lifetimes in constant evaluation [PR70331,PR96630,PR98675] Jason Merrill
2023-07-22  5:28     ` Nathaniel Shead
2023-07-20  9:36 ` [PATCH v4 2/3] c++: Improve constexpr error for dangling local variables [PR110619] Nathaniel Shead
2023-07-20 15:46   ` Jason Merrill
2023-07-21  5:39     ` Nathaniel Shead
2023-07-21 21:44       ` Jason Merrill
2023-07-22  5:20         ` Nathaniel Shead
2023-07-20  9:37 ` [PATCH v4 3/3] c++: Improve location information in constant evaluation Nathaniel Shead
2023-07-20 17:00   ` Jason Merrill
2023-07-22  5:26     ` Nathaniel Shead

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZLj/54VvX7Xz7wRk@Thaum.localdomain \
    --to=nathanieloshead@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    --cc=ppalka@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).