tbaeder updated this revision to Diff 557773. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154581/new/
https://reviews.llvm.org/D154581 Files: clang/lib/AST/Interp/Descriptor.cpp clang/lib/AST/Interp/Descriptor.h clang/lib/AST/Interp/EvalEmitter.cpp clang/lib/AST/Interp/EvalEmitter.h clang/lib/AST/Interp/InterpBlock.h clang/lib/AST/Interp/InterpFrame.cpp clang/lib/AST/Interp/InterpState.cpp clang/lib/AST/Interp/Pointer.cpp clang/lib/AST/Interp/Pointer.h clang/test/AST/Interp/arrays.cpp
Index: clang/test/AST/Interp/arrays.cpp =================================================================== --- clang/test/AST/Interp/arrays.cpp +++ clang/test/AST/Interp/arrays.cpp @@ -410,3 +410,52 @@ static_assert(b[4] == '\0', ""); static_assert(b[5] == '\0', ""); } + +namespace NoInitMapLeak { +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdivision-by-zero" +#pragma clang diagnostic ignored "-Wc++20-extensions" + constexpr int testLeak() { // expected-error {{never produces a constant expression}} \ + // ref-error {{never produces a constant expression}} + int a[2]; + a[0] = 1; + // interrupts interpretation. + (void)(1 / 0); // expected-note 2{{division by zero}} \ + // ref-note 2{{division by zero}} + + + return 1; + } +#pragma clang diagnostic pop + static_assert(testLeak() == 1, ""); // expected-error {{not an integral constant expression}} \ + // expected-note {{in call to 'testLeak()'}} \ + // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to 'testLeak()'}} + + + constexpr int a[] = {1,2,3,4/0,5}; // expected-error {{must be initialized by a constant expression}} \ + // expected-note {{division by zero}} \ + // ref-error {{must be initialized by a constant expression}} \ + // ref-note {{division by zero}} \ + // ref-note {{declared here}} + + /// FIXME: This should fail in the new interpreter as well. + constexpr int b = a[0]; // ref-error {{must be initialized by a constant expression}} \ + // ref-note {{is not a constant expression}} \ + // ref-note {{declared here}} + static_assert(b == 1, ""); // ref-error {{not an integral constant expression}} \ + // ref-note {{not a constant expression}} + + constexpr int f() { // expected-error {{never produces a constant expression}} \ + // ref-error {{never produces a constant expression}} + int a[] = {19,2,3/0,4}; // expected-note 2{{division by zero}} \ + // expected-warning {{is undefined}} \ + // ref-note 2{{division by zero}} \ + // ref-warning {{is undefined}} + return 1; + } + static_assert(f() == 1, ""); // expected-error {{not an integral constant expression}} \ + // expected-note {{in call to}} \ + // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to}} +} Index: clang/lib/AST/Interp/Pointer.h =================================================================== --- clang/lib/AST/Interp/Pointer.h +++ clang/lib/AST/Interp/Pointer.h @@ -108,7 +108,7 @@ if (getFieldDesc()->ElemDesc) Off += sizeof(InlineDescriptor); else - Off += sizeof(InitMap *); + Off += sizeof(InitMapPtr); return Pointer(Pointee, Base, Base + Off); } @@ -147,7 +147,7 @@ if (inPrimitiveArray()) { if (Offset != Base) return *this; - return Pointer(Pointee, Base, Offset + sizeof(InitMap *)); + return Pointer(Pointee, Base, Offset + sizeof(InitMapPtr)); } // Pointer is to a field or array element - enter it. @@ -168,7 +168,7 @@ // Revert to an outer one-past-end pointer. unsigned Adjust; if (inPrimitiveArray()) - Adjust = sizeof(InitMap *); + Adjust = sizeof(InitMapPtr); else Adjust = sizeof(InlineDescriptor); return Pointer(Pointee, Base, Base + getSize() + Adjust); @@ -257,7 +257,7 @@ if (getFieldDesc()->ElemDesc) Adjust = sizeof(InlineDescriptor); else - Adjust = sizeof(InitMap *); + Adjust = sizeof(InitMapPtr); } return Offset - Base - Adjust; } @@ -359,7 +359,7 @@ assert(isLive() && "Invalid pointer"); if (isArrayRoot()) return *reinterpret_cast<T *>(Pointee->rawData() + Base + - sizeof(InitMap *)); + sizeof(InitMapPtr)); return *reinterpret_cast<T *>(Pointee->rawData() + Offset); } @@ -367,7 +367,7 @@ /// Dereferences a primitive element. template <typename T> T &elem(unsigned I) const { assert(I < getNumElems()); - return reinterpret_cast<T *>(Pointee->data() + sizeof(InitMap *))[I]; + return reinterpret_cast<T *>(Pointee->data() + sizeof(InitMapPtr))[I]; } /// Initializes a field. @@ -418,6 +418,7 @@ private: friend class Block; friend class DeadBlock; + friend struct InitMap; Pointer(Block *Pointee, unsigned Base, unsigned Offset); @@ -431,9 +432,9 @@ 1; } - /// Returns a reference to the pointer which stores the initialization map. - InitMap *&getInitMap() const { - return *reinterpret_cast<InitMap **>(Pointee->rawData() + Base); + /// Returns a reference to the InitMapPtr which stores the initialization map. + InitMapPtr &getInitMap() const { + return *reinterpret_cast<InitMapPtr *>(Pointee->rawData() + Base); } /// The block the pointer is pointing to. Index: clang/lib/AST/Interp/Pointer.cpp =================================================================== --- clang/lib/AST/Interp/Pointer.cpp +++ clang/lib/AST/Interp/Pointer.cpp @@ -164,13 +164,16 @@ 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 @@ 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; } Index: clang/lib/AST/Interp/InterpState.cpp =================================================================== --- clang/lib/AST/Interp/InterpState.cpp +++ clang/lib/AST/Interp/InterpState.cpp @@ -65,9 +65,9 @@ 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(); } } Index: clang/lib/AST/Interp/InterpFrame.cpp =================================================================== --- clang/lib/AST/Interp/InterpFrame.cpp +++ clang/lib/AST/Interp/InterpFrame.cpp @@ -72,6 +72,19 @@ 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) { Index: clang/lib/AST/Interp/InterpBlock.h =================================================================== --- clang/lib/AST/Interp/InterpBlock.h +++ clang/lib/AST/Interp/InterpBlock.h @@ -71,6 +71,7 @@ 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 @@ 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 @@ 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; }; Index: clang/lib/AST/Interp/EvalEmitter.h =================================================================== --- clang/lib/AST/Interp/EvalEmitter.h +++ clang/lib/AST/Interp/EvalEmitter.h @@ -40,7 +40,7 @@ EvalEmitter(Context &Ctx, Program &P, State &Parent, InterpStack &Stk, APValue &Result); - virtual ~EvalEmitter() {} + virtual ~EvalEmitter(); /// Define a label. void emitLabel(LabelTy Label); Index: clang/lib/AST/Interp/EvalEmitter.cpp =================================================================== --- clang/lib/AST/Interp/EvalEmitter.cpp +++ clang/lib/AST/Interp/EvalEmitter.cpp @@ -25,6 +25,14 @@ 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; Index: clang/lib/AST/Interp/Descriptor.h =================================================================== --- clang/lib/AST/Interp/Descriptor.h +++ clang/lib/AST/Interp/Descriptor.h @@ -20,10 +20,12 @@ 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,9 +195,6 @@ }; /// 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. @@ -203,26 +202,29 @@ /// 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); +private: + friend class Pointer; + /// Returns a pointer to storage. T *data(); const T *data() const; -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: + static constexpr size_t numFields(unsigned N) { + return (N + PER_FIELD - 1) / PER_FIELD; + } /// Number of fields initialized. unsigned UninitFields; + std::unique_ptr<T[]> Data; }; } // namespace interp Index: clang/lib/AST/Interp/Descriptor.cpp =================================================================== --- clang/lib/AST/Interp/Descriptor.cpp +++ clang/lib/AST/Interp/Descriptor.cpp @@ -40,6 +40,9 @@ 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(); } @@ -47,11 +50,11 @@ 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 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 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 @@ 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, 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, 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,16 @@ 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); -} +InitMap::T *InitMap::data() { return Data.get(); } -const InitMap::T *InitMap::data() const { - auto *Start = reinterpret_cast<const char *>(this) + align(sizeof(InitMap)); - return reinterpret_cast<const T *>(Start); -} +const InitMap::T *InitMap::data() const { return Data.get(); } -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 +339,7 @@ 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); -}
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits