llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) <details> <summary>Changes</summary> Some time ago, I did a similar patch for local variables. Initializing global variables can fail as well: ```c++ constexpr int a = 1/0; static_assert(a == 0); ``` ... would succeed in the new interpreter, because we never saved the fact that `a` has not been successfully initialized. (Side note: The changes in `Pointer.h` make me want to refactor `Pointer::Base` to simply be relative to `Pointee->data()` and not `Pointee->rawData()`. That way, the 'root' value would always be `0` and not `0 || sizeof(InlineDescriptor)`.) --- Patch is 23.91 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/72892.diff 11 Files Affected: - (modified) clang/lib/AST/Interp/ByteCodeExprGen.cpp (+14-1) - (modified) clang/lib/AST/Interp/Descriptor.cpp (+14-13) - (modified) clang/lib/AST/Interp/Descriptor.h (+4-2) - (modified) clang/lib/AST/Interp/Interp.cpp (+16) - (modified) clang/lib/AST/Interp/Interp.h (+18-5) - (modified) clang/lib/AST/Interp/Pointer.cpp (+3-1) - (modified) clang/lib/AST/Interp/Pointer.h (+21-9) - (modified) clang/lib/AST/Interp/Program.cpp (+41-15) - (modified) clang/test/AST/Interp/cxx17.cpp (+20-3) - (modified) clang/test/AST/Interp/cxx23.cpp (+17-10) - (modified) clang/test/AST/Interp/literals.cpp (+17) ``````````diff diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp index 5dc1f9dfb10ff32..180749a75ebd22e 100644 --- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -662,13 +662,26 @@ bool ByteCodeExprGen<Emitter>::VisitInitListExpr(const InitListExpr *E) { return this->visitInitList(E->inits(), E); if (T->isArrayType()) { - // FIXME: Array fillers. unsigned ElementIndex = 0; for (const Expr *Init : E->inits()) { if (!this->visitArrayElemInit(ElementIndex, Init)) return false; ++ElementIndex; } + + // Expand the filler expression. + // FIXME: This should go away. + if (const Expr *Filler = E->getArrayFiller()) { + const ConstantArrayType *CAT = + Ctx.getASTContext().getAsConstantArrayType(E->getType()); + uint64_t NumElems = CAT->getSize().getZExtValue(); + + for (; ElementIndex != NumElems; ++ElementIndex) { + if (!this->visitArrayElemInit(ElementIndex, Filler)) + return false; + } + } + return true; } diff --git a/clang/lib/AST/Interp/Descriptor.cpp b/clang/lib/AST/Interp/Descriptor.cpp index 59a952135a2d809..7330295132618ed 100644 --- a/clang/lib/AST/Interp/Descriptor.cpp +++ b/clang/lib/AST/Interp/Descriptor.cpp @@ -243,18 +243,19 @@ 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(InitMapPtr) + MDSize), IsConst(IsConst), - IsMutable(IsMutable), IsTemporary(IsTemporary), IsArray(true), - CtorFn(getCtorArrayPrim(Type)), DtorFn(getDtorArrayPrim(Type)), - MoveFn(getMoveArrayPrim(Type)) { + AllocSize(align(MDSize) + align(Size) + sizeof(InitMapPtr)), + IsConst(IsConst), IsMutable(IsMutable), IsTemporary(IsTemporary), + IsArray(true), CtorFn(getCtorArrayPrim(Type)), + DtorFn(getDtorArrayPrim(Type)), MoveFn(getMoveArrayPrim(Type)) { assert(Source && "Missing source"); } /// Primitive unknown-size arrays. -Descriptor::Descriptor(const DeclTy &D, PrimType Type, bool IsTemporary, - UnknownSize) - : Source(D), ElemSize(primSize(Type)), Size(UnknownSizeMark), MDSize(0), - AllocSize(alignof(void *) + sizeof(InitMapPtr)), IsConst(true), +Descriptor::Descriptor(const DeclTy &D, PrimType Type, MetadataSize MD, + bool IsTemporary, UnknownSize) + : Source(D), ElemSize(primSize(Type)), Size(UnknownSizeMark), + MDSize(MD.value_or(0)), + AllocSize(MDSize + sizeof(InitMapPtr) + alignof(void *)), IsConst(true), IsMutable(false), IsTemporary(IsTemporary), IsArray(true), CtorFn(getCtorArrayPrim(Type)), DtorFn(getDtorArrayPrim(Type)), MoveFn(getMoveArrayPrim(Type)) { @@ -275,12 +276,12 @@ Descriptor::Descriptor(const DeclTy &D, const Descriptor *Elem, MetadataSize MD, } /// Unknown-size arrays of composite elements. -Descriptor::Descriptor(const DeclTy &D, Descriptor *Elem, bool IsTemporary, - UnknownSize) +Descriptor::Descriptor(const DeclTy &D, Descriptor *Elem, MetadataSize MD, + bool IsTemporary, UnknownSize) : Source(D), ElemSize(Elem->getAllocSize() + sizeof(InlineDescriptor)), - Size(UnknownSizeMark), MDSize(0), - AllocSize(alignof(void *) + sizeof(InitMapPtr)), ElemDesc(Elem), - IsConst(true), IsMutable(false), IsTemporary(IsTemporary), IsArray(true), + Size(UnknownSizeMark), MDSize(MD.value_or(0)), + AllocSize(MDSize + alignof(void *)), ElemDesc(Elem), IsConst(true), + IsMutable(false), IsTemporary(IsTemporary), IsArray(true), CtorFn(ctorArrayDesc), DtorFn(dtorArrayDesc), MoveFn(moveArrayDesc) { assert(Source && "Missing source"); } diff --git a/clang/lib/AST/Interp/Descriptor.h b/clang/lib/AST/Interp/Descriptor.h index 8135f3d12f7035c..b0ffc21b7ea960f 100644 --- a/clang/lib/AST/Interp/Descriptor.h +++ b/clang/lib/AST/Interp/Descriptor.h @@ -128,14 +128,16 @@ struct Descriptor final { bool IsConst, bool IsTemporary, bool IsMutable); /// Allocates a descriptor for an array of primitives of unknown size. - Descriptor(const DeclTy &D, PrimType Type, bool IsTemporary, UnknownSize); + Descriptor(const DeclTy &D, PrimType Type, MetadataSize MDSize, + bool IsTemporary, UnknownSize); /// Allocates a descriptor for an array of composites. Descriptor(const DeclTy &D, const Descriptor *Elem, MetadataSize MD, unsigned NumElems, bool IsConst, bool IsTemporary, bool IsMutable); /// Allocates a descriptor for an array of composites of unknown size. - Descriptor(const DeclTy &D, Descriptor *Elem, bool IsTemporary, UnknownSize); + Descriptor(const DeclTy &D, Descriptor *Elem, MetadataSize MD, + bool IsTemporary, UnknownSize); /// Allocates a descriptor for a record. Descriptor(const DeclTy &D, Record *R, MetadataSize MD, bool IsConst, diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp index 13b77e9a87725c7..69d6504696311df 100644 --- a/clang/lib/AST/Interp/Interp.cpp +++ b/clang/lib/AST/Interp/Interp.cpp @@ -289,6 +289,22 @@ bool CheckInitialized(InterpState &S, CodePtr OpPC, const Pointer &Ptr, return false; } +bool CheckGlobalInitialized(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { + if (Ptr.isInitialized()) + return true; + + const VarDecl *VD = cast<VarDecl>(Ptr.getDeclDesc()->asValueDecl()); + if ((S.getLangOpts().CPlusPlus && !VD->hasConstantInitialization() && + VD->mightBeUsableInConstantExpressions(S.getCtx())) || + ((S.getLangOpts().CPlusPlus || S.getLangOpts().OpenCL) && + !S.getLangOpts().CPlusPlus11 && !VD->hasICEInitializer(S.getCtx()))) { + const SourceInfo &Loc = S.Current->getSource(OpPC); + S.FFDiag(Loc, diag::note_constexpr_var_init_non_constant, 1) << VD; + S.Note(VD->getLocation(), diag::note_declared_at); + } + return false; +} + bool CheckLoad(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { if (!CheckDummy(S, OpPC, Ptr)) return false; diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h index 4f7778bdd2ff333..3a665c12d663b25 100644 --- a/clang/lib/AST/Interp/Interp.h +++ b/clang/lib/AST/Interp/Interp.h @@ -86,6 +86,8 @@ bool CheckLoad(InterpState &S, CodePtr OpPC, const Pointer &Ptr); bool CheckInitialized(InterpState &S, CodePtr OpPC, const Pointer &Ptr, AccessKinds AK); +/// Check if a global variable is initialized. +bool CheckGlobalInitialized(InterpState &S, CodePtr OpPC, const Pointer &Ptr); /// Checks if a value can be stored in a block. bool CheckStore(InterpState &S, CodePtr OpPC, const Pointer &Ptr); @@ -1004,10 +1006,16 @@ bool SetThisField(InterpState &S, CodePtr OpPC, uint32_t I) { template <PrimType Name, class T = typename PrimConv<Name>::T> bool GetGlobal(InterpState &S, CodePtr OpPC, uint32_t I) { - const Block *B = S.P.getGlobal(I); - if (B->isExtern()) + const Pointer &Ptr = S.P.getPtrGlobal(I); + if (Ptr.isExtern()) return false; - S.Stk.push<T>(B->deref<T>()); + + // If a global variable is uninitialized, that means the initialize we've + // compiled for it wasn't a constant expression. Diagnose that. + if (!CheckGlobalInitialized(S, OpPC, Ptr)) + return false; + + S.Stk.push<T>(Ptr.deref<T>()); return true; } @@ -1019,7 +1027,9 @@ bool SetGlobal(InterpState &S, CodePtr OpPC, uint32_t I) { template <PrimType Name, class T = typename PrimConv<Name>::T> bool InitGlobal(InterpState &S, CodePtr OpPC, uint32_t I) { - S.P.getGlobal(I)->deref<T>() = S.Stk.pop<T>(); + const Pointer &P = S.P.getGlobal(I); + P.deref<T>() = S.Stk.pop<T>(); + P.initialize(); return true; } @@ -1035,7 +1045,10 @@ bool InitGlobalTemp(InterpState &S, CodePtr OpPC, uint32_t I, APValue *Cached = Temp->getOrCreateValue(true); *Cached = APV; - S.P.getGlobal(I)->deref<T>() = S.Stk.pop<T>(); + const Pointer &P = S.P.getGlobal(I); + P.deref<T>() = S.Stk.pop<T>(); + P.initialize(); + return true; } diff --git a/clang/lib/AST/Interp/Pointer.cpp b/clang/lib/AST/Interp/Pointer.cpp index e979b99b0fdd0a0..88b945b7902cf22 100644 --- a/clang/lib/AST/Interp/Pointer.cpp +++ b/clang/lib/AST/Interp/Pointer.cpp @@ -19,7 +19,9 @@ using namespace clang; using namespace clang::interp; -Pointer::Pointer(Block *Pointee) : Pointer(Pointee, 0, 0) {} +Pointer::Pointer(Block *Pointee) + : Pointer(Pointee, Pointee->getDescriptor()->getMetadataSize(), + Pointee->getDescriptor()->getMetadataSize()) {} Pointer::Pointer(Block *Pointee, unsigned BaseAndOffset) : Pointer(Pointee, BaseAndOffset, BaseAndOffset) {} diff --git a/clang/lib/AST/Interp/Pointer.h b/clang/lib/AST/Interp/Pointer.h index a8f6e62fa76d356..f5e76b253931c8b 100644 --- a/clang/lib/AST/Interp/Pointer.h +++ b/clang/lib/AST/Interp/Pointer.h @@ -134,7 +134,8 @@ class Pointer { // Pointer to an array of base types - enter block. if (Base == RootPtrMark) - return Pointer(Pointee, 0, Offset == 0 ? Offset : PastEndMark); + return Pointer(Pointee, sizeof(InlineDescriptor), + Offset == 0 ? Offset : PastEndMark); // Pointer is one past end - magic offset marks that. if (isOnePastEnd()) @@ -179,7 +180,7 @@ class Pointer { return *this; // If at base, point to an array of base types. - if (Base == 0) + if (Base == 0 || Base == sizeof(InlineDescriptor)) return Pointer(Pointee, RootPtrMark, 0); // Step into the containing array, if inside one. @@ -196,7 +197,10 @@ class Pointer { /// Checks if the pointer is live. bool isLive() const { return Pointee && !Pointee->IsDead; } /// Checks if the item is a field in an object. - bool isField() const { return Base != 0 && Base != RootPtrMark; } + bool isField() const { + return Base != 0 && Base != sizeof(InlineDescriptor) && + Base != RootPtrMark && getFieldDesc()->asDecl(); + } /// Accessor for information about the declaration site. const Descriptor *getDeclDesc() const { @@ -227,7 +231,7 @@ class Pointer { /// Accessors for information about the innermost field. const Descriptor *getFieldDesc() const { - if (Base == 0 || Base == RootPtrMark) + if (Base == 0 || Base == sizeof(InlineDescriptor) || Base == RootPtrMark) return getDeclDesc(); return getInlineDesc()->Desc; } @@ -282,7 +286,9 @@ class Pointer { bool isArrayElement() const { return inArray() && Base != Offset; } /// Pointer points directly to a block. bool isRoot() const { - return (Base == 0 || Base == RootPtrMark) && Offset == 0; + return (Base == 0 || Base == sizeof(InlineDescriptor) || + Base == RootPtrMark) && + Offset == 0; } /// Returns the record descriptor of a class. @@ -315,12 +321,16 @@ class Pointer { /// Checks if the field is mutable. bool isMutable() const { - return Base != 0 && getInlineDesc()->IsFieldMutable; + return Base != 0 && Base != sizeof(InlineDescriptor) && + getInlineDesc()->IsFieldMutable; } /// Checks if an object was initialized. bool isInitialized() const; /// Checks if the object is active. - bool isActive() const { return Base == 0 || getInlineDesc()->IsActive; } + bool isActive() const { + return Base == 0 || Base == sizeof(InlineDescriptor) || + getInlineDesc()->IsActive; + } /// Checks if a structure is a base class. bool isBaseClass() const { return isField() && getInlineDesc()->IsBase; } /// Checks if the pointer pointers to a dummy value. @@ -328,7 +338,9 @@ class Pointer { /// Checks if an object or a subfield is mutable. bool isConst() const { - return Base == 0 ? getDeclDesc()->IsConst : getInlineDesc()->IsConst; + return (Base == 0 || Base == sizeof(InlineDescriptor)) + ? getDeclDesc()->IsConst + : getInlineDesc()->IsConst; } /// Returns the declaration ID. @@ -353,7 +365,7 @@ class Pointer { return 1; // narrow()ed element in a composite array. - if (Base > 0 && Base == Offset) + if (Base > sizeof(InlineDescriptor) && Base == Offset) return 0; if (auto ElemSize = elemSize()) diff --git a/clang/lib/AST/Interp/Program.cpp b/clang/lib/AST/Interp/Program.cpp index 52e13398163ecf7..fbcc358f9ae9348 100644 --- a/clang/lib/AST/Interp/Program.cpp +++ b/clang/lib/AST/Interp/Program.cpp @@ -54,11 +54,11 @@ unsigned Program::createGlobalString(const StringLiteral *S) { } // Create a descriptor for the string. - Descriptor *Desc = - allocateDescriptor(S, CharType, std::nullopt, S->getLength() + 1, - /*isConst=*/true, - /*isTemporary=*/false, - /*isMutable=*/false); + Descriptor *Desc = allocateDescriptor(S, CharType, Descriptor::InlineDescMD, + S->getLength() + 1, + /*isConst=*/true, + /*isTemporary=*/false, + /*isMutable=*/false); // Allocate storage for the string. // The byte length does not include the null terminator. @@ -67,6 +67,16 @@ unsigned Program::createGlobalString(const StringLiteral *S) { auto *G = new (Allocator, Sz) Global(Desc, /*isStatic=*/true, /*isExtern=*/false); G->block()->invokeCtor(); + + InlineDescriptor *ID = + reinterpret_cast<InlineDescriptor *>(G->block()->rawData()); + ID->Offset = sizeof(InlineDescriptor); + ID->Desc = Desc; + ID->IsConst = true; + ID->IsInitialized = true; + ID->IsBase = false; + ID->IsActive = true; + ID->IsFieldMutable = false; Globals.push_back(G); // Construct the string in storage. @@ -78,16 +88,19 @@ unsigned Program::createGlobalString(const StringLiteral *S) { case PT_Sint8: { using T = PrimConv<PT_Sint8>::T; Field.deref<T>() = T::from(CodePoint, BitWidth); + Field.initialize(); break; } case PT_Uint16: { using T = PrimConv<PT_Uint16>::T; Field.deref<T>() = T::from(CodePoint, BitWidth); + Field.initialize(); break; } case PT_Uint32: { using T = PrimConv<PT_Uint32>::T; Field.deref<T>() = T::from(CodePoint, BitWidth); + Field.initialize(); break; } default: @@ -190,12 +203,13 @@ std::optional<unsigned> Program::createGlobal(const DeclTy &D, QualType Ty, Descriptor *Desc; const bool IsConst = Ty.isConstQualified(); const bool IsTemporary = D.dyn_cast<const Expr *>(); - if (auto T = Ctx.classify(Ty)) { - Desc = createDescriptor(D, *T, std::nullopt, IsConst, IsTemporary); - } else { - Desc = createDescriptor(D, Ty.getTypePtr(), std::nullopt, IsConst, - IsTemporary); - } + if (std::optional<PrimType> T = Ctx.classify(Ty)) + Desc = + createDescriptor(D, *T, Descriptor::InlineDescMD, IsConst, IsTemporary); + else + Desc = createDescriptor(D, Ty.getTypePtr(), Descriptor::InlineDescMD, + IsConst, IsTemporary); + if (!Desc) return std::nullopt; @@ -206,6 +220,18 @@ std::optional<unsigned> Program::createGlobal(const DeclTy &D, QualType Ty, Global(getCurrentDecl(), Desc, IsStatic, IsExtern); G->block()->invokeCtor(); + // Initialize InlineDescriptor fields. + InlineDescriptor *ID = + reinterpret_cast<InlineDescriptor *>(G->block()->rawData()); + ID->Offset = sizeof(InlineDescriptor); + ID->Desc = Desc; + ID->IsConst = true; + ID->IsInitialized = true; + ID->IsInitialized = false; + ID->IsBase = false; + ID->IsActive = true; + ID->IsFieldMutable = false; + Globals.push_back(G); return I; @@ -338,8 +364,8 @@ Descriptor *Program::createDescriptor(const DeclTy &D, const Type *Ty, } else { // Arrays of composites. In this case, the array is a list of pointers, // followed by the actual elements. - Descriptor *ElemDesc = createDescriptor( - D, ElemTy.getTypePtr(), std::nullopt, IsConst, IsTemporary); + Descriptor *ElemDesc = createDescriptor(D, ElemTy.getTypePtr(), MDSize, + IsConst, IsTemporary); if (!ElemDesc) return nullptr; unsigned ElemSize = @@ -355,14 +381,14 @@ Descriptor *Program::createDescriptor(const DeclTy &D, const Type *Ty, // is forbidden on pointers to such objects. if (isa<IncompleteArrayType>(ArrayType)) { if (std::optional<PrimType> T = Ctx.classify(ElemTy)) { - return allocateDescriptor(D, *T, IsTemporary, + return allocateDescriptor(D, *T, MDSize, IsTemporary, Descriptor::UnknownSize{}); } else { Descriptor *Desc = createDescriptor(D, ElemTy.getTypePtr(), MDSize, IsConst, IsTemporary); if (!Desc) return nullptr; - return allocateDescriptor(D, Desc, IsTemporary, + return allocateDescriptor(D, Desc, MDSize, IsTemporary, Descriptor::UnknownSize{}); } } diff --git a/clang/test/AST/Interp/cxx17.cpp b/clang/test/AST/Interp/cxx17.cpp index e1f578a4418d9fe..76d985eb22e178a 100644 --- a/clang/test/AST/Interp/cxx17.cpp +++ b/clang/test/AST/Interp/cxx17.cpp @@ -1,9 +1,6 @@ // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++17 -verify %s // RUN: %clang_cc1 -std=c++17 -verify=ref %s -// ref-no-diagnostics -// expected-no-diagnostics - struct F { int a; int b;}; constexpr F getF() { return {12, 3}; @@ -83,3 +80,23 @@ constexpr int b() { return a[0] + a[1]; } static_assert(b() == 11); + +/// The diagnostics between the two interpreters are different here. +struct S { int a; }; +constexpr S getS() { // expected-error {{constexpr function never produces a constant expression}} \\ + // ref-error {{constexpr function never produces a constant expression}} + (void)(1/0); // expected-note 2{{division by zero}} \ + // expected-warning {{division by zero}} \ + // ref-note 2{{division by zero}} \ + // ref-warning {{division by zero}} + return S{12}; +} +constexpr S s = getS(); // expected-error {{must be initialized by a constant expression}} \ + // expected-note {{in call to 'getS()'}} \ + // ref-error {{must be initialized by a constant expression}} \\ + // ref-note {{in call to 'getS()'}} \ + // ref-note {{declared here}} +static_assert(s.a == 12, ""); // expected-error {{not an integral constant expression}} \ + // expected-note {{read of uninitialized object}} \ + // ref-error {{not an integral constant expression}} \ + // ref-note {{initializer of 's' is not a constant expression}} diff --git a/clang/test/AST/Interp/cxx23.cpp b/clang/test/AST/Interp/cxx23.cpp index e284a66626fb331..8f9ffbe5d100b9d 100644 --- a/clang/test/AST/Interp/cxx23.cpp +++ b/clang/test/AST/Interp/cxx23.cpp @@ -3,28 +3,35 @@ // RUN: %clang_cc1 -std=c++20 -fsyntax-only -fcxx-exceptions -verify=expected20 %s -fexperimental-new-constant-interpreter // RUN: %clang_cc1 -std=c++23 -fsyntax-only -fcxx-exceptions -verify=expected23 %s -fexperimental-new-constant-interpreter - -// expected23-no-diagnostics - - /// FIXME: The new interpreter is missing all the 'control flows through...' diagnostics. constexpr int f(int n) { // ref20-error {{constexpr function never produces a constant expression}} \ - // ref23-error {{constexpr function never produces a constant expression}} + // ref23-error {{constexpr function never produces a constant expression}} \ + // expected20-error {{constexpr function never produces a constant expression}} \ + // expected23-error {{constexpr function never produces a constant expression}} static const int m = n; // ref20-note {{control flows through the definition of a static v... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/72892 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
