[clang][Interp] Use different inline descriptors for global variables

Most of the InlineDescriptor fields were unused for global variables.
But more importantly, we need to differentiate between global variables
that are uninitialized because they didn't have an initializer when we
originally created them, and ones that are uninitialized because they
DID have an initializer, but evaluating it failed.

clang/lib/AST/Interp/ByteCodeExprGen.cpp

@@ -30,15 +30,21 @@ template <class Emitter> class DeclScope final : public VariableScope<Emitter> {
 public:
   DeclScope(ByteCodeExprGen<Emitter> *Ctx, const ValueDecl *VD)
       : VariableScope<Emitter>(Ctx, nullptr), Scope(Ctx->P, VD),
-        OldGlobalDecl(Ctx->GlobalDecl) {
+        OldGlobalDecl(Ctx->GlobalDecl),
+        OldInitializingDecl(Ctx->InitializingDecl) {
     Ctx->GlobalDecl = Context::shouldBeGloballyIndexed(VD);
+    Ctx->InitializingDecl = VD;
   }
 
-  ~DeclScope() { this->Ctx->GlobalDecl = OldGlobalDecl; }
+  ~DeclScope() {
+    this->Ctx->GlobalDecl = OldGlobalDecl;
+    this->Ctx->InitializingDecl = OldInitializingDecl;
+  }
 
 private:
   Program::DeclScope Scope;
   bool OldGlobalDecl;
+  const ValueDecl *OldInitializingDecl;
 };
 
 /// Scope used to handle initialization methods.
@@ -3125,11 +3131,16 @@ template <class Emitter>
 bool ByteCodeExprGen<Emitter>::visitDecl(const VarDecl *VD) {
   assert(!VD->isInvalidDecl() && "Trying to constant evaluate an invalid decl");
 
-  // Global variable we've already seen but that's uninitialized means
-  // evaluating the initializer failed. Just return failure.
-  if (std::optional<unsigned> Index = P.getGlobal(VD);
-      Index && !P.getPtrGlobal(*Index).isInitialized())
-    return false;
+  // If we've seen the global variable already and the initializer failed,
+  // just return false immediately.
+  if (std::optional<unsigned> Index = P.getGlobal(VD)) {
+    const Pointer &Ptr = P.getPtrGlobal(*Index);
+    const GlobalInlineDescriptor &GD =
+        *reinterpret_cast<const GlobalInlineDescriptor *>(
+            Ptr.block()->rawData());
+    if (GD.InitState == GlobalInitState::InitializerFailed)
+      return false;
+  }
 
   // Create and initialize the variable.
   if (!this->visitVarDecl(VD))
@@ -3167,13 +3178,15 @@ bool ByteCodeExprGen<Emitter>::visitDecl(const VarDecl *VD) {
       auto GlobalIndex = P.getGlobal(VD);
       assert(GlobalIndex);
       Block *GlobalBlock = P.getGlobal(*GlobalIndex);
-      InlineDescriptor &ID =
-          *reinterpret_cast<InlineDescriptor *>(GlobalBlock->rawData());
-      ID.IsInitialized = false;
+      GlobalInlineDescriptor &GD =
+          *reinterpret_cast<GlobalInlineDescriptor *>(GlobalBlock->rawData());
+
+      GD.InitState = GlobalInitState::InitializerFailed;
       GlobalBlock->invokeDtor();
     }
     return false;
   }
+
   return true;
 }
 
@@ -3967,36 +3980,39 @@ bool ByteCodeExprGen<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) {
     }
   }
 
