diff mbox series

[C++] Avoid caching constexpr calls that allocate something that they don't deallocate or vice versa (PR c++/91369)

Message ID 20200105130447.GV10088@tucnak
State New
Headers show
Series [C++] Avoid caching constexpr calls that allocate something that they don't deallocate or vice versa (PR c++/91369) | expand

Commit Message

Jakub Jelinek Jan. 5, 2020, 1:04 p.m. UTC 
Hi!

The caching of constexpr calls breaks the following testcase.
The problem is that constexpr calls that allocate from heap something
that they don't deallocate or calls that deallocate something they haven't
allocated aren't stateless for the constexpr context and in each outermost
constexpr evaluation the allocations will produce different artificial heap
VAR_DECLs.  In the testcase a function allocates something that the caller
is supposed to deallocate, so when we call it second time (e.g. from
different function) with the same constant arguments, we get the cached
result that contains address of the heap VAR_DECL from the earlier
evaluation and error trying to free something we haven't allocated (in the
same outermost constexpr evaluation).  Similarly, a constexpr call could
deallocate something the caller is supposed to allocate, if we cache such
call, we wouldn't actually deallocate it and complain the allocation has not
been deallocated.

Fixed by making sure we only cache calls that either don't perform any
allocations/deallocations, or perform exactly as many allocations and
deallocations and all those new allocations (new additions to heap_vars,
to which we only add, never remove) have been deallocated (checking just
numbers is not good enough, we might perform 3 allocations and 3
deallocations, e.g. deallocate 2 of those allocations and one passed to the
call from the caller and pass the still allocated address to the caller.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2020-01-05  Jakub Jelinek  <jakub@redhat.com>

	PR c++/91369
	* constexpr.c (struct constexpr_global_ctx): Add heap_alloc_count
	member, initialize it to zero in ctor.
	(cxx_eval_call_expression): Bump heap_dealloc_count when deleting
	a heap object.  Don't cache calls to functions which allocate some
	heap objects and don't deallocate them or deallocate some heap
	objects they didn't allocate.

	* g++.dg/cpp1y/constexpr-new.C: Expect an error explaining why
	static_assert failed for C++2a.
	* g++.dg/cpp2a/constexpr-new9.C: New test.


	Jakub

Comments

Jason Merrill Jan. 6, 2020, 10:29 p.m. UTC | #1 
On 1/5/20 8:04 AM, Jakub Jelinek wrote:
> Hi!
> 
> The caching of constexpr calls breaks the following testcase.
> The problem is that constexpr calls that allocate from heap something
> that they don't deallocate or calls that deallocate something they haven't
> allocated aren't stateless for the constexpr context and in each outermost
> constexpr evaluation the allocations will produce different artificial heap
> VAR_DECLs.  In the testcase a function allocates something that the caller
> is supposed to deallocate, so when we call it second time (e.g. from
> different function) with the same constant arguments, we get the cached
> result that contains address of the heap VAR_DECL from the earlier
> evaluation and error trying to free something we haven't allocated (in the
> same outermost constexpr evaluation).  Similarly, a constexpr call could
> deallocate something the caller is supposed to allocate, if we cache such
> call, we wouldn't actually deallocate it and complain the allocation has not
> been deallocated.
> 
> Fixed by making sure we only cache calls that either don't perform any
> allocations/deallocations, or perform exactly as many allocations and
> deallocations and all those new allocations (new additions to heap_vars,
> to which we only add, never remove) have been deallocated (checking just
> numbers is not good enough, we might perform 3 allocations and 3
> deallocations, e.g. deallocate 2 of those allocations and one passed to the
> call from the caller and pass the still allocated address to the caller.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

> 2020-01-05  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/91369
> 	* constexpr.c (struct constexpr_global_ctx): Add heap_alloc_count
> 	member, initialize it to zero in ctor.
> 	(cxx_eval_call_expression): Bump heap_dealloc_count when deleting
> 	a heap object.  Don't cache calls to functions which allocate some
> 	heap objects and don't deallocate them or deallocate some heap
> 	objects they didn't allocate.
diff mbox series

Patch

--- gcc/cp/constexpr.c.jj	2020-01-01 12:22:34.002475595 +0100
+++ gcc/cp/constexpr.c	2020-01-04 11:56:07.619124987 +0100
@@ -1041,8 +1041,11 @@  struct constexpr_global_ctx {
   auto_vec<tree, 16> heap_vars;
   /* Cleanups that need to be evaluated at the end of CLEANUP_POINT_EXPR.  */
   vec<tree> *cleanups;
+  /* Number of heap VAR_DECL deallocations.  */
+  unsigned heap_dealloc_count;
   /* Constructor.  */
-  constexpr_global_ctx () : constexpr_ops_count (0), cleanups (NULL) {}
+  constexpr_global_ctx ()
+    : constexpr_ops_count (0), cleanups (NULL), heap_dealloc_count (0) {}
 };
 
 /* The constexpr expansion context.  CALL is the current function
@@ -2056,6 +2059,7 @@  cxx_eval_call_expression (const constexp
 		    {
 		      DECL_NAME (var) = heap_deleted_identifier;
 		      ctx->global->values.remove (var);
+		      ctx->global->heap_dealloc_count++;
 		      return void_node;
 		    }
 		  else if (DECL_NAME (var) == heap_deleted_identifier)
@@ -2281,6 +2285,7 @@  cxx_eval_call_expression (const constexp
     }
   else
     {
+      bool cacheable = true;
       if (result && result != error_mark_node)
 	/* OK */;
       else if (!DECL_SAVED_TREE (fun))
@@ -2346,6 +2351,8 @@  cxx_eval_call_expression (const constexp
 	  auto_vec<tree, 10> save_exprs;
 	  ctx_with_save_exprs.save_exprs = &save_exprs;
 	  ctx_with_save_exprs.call = &new_call;
+	  unsigned save_heap_alloc_count = ctx->global->heap_vars.length ();
+	  unsigned save_heap_dealloc_count = ctx->global->heap_dealloc_count;
 
 	  tree jump_target = NULL_TREE;
 	  cxx_eval_constant_expression (&ctx_with_save_exprs, body,
@@ -2417,6 +2424,33 @@  cxx_eval_call_expression (const constexp
 
 	  /* Make the unshared function copy we used available for re-use.  */
 	  save_fundef_copy (fun, copy);
+
+	  /* If the call allocated some heap object that hasn't been
+	     deallocated during the call, or if it deallocated some heap
+	     object it has not allocated, the call isn't really stateless
+	     for the constexpr evaluation and should not be cached.
+	     It is fine if the call allocates something and deallocates it
+	     too.  */
+	  if (entry
+	      && (save_heap_alloc_count != ctx->global->heap_vars.length ()
+		  || (save_heap_dealloc_count
+		      != ctx->global->heap_dealloc_count)))
+	    {
+	      tree heap_var;
+	      unsigned int i;
+	      if ((ctx->global->heap_vars.length ()
+		   - ctx->global->heap_dealloc_count)
+		  != save_heap_alloc_count - save_heap_dealloc_count)
+		cacheable = false;
+	      else
+		FOR_EACH_VEC_ELT_FROM (ctx->global->heap_vars, i, heap_var,
+				       save_heap_alloc_count)
+		  if (DECL_NAME (heap_var) != heap_deleted_identifier)
+		    {
+		      cacheable = false;
+		      break;
+		    }
+	    }
 	}
 
       if (result == error_mark_node)
@@ -2426,7 +2460,7 @@  cxx_eval_call_expression (const constexp
       else if (!result)
 	result = void_node;
       if (entry)
-	entry->result = result;
+	entry->result = cacheable ? result : error_mark_node;
     }
 
   /* The result of a constexpr function must be completely initialized.
--- gcc/testsuite/g++.dg/cpp1y/constexpr-new.C.jj	2019-10-05 09:36:39.250674497 +0200
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-new.C	2020-01-04 12:15:20.628862418 +0100
@@ -5,7 +5,7 @@  constexpr int *f4(bool b) {
     return nullptr;
   } else {
     return new int{42}; // { dg-error "call to non-.constexpr." "" { target c++17_down } }
-  }
+  }			// { dg-error "is not a constant expression because allocated storage has not been deallocated" "" { target c++2a } .-1 }
 }
 static_assert(f4(true) == nullptr, "");
 static_assert(f4(false) == nullptr, ""); // { dg-error "non-.constant. condition|" }
--- gcc/testsuite/g++.dg/cpp2a/constexpr-new9.C.jj	2020-01-04 11:58:16.365198021 +0100
+++ gcc/testsuite/g++.dg/cpp2a/constexpr-new9.C	2020-01-04 11:58:11.861265422 +0100
@@ -0,0 +1,15 @@ 
+// PR c++/91369
+// { dg-do compile { target c++2a } }
+
+struct S {
+  constexpr S (int *i) : i{i} {}
+  constexpr ~S () { delete[] i; }
+  int *i;
+};
+
+constexpr S foo (int x) { return { new int[x] () }; }
+constexpr bool bar () { foo (1); return true; }
+constexpr bool baz () { foo (1); return false; }
+
+static_assert (bar ());
+static_assert (!baz ());