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: Nathaniel Shead <nathanieloshead@gmail.com>
Subject: [PATCH v2 1/3] c++: Track lifetimes in constant evaluation [PR70331,PR96630,PR98675]
Date: Wed, 29 Mar 2023 13:32:56 +1100	[thread overview]
Message-ID: <20230329023258.13487-2-nathanieloshead@gmail.com> (raw)
In-Reply-To: <20230329023258.13487-1-nathanieloshead@gmail.com>

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 ending
lifetimes (e.g. explicit destructor calls).

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

gcc/cp/ChangeLog:

	* constexpr.cc (constexpr_global_ctx::put_value): Mark value as
	in lifetime.
	(constexpr_global_ctx::remove_value): Mark value as expired.
	(cxx_eval_call_expression): Remove comment that is no longer
	applicable.
	(non_const_var_error): Add check for expired values.
	(cxx_eval_constant_expression): Add checks for expired values. Forget
	local variables at end of bind expressions. Forget temporaries at end
	of cleanup points.
	* cp-tree.h (struct lang_decl_base): New flag to track expired values
	in constant evaluation.
	(DECL_EXPIRED_P): Access the new flag.
	(SET_DECL_EXPIRED_P): Modify the new flag.
	* module.cc (trees_out::lang_decl_bools): Write out the new flag.
	(trees_in::lang_decl_bools): Read in the new flag.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/constexpr-ice20.C: Update error raised by test.
	* 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.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/constexpr.cc                           | 69 +++++++++++++++----
 gcc/cp/cp-tree.h                              | 10 ++-
 gcc/cp/module.cc                              |  2 +
 gcc/testsuite/g++.dg/cpp0x/constexpr-ice20.C  |  2 +-
 .../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 +++
 9 files changed, 137 insertions(+), 14 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

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 3de60cfd0f8..bdbc12144a7 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -1185,10 +1185,22 @@ public:
   void put_value (tree t, tree v)
   {
     bool already_in_map = values.put (t, v);
+    if (!already_in_map && DECL_P (t))
+      {
+	if (!DECL_LANG_SPECIFIC (t))
+	  retrofit_lang_decl (t);
+	if (DECL_LANG_SPECIFIC (t))
+	  SET_DECL_EXPIRED_P (t, false);
+      }
     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) && DECL_LANG_SPECIFIC (t))
+      SET_DECL_EXPIRED_P (t, true);
+    values.remove (t);
+  }
 };
 
 /* Helper class for constexpr_global_ctx.  In some cases we want to avoid
@@ -3157,10 +3169,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 	  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);
@@ -5708,6 +5717,13 @@ non_const_var_error (location_t loc, tree r, bool fundef_p)
 	inform (DECL_SOURCE_LOCATION (r), "allocated here");
       return;
     }
+  if (DECL_EXPIRED_P (r))
+    {
+      if (constexpr_error (loc, fundef_p, "accessing object outside its "
+			   "lifetime"))
+	inform (DECL_SOURCE_LOCATION (r), "declared here");
+      return;
+    }
   if (!constexpr_error (loc, fundef_p, "the value of %qD is not usable in "
 			"a constant expression", r))
     return;
@@ -7048,6 +7064,13 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	  r = build_constructor (TREE_TYPE (t), NULL);
 	  TREE_CONSTANT (r) = true;
 	}
+      else if (DECL_EXPIRED_P (t))
+	{
+	  if (!ctx->quiet)
+	    non_const_var_error (loc, r, /*fundef_p*/false);
+	  *non_constant_p = true;
+	  break;
+	}
       else if (ctx->strict)
 	r = decl_really_constant_value (t, /*unshare_p=*/false);
       else
@@ -7093,7 +7116,15 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
       else
 	{
 	  if (!ctx->quiet)
-	    error ("%qE is not a constant expression", t);
+	    {
+	      if (DECL_EXPIRED_P (r))
+		{
+		  error_at (loc, "accessing object outside its lifetime");
+		  inform (DECL_SOURCE_LOCATION (r), "declared here");
+		}
+	      else
+		error_at (loc, "%qE is not a constant expression", t);
+	    }
 	  *non_constant_p = true;
 	}
       break;
@@ -7315,17 +7346,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;
 
