[clang] [clang][bytecode] Fix an assertion failure with double deletions (PR #206424)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

<details>
<summary>Changes</summary>

See the comment.

---
Full diff: https://github.com/llvm/llvm-project/pull/206424.diff


5 Files Affected:

- (modified) clang/lib/AST/ByteCode/DynamicAllocator.cpp (+5-2) 
- (modified) clang/lib/AST/ByteCode/DynamicAllocator.h (+1-2) 
- (modified) clang/lib/AST/ByteCode/Interp.cpp (+1-1) 
- (modified) clang/lib/AST/ByteCode/InterpBuiltin.cpp (+1-1) 
- (modified) clang/test/AST/ByteCode/new-delete.cpp (+16) 


``````````diff
diff --git a/clang/lib/AST/ByteCode/DynamicAllocator.cpp b/clang/lib/AST/ByteCode/DynamicAllocator.cpp
index a3a0dcd489a76..d70202b92e6e9 100644
--- a/clang/lib/AST/ByteCode/DynamicAllocator.cpp
+++ b/clang/lib/AST/ByteCode/DynamicAllocator.cpp
@@ -110,7 +110,7 @@ Block *DynamicAllocator::allocate(const Descriptor *D, unsigned EvalID,
 }
 
 bool DynamicAllocator::deallocate(const Expr *Source,
-                                  const Block *BlockToDelete, InterpState &S) {
+                                  const Block *BlockToDelete) {
   auto It = AllocationSites.find(Source);
   if (It == AllocationSites.end())
     return false;
@@ -123,7 +123,10 @@ bool DynamicAllocator::deallocate(const Expr *Source,
     return BlockToDelete == A.block();
   });
 
-  assert(AllocIt != Site.Allocations.end());
+  // The allocation site it fine, but this block doesn't belong to it. Must've
+  // already been deleted.
+  if (AllocIt == Site.Allocations.end())
+    return false;
 
   Block *B = AllocIt->block();
   assert(B->isInitialized());
diff --git a/clang/lib/AST/ByteCode/DynamicAllocator.h b/clang/lib/AST/ByteCode/DynamicAllocator.h
index ab1058bdf9f22..2336c3f3316c3 100644
--- a/clang/lib/AST/ByteCode/DynamicAllocator.h
+++ b/clang/lib/AST/ByteCode/DynamicAllocator.h
@@ -80,8 +80,7 @@ class DynamicAllocator final {
 
   /// Deallocate the given source+block combination.
   /// Returns \c true if anything has been deallocatd, \c false otherwise.
-  bool deallocate(const Expr *Source, const Block *BlockToDelete,
-                  InterpState &S);
+  bool deallocate(const Expr *Source, const Block *BlockToDelete);
 
   /// Checks whether the allocation done at the given source is an array
   /// allocation.
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 2f09c19178f09..0396482517046 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -1485,7 +1485,7 @@ bool Free(InterpState &S, CodePtr OpPC, bool DeleteIsArrayForm,
   if (!RunDestructors(S, OpPC, BlockToDelete))
     return false;
 
-  if (!Allocator.deallocate(Source, BlockToDelete, S)) {
+  if (!Allocator.deallocate(Source, BlockToDelete)) {
     // Nothing has been deallocated, this must be a double-delete.
     const SourceInfo &Loc = S.Current->getSource(OpPC);
     S.FFDiag(Loc, diag::note_constexpr_double_delete);
diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
index b76f13833da14..73952e032f1eb 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
@@ -1681,7 +1681,7 @@ static bool interp__builtin_operator_delete(InterpState &S, CodePtr OpPC,
   std::optional<DynamicAllocator::Form> AllocForm =
       Allocator.getAllocationForm(Source);
 
-  if (!Allocator.deallocate(Source, BlockToDelete, S)) {
+  if (!Allocator.deallocate(Source, BlockToDelete)) {
     // Nothing has been deallocated, this must be a double-delete.
     const SourceInfo &Loc = S.Current->getSource(OpPC);
     S.FFDiag(Loc, diag::note_constexpr_double_delete);
diff --git a/clang/test/AST/ByteCode/new-delete.cpp b/clang/test/AST/ByteCode/new-delete.cpp
index b9c1e089e6536..24195d77962b8 100644
--- a/clang/test/AST/ByteCode/new-delete.cpp
+++ b/clang/test/AST/ByteCode/new-delete.cpp
@@ -112,6 +112,22 @@ consteval int doubleDelete() { // both-error {{never produces a constant express
 static_assert(doubleDelete() == 1); // both-error {{not an integral constant expression}} \
                                     // both-note {{in call to 'doubleDelete()'}}
 
+template <int N>
+constexpr bool doubleDelete2(const char (&x)[N]) {
+  int *p[N];
+  for (int i = 0; i < N; i++)
+    p[i] = new int(x[i]);
+
+  delete p[0];
+  delete p[0]; // both-note {{delete of pointer that has already been deleted}}
+
+  return true;
+}
+static_assert(doubleDelete2("foo")); // both-error {{not an integral constant expression}} \
+                                     // both-note {{in call to}}
+
+
+
 constexpr int AutoArray() {
   auto array = new int[]{0, 1, 2, 3};
   int ret = array[3];

``````````

</details>


https://github.com/llvm/llvm-project/pull/206424