-  // Try to lazily visit (or emit dummy pointers for) declarations
-  // we haven't seen yet.
-  if (Ctx.getLangOpts().CPlusPlus) {
-    if (const auto *VD = dyn_cast<VarDecl>(D)) {
-      const auto typeShouldBeVisited = [&](QualType T) -> bool {
-        if (T.isConstant(Ctx.getASTContext()))
-          return true;
-        if (const auto *RT = T->getAs<ReferenceType>())
-          return RT->getPointeeType().isConstQualified();
-        return false;
-      };
+  if (D != InitializingDecl) {
+    // Try to lazily visit (or emit dummy pointers for) declarations
+    // we haven't seen yet.
+    if (Ctx.getLangOpts().CPlusPlus) {
+      if (const auto *VD = dyn_cast<VarDecl>(D)) {
+        const auto typeShouldBeVisited = [&](QualType T) -> bool {
+          if (T.isConstant(Ctx.getASTContext()))
+            return true;
+          if (const auto *RT = T->getAs<ReferenceType>())
+            return RT->getPointeeType().isConstQualified();
+          return false;
+        };
 
-      // Visit local const variables like normal.
-      if ((VD->isLocalVarDecl() || VD->isStaticDataMember()) &&
-          typeShouldBeVisited(VD->getType())) {
+        // Visit local const variables like normal.
+        if ((VD->hasGlobalStorage() || VD->isLocalVarDecl() ||
+             VD->isStaticDataMember()) &&
+            typeShouldBeVisited(VD->getType())) {
+          if (!this->visitVarDecl(VD))
+            return false;
+          // Retry.
+          return this->visitDeclRef(VD, E);
+        }
+      }
+    } else {
+      if (const auto *VD = dyn_cast<VarDecl>(D);
+          VD && VD->getAnyInitializer() &&
+          VD->getType().isConstant(Ctx.getASTContext()) && !VD->isWeak()) {
         if (!this->visitVarDecl(VD))
           return false;
         // Retry.
         return this->visitDeclRef(VD, E);
       }
     }
-  } else {
-    if (const auto *VD = dyn_cast<VarDecl>(D);
-        VD && VD->getAnyInitializer() &&
-        VD->getType().isConstant(Ctx.getASTContext()) && !VD->isWeak()) {
-      if (!this->visitVarDecl(VD))
-        return false;
-      // Retry.
-      return this->visitDeclRef(VD, E);
-    }
   }
 
   if (std::optional<unsigned> I = P.getOrCreateDummy(D)) {

clang/lib/AST/Interp/ByteCodeExprGen.h

@@ -321,6 +321,7 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>,
   /// Flag inidicating if we're initializing an already created
   /// variable. This is set in visitInitializer().
   bool Initializing = false;
+  const ValueDecl *InitializingDecl = nullptr;
 
   /// Flag indicating if we're initializing a global variable.
   bool GlobalDecl = false;

clang/lib/AST/Interp/Descriptor.h

@@ -47,6 +47,18 @@ using BlockMoveFn = void (*)(Block *Storage, const std::byte *SrcFieldPtr,
                              std::byte *DstFieldPtr,
                              const Descriptor *FieldDesc);
 
+enum class GlobalInitState {
+  Initialized,
+  NoInitializer,
+  InitializerFailed,
+};
+
+/// Descriptor used for global variables.
+struct alignas(void *) GlobalInlineDescriptor {
+  GlobalInitState InitState = GlobalInitState::InitializerFailed;
+};
+static_assert(sizeof(GlobalInlineDescriptor) == sizeof(void *), "");
+
 /// Inline descriptor embedded in structures and arrays.
 ///
 /// Such descriptors precede all composite array elements and structure fields.
@@ -86,6 +98,7 @@ struct InlineDescriptor {
   void dump() const { dump(llvm::errs()); }
   void dump(llvm::raw_ostream &OS) const;
 };
+static_assert(sizeof(GlobalInlineDescriptor) != sizeof(InlineDescriptor), "");
 
 /// Describes a memory block created by an allocation site.
 struct Descriptor final {
@@ -110,6 +123,7 @@ struct Descriptor final {
 
   using MetadataSize = std::optional<unsigned>;
   static constexpr MetadataSize InlineDescMD = sizeof(InlineDescriptor);
+  static constexpr MetadataSize GlobalMD = sizeof(GlobalInlineDescriptor);
 
   /// Pointer to the record, if block contains records.
   const Record *const ElemRecord = nullptr;

clang/lib/AST/Interp/Pointer.cpp

@@ -243,8 +243,17 @@ bool Pointer::isInitialized() const {
     return IM->second->isElementInitialized(getIndex());
   }
 
+  if (asBlockPointer().Base == 0)
+    return true;
+
+  if (isRoot() && PointeeStorage.BS.Base == sizeof(GlobalInlineDescriptor)) {
+    const GlobalInlineDescriptor &GD =
+        *reinterpret_cast<const GlobalInlineDescriptor *>(block()->rawData());
+    return GD.InitState == GlobalInitState::Initialized;
+  }
+
   // Field has its bit in an inline descriptor.
-  return PointeeStorage.BS.Base == 0 || getInlineDesc()->IsInitialized;
+  return getInlineDesc()->IsInitialized;
 }
 
 void Pointer::initialize() const {
@@ -282,6 +291,13 @@ void Pointer::initialize() const {
     return;
   }
 
+  if (isRoot() && PointeeStorage.BS.Base == sizeof(GlobalInlineDescriptor)) {
+    GlobalInlineDescriptor &GD = *reinterpret_cast<GlobalInlineDescriptor *>(
+        asBlockPointer().Pointee->rawData());
+    GD.InitState = GlobalInitState::Initialized;
+    return;
+  }
+
   // Field has its bit in an inline descriptor.
   assert(PointeeStorage.BS.Base != 0 &&
          "Only composite fields can be initialised");
@@ -292,6 +308,10 @@ void Pointer::activate() const {
   // Field has its bit in an inline descriptor.
   assert(PointeeStorage.BS.Base != 0 &&
          "Only composite fields can be initialised");
+
+  if (isRoot() && PointeeStorage.BS.Base == sizeof(GlobalInlineDescriptor))
+    return;
+
   getInlineDesc()->IsActive = true;
 }
 

clang/lib/AST/Interp/Pointer.h

@@ -256,9 +256,7 @@ class Pointer {
     if (isIntegralPointer())
       return false;
 
-    unsigned Base = asBlockPointer().Base;
-    return Base != 0 && Base != sizeof(InlineDescriptor) &&
-           Base != RootPtrMark && getFieldDesc()->asDecl();
+    return !isRoot() && getFieldDesc()->asDecl();
   }
 
   /// Accessor for information about the declaration site.
@@ -462,9 +460,7 @@ class Pointer {
   bool isMutable() const {
     if (!isBlockPointer())
       return false;
-    return asBlockPointer().Base != 0 &&
-           asBlockPointer().Base != sizeof(InlineDescriptor) &&
-           getInlineDesc()->IsFieldMutable;
+    return !isRoot() && getInlineDesc()->IsFieldMutable;
   }
 
   bool isWeak() const {
@@ -627,6 +623,7 @@ class Pointer {
 
   /// Returns the embedded descriptor preceding a field.
   InlineDescriptor *getInlineDesc() const {
+    assert(asBlockPointer().Base != sizeof(GlobalInlineDescriptor));
     return getDescriptor(asBlockPointer().Base);
   }
 

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, Descriptor::InlineDescMD,
-                                        S->getLength() + 1,
-                                        /*isConst=*/true,
-                                        /*isTemporary=*/false,
-                                        /*isMutable=*/false);
+  Descriptor *Desc =
+      allocateDescriptor(S, CharType, Descriptor::GlobalMD, S->getLength() + 1,
+                         /*isConst=*/true,
+                         /*isTemporary=*/false,
+                         /*isMutable=*/false);
 
   // Allocate storage for the string.
   // The byte length does not include the null terminator.
@@ -207,11 +207,10 @@ std::optional<unsigned> Program::createGlobal(const DeclTy &D, QualType Ty,
   const bool IsConst = Ty.isConstQualified();
   const bool IsTemporary = D.dyn_cast<const Expr *>();
   if (std::optional<PrimType> T = Ctx.classify(Ty))
-    Desc =
-        createDescriptor(D, *T, Descriptor::InlineDescMD, IsConst, IsTemporary);
+    Desc = createDescriptor(D, *T, Descriptor::GlobalMD, IsConst, IsTemporary);
   else
-    Desc = createDescriptor(D, Ty.getTypePtr(), Descriptor::InlineDescMD,
-                            IsConst, IsTemporary);
+    Desc = createDescriptor(D, Ty.getTypePtr(), Descriptor::GlobalMD, IsConst,
+                            IsTemporary);
 
   if (!Desc)
     return std::nullopt;
@@ -224,7 +223,9 @@ std::optional<unsigned> Program::createGlobal(const DeclTy &D, QualType Ty,
   G->block()->invokeCtor();
 
   // Initialize InlineDescriptor fields.
-  new (G->block()->rawData()) InlineDescriptor(Desc);
+  auto *GD = new (G->block()->rawData()) GlobalInlineDescriptor();
+  if (!Init)
+    GD->InitState = GlobalInitState::NoInitializer;
   Globals.push_back(G);
 
   return I;

clang/test/AST/Interp/cxx98.cpp

@@ -18,11 +18,11 @@ template struct C<cval>;
 
 /// FIXME: This example does not get properly diagnosed in the new interpreter.
 extern const int recurse1;
-const int recurse2 = recurse1; // ref-note {{declared here}}
+const int recurse2 = recurse1; // both-note {{declared here}}
 const int recurse1 = 1;
 int array1[recurse1];
 int array2[recurse2]; // ref-warning 2{{variable length array}} \
-                      // ref-note {{initializer of 'recurse2' is not a constant expression}} \
+                      // both-note {{initializer of 'recurse2' is not a constant expression}} \
                       // expected-warning {{variable length array}} \
                       // expected-error {{variable length array}}
 

clang/unittests/AST/Interp/Descriptor.cpp

@@ -53,7 +53,7 @@ TEST(Descriptor, Primitives) {
   ASSERT_FALSE(GlobalDesc->asRecordDecl());
 
   // Still true because this is a global variable.
-  ASSERT_TRUE(GlobalDesc->getMetadataSize() == sizeof(InlineDescriptor));
+  ASSERT_TRUE(GlobalDesc->getMetadataSize() == sizeof(GlobalInlineDescriptor));
   ASSERT_FALSE(GlobalDesc->isPrimitiveArray());
   ASSERT_FALSE(GlobalDesc->isCompositeArray());
   ASSERT_FALSE(GlobalDesc->isZeroSizeArray());