c++: End lifetime of objects in constexpr after destructor call [PR71093]

This patch adds checks for using objects after they've been manually
destroyed via explicit destructor call. Currently this is only
implemented for 'top-level' objects; FIELD_DECLs and individual elements
of arrays will need a lot more work to track correctly and are left for
a future patch.

The other limitation is that destruction of parameter objects is checked
too 'early', happening at the end of the function call rather than the
end of the owning full-expression as they should be for consistency;
see cpp2a/constexpr-lifetime2.C. This is because I wasn't able to find a
good way to link the constructed parameter declarations with the
variable declarations that are actually destroyed later on to propagate
their lifetime status, so I'm leaving this for a later patch.

	PR c++/71093

gcc/cp/ChangeLog:

	* constexpr.cc (constexpr_global_ctx::get_value_ptr): Don't
	return NULL_TREE for objects we're initializing.
	(constexpr_global_ctx::destroy_value): Rename from remove_value.
	Only mark real variables as outside lifetime.
	(constexpr_global_ctx::clear_value): New function.
	(destroy_value_checked): New function.
	(cxx_eval_call_expression): Defer complaining about non-constant
	arg0 for operator delete. Use remove_value_safe.
	(cxx_fold_indirect_ref_1): Handle conversion to 'as base' type.
	(outside_lifetime_error): Include name of object we're
	accessing.
	(cxx_eval_store_expression): Handle clobbers. Improve error
	messages.
	(cxx_eval_constant_expression): Use remove_value_safe. Clear
	bind variables before entering body.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp1y/constexpr-lifetime1.C: Improve error message.
	* g++.dg/cpp1y/constexpr-lifetime2.C: Likewise.
	* g++.dg/cpp1y/constexpr-lifetime3.C: Likewise.
	* g++.dg/cpp1y/constexpr-lifetime4.C: Likewise.
	* g++.dg/cpp2a/bitfield2.C: Likewise.
	* g++.dg/cpp2a/constexpr-new3.C: Likewise. New check.
	* g++.dg/cpp1y/constexpr-lifetime7.C: New test.
	* g++.dg/cpp2a/constexpr-lifetime1.C: New test.
	* g++.dg/cpp2a/constexpr-lifetime2.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>

1 files changed, 126 insertions, 22 deletions

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 9d9e96c..e1b2d27 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -1193,13 +1193,20 @@ public:
 	return *p;
     return NULL_TREE;
   }
-  tree *get_value_ptr (tree t)
+  tree *get_value_ptr (tree t, bool initializing)
   {
     if (modifiable && !modifiable->contains (t))
       return nullptr;
     if (tree *p = values.get (t))
-      if (*p != void_node)
-	return p;
+      {
+	if (*p != void_node)
+	  return p;
+	else if (initializing)
+	  {
+	    *p = NULL_TREE;
+	    return p;
+	  }
+      }
     return nullptr;
   }
   void put_value (tree t, tree v)
@@ -1208,13 +1215,19 @@ public:
     if (!already_in_map && modifiable)
       modifiable->add (t);
   }
-  void remove_value (tree t)
+  void destroy_value (tree t)
   {
-    if (DECL_P (t))
+    if (TREE_CODE (t) == VAR_DECL
+	|| TREE_CODE (t) == PARM_DECL
+	|| TREE_CODE (t) == RESULT_DECL)
       values.put (t, void_node);
     else
       values.remove (t);
   }
+  void clear_value (tree t)
+  {
+    values.remove (t);
+  }
 };
 
 /* Helper class for constexpr_global_ctx.  In some cases we want to avoid
@@ -1238,7 +1251,7 @@ public:
   ~modifiable_tracker ()
   {
     for (tree t: set)
-      global->remove_value (t);
+      global->clear_value (t);
     global->modifiable = nullptr;
   }
 };
@@ -1278,6 +1291,40 @@ struct constexpr_ctx {
   mce_value manifestly_const_eval;
 };
 
+/* Remove T from the global values map, checking for attempts to destroy
+   a value that has already finished its lifetime.  */
+
+static void
+destroy_value_checked (const constexpr_ctx* ctx, tree t, bool *non_constant_p)
+{
+  if (t == error_mark_node || TREE_TYPE (t) == error_mark_node)
+    return;
+
+  /* Don't error again here if we've already reported a problem.  */
+  if (!*non_constant_p
+      && DECL_P (t)
+      /* Non-trivial destructors have their lifetimes ended explicitly
+	 with a clobber, so don't worry about it here.  */
+      && (!TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (t))
+	  /* ...except parameters are remapped in cxx_eval_call_expression,
+	     and the destructor call during cleanup won't be able to tell that
+	     this value has already been destroyed, so complain now.  This is
+	     not quite unobservable, but is extremely unlikely to crop up in
+	     practice; see g++.dg/cpp2a/constexpr-lifetime2.C.  */
+	  || TREE_CODE (t) == PARM_DECL)
+      && ctx->global->is_outside_lifetime (t))
+    {
+      if (!ctx->quiet)
+	{
+	  auto_diagnostic_group d;
+	  error ("destroying %qE outside its lifetime", t);
+	  inform (DECL_SOURCE_LOCATION (t), "declared here");
+	}
+      *non_constant_p = true;
+    }
+  ctx->global->destroy_value (t);
+}
+
 /* This internal flag controls whether we should avoid doing anything during
    constexpr evaluation that would cause extra DECL_UID generation, such as
    template instantiation and function body copying.  */