@@ -7831,10 +7873,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/cp/cp-tree.h b/gcc/cp/cp-tree.h
index b74c18b03ad..3cc08da816f 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -2860,6 +2860,7 @@ struct GTY(()) lang_decl_base {
   unsigned concept_p : 1;                  /* applies to vars and functions */
   unsigned var_declared_inline_p : 1;	   /* var */
   unsigned dependent_init_p : 1;	   /* var */
+  unsigned expired_p : 1;		   /* var or parm */
 
   /* The following apply to VAR, FUNCTION, TYPE, CONCEPT, & NAMESPACE
      decls.  */
@@ -2871,7 +2872,7 @@ struct GTY(()) lang_decl_base {
   /* VAR_DECL or FUNCTION_DECL has keyed decls.     */
   unsigned module_keyed_decls_p : 1;
 
-  /* 12 spare bits.  */
+  /* 11 spare bits.  */
 };
 
 /* True for DECL codes which have template info and access.  */
@@ -4366,6 +4367,13 @@ get_vec_init_expr (tree t)
 #define SET_DECL_DEPENDENT_INIT_P(NODE, X) \
   (DECL_LANG_SPECIFIC (VAR_DECL_CHECK (NODE))->u.base.dependent_init_p = (X))
 
+/* Nonzero if NODE is a VAR_DECL, PARM_DECL, or FIELD_DECL that is within
+   its lifetime for constant evaluation purposes.  */
+#define DECL_EXPIRED_P(NODE) \
+  (DECL_LANG_SPECIFIC (NODE) && DECL_LANG_SPECIFIC (NODE)->u.base.expired_p)
+#define SET_DECL_EXPIRED_P(NODE, X) \
+  (DECL_LANG_SPECIFIC (NODE)->u.base.expired_p = (X))
+
 /* Nonzero if NODE is an artificial VAR_DECL for a C++17 structured binding
    declaration or one of VAR_DECLs for the user identifiers in it.  */
 #define DECL_DECOMPOSITION_P(NODE) \
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index ac2fe66b080..7af43b5736d 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -5654,6 +5654,7 @@ trees_out::lang_decl_bools (tree t)
   WB (lang->u.base.concept_p);
   WB (lang->u.base.var_declared_inline_p);
   WB (lang->u.base.dependent_init_p);
+  WB (lang->u.base.expired_p);
   /* When building a header unit, everthing is marked as purview, (so
      we know which decls to write).  But when we import them we do not
      want to mark them as in module purview.  */
@@ -5728,6 +5729,7 @@ trees_in::lang_decl_bools (tree t)
   RB (lang->u.base.concept_p);
   RB (lang->u.base.var_declared_inline_p);
   RB (lang->u.base.dependent_init_p);
+  RB (lang->u.base.expired_p);
   RB (lang->u.base.module_purview_p);
   RB (lang->u.base.module_attach_p);
   if (VAR_OR_FUNCTION_DECL_P (t))
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-ice20.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-ice20.C
index e2d4853a284..ebaa95e5324 100644
--- a/gcc/testsuite/g++.dg/cpp0x/constexpr-ice20.C
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-ice20.C
@@ -4,4 +4,4 @@
 typedef bool (*Function)(int);
 constexpr bool check(int x, Function p) { return p(x); }  // { dg-message "in .constexpr. expansion of" }
 
-static_assert(check(2, check), "");  // { dg-error "conversion|constant|in .constexpr. expansion of" }
+static_assert(check(2, check), "");  // { dg-error "conversion|constant|lifetime|in .constexpr. expansion of" }
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 "" }
-- 
2.34.1


  reply	other threads:[~2023-03-29  2:33 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-29  2:32 [PATCH v2 0/3] c++: Track lifetimes in constant evaluation [PR70331,...] Nathaniel Shead
2023-03-29  2:32 ` Nathaniel Shead [this message]
2023-06-23 16:43   ` [PATCH v2 1/3] c++: Track lifetimes in constant evaluation [PR70331, PR96630, PR98675] Patrick Palka
2023-06-24 14:02     ` Nathaniel Shead
2023-06-26 19:37       ` Patrick Palka
2023-06-30 15:45         ` Nathaniel Shead
2023-03-29  2:32 ` [PATCH v2 2/3] c++: Improve constexpr error for dangling local variables Nathaniel Shead
2023-06-23 16:57   ` Patrick Palka
2023-03-29  2:32 ` [PATCH v2 3/3] c++: Improve location information in constexpr evaluation Nathaniel Shead
2023-06-23 17:09   ` Patrick Palka
2023-06-30 15:50     ` Nathaniel Shead
2023-05-08 10:48 ` [PATCH v2 0/3] c++: Track lifetimes in constant evaluation [PR70331,...] Nathaniel Shead
2023-06-13 10:11 ` 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=20230329023258.13487-2-nathanieloshead@gmail.com \
    --to=nathanieloshead@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    /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).