[clang][Interp] Call destructors of local variables

Differential Revision: https://reviews.llvm.org/D154581

Author: tbaederr

clang/lib/AST/Interp/Context.cpp

@@ -160,8 +160,16 @@ const llvm::fltSemantics &Context::getFloatSemantics(QualType T) const {
 bool Context::Run(State &Parent, const Function *Func, APValue &Result) {
   InterpState State(Parent, *P, Stk, *this);
   State.Current = new InterpFrame(State, Func, /*Caller=*/nullptr, {});
-  if (Interpret(State, Result))
+  if (Interpret(State, Result)) {
+    assert(Stk.empty());
     return true;
+  }
+
+  // We explicitly delete our state here, so the Stk.clear() call
+  // below doesn't violently free values the destructor would
+  // otherwise access.
+  State.~InterpState();
+
   Stk.clear();
   return false;
 }

clang/lib/AST/Interp/Descriptor.cpp

@@ -40,18 +40,21 @@ static void moveTy(Block *, const std::byte *Src, std::byte *Dst,
 template <typename T>
 static void ctorArrayTy(Block *, std::byte *Ptr, bool, bool, bool,
                         const Descriptor *D) {
+  new (Ptr) InitMapPtr(std::nullopt);
+
+  Ptr += sizeof(InitMapPtr);
   for (unsigned I = 0, NE = D->getNumElems(); I < NE; ++I) {
     new (&reinterpret_cast<T *>(Ptr)[I]) T();
   }
 }
 
 template <typename T>
 static void dtorArrayTy(Block *, std::byte *Ptr, const Descriptor *D) {
-  InitMap *IM = *reinterpret_cast<InitMap **>(Ptr);
-  if (IM != (InitMap *)-1)
-    free(IM);
+  InitMapPtr &IMP = *reinterpret_cast<InitMapPtr *>(Ptr);
 
-  Ptr += sizeof(InitMap *);
+  if (IMP)
+    IMP = std::nullopt;
+  Ptr += sizeof(InitMapPtr);
   for (unsigned I = 0, NE = D->getNumElems(); I < NE; ++I) {
     reinterpret_cast<T *>(Ptr)[I].~T();
   }
@@ -209,7 +212,8 @@ static BlockMoveFn getMovePrim(PrimType Type) {
 }
 
 static BlockCtorFn getCtorArrayPrim(PrimType Type) {
-  COMPOSITE_TYPE_SWITCH(Type, return ctorArrayTy<T>, return nullptr);
+  TYPE_SWITCH(Type, return ctorArrayTy<T>);
+  llvm_unreachable("unknown Expr");
 }
 
 static BlockDtorFn getDtorArrayPrim(PrimType Type) {
@@ -218,7 +222,8 @@ static BlockDtorFn getDtorArrayPrim(PrimType Type) {
 }
 
 static BlockMoveFn getMoveArrayPrim(PrimType Type) {
-  COMPOSITE_TYPE_SWITCH(Type, return moveArrayTy<T>, return nullptr);
+  TYPE_SWITCH(Type, return moveArrayTy<T>);
+  llvm_unreachable("unknown Expr");
 }
 
 /// Primitives.
@@ -238,7 +243,7 @@ Descriptor::Descriptor(const DeclTy &D, PrimType Type, MetadataSize MD,
                        bool IsMutable)
     : Source(D), ElemSize(primSize(Type)), Size(ElemSize * NumElems),
       MDSize(MD.value_or(0)),
-      AllocSize(align(Size) + sizeof(InitMap *) + MDSize), IsConst(IsConst),
+      AllocSize(align(Size) + sizeof(InitMapPtr) + MDSize), IsConst(IsConst),
       IsMutable(IsMutable), IsTemporary(IsTemporary), IsArray(true),
       CtorFn(getCtorArrayPrim(Type)), DtorFn(getDtorArrayPrim(Type)),
       MoveFn(getMoveArrayPrim(Type)) {
@@ -249,9 +254,10 @@ Descriptor::Descriptor(const DeclTy &D, PrimType Type, MetadataSize MD,
 Descriptor::Descriptor(const DeclTy &D, PrimType Type, bool IsTemporary,
                        UnknownSize)
     : Source(D), ElemSize(primSize(Type)), Size(UnknownSizeMark), MDSize(0),
-      AllocSize(alignof(void *)), IsConst(true), IsMutable(false),
-      IsTemporary(IsTemporary), IsArray(true), CtorFn(getCtorArrayPrim(Type)),
-      DtorFn(getDtorArrayPrim(Type)), MoveFn(getMoveArrayPrim(Type)) {
+      AllocSize(alignof(void *) + sizeof(InitMapPtr)), IsConst(true),
+      IsMutable(false), IsTemporary(IsTemporary), IsArray(true),
+      CtorFn(getCtorArrayPrim(Type)), DtorFn(getDtorArrayPrim(Type)),
+      MoveFn(getMoveArrayPrim(Type)) {
   assert(Source && "Missing source");
 }
 
@@ -272,10 +278,10 @@ Descriptor::Descriptor(const DeclTy &D, Descriptor *Elem, MetadataSize MD,
 Descriptor::Descriptor(const DeclTy &D, Descriptor *Elem, bool IsTemporary,
                        UnknownSize)
     : Source(D), ElemSize(Elem->getAllocSize() + sizeof(InlineDescriptor)),
-      Size(UnknownSizeMark), MDSize(0), AllocSize(alignof(void *)),
-      ElemDesc(Elem), IsConst(true), IsMutable(false), IsTemporary(IsTemporary),
-      IsArray(true), CtorFn(ctorArrayDesc), DtorFn(dtorArrayDesc),
-      MoveFn(moveArrayDesc) {
+      Size(UnknownSizeMark), MDSize(0),
+      AllocSize(alignof(void *) + sizeof(InitMapPtr)), ElemDesc(Elem),
+      IsConst(true), IsMutable(false), IsTemporary(IsTemporary), IsArray(true),
+      CtorFn(ctorArrayDesc), DtorFn(dtorArrayDesc), MoveFn(moveArrayDesc) {
   assert(Source && "Missing source");
 }
 
@@ -314,21 +320,12 @@ SourceLocation Descriptor::getLocation() const {
   llvm_unreachable("Invalid descriptor type");
 }
 
-InitMap::InitMap(unsigned N) : UninitFields(N) {
-  std::fill_n(data(), (N + PER_FIELD - 1) / PER_FIELD, 0);
+InitMap::InitMap(unsigned N)
+    : UninitFields(N), Data(std::make_unique<T[]>(numFields(N))) {
+  std::fill_n(data(), numFields(N), 0);
 }
 
-InitMap::T *InitMap::data() {
-  auto *Start = reinterpret_cast<char *>(this) + align(sizeof(InitMap));
-  return reinterpret_cast<T *>(Start);
-}
-
-const InitMap::T *InitMap::data() const {
-  auto *Start = reinterpret_cast<const char *>(this) + align(sizeof(InitMap));
-  return reinterpret_cast<const T *>(Start);
-}
-
-bool InitMap::initialize(unsigned I) {
+bool InitMap::initializeElement(unsigned I) {
   unsigned Bucket = I / PER_FIELD;
   T Mask = T(1) << (I % PER_FIELD);
   if (!(data()[Bucket] & Mask)) {
@@ -338,13 +335,7 @@ bool InitMap::initialize(unsigned I) {
   return UninitFields == 0;
 }
 
-bool InitMap::isInitialized(unsigned I) const {
+bool InitMap::isElementInitialized(unsigned I) const {
   unsigned Bucket = I / PER_FIELD;
   return data()[Bucket] & (T(1) << (I % PER_FIELD));
 }
-
-InitMap *InitMap::allocate(unsigned N) {
-  const size_t NumFields = ((N + PER_FIELD - 1) / PER_FIELD);
-  const size_t Size = align(sizeof(InitMap)) + NumFields * PER_FIELD;
-  return new (malloc(Size)) InitMap(N);
-}

clang/lib/AST/Interp/Descriptor.h

@@ -20,10 +20,12 @@ namespace clang {
 namespace interp {
 class Block;
 class Record;
+struct InitMap;
 struct Descriptor;
 enum PrimType : unsigned;
 
 using DeclTy = llvm::PointerUnion<const Decl *, const Expr *>;
+using InitMapPtr = std::optional<std::pair<bool, std::shared_ptr<InitMap>>>;
 
 /// Invoked whenever a block is created. The constructor method fills in the
 /// inline descriptors of all fields and array elements. It also initializes
@@ -193,36 +195,36 @@ struct Descriptor final {
 };
 
 /// Bitfield tracking the initialisation status of elements of primitive arrays.
-/// A pointer to this is embedded at the end of all primitive arrays.
-/// If the map was not yet created and nothing was initialized, the pointer to
-/// this structure is 0. If the object was fully initialized, the pointer is -1.
 struct InitMap final {
 private:
   /// Type packing bits.
   using T = uint64_t;
   /// Bits stored in a single field.
   static constexpr uint64_t PER_FIELD = sizeof(T) * CHAR_BIT;
 
+public:
   /// Initializes the map with no fields set.
-  InitMap(unsigned N);
+  explicit InitMap(unsigned N);
+
+private:
+  friend class Pointer;
 
   /// Returns a pointer to storage.
-  T *data();
-  const T *data() const;
+  T *data() { return Data.get(); }
+  const T *data() const { return Data.get(); }
 
-public:
   /// Initializes an element. Returns true when object if fully initialized.
-  bool initialize(unsigned I);
+  bool initializeElement(unsigned I);
 
   /// Checks if an element was initialized.
-  bool isInitialized(unsigned I) const;
+  bool isElementInitialized(unsigned I) const;
 
-  /// Allocates a map holding N elements.
-  static InitMap *allocate(unsigned N);
-
-private:
-  /// Number of fields initialized.
+  static constexpr size_t numFields(unsigned N) {
+    return (N + PER_FIELD - 1) / PER_FIELD;
+  }
+  /// Number of fields not initialized.
   unsigned UninitFields;
+  std::unique_ptr<T[]> Data;
 };
 
 } // namespace interp

clang/lib/AST/Interp/EvalEmitter.cpp

@@ -25,6 +25,14 @@ EvalEmitter::EvalEmitter(Context &Ctx, Program &P, State &Parent,
       new InterpFrame(S, /*Func=*/nullptr, /*Caller=*/nullptr, CodePtr());
 }
 
+EvalEmitter::~EvalEmitter() {
+  for (auto &[K, V] : Locals) {
+    Block *B = reinterpret_cast<Block *>(V.get());
+    if (B->isInitialized())
+      B->invokeDtor();
+  }
+}
+
 llvm::Expected<bool> EvalEmitter::interpretExpr(const Expr *E) {
   if (this->visitExpr(E))
     return true;

clang/lib/AST/Interp/EvalEmitter.h

@@ -40,7 +40,7 @@ class EvalEmitter : public SourceMapper {
   EvalEmitter(Context &Ctx, Program &P, State &Parent, InterpStack &Stk,
               APValue &Result);
 
-  virtual ~EvalEmitter() {}
+  virtual ~EvalEmitter();
 
   /// Define a label.
   void emitLabel(LabelTy Label);

clang/lib/AST/Interp/InterpBlock.h

@@ -71,6 +71,7 @@ class Block final {
   unsigned getSize() const { return Desc->getAllocSize(); }
   /// Returns the declaration ID.
   std::optional<unsigned> getDeclID() const { return DeclID; }
+  bool isInitialized() const { return IsInitialized; }
 
   /// Returns a pointer to the stored data.
   /// You are allowed to read Desc->getSize() bytes from this address.
@@ -104,12 +105,14 @@ class Block final {
     if (Desc->CtorFn)
       Desc->CtorFn(this, data(), Desc->IsConst, Desc->IsMutable,
                    /*isActive=*/true, Desc);
+    IsInitialized = true;
   }
 
   /// Invokes the Destructor.
   void invokeDtor() {
     if (Desc->DtorFn)
       Desc->DtorFn(this, data(), Desc);
+    IsInitialized = false;
   }
 
 protected:
@@ -139,8 +142,12 @@ class Block final {
   bool IsStatic = false;
   /// Flag indicating if the block is an extern.
   bool IsExtern = false;
-  /// Flag indicating if the pointer is dead.
+  /// Flag indicating if the pointer is dead. This is only ever
+  /// set once, when converting the Block to a DeadBlock.
   bool IsDead = false;
+  /// Flag indicating if the block contents have been initialized
+  /// via invokeCtor.
+  bool IsInitialized = false;
   /// Pointer to the stack slot descriptor.
   Descriptor *Desc;
 };

clang/lib/AST/Interp/InterpFrame.cpp

@@ -72,6 +72,19 @@ InterpFrame::InterpFrame(InterpState &S, const Function *Func, CodePtr RetPC)
 InterpFrame::~InterpFrame() {
   for (auto &Param : Params)
     S.deallocate(reinterpret_cast<Block *>(Param.second.get()));
+
+  // When destroying the InterpFrame, call the Dtor for all block
+  // that haven't been destroyed via a destroy() op yet.
+  // This happens when the execution is interruped midway-through.
+  if (Func) {
+    for (auto &Scope : Func->scopes()) {
+      for (auto &Local : Scope.locals()) {
+        Block *B = localBlock(Local.Offset);
+        if (B->isInitialized())
+          B->invokeDtor();
+      }
+    }
+  }
 }
 
 void InterpFrame::destroy(unsigned Idx) {

clang/lib/AST/Interp/InterpState.cpp

@@ -65,9 +65,9 @@ void InterpState::deallocate(Block *B) {
         std::memcpy(D->rawData(), B->rawData(), Desc->getMetadataSize());
     }
 
+    // We moved the contents over to the DeadBlock.
+    B->IsInitialized = false;
   } else {
-    // Free storage, if necessary.
-    if (Desc->DtorFn)
-      Desc->DtorFn(B, B->data(), Desc);
+    B->invokeDtor();
   }
 }

clang/lib/AST/Interp/Pointer.cpp

@@ -164,13 +164,16 @@ bool Pointer::isInitialized() const {
   if (Desc->isPrimitiveArray()) {
     if (isStatic() && Base == 0)
       return true;
-    // Primitive array field are stored in a bitset.
-    InitMap *Map = getInitMap();
-    if (!Map)
+
+    InitMapPtr &IM = getInitMap();
+
+    if (!IM)
       return false;
-    if (Map == (InitMap *)-1)
+
+    if (IM->first)
       return true;
-    return Map->isInitialized(getIndex());
+
+    return IM->second->isElementInitialized(getIndex());
   }
 
   // Field has its bit in an inline descriptor.
@@ -187,15 +190,20 @@ void Pointer::initialize() const {
     if (isStatic() && Base == 0)
       return;
 
-    // Primitive array initializer.
-    InitMap *&Map = getInitMap();
-    if (Map == (InitMap *)-1)
+    InitMapPtr &IM = getInitMap();
+    if (!IM)
+      IM =
+          std::make_pair(false, std::make_shared<InitMap>(Desc->getNumElems()));
+
+    assert(IM);
+
+    // All initialized.
+    if (IM->first)
       return;
-    if (Map == nullptr)
-      Map = InitMap::allocate(Desc->getNumElems());
-    if (Map->initialize(getIndex())) {
-      free(Map);
-      Map = (InitMap *)-1;
+
+    if (IM->second->initializeElement(getIndex())) {
+      IM->first = true;
+      IM->second.reset();
     }
     return;
   }