[clang][bytecode] Always track item types in InterpStack

clang/lib/AST/ByteCode/Compiler.cpp

@@ -2974,20 +2974,25 @@ bool Compiler<Emitter>::VisitCompoundLiteralExpr(const CompoundLiteralExpr *E) {
     if (T && !E->isLValue())
       return this->delegate(Init);
 
-    if (std::optional<unsigned> GlobalIndex = P.createGlobal(E)) {
-      if (!this->emitGetPtrGlobal(*GlobalIndex, E))
-        return false;
+    std::optional<unsigned> GlobalIndex = P.createGlobal(E);
+    if (!GlobalIndex)
+      return false;
 
-      if (T) {
-        if (!this->visit(Init))
-          return false;
-        return this->emitInitGlobal(*T, *GlobalIndex, E);
-      }
+    if (!this->emitGetPtrGlobal(*GlobalIndex, E))
+      return false;
+
+    // Since this is a global variable, we might've already seen,
+    // don't do it again.
+    if (P.isGlobalInitialized(*GlobalIndex))
+      return true;
 
-      return this->visitInitializer(Init) && this->emitFinishInit(E);
+    if (T) {
+      if (!this->visit(Init))
+        return false;
+      return this->emitInitGlobal(*T, *GlobalIndex, E);
     }
 
-    return false;
+    return this->visitInitializer(Init) && this->emitFinishInit(E);
   }
 
   // Otherwise, use a local variable.

clang/lib/AST/ByteCode/Context.cpp

@@ -398,17 +398,11 @@ const llvm::fltSemantics &Context::getFloatSemantics(QualType T) const {
 }
 
 bool Context::Run(State &Parent, const Function *Func) {
-
-  {
-    InterpState State(Parent, *P, Stk, *this, Func);
-    if (Interpret(State)) {
-      assert(Stk.empty());
-      return true;
-    }
-    // State gets destroyed here, so the Stk.clear() below doesn't accidentally
-    // remove values the State's destructor might access.
+  InterpState State(Parent, *P, Stk, *this, Func);
+  if (Interpret(State)) {
+    assert(Stk.empty());
+    return true;
   }
-
   Stk.clear();
   return false;
 }

clang/lib/AST/ByteCode/Context.h

@@ -30,7 +30,7 @@ namespace interp {
 class Function;
 class Program;
 class State;
-enum PrimType : unsigned;
+enum PrimType : uint8_t;
 
 struct ParamOffset {
   unsigned Offset;

clang/lib/AST/ByteCode/Descriptor.h

@@ -24,7 +24,7 @@ class Record;
 class SourceInfo;
 struct InitMap;
 struct Descriptor;
-enum PrimType : unsigned;
+enum PrimType : uint8_t;
 
 using DeclTy = llvm::PointerUnion<const Decl *, const Expr *>;
 using InitMapPtr = std::optional<std::pair<bool, std::shared_ptr<InitMap>>>;

clang/lib/AST/ByteCode/Function.h

@@ -28,7 +28,7 @@ namespace interp {
 class Program;
 class ByteCodeEmitter;
 class Pointer;
-enum PrimType : uint32_t;
+enum PrimType : uint8_t;
 
 /// Describes a scope block.
 ///

clang/lib/AST/ByteCode/InterpBlock.cpp

@@ -18,10 +18,6 @@ using namespace clang::interp;
 
 void Block::addPointer(Pointer *P) {
   assert(P);
-  if (IsStatic) {
-    assert(!Pointers);
-    return;
-  }
 
 #ifndef NDEBUG
   assert(!hasPointer(P));
@@ -39,10 +35,6 @@ void Block::addPointer(Pointer *P) {
 void Block::removePointer(Pointer *P) {
   assert(P->isBlockPointer());
   assert(P);
-  if (IsStatic) {
-    assert(!Pointers);
-    return;
-  }
 
 #ifndef NDEBUG
   assert(hasPointer(P));
@@ -74,10 +66,6 @@ void Block::replacePointer(Pointer *Old, Pointer *New) {
   assert(New);
   assert(New->isBlockPointer());
   assert(Old != New);
-  if (IsStatic) {
-    assert(!Pointers);
-    return;
-  }
 #ifndef NDEBUG
   assert(hasPointer(Old));
 #endif

clang/lib/AST/ByteCode/InterpBlock.h

@@ -22,7 +22,7 @@ class Block;
 class DeadBlock;
 class InterpState;
 class Pointer;
-enum PrimType : unsigned;
+enum PrimType : uint8_t;
 
 /// A memory block, either on the stack or in the heap.
 ///

clang/lib/AST/ByteCode/InterpStack.cpp

@@ -26,33 +26,33 @@ InterpStack::~InterpStack() {
     std::free(Chunk);
   Chunk = nullptr;
   StackSize = 0;
-#ifndef NDEBUG
   ItemTypes.clear();
-#endif
 }
 
 // We keep the last chunk around to reuse.
 void InterpStack::clear() {
-  if (!Chunk)
-    return;
-
-  if (Chunk->Next)
-    std::free(Chunk->Next);
-
-  assert(Chunk);
-  StackSize = 0;
-#ifndef NDEBUG
-  ItemTypes.clear();
-#endif
+  for (PrimType Item : llvm::reverse(ItemTypes)) {
+    TYPE_SWITCH(Item, { this->discard<T>(); });
+  }
+  assert(ItemTypes.empty());
+  assert(empty());
 }
 
 void InterpStack::clearTo(size_t NewSize) {
-  assert(NewSize <= size());
-  size_t ToShrink = size() - NewSize;
-  if (ToShrink == 0)
+  if (NewSize == 0)
+    return clear();
+  if (NewSize == size())
     return;
 
-  shrink(ToShrink);
+  assert(NewSize <= size());
+  for (PrimType Item : llvm::reverse(ItemTypes)) {
+    TYPE_SWITCH(Item, { this->discard<T>(); });
+
+    if (size() == NewSize)
+      break;
+  }
+
+  // Note: discard() above already removed the types from ItemTypes.
   assert(size() == NewSize);
 }
 
@@ -105,25 +105,9 @@ void InterpStack::shrink(size_t Size) {
 
   Chunk->End -= Size;
   StackSize -= Size;
-
-#ifndef NDEBUG
-  size_t TypesSize = 0;
-  for (PrimType T : ItemTypes)
-    TYPE_SWITCH(T, { TypesSize += aligned_size<T>(); });
-
-  size_t StackSize = size();
-  while (TypesSize > StackSize) {
-    TYPE_SWITCH(ItemTypes.back(), {
-      TypesSize -= aligned_size<T>();
-      ItemTypes.pop_back();
-    });
-  }
-  assert(TypesSize == StackSize);
-#endif
 }
 
 void InterpStack::dump() const {
-#ifndef NDEBUG
   llvm::errs() << "Items: " << ItemTypes.size() << ". Size: " << size() << '\n';
   if (ItemTypes.empty())
     return;
@@ -133,17 +117,16 @@ void InterpStack::dump() const {
 
   // The type of the item on the top of the stack is inserted to the back
   // of the vector, so the iteration has to happen backwards.
-  for (auto TyIt = ItemTypes.rbegin(); TyIt != ItemTypes.rend(); ++TyIt) {
-    Offset += align(primSize(*TyIt));
+  for (PrimType Item : llvm::reverse(ItemTypes)) {
+    Offset += align(primSize(Item));
 
     llvm::errs() << Index << '/' << Offset << ": ";
-    TYPE_SWITCH(*TyIt, {
+    TYPE_SWITCH(Item, {
       const T &V = peek<T>(Offset);
       llvm::errs() << V;
     });
     llvm::errs() << '\n';
 
     ++Index;
   }
-#endif
 }

clang/lib/AST/ByteCode/InterpStack.h

@@ -17,7 +17,6 @@
 #include "IntegralAP.h"
 #include "MemberPointer.h"
 #include "PrimType.h"
-#include <vector>
 
 namespace clang {
 namespace interp {
@@ -33,18 +32,14 @@ class InterpStack final {
   /// Constructs a value in place on the top of the stack.
   template <typename T, typename... Tys> void push(Tys &&...Args) {
     new (grow(aligned_size<T>())) T(std::forward<Tys>(Args)...);
-#ifndef NDEBUG
     ItemTypes.push_back(toPrimType<T>());
-#endif
   }
 
   /// Returns the value from the top of the stack and removes it.
   template <typename T> T pop() {
-#ifndef NDEBUG
     assert(!ItemTypes.empty());
     assert(ItemTypes.back() == toPrimType<T>());
     ItemTypes.pop_back();
-#endif
     T *Ptr = &peekInternal<T>();
     T Value = std::move(*Ptr);
     shrink(aligned_size<T>());
@@ -53,22 +48,20 @@ class InterpStack final {
 
   /// Discards the top value from the stack.
   template <typename T> void discard() {
-#ifndef NDEBUG
     assert(!ItemTypes.empty());
     assert(ItemTypes.back() == toPrimType<T>());
     ItemTypes.pop_back();
-#endif
     T *Ptr = &peekInternal<T>();
-    Ptr->~T();
+    if constexpr (!std::is_trivially_destructible_v<T>) {
+      Ptr->~T();
+    }
     shrink(aligned_size<T>());
   }
 
   /// Returns a reference to the value on the top of the stack.
   template <typename T> T &peek() const {
-#ifndef NDEBUG
     assert(!ItemTypes.empty());
     assert(ItemTypes.back() == toPrimType<T>());
-#endif
     return peekInternal<T>();
   }
 
@@ -83,7 +76,7 @@ class InterpStack final {
   /// Returns the size of the stack in bytes.
   size_t size() const { return StackSize; }
 
-  /// Clears the stack without calling any destructors.
+  /// Clears the stack.
   void clear();
   void clearTo(size_t NewSize);
 
@@ -146,9 +139,11 @@ class InterpStack final {
   /// Total size of the stack.
   size_t StackSize = 0;
 
-#ifndef NDEBUG
-  /// vector recording the type of data we pushed into the stack.
-  std::vector<PrimType> ItemTypes;
+  /// SmallVector recording the type of data we pushed into the stack.
+  /// We don't usually need this during normal code interpretation but
+  /// when aborting, we need type information to call the destructors
+  /// for what's left on the stack.
+  llvm::SmallVector<PrimType> ItemTypes;
 
   template <typename T> static constexpr PrimType toPrimType() {
     if constexpr (std::is_same_v<T, Pointer>)
@@ -192,7 +187,6 @@ class InterpStack final {
 
     llvm_unreachable("unknown type push()'ed into InterpStack");
   }
-#endif
 };
 
 } // namespace interp

clang/lib/AST/ByteCode/InterpState.cpp

@@ -45,6 +45,12 @@ InterpState::~InterpState() {
 
   while (DeadBlocks) {
     DeadBlock *Next = DeadBlocks->Next;
+
+    // There might be a pointer in a global structure pointing to the dead
+    // block.
+    for (Pointer *P = DeadBlocks->B.Pointers; P; P = P->asBlockPointer().Next)
+      DeadBlocks->B.removePointer(P);
+
     std::free(DeadBlocks);
     DeadBlocks = Next;
   }
@@ -53,12 +59,6 @@ InterpState::~InterpState() {
 void InterpState::cleanup() {
   // As a last resort, make sure all pointers still pointing to a dead block
   // don't point to it anymore.
-  for (DeadBlock *DB = DeadBlocks; DB; DB = DB->Next) {
-    for (Pointer *P = DB->B.Pointers; P; P = P->asBlockPointer().Next) {
-      P->PointeeStorage.BS.Pointee = nullptr;
-    }
-  }
-
   Alloc.cleanup();
 }
 

clang/lib/AST/ByteCode/Pointer.h

@@ -29,7 +29,7 @@ class DeadBlock;
 class Pointer;
 class Context;
 template <unsigned A, bool B> class Integral;
-enum PrimType : unsigned;
+enum PrimType : uint8_t;
 
 class Pointer;
 inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Pointer &P);

clang/lib/AST/ByteCode/PrimType.h

@@ -31,7 +31,7 @@ template <bool Signed> class IntegralAP;
 template <unsigned Bits, bool Signed> class Integral;
 
 /// Enumeration of the primitive types of the VM.
-enum PrimType : unsigned {
+enum PrimType : uint8_t {
   PT_Sint8 = 0,
   PT_Uint8 = 1,
   PT_Sint16 = 2,
@@ -51,14 +51,15 @@ enum PrimType : unsigned {
 
 // Like std::optional<PrimType>, but only sizeof(PrimType).
 class OptPrimType final {
-  unsigned V = ~0u;
+  static constexpr uint8_t None = 0xFF;
+  uint8_t V = None;
 
 public:
   OptPrimType() = default;
   OptPrimType(std::nullopt_t) {}
   OptPrimType(PrimType T) : V(static_cast<unsigned>(T)) {}
 
-  explicit constexpr operator bool() const { return V != ~0u; }
+  explicit constexpr operator bool() const { return V != None; }
   PrimType operator*() const {
     assert(operator bool());
     return static_cast<PrimType>(V);

clang/lib/AST/ByteCode/Program.h

@@ -73,6 +73,10 @@ class Program final {
     return Globals[Idx]->block();
   }
 
+  bool isGlobalInitialized(unsigned Index) const {
+    return getPtrGlobal(Index).isInitialized();
+  }
+
   /// Finds a global's index.
   std::optional<unsigned> getGlobal(const ValueDecl *VD);
   std::optional<unsigned> getGlobal(const Expr *E);

clang/test/AST/ByteCode/c.c

@@ -329,3 +329,12 @@ void foo3 (void)
  void* x = 0;
  void* y = &*x;
 }
+
+static void *FooTable[1] = {
+    [0] = (void *[1]) { // 1
+        [0] = (void *[1]) { // 2
+            [0] = (void *[1]) {} // pedantic-warning {{use of an empty initializer}}
+        },
+    }
+};
+