diff --git a/clang/lib/AST/Interp/Context.cpp b/clang/lib/AST/Interp/Context.cpp index 04710cd0f2c28..3ad63f40b0c72 100644 --- a/clang/lib/AST/Interp/Context.cpp +++ b/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; } diff --git a/clang/lib/AST/Interp/Descriptor.cpp b/clang/lib/AST/Interp/Descriptor.cpp index 3990282686fe3..abd75a8d67bbd 100644 --- a/clang/lib/AST/Interp/Descriptor.cpp +++ b/clang/lib/AST/Interp/Descriptor.cpp @@ -40,6 +40,9 @@ static void moveTy(Block *, const std::byte *Src, std::byte *Dst, template 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(Ptr)[I]) T(); } @@ -47,11 +50,11 @@ static void ctorArrayTy(Block *, std::byte *Ptr, bool, bool, bool, template static void dtorArrayTy(Block *, std::byte *Ptr, const Descriptor *D) { - InitMap *IM = *reinterpret_cast(Ptr); - if (IM != (InitMap *)-1) - free(IM); + InitMapPtr &IMP = *reinterpret_cast(Ptr); - Ptr += sizeof(InitMap *); + if (IMP) + IMP = std::nullopt; + Ptr += sizeof(InitMapPtr); for (unsigned I = 0, NE = D->getNumElems(); I < NE; ++I) { reinterpret_cast(Ptr)[I].~T(); } @@ -209,7 +212,8 @@ static BlockMoveFn getMovePrim(PrimType Type) { } static BlockCtorFn getCtorArrayPrim(PrimType Type) { - COMPOSITE_TYPE_SWITCH(Type, return ctorArrayTy, return nullptr); + TYPE_SWITCH(Type, return ctorArrayTy); + 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, return nullptr); + TYPE_SWITCH(Type, return moveArrayTy); + 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(numFields(N))) { + std::fill_n(data(), numFields(N), 0); } -InitMap::T *InitMap::data() { - auto *Start = reinterpret_cast(this) + align(sizeof(InitMap)); - return reinterpret_cast(Start); -} - -const InitMap::T *InitMap::data() const { - auto *Start = reinterpret_cast(this) + align(sizeof(InitMap)); - return reinterpret_cast(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); -} diff --git a/clang/lib/AST/Interp/Descriptor.h b/clang/lib/AST/Interp/Descriptor.h index 55a754c3505cc..be9a380138a7b 100644 --- a/clang/lib/AST/Interp/Descriptor.h +++ b/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; +using InitMapPtr = std::optional>>; /// 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 @@ 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. @@ -203,26 +202,29 @@ struct InitMap final { /// 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 Data; }; } // namespace interp diff --git a/clang/lib/AST/Interp/EvalEmitter.cpp b/clang/lib/AST/Interp/EvalEmitter.cpp index f46ef1067cf52..f8942291b3b16 100644 --- a/clang/lib/AST/Interp/EvalEmitter.cpp +++ b/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(V.get()); + if (B->isInitialized()) + B->invokeDtor(); + } +} + llvm::Expected EvalEmitter::interpretExpr(const Expr *E) { if (this->visitExpr(E)) return true; diff --git a/clang/lib/AST/Interp/EvalEmitter.h b/clang/lib/AST/Interp/EvalEmitter.h index 1128e0fb09f0d..5a9be18c34a03 100644 --- a/clang/lib/AST/Interp/EvalEmitter.h +++ b/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); diff --git a/clang/lib/AST/Interp/InterpBlock.h b/clang/lib/AST/Interp/InterpBlock.h index 5112108e6f010..89f937b588fb6 100644 --- a/clang/lib/AST/Interp/InterpBlock.h +++ b/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 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; }; diff --git a/clang/lib/AST/Interp/InterpFrame.cpp b/clang/lib/AST/Interp/InterpFrame.cpp index 49db83a965ab0..b06923114c7a2 100644 --- a/clang/lib/AST/Interp/InterpFrame.cpp +++ b/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(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) { diff --git a/clang/lib/AST/Interp/InterpState.cpp b/clang/lib/AST/Interp/InterpState.cpp index 4f050baea39e7..2cb87ef07fe58 100644 --- a/clang/lib/AST/Interp/InterpState.cpp +++ b/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(); } } diff --git a/clang/lib/AST/Interp/Pointer.cpp b/clang/lib/AST/Interp/Pointer.cpp index e04d29b8bd81b..d1af58203bec6 100644 --- a/clang/lib/AST/Interp/Pointer.cpp +++ b/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(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; } diff --git a/clang/lib/AST/Interp/Pointer.h b/clang/lib/AST/Interp/Pointer.h index d5279e757f047..3b21290332a9d 100644 --- a/clang/lib/AST/Interp/Pointer.h +++ b/clang/lib/AST/Interp/Pointer.h @@ -108,7 +108,7 @@ class Pointer { if (getFieldDesc()->ElemDesc) Off += sizeof(InlineDescriptor); else - Off += sizeof(InitMap *); + Off += sizeof(InitMapPtr); return Pointer(Pointee, Base, Base + Off); } @@ -147,7 +147,7 @@ class Pointer { 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 @@ class Pointer { // 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 @@ class Pointer { if (getFieldDesc()->ElemDesc) Adjust = sizeof(InlineDescriptor); else - Adjust = sizeof(InitMap *); + Adjust = sizeof(InitMapPtr); } return Offset - Base - Adjust; } @@ -359,7 +359,7 @@ class Pointer { assert(isLive() && "Invalid pointer"); if (isArrayRoot()) return *reinterpret_cast(Pointee->rawData() + Base + - sizeof(InitMap *)); + sizeof(InitMapPtr)); return *reinterpret_cast(Pointee->rawData() + Offset); } @@ -367,7 +367,7 @@ class Pointer { /// Dereferences a primitive element. template T &elem(unsigned I) const { assert(I < getNumElems()); - return reinterpret_cast(Pointee->data() + sizeof(InitMap *))[I]; + return reinterpret_cast(Pointee->data() + sizeof(InitMapPtr))[I]; } /// Initializes a field. @@ -418,6 +418,7 @@ class Pointer { private: friend class Block; friend class DeadBlock; + friend struct InitMap; Pointer(Block *Pointee, unsigned Base, unsigned Offset); @@ -431,9 +432,9 @@ class Pointer { 1; } - /// Returns a reference to the pointer which stores the initialization map. - InitMap *&getInitMap() const { - return *reinterpret_cast(Pointee->rawData() + Base); + /// Returns a reference to the InitMapPtr which stores the initialization map. + InitMapPtr &getInitMap() const { + return *reinterpret_cast(Pointee->rawData() + Base); } /// The block the pointer is pointing to. diff --git a/clang/test/AST/Interp/arrays.cpp b/clang/test/AST/Interp/arrays.cpp index ba29632e97c30..7110785ea4c66 100644 --- a/clang/test/AST/Interp/arrays.cpp +++ b/clang/test/AST/Interp/arrays.cpp @@ -406,3 +406,52 @@ namespace StringZeroFill { 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}} +}