From 7f677fe3100131214386f9ce1fa308c235a595e9 Mon Sep 17 00:00:00 2001 From: Timm Baeder Date: Thu, 26 Oct 2023 15:15:25 +0200 Subject: [PATCH] [clang][Interp] Add explicit dummy descriptors (#68888) Instead of (ab)using incomplete array types for this, add a 'Dummy' bit to Descriptor. We need to be able to differentiate between the two when adding an offset. --- clang/lib/AST/Interp/Descriptor.cpp | 7 +++++++ clang/lib/AST/Interp/Descriptor.h | 6 ++++++ clang/lib/AST/Interp/Interp.cpp | 6 ++++++ clang/lib/AST/Interp/Interp.h | 20 ++++++++++++++------ clang/lib/AST/Interp/InterpBuiltin.cpp | 3 +++ clang/lib/AST/Interp/Pointer.h | 2 ++ clang/lib/AST/Interp/Program.cpp | 20 +++++++++++--------- clang/test/AST/Interp/c.c | 20 ++++++++++++++++++++ 8 files changed, 69 insertions(+), 15 deletions(-) diff --git a/clang/lib/AST/Interp/Descriptor.cpp b/clang/lib/AST/Interp/Descriptor.cpp index 2a21f60588d46c..59a952135a2d80 100644 --- a/clang/lib/AST/Interp/Descriptor.cpp +++ b/clang/lib/AST/Interp/Descriptor.cpp @@ -296,6 +296,13 @@ Descriptor::Descriptor(const DeclTy &D, Record *R, MetadataSize MD, assert(Source && "Missing source"); } +Descriptor::Descriptor(const DeclTy &D, MetadataSize MD) + : Source(D), ElemSize(1), Size(ElemSize), MDSize(MD.value_or(0)), + AllocSize(Size + MDSize), ElemRecord(nullptr), IsConst(true), + IsMutable(false), IsTemporary(false), 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 6ee9bbe85d71d6..8135f3d12f7035 100644 --- a/clang/lib/AST/Interp/Descriptor.h +++ b/clang/lib/AST/Interp/Descriptor.h @@ -111,6 +111,8 @@ struct Descriptor final { const bool IsTemporary = false; /// Flag indicating if the block is an array. const bool IsArray = false; + /// Flag indicating if this is a dummy descriptor. + const bool IsDummy = false; /// Storage management methods. const BlockCtorFn CtorFn = nullptr; @@ -139,6 +141,8 @@ struct Descriptor final { Descriptor(const DeclTy &D, Record *R, MetadataSize MD, bool IsConst, bool IsTemporary, bool IsMutable); + Descriptor(const DeclTy &D, MetadataSize MD); + QualType getType() const; QualType getElemQualType() const; SourceLocation getLocation() const; @@ -192,6 +196,8 @@ struct Descriptor final { bool isArray() const { return IsArray; } /// Checks if the descriptor is of a record. bool isRecord() const { return !IsArray && ElemRecord; } + /// Checks if this is a dummy descriptor. + bool isDummy() const { return IsDummy; } }; /// Bitfield tracking the initialisation status of elements of primitive arrays. diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp index 4a4c0922758c91..31d43b6010c187 100644 --- a/clang/lib/AST/Interp/Interp.cpp +++ b/clang/lib/AST/Interp/Interp.cpp @@ -215,6 +215,10 @@ bool CheckLive(InterpState &S, CodePtr OpPC, const Pointer &Ptr, return true; } +bool CheckDummy(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { + return !Ptr.isDummy(); +} + bool CheckNull(InterpState &S, CodePtr OpPC, const Pointer &Ptr, CheckSubobjectKind CSK) { if (!Ptr.isZero()) @@ -297,6 +301,8 @@ bool CheckInitialized(InterpState &S, CodePtr OpPC, const Pointer &Ptr, } bool CheckLoad(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { + if (!CheckDummy(S, OpPC, Ptr)) + return false; if (!CheckLive(S, OpPC, Ptr, AK_Read)) return false; if (!CheckExtern(S, OpPC, Ptr)) diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h index 86cc2676529511..3e1b4f32e8b691 100644 --- a/clang/lib/AST/Interp/Interp.h +++ b/clang/lib/AST/Interp/Interp.h @@ -55,6 +55,10 @@ bool CheckArray(InterpState &S, CodePtr OpPC, const Pointer &Ptr); /// Checks if a pointer is live and accessible. bool CheckLive(InterpState &S, CodePtr OpPC, const Pointer &Ptr, AccessKinds AK); + +/// Checks if a pointer is a dummy pointer. +bool CheckDummy(InterpState &S, CodePtr OpPC, const Pointer &Ptr); + /// Checks if a pointer is null. bool CheckNull(InterpState &S, CodePtr OpPC, const Pointer &Ptr, CheckSubobjectKind CSK); @@ -1415,8 +1419,9 @@ bool OffsetHelper(InterpState &S, CodePtr OpPC, const T &Offset, // Compute the largest index into the array. T MaxIndex = T::from(Ptr.getNumElems(), Offset.bitWidth()); + bool Invalid = false; // Helper to report an invalid offset, computed as APSInt. - auto InvalidOffset = [&]() { + auto DiagInvalidOffset = [&]() -> void { const unsigned Bits = Offset.bitWidth(); APSInt APOffset(Offset.toAPSInt().extend(Bits + 2), false); APSInt APIndex(Index.toAPSInt().extend(Bits + 2), false); @@ -1426,28 +1431,31 @@ bool OffsetHelper(InterpState &S, CodePtr OpPC, const T &Offset, << NewIndex << /*array*/ static_cast(!Ptr.inArray()) << static_cast(MaxIndex); - return false; + Invalid = true; }; T MaxOffset = T::from(MaxIndex - Index, Offset.bitWidth()); if constexpr (Op == ArithOp::Add) { // If the new offset would be negative, bail out. if (Offset.isNegative() && (Offset.isMin() || -Offset > Index)) - return InvalidOffset(); + DiagInvalidOffset(); // If the new offset would be out of bounds, bail out. if (Offset.isPositive() && Offset > MaxOffset) - return InvalidOffset(); + DiagInvalidOffset(); } else { // If the new offset would be negative, bail out. if (Offset.isPositive() && Index < Offset) - return InvalidOffset(); + DiagInvalidOffset(); // If the new offset would be out of bounds, bail out. if (Offset.isNegative() && (Offset.isMin() || -Offset > MaxOffset)) - return InvalidOffset(); + DiagInvalidOffset(); } + if (Invalid && !Ptr.isDummy()) + return false; + // Offset is valid - compute it on unsigned. int64_t WideIndex = static_cast(Index); int64_t WideOffset = static_cast(Offset); diff --git a/clang/lib/AST/Interp/InterpBuiltin.cpp b/clang/lib/AST/Interp/InterpBuiltin.cpp index 7552c1b88cff60..e329794cb79243 100644 --- a/clang/lib/AST/Interp/InterpBuiltin.cpp +++ b/clang/lib/AST/Interp/InterpBuiltin.cpp @@ -152,6 +152,9 @@ static bool interp__builtin_strlen(InterpState &S, CodePtr OpPC, if (!CheckLive(S, OpPC, StrPtr, AK_Read)) return false; + if (!CheckDummy(S, OpPC, StrPtr)) + return false; + assert(StrPtr.getFieldDesc()->isPrimitiveArray()); size_t Len = 0; diff --git a/clang/lib/AST/Interp/Pointer.h b/clang/lib/AST/Interp/Pointer.h index 65d710077fd1cb..b371b306fe7a70 100644 --- a/clang/lib/AST/Interp/Pointer.h +++ b/clang/lib/AST/Interp/Pointer.h @@ -314,6 +314,8 @@ class Pointer { bool isActive() const { return Base == 0 || 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. + bool isDummy() const { return getDeclDesc()->isDummy(); } /// Checks if an object or a subfield is mutable. bool isConst() const { diff --git a/clang/lib/AST/Interp/Program.cpp b/clang/lib/AST/Interp/Program.cpp index 65e170881e313d..c6d19afd7d2219 100644 --- a/clang/lib/AST/Interp/Program.cpp +++ b/clang/lib/AST/Interp/Program.cpp @@ -144,16 +144,18 @@ std::optional Program::getOrCreateDummy(const ValueDecl *PD) { It != DummyParams.end()) return It->second; - // Create a pointer to an incomplete array of the specified elements. - QualType ElemTy = PD->getType(); - QualType Ty = - Ctx.getASTContext().getIncompleteArrayType(ElemTy, ArrayType::Normal, 0); + // Create dummy descriptor. + Descriptor *Desc = allocateDescriptor(PD, std::nullopt); + // Allocate a block for storage. + unsigned I = Globals.size(); - if (auto Idx = createGlobal(PD, Ty, /*isStatic=*/true, /*isExtern=*/true)) { - DummyParams[PD] = *Idx; - return Idx; - } - return std::nullopt; + auto *G = new (Allocator, Desc->getAllocSize()) + Global(getCurrentDecl(), Desc, /*IsStatic=*/true, /*IsExtern=*/false); + G->block()->invokeCtor(); + + Globals.push_back(G); + DummyParams[PD] = I; + return I; } std::optional Program::createGlobal(const ValueDecl *VD, diff --git a/clang/test/AST/Interp/c.c b/clang/test/AST/Interp/c.c index 974ca72702f7dd..e8aa8b8599f213 100644 --- a/clang/test/AST/Interp/c.c +++ b/clang/test/AST/Interp/c.c @@ -47,3 +47,23 @@ _Static_assert(&a != 0, ""); // ref-warning {{always true}} \ // expected-warning {{always true}} \ // pedantic-expected-warning {{always true}} \ // pedantic-expected-warning {{is a GNU extension}} +_Static_assert((&c + 1) != 0, ""); // pedantic-ref-warning {{is a GNU extension}} \ + // pedantic-expected-warning {{is a GNU extension}} +_Static_assert((&a + 100) != 0, ""); // pedantic-ref-warning {{is a GNU extension}} \ + // pedantic-ref-note {{100 of non-array}} \ + // pedantic-expected-note {{100 of non-array}} \ + // pedantic-expected-warning {{is a GNU extension}} +_Static_assert((&a - 100) != 0, ""); // pedantic-ref-warning {{is a GNU extension}} \ + // pedantic-expected-warning {{is a GNU extension}} \ + // pedantic-ref-note {{-100 of non-array}} \ + // pedantic-expected-note {{-100 of non-array}} +/// extern variable of a composite type. +/// FIXME: The 'cast from void*' note is missing in the new interpreter. +extern struct Test50S Test50; +_Static_assert(&Test50 != (void*)0, ""); // ref-warning {{always true}} \ + // pedantic-ref-warning {{always true}} \ + // pedantic-ref-warning {{is a GNU extension}} \ + // pedantic-ref-note {{cast from 'void *' is not allowed}} \ + // expected-warning {{always true}} \ + // pedantic-expected-warning {{always true}} \ + // pedantic-expected-warning {{is a GNU extension}}