Author: Timm Bäder Date: 2024-04-22T10:32:31+02:00 New Revision: 6195e228eb2a7085fac53603f534d2401ab1ac39
URL: https://github.com/llvm/llvm-project/commit/6195e228eb2a7085fac53603f534d2401ab1ac39 DIFF: https://github.com/llvm/llvm-project/commit/6195e228eb2a7085fac53603f534d2401ab1ac39.diff LOG: Revert "[clang][Interp] Create full type info for dummy pointers" This reverts commit eef5798844a6ed489c28b37113f3bcaafd1d6e68. This breaks two tests on an arm builder: https://lab.llvm.org/buildbot/#/builders/245/builds/23496 Added: Modified: clang/lib/AST/Interp/Descriptor.cpp clang/lib/AST/Interp/Descriptor.h clang/lib/AST/Interp/Interp.h clang/lib/AST/Interp/Pointer.cpp clang/lib/AST/Interp/Program.cpp clang/test/AST/Interp/builtin-align-cxx.cpp clang/test/AST/Interp/c.c Removed: ################################################################################ diff --git a/clang/lib/AST/Interp/Descriptor.cpp b/clang/lib/AST/Interp/Descriptor.cpp index 6704444fccdd6a..a4ccc0236d292c 100644 --- a/clang/lib/AST/Interp/Descriptor.cpp +++ b/clang/lib/AST/Interp/Descriptor.cpp @@ -309,6 +309,14 @@ Descriptor::Descriptor(const DeclTy &D) assert(Source && "Missing source"); } +/// Dummy array. +Descriptor::Descriptor(const DeclTy &D, UnknownSize) + : Source(D), ElemSize(1), Size(UnknownSizeMark), MDSize(0), + AllocSize(MDSize), ElemRecord(nullptr), IsConst(true), IsMutable(false), + IsTemporary(false), IsArray(true), IsDummy(true) { + assert(Source && "Missing source"); +} + QualType Descriptor::getType() const { if (auto *E = asExpr()) return E->getType(); diff --git a/clang/lib/AST/Interp/Descriptor.h b/clang/lib/AST/Interp/Descriptor.h index f8a574c9b3a023..c386fc8ac7b09d 100644 --- a/clang/lib/AST/Interp/Descriptor.h +++ b/clang/lib/AST/Interp/Descriptor.h @@ -125,7 +125,7 @@ struct Descriptor final { /// Flag indicating if the block is an array. const bool IsArray = false; /// Flag indicating if this is a dummy descriptor. - bool IsDummy = false; + const bool IsDummy = false; /// Storage management methods. const BlockCtorFn CtorFn = nullptr; @@ -159,8 +159,8 @@ struct Descriptor final { /// Allocates a dummy descriptor. Descriptor(const DeclTy &D); - /// Make this descriptor a dummy descriptor. - void makeDummy() { IsDummy = true; } + /// Allocates a dummy array descriptor. + Descriptor(const DeclTy &D, UnknownSize); QualType getType() const; QualType getElemQualType() const; diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h index 813bd02030cbfa..cebedf59e0593f 100644 --- a/clang/lib/AST/Interp/Interp.h +++ b/clang/lib/AST/Interp/Interp.h @@ -823,9 +823,9 @@ inline bool CmpHelperEQ<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) { // element in the same array are NOT equal. They have the same Base value, // but a diff erent Offset. This is a pretty rare case, so we fix this here // by comparing pointers to the first elements. - if (!LHS.isZero() && LHS.isArrayRoot()) + if (!LHS.isZero() && !LHS.isDummy() && LHS.isArrayRoot()) VL = LHS.atIndex(0).getByteOffset(); - if (!RHS.isZero() && RHS.isArrayRoot()) + if (!RHS.isZero() && !RHS.isDummy() && RHS.isArrayRoot()) VR = RHS.atIndex(0).getByteOffset(); S.Stk.push<BoolT>(BoolT::from(Fn(Compare(VL, VR)))); @@ -1241,16 +1241,14 @@ inline bool GetPtrField(InterpState &S, CodePtr OpPC, uint32_t Off) { !CheckNull(S, OpPC, Ptr, CSK_Field)) return false; - if (!CheckExtern(S, OpPC, Ptr)) - return false; - if (!CheckRange(S, OpPC, Ptr, CSK_Field)) - return false; - if (!CheckSubobject(S, OpPC, Ptr, CSK_Field)) - return false; - - if (Ptr.isBlockPointer() && Off > Ptr.block()->getSize()) - return false; - + if (CheckDummy(S, OpPC, Ptr)) { + if (!CheckExtern(S, OpPC, Ptr)) + return false; + if (!CheckRange(S, OpPC, Ptr, CSK_Field)) + return false; + if (!CheckSubobject(S, OpPC, Ptr, CSK_Field)) + return false; + } S.Stk.push<Pointer>(Ptr.atField(Off)); return true; } @@ -1994,6 +1992,11 @@ inline bool ArrayElemPtr(InterpState &S, CodePtr OpPC) { if (!Ptr.isZero()) { if (!CheckArray(S, OpPC, Ptr)) return false; + + if (Ptr.isDummy()) { + S.Stk.push<Pointer>(Ptr); + return true; + } } if (!OffsetHelper<T, ArithOp::Add>(S, OpPC, Offset, Ptr)) @@ -2010,6 +2013,11 @@ inline bool ArrayElemPtrPop(InterpState &S, CodePtr OpPC) { if (!Ptr.isZero()) { if (!CheckArray(S, OpPC, Ptr)) return false; + + if (Ptr.isDummy()) { + S.Stk.push<Pointer>(Ptr); + return true; + } } if (!OffsetHelper<T, ArithOp::Add>(S, OpPC, Offset, Ptr)) @@ -2045,12 +2053,12 @@ inline bool ArrayElemPop(InterpState &S, CodePtr OpPC, uint32_t Index) { inline bool ArrayDecay(InterpState &S, CodePtr OpPC) { const Pointer &Ptr = S.Stk.pop<Pointer>(); - if (Ptr.isZero()) { + if (Ptr.isZero() || Ptr.isDummy()) { S.Stk.push<Pointer>(Ptr); return true; } - if (!Ptr.isUnknownSizeArray() || Ptr.isDummy()) { + if (!Ptr.isUnknownSizeArray()) { S.Stk.push<Pointer>(Ptr.atIndex(0)); return true; } diff --git a/clang/lib/AST/Interp/Pointer.cpp b/clang/lib/AST/Interp/Pointer.cpp index c7708de8d8c79c..5ef31671ae7be5 100644 --- a/clang/lib/AST/Interp/Pointer.cpp +++ b/clang/lib/AST/Interp/Pointer.cpp @@ -139,10 +139,9 @@ APValue Pointer::toAPValue() const { else llvm_unreachable("Invalid allocation type"); - if (isDummy() || isUnknownSizeArray() || Desc->asExpr()) { + if (isDummy() || isUnknownSizeArray() || Desc->asExpr()) return APValue(Base, CharUnits::Zero(), Path, /*IsOnePastEnd=*/false, /*IsNullPtr=*/false); - } // TODO: compute the offset into the object. CharUnits Offset = CharUnits::Zero(); diff --git a/clang/lib/AST/Interp/Program.cpp b/clang/lib/AST/Interp/Program.cpp index f4c004dd082169..2c8c6781b34833 100644 --- a/clang/lib/AST/Interp/Program.cpp +++ b/clang/lib/AST/Interp/Program.cpp @@ -148,20 +148,17 @@ std::optional<unsigned> Program::getOrCreateDummy(const ValueDecl *VD) { if (auto It = DummyVariables.find(VD); It != DummyVariables.end()) return It->second; + // Create dummy descriptor. + // We create desriptors of 'array of unknown size' if the type is an array + // type _and_ the size isn't known (it's not a ConstantArrayType). If the size + // is known however, we create a regular dummy pointer. Descriptor *Desc; - if (std::optional<PrimType> T = Ctx.classify(VD->getType())) - Desc = createDescriptor(VD, *T, std::nullopt, true, false); + if (const auto *AT = VD->getType()->getAsArrayTypeUnsafe(); + AT && !isa<ConstantArrayType>(AT)) + Desc = allocateDescriptor(VD, Descriptor::UnknownSize{}); else - Desc = createDescriptor(VD, VD->getType().getTypePtr(), std::nullopt, true, - false); - if (!Desc) Desc = allocateDescriptor(VD); - assert(Desc); - Desc->makeDummy(); - - assert(Desc->isDummy()); - // Allocate a block for storage. unsigned I = Globals.size(); @@ -317,7 +314,8 @@ Record *Program::getOrCreateRecord(const RecordDecl *RD) { for (const FieldDecl *FD : RD->fields()) { // Note that we DO create fields and descriptors // for unnamed bitfields here, even though we later ignore - // them everywhere. That's so the FieldDecl's getFieldIndex() matches. + // them everywhere. That's because so the FieldDecl's + // getFieldIndex() matches. // Reserve space for the field's descriptor and the offset. BaseSize += align(sizeof(InlineDescriptor)); @@ -350,7 +348,6 @@ Descriptor *Program::createDescriptor(const DeclTy &D, const Type *Ty, Descriptor::MetadataSize MDSize, bool IsConst, bool IsTemporary, bool IsMutable, const Expr *Init) { - // Classes and structures. if (const auto *RT = Ty->getAs<RecordType>()) { if (const auto *Record = getOrCreateRecord(RT->getDecl())) diff --git a/clang/test/AST/Interp/builtin-align-cxx.cpp b/clang/test/AST/Interp/builtin-align-cxx.cpp index c4103953df026a..62d73dba929b2c 100644 --- a/clang/test/AST/Interp/builtin-align-cxx.cpp +++ b/clang/test/AST/Interp/builtin-align-cxx.cpp @@ -2,6 +2,19 @@ // RUN: %clang_cc1 -triple=x86_64-unknown-unknown -std=c++11 %s -fsyntax-only -verify=expected,both -fexperimental-new-constant-interpreter // RUN: %clang_cc1 -triple=x86_64-unknown-unknown -std=c++11 %s -fsyntax-only -verify=ref,both + +/// This is just a copy of the one from test/SemaCXX/ with some of the +/// diagnostic output adapted. +/// Also, align32array has an initializer now, which means it's not just +/// a dummy pointer for us and we do actually have type information for it. +/// In the future, we need to retain type information for dummy pointers as +/// well, so here is a test that will break once we do that: +namespace { + _Alignas(32) char heh[4]; + static_assert(!__builtin_is_aligned(&heh[1], 4), ""); // expected-error {{failed}} +} + + // Check that we don't crash when using dependent types in __builtin_align: template <typename a, a b> void *c(void *d) { // both-note{{candidate template ignored}} @@ -164,7 +177,7 @@ static_assert(wrap_align_up(static_cast<bool>(1), const_value(1 << 21)), ""); // // both-note@-1{{in instantiation of function template specialization 'wrap_align_up<bool>' requested here}} // Check constant evaluation for pointers: -_Alignas(32) char align32array[128]; +_Alignas(32) char align32array[128] = {}; static_assert(&align32array[0] == &align32array[0], ""); // __builtin_align_up/down can be constant evaluated as a no-op for values // that are known to have greater alignment: diff --git a/clang/test/AST/Interp/c.c b/clang/test/AST/Interp/c.c index db6739e8b19cb9..5ae9b1dc7bff86 100644 --- a/clang/test/AST/Interp/c.c +++ b/clang/test/AST/Interp/c.c @@ -257,7 +257,3 @@ int Y __attribute__((annotate( 42, (struct TestStruct) { .a = 1, .b = 2 } ))); - -/// This tests that we have full type info, even for values we cannot read. -int dummyarray[5]; -_Static_assert(&dummyarray[0] < &dummyarray[1], ""); // pedantic-warning {{GNU extension}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits