[clang] 193db1e - [clang][Interp][NFC] Make pointer chain management more robust

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-07-09T16:27:49+02:00
New Revision: 193db1ebf09ac8a7661b4273e2847de34430a9af

URL: https://github.com/llvm/llvm-project/commit/193db1ebf09ac8a7661b4273e2847de34430a9af
DIFF: https://github.com/llvm/llvm-project/commit/193db1ebf09ac8a7661b4273e2847de34430a9af.diff

LOG: [clang][Interp][NFC] Make pointer chain management more robust

Add a bunch of assertions to ensure we're doing the right thing. Also,
rename movePointer to replacePointer, since the old name didn't make
much sense.

Added: 
    

Modified: 
    clang/lib/AST/Interp/InterpBlock.cpp
    clang/lib/AST/Interp/InterpBlock.h
    clang/lib/AST/Interp/Pointer.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/Interp/InterpBlock.cpp b/clang/lib/AST/Interp/InterpBlock.cpp
index c2a2ba70f4cd32..a62128d9cfaedd 100644
--- a/clang/lib/AST/Interp/InterpBlock.cpp
+++ b/clang/lib/AST/Interp/InterpBlock.cpp
@@ -16,11 +16,16 @@
 using namespace clang;
 using namespace clang::interp;
 
-
-
 void Block::addPointer(Pointer *P) {
-  if (IsStatic)
+  assert(P);
+  if (IsStatic) {
+    assert(!Pointers);
     return;
+  }
+
+#ifndef NDEBUG
+  assert(!hasPointer(P));
+#endif
   if (Pointers)
     Pointers->Prev = P;
   P->Next = Pointers;
@@ -29,10 +34,19 @@ void Block::addPointer(Pointer *P) {
 }
 
 void Block::removePointer(Pointer *P) {
-  if (IsStatic)
+  assert(P);
+  if (IsStatic) {
+    assert(!Pointers);
     return;
+  }
+
+#ifndef NDEBUG
+  assert(hasPointer(P));
+#endif
+
   if (Pointers == P)
     Pointers = P->Next;
+
   if (P->Prev)
     P->Prev->Next = P->Next;
   if (P->Next)
@@ -44,21 +58,38 @@ void Block::cleanup() {
     (reinterpret_cast<DeadBlock *>(this + 1) - 1)->free();
 }
 
-void Block::movePointer(Pointer *From, Pointer *To) {
-  if (IsStatic)
+void Block::replacePointer(Pointer *Old, Pointer *New) {
+  assert(Old);
+  assert(New);
+  if (IsStatic) {
+    assert(!Pointers);
     return;
-  To->Prev = From->Prev;
-  if (To->Prev)
-    To->Prev->Next = To;
-  To->Next = From->Next;
-  if (To->Next)
-    To->Next->Prev = To;
-  if (Pointers == From)
-    Pointers = To;
-
-  From->Prev = nullptr;
-  From->Next = nullptr;
+  }
+
+#ifndef NDEBUG
+  assert(hasPointer(Old));
+#endif
+
+  removePointer(Old);
+  addPointer(New);
+
+  Old->Pointee = nullptr;
+
+#ifndef NDEBUG
+  assert(!hasPointer(Old));
+  assert(hasPointer(New));
+#endif
+}
+
+#ifndef NDEBUG
+bool Block::hasPointer(const Pointer *P) const {
+  for (const Pointer *C = Pointers; C; C = C->Next) {
+    if (C == P)
+      return true;
+  }
+  return false;
 }
+#endif
 
 DeadBlock::DeadBlock(DeadBlock *&Root, Block *Blk)
     : Root(Root), B(Blk->Desc, Blk->IsStatic, Blk->IsExtern, /*isDead=*/true) {

diff  --git a/clang/lib/AST/Interp/InterpBlock.h b/clang/lib/AST/Interp/InterpBlock.h
index 059a32491cfebb..0080dad718edd2 100644
--- a/clang/lib/AST/Interp/InterpBlock.h
+++ b/clang/lib/AST/Interp/InterpBlock.h
@@ -124,7 +124,10 @@ class Block final {
   /// Pointer chain management.
   void addPointer(Pointer *P);
   void removePointer(Pointer *P);
-  void movePointer(Pointer *From, Pointer *To);
+  void replacePointer(Pointer *Old, Pointer *New);
+#ifndef NDEBUG
+  bool hasPointer(const Pointer *P) const;
+#endif
 
   /// Start of the chain of pointers.
   Pointer *Pointers = nullptr;

diff  --git a/clang/lib/AST/Interp/Pointer.cpp b/clang/lib/AST/Interp/Pointer.cpp
index 5b304019b511e0..00943dc846d3b2 100644
--- a/clang/lib/AST/Interp/Pointer.cpp
+++ b/clang/lib/AST/Interp/Pointer.cpp
@@ -24,7 +24,7 @@ Pointer::Pointer(const Pointer &P) : Pointer(P.Pointee, P.Base, P.Offset) {}
 Pointer::Pointer(Pointer &&P)
     : Pointee(P.Pointee), Base(P.Base), Offset(P.Offset) {
   if (Pointee)
-    Pointee->movePointer(&P, this);
+    Pointee->replacePointer(&P, this);
 }
 
 Pointer::Pointer(Block *Pointee, unsigned Base, unsigned Offset)
@@ -69,7 +69,7 @@ void Pointer::operator=(Pointer &&P) {
 
   Pointee = P.Pointee;
   if (Pointee)
-    Pointee->movePointer(&P, this);
+    Pointee->replacePointer(&P, this);
 
   if (Old)
     Old->cleanup();