@@ -2806,6 +2853,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 	  && (CALL_FROM_NEW_OR_DELETE_P (t)
 	      || is_std_allocator_allocate (ctx->call)))
 	{
+	  const bool new_op_p = IDENTIFIER_NEW_OP_P (DECL_NAME (fun));
 	  const int nargs = call_expr_nargs (t);
 	  tree arg0 = NULL_TREE;
 	  for (int i = 0; i < nargs; ++i)
@@ -2813,12 +2861,15 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 	      tree arg = CALL_EXPR_ARG (t, i);
 	      arg = cxx_eval_constant_expression (ctx, arg, vc_prvalue,
 						  non_constant_p, overflow_p);
-	      VERIFY_CONSTANT (arg);
+	      /* Deleting a non-constant pointer has a better error message
+		 below.  */
+	      if (new_op_p || i != 0)
+		VERIFY_CONSTANT (arg);
 	      if (i == 0)
 		arg0 = arg;
 	    }
 	  gcc_assert (arg0);
-	  if (IDENTIFIER_NEW_OP_P (DECL_NAME (fun)))
+	  if (new_op_p)
 	    {
 	      tree type = build_array_type_nelts (char_type_node,
 						  tree_to_uhwi (arg0));
@@ -2867,7 +2918,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 			  return t;
 			}
 		      DECL_NAME (var) = heap_deleted_identifier;
-		      ctx->global->remove_value (var);
+		      ctx->global->destroy_value (var);
 		      ctx->global->heap_dealloc_count++;
 		      return void_node;
 		    }
@@ -2890,7 +2941,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 			  return t;
 			}
 		      DECL_NAME (var) = heap_deleted_identifier;
-		      ctx->global->remove_value (var);
+		      ctx->global->destroy_value (var);
 		      ctx->global->heap_dealloc_count++;
 		      return void_node;
 		    }
@@ -3255,9 +3306,9 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t,
 				      non_constant_p, overflow_p);
 
 	  /* Remove the parms/result from the values map.  */
-	  ctx->global->remove_value (res);
+	  destroy_value_checked (ctx, res, non_constant_p);
 	  for (tree parm = parms; parm; parm = TREE_CHAIN (parm))
-	    ctx->global->remove_value (parm);
+	    destroy_value_checked (ctx, parm, non_constant_p);
 
 	  /* Free any parameter CONSTRUCTORs we aren't returning directly.  */
 	  while (!ctors->is_empty ())
@@ -5657,6 +5708,10 @@ cxx_fold_indirect_ref_1 (const constexpr_ctx *ctx, location_t loc, tree type,
 	      }
 	  }
 
+      /* Handle conversion to "as base" type.  */
+      if (CLASSTYPE_AS_BASE (optype) == type)
+	return op;
+
       /* Handle conversion to an empty base class, which is represented with a
 	 NOP_EXPR.  Do this before spelunking into the non-empty subobjects,
 	 which is likely to be a waste of time (109678).  */
@@ -5908,7 +5963,7 @@ outside_lifetime_error (location_t loc, tree r)
     }
   else
     {
-      error_at (loc, "accessing object outside its lifetime");
+      error_at (loc, "accessing %qE outside its lifetime", r);
       inform (DECL_SOURCE_LOCATION (r), "declared here");
     }
 }
@@ -6125,8 +6180,10 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
   constexpr_ctx new_ctx = *ctx;
 
   tree init = TREE_OPERAND (t, 1);
