[clang] 6195e22 - Revert "[clang][Interp] Create full type info for dummy pointers"

Timm Bäder via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 22 01:34:01 PDT 2024


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 at -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}}


        


More information about the cfe-commits mailing list