[clang][Interp] Add explicit dummy descriptors

[tbaederr]

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.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/68888.diff

8 Files Affected:

• (modified) clang/lib/AST/Interp/Descriptor.cpp (+7)
• (modified) clang/lib/AST/Interp/Descriptor.h (+6)
• (modified) clang/lib/AST/Interp/Interp.cpp (+6)
• (modified) clang/lib/AST/Interp/Interp.h (+14-6)
• (modified) clang/lib/AST/Interp/InterpBuiltin.cpp (+3)
• (modified) clang/lib/AST/Interp/Pointer.h (+2)
• (modified) clang/lib/AST/Interp/Program.cpp (+11-9)
• (modified) clang/test/AST/Interp/c.c (+10)

diff --git a/clang/lib/AST/Interp/Descriptor.cpp b/clang/lib/AST/Interp/Descriptor.cpp
index 4ecb7466998e705..56e03a32abe5c34 100644
--- a/clang/lib/AST/Interp/Descriptor.cpp
+++ b/clang/lib/AST/Interp/Descriptor.cpp
@@ -284,6 +284,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 55a754c3505cce7..8de637a31a0ffff 100644
--- a/clang/lib/AST/Interp/Descriptor.h
+++ b/clang/lib/AST/Interp/Descriptor.h
@@ -109,6 +109,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;
@@ -137,6 +139,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;
@@ -190,6 +194,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 a4d6844ebe61722..1d14241106a63eb 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -186,6 +186,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())
@@ -268,6 +272,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 1ad3b8bfc7711d3..4d2cd9dc2ae8c1d 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);
@@ -1423,8 +1427,9 @@ bool OffsetHelper(InterpState &S, CodePtr OpPC, const T &Offset,
   // Compute the largest index into the array.
   unsigned MaxIndex = Ptr.getNumElems();
 
+  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);
@@ -1434,28 +1439,31 @@ bool OffsetHelper(InterpState &S, CodePtr OpPC, const T &Offset,
         << NewIndex
         << /*array*/ static_cast<int>(!Ptr.inArray())
         << static_cast<unsigned>(MaxIndex);
-    return false;
+    Invalid = true;
   };
 
   unsigned MaxOffset = MaxIndex - Ptr.getIndex();
   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<int64_t>(Index);
   int64_t WideOffset = static_cast<int64_t>(Offset);
diff --git a/clang/lib/AST/Interp/InterpBuiltin.cpp b/clang/lib/AST/Interp/InterpBuiltin.cpp
index 7552c1b88cff60c..e329794cb79243d 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 d5279e757f04764..52c44915c4a3775 100644
--- a/clang/lib/AST/Interp/Pointer.h
+++ b/clang/lib/AST/Interp/Pointer.h
@@ -313,6 +313,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 65e170881e313d7..c6d19afd7d2219d 100644
--- a/clang/lib/AST/Interp/Program.cpp
+++ b/clang/lib/AST/Interp/Program.cpp
@@ -144,16 +144,18 @@ std::optional<unsigned> 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<unsigned> Program::createGlobal(const ValueDecl *VD,
diff --git a/clang/test/AST/Interp/c.c b/clang/test/AST/Interp/c.c
index 974ca72702f7dd0..ae6001aa3b4eee8 100644
--- a/clang/test/AST/Interp/c.c
+++ b/clang/test/AST/Interp/c.c
@@ -47,3 +47,13 @@ _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}}

[tbaederr]

Ping

[tbaederr]

Ping

[AaronBallman]

LGTM