-  if (TREE_CLOBBER_P (init))
-    /* Just ignore clobbers.  */
+
+  if (TREE_CLOBBER_P (init)
+      && CLOBBER_KIND (init) < CLOBBER_OBJECT_END)
+    /* Only handle clobbers ending the lifetime of objects.  */
     return void_node;
 
   /* First we figure out where we're storing to.  */
@@ -6136,7 +6193,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
 
   tree type = TREE_TYPE (target);
   bool preeval = SCALAR_TYPE_P (type) || TREE_CODE (t) == MODIFY_EXPR;
-  if (preeval)
+  if (preeval && !TREE_CLOBBER_P (init))
     {
       /* Evaluate the value to be stored without knowing what object it will be
 	 stored in, so that any side-effects happen first.  */
@@ -6244,11 +6301,18 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
       && const_object_being_modified == NULL_TREE)
     const_object_being_modified = object;
 
+  if (DECL_P (object)
+      && TREE_CLOBBER_P (init)
+      && DECL_NAME (object) == heap_deleted_identifier)
+    /* Ignore clobbers of deleted allocations for now; we'll get a better error
+       message later when operator delete is called.  */
+    return void_node;
+
   /* And then find/build up our initializer for the path to the subobject
      we're initializing.  */
   tree *valp;
   if (DECL_P (object))
-    valp = ctx->global->get_value_ptr (object);
+    valp = ctx->global->get_value_ptr (object, TREE_CODE (t) == INIT_EXPR);
   else
     valp = NULL;
   if (!valp)
@@ -6256,10 +6320,45 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
       /* A constant-expression cannot modify objects from outside the
 	 constant-expression.  */
       if (!ctx->quiet)
-	error ("modification of %qE is not a constant expression", object);
+	{
+	  auto_diagnostic_group d;
+	  if (DECL_P (object) && DECL_NAME (object) == heap_deleted_identifier)
+	    {
+	      error ("modification of allocated storage after deallocation "
+		     "is not a constant expression");
+	      inform (DECL_SOURCE_LOCATION (object), "allocated here");
+	    }
+	  else if (DECL_P (object) && ctx->global->is_outside_lifetime (object))
+	    {
+	      if (TREE_CLOBBER_P (init))
+		error ("destroying %qE outside its lifetime", object);
+	      else
+		error ("modification of %qE outside its lifetime "
+		       "is not a constant expression", object);
+	      inform (DECL_SOURCE_LOCATION (object), "declared here");
+	    }
+	  else
+	    {
+	      if (TREE_CLOBBER_P (init))
+		error ("destroying %qE from outside current evaluation "
+		       "is not a constant expression", object);
+	      else
+		error ("modification of %qE from outside current evaluation "
+		       "is not a constant expression", object);
+	    }
+	}
       *non_constant_p = true;
       return t;
     }
+
+  /* Handle explicit end-of-lifetime.  */
+  if (TREE_CLOBBER_P (init))
+    {
+      if (refs->is_empty ())
+	ctx->global->destroy_value (object);
+      return void_node;
+    }
+
   type = TREE_TYPE (object);
   bool no_zero_init = true;
 
@@ -6533,7 +6632,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
       /* The hash table might have moved since the get earlier, and the
 	 initializer might have mutated the underlying CONSTRUCTORs, so we must
 	 recompute VALP. */
-      valp = ctx->global->get_value_ptr (object);
+      valp = ctx->global->get_value_ptr (object, TREE_CODE (t) == INIT_EXPR);
       for (unsigned i = 0; i < vec_safe_length (indexes); i++)
 	{
 	  ctors[i] = valp;
@@ -7650,7 +7749,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	/* Forget SAVE_EXPRs and TARGET_EXPRs created by this
 	   full-expression.  */
 	for (tree save_expr : save_exprs)
-	  ctx->global->remove_value (save_expr);
+	  destroy_value_checked (ctx, save_expr, non_constant_p);
       }
       break;
 
@@ -8203,13 +8302,18 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 				      non_constant_p, overflow_p, jump_target);
 
     case BIND_EXPR:
+      /* Pre-emptively clear the vars declared by this BIND_EXPR from the value
+	 map, so that when checking whether they're already destroyed later we
+	 don't get confused by remnants of previous calls.  */
+      for (tree decl = BIND_EXPR_VARS (t); decl; decl = DECL_CHAIN (decl))
+	ctx->global->clear_value (decl);
       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;
+	destroy_value_checked (ctx, decl, non_constant_p);
+      break;
 
     case PREINCREMENT_EXPR:
     case POSTINCREMENT_EXPR: