[clang][bytecode] Add lifetime information for primitive array elements

[tbaederr]

Double the allocated size of the InitMap and use the second half for lifetime information.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

Double the allocated size of the InitMap and use the second half for lifetime information.

---

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

11 Files Affected:

• (modified) clang/lib/AST/ByteCode/Descriptor.h (-1)
• (modified) clang/lib/AST/ByteCode/InitMap.cpp (+26-3)
• (modified) clang/lib/AST/ByteCode/InitMap.h (+24-8)
• (modified) clang/lib/AST/ByteCode/Interp.cpp (+28-8)
• (modified) clang/lib/AST/ByteCode/Interp.h (+1-32)
• (modified) clang/lib/AST/ByteCode/Pointer.cpp (+30-1)
• (modified) clang/lib/AST/ByteCode/Pointer.h (+39-1)
• (modified) clang/lib/AST/CMakeLists.txt (+1)
• (modified) clang/test/AST/ByteCode/builtin-functions.cpp (+4-6)
• (modified) clang/test/AST/ByteCode/cxx23.cpp (+33)
• (modified) clang/test/AST/ByteCode/lifetimes26.cpp (+39-12)

diff --git a/clang/lib/AST/ByteCode/Descriptor.h b/clang/lib/AST/ByteCode/Descriptor.h
index 057f40f8f1f69..5e4c2a8d94774 100644
--- a/clang/lib/AST/ByteCode/Descriptor.h
+++ b/clang/lib/AST/ByteCode/Descriptor.h
@@ -275,7 +275,6 @@ struct Descriptor final {
   void dump(llvm::raw_ostream &OS) const;
   void dumpFull(unsigned Offset = 0, unsigned Indent = 0) const;
 };
-
 } // namespace interp
 } // namespace clang
 
diff --git a/clang/lib/AST/ByteCode/InitMap.cpp b/clang/lib/AST/ByteCode/InitMap.cpp
index ae841dd6df189..b62e30a46535f 100644
--- a/clang/lib/AST/ByteCode/InitMap.cpp
+++ b/clang/lib/AST/ByteCode/InitMap.cpp
@@ -10,9 +10,6 @@
 using namespace clang;
 using namespace clang::interp;
 
-InitMap::InitMap(unsigned N)
-    : UninitFields(N), Data(std::make_unique<T[]>(numFields(N))) {}
-
 bool InitMap::initializeElement(unsigned I) {
   unsigned Bucket = I / PER_FIELD;
   T Mask = T(1) << (I % PER_FIELD);
@@ -29,3 +26,29 @@ bool InitMap::isElementInitialized(unsigned I) const {
   unsigned Bucket = I / PER_FIELD;
   return data()[Bucket] & (T(1) << (I % PER_FIELD));
 }
+
+// Values in the second half of data() are inverted,
+// i.e. 0 means "lifetime started".
+void InitMap::startElementLifetime(unsigned I) {
+  unsigned LifetimeIndex = NumElems + I;
+
+  unsigned Bucket = LifetimeIndex / PER_FIELD;
+  T Mask = T(1) << (LifetimeIndex % PER_FIELD);
+  if ((data()[Bucket] & Mask)) {
+    data()[Bucket] &= ~Mask;
+    --DeadFields;
+  }
+}
+
+// Values in the second half of data() are inverted,
+// i.e. 0 means "lifetime started".
+void InitMap::endElementLifetime(unsigned I) {
+  unsigned LifetimeIndex = NumElems + I;
+
+  unsigned Bucket = LifetimeIndex / PER_FIELD;
+  T Mask = T(1) << (LifetimeIndex % PER_FIELD);
+  if (!(data()[Bucket] & Mask)) {
+    data()[Bucket] |= Mask;
+    ++DeadFields;
+  }
+}
diff --git a/clang/lib/AST/ByteCode/InitMap.h b/clang/lib/AST/ByteCode/InitMap.h
index 44184f4ba18a0..ade6600a54e59 100644
--- a/clang/lib/AST/ByteCode/InitMap.h
+++ b/clang/lib/AST/ByteCode/InitMap.h
@@ -24,20 +24,32 @@ struct InitMap final {
   using T = uint64_t;
   /// Bits stored in a single field.
   static constexpr uint64_t PER_FIELD = sizeof(T) * CHAR_BIT;
+  /// Number of fields in the init map.
+  unsigned NumElems;
   /// Number of fields not initialized.
   unsigned UninitFields;
+  unsigned DeadFields = 0;
   std::unique_ptr<T[]> Data;
 
 public:
   /// Initializes the map with no fields set.
-  explicit InitMap(unsigned N);
-
-private:
-  friend class Pointer;
+  explicit InitMap(unsigned N)
+      : NumElems(N), UninitFields(N),
+        Data(std::make_unique<T[]>(numFields(N))) {}
+  explicit InitMap(unsigned N, bool AllInitialized)
+      : NumElems(N), UninitFields(AllInitialized ? 0 : N),
+        Data(std::make_unique<T[]>(numFields(N))) {}
+
+  void startElementLifetime(unsigned I);
+  void endElementLifetime(unsigned I);
+
+  bool isElementAlive(unsigned I) const {
+    unsigned LifetimeIndex = NumElems + I;
+    unsigned Bucket = LifetimeIndex / PER_FIELD;
+    return !(data()[Bucket] & (T(1) << (LifetimeIndex % PER_FIELD)));
+  }
 
-  /// Returns a pointer to storage.
-  T *data() { return Data.get(); }
-  const T *data() const { return Data.get(); }
+  bool allElementsAlive() const { return DeadFields == 0; }
 
   /// Initializes an element. Returns true when object if fully initialized.
   bool initializeElement(unsigned I);
@@ -45,6 +57,11 @@ struct InitMap final {
   /// Checks if an element was initialized.
   bool isElementInitialized(unsigned I) const;
 
+private:
+  /// Returns a pointer to storage.
+  T *data() { return Data.get(); }
+  const T *data() const { return Data.get(); }
+
   static constexpr size_t numFields(unsigned N) {
     return ((N + PER_FIELD - 1) / PER_FIELD) * 2;
   }
@@ -94,7 +111,6 @@ struct InitMapPtr final {
   };
 };
 static_assert(sizeof(InitMapPtr) == sizeof(void *));
-
 } // namespace interp
 } // namespace clang
 
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 889ac1e1a9a7e..69423a13cb090 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -1907,10 +1907,6 @@ bool EndLifetime(InterpState &S, CodePtr OpPC) {
   if (Ptr.isBlockPointer() && !CheckDummy(S, OpPC, Ptr.block(), AK_Destroy))
     return false;
 
-  // FIXME: We need per-element lifetime information for primitive arrays.
-  if (Ptr.isArrayElement())
-    return true;
-
   endLifetimeRecurse(Ptr.narrow());
   return true;
 }
@@ -1921,10 +1917,6 @@ bool EndLifetimePop(InterpState &S, CodePtr OpPC) {
   if (Ptr.isBlockPointer() && !CheckDummy(S, OpPC, Ptr.block(), AK_Destroy))
     return false;
 
-  // FIXME: We need per-element lifetime information for primitive arrays.
-  if (Ptr.isArrayElement())
-    return true;
-
   endLifetimeRecurse(Ptr.narrow());
   return true;
 }
@@ -2324,6 +2316,34 @@ bool FinishInitGlobal(InterpState &S, CodePtr OpPC) {
   return true;
 }
 
+bool Destroy(InterpState &S, CodePtr OpPC, uint32_t I) {
+  assert(S.Current->getFunction());
+  // FIXME: We iterate the scope once here and then again in the destroy() call
+  // below.
+  for (auto &Local : S.Current->getFunction()->getScope(I).locals_reverse()) {
+    if (!S.Current->getLocalBlock(Local.Offset)->isInitialized())
+      continue;
+    const Pointer &Ptr = S.Current->getLocalPointer(Local.Offset);
+    if (Ptr.getLifetime() == Lifetime::Ended) {
+      // Try to use the declaration for better diagnostics
+      if (const Decl *D = Ptr.getDeclDesc()->asDecl()) {
+        auto *ND = cast<NamedDecl>(D);
+        S.FFDiag(ND->getLocation(),
+                 diag::note_constexpr_destroy_out_of_lifetime)
+            << ND->getNameAsString();
+      } else {
+        S.FFDiag(Ptr.getDeclDesc()->getLocation(),
+                 diag::note_constexpr_destroy_out_of_lifetime)
+            << Ptr.toDiagnosticString(S.getASTContext());
+      }
+      return false;
+    }
+  }
+
+  S.Current->destroy(I);
+  return true;
+}
+
 // https://github.com/llvm/llvm-project/issues/102513
 #if defined(_MSC_VER) && !defined(__clang__) && !defined(NDEBUG)
 #pragma optimize("", off)
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 427f694319b6c..f018efdd454b2 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -122,6 +122,7 @@ bool CheckFunctionDecl(InterpState &S, CodePtr OpPC, const FunctionDecl *FD);
 bool handleFixedPointOverflow(InterpState &S, CodePtr OpPC,
                               const FixedPoint &FP);
 
+bool Destroy(InterpState &S, CodePtr OpPC, uint32_t I);
 bool isConstexprUnknown(const Pointer &P);
 
 enum class ShiftDir { Left, Right };
@@ -2436,38 +2437,6 @@ inline bool SubPtr(InterpState &S, CodePtr OpPC, bool ElemSizeIsZero) {
   return true;
 }
 
-//===----------------------------------------------------------------------===//
-// Destroy
-//===----------------------------------------------------------------------===//
-
-inline bool Destroy(InterpState &S, CodePtr OpPC, uint32_t I) {
-  assert(S.Current->getFunction());
-
-  // FIXME: We iterate the scope once here and then again in the destroy() call
-  // below.
-  for (auto &Local : S.Current->getFunction()->getScope(I).locals_reverse()) {
-    const Pointer &Ptr = S.Current->getLocalPointer(Local.Offset);
-
-    if (Ptr.getLifetime() == Lifetime::Ended) {
-      // Try to use the declaration for better diagnostics
-      if (const Decl *D = Ptr.getDeclDesc()->asDecl()) {
-        auto *ND = cast<NamedDecl>(D);
-        S.FFDiag(ND->getLocation(),
-                 diag::note_constexpr_destroy_out_of_lifetime)
-            << ND->getNameAsString();
-      } else {
-        S.FFDiag(Ptr.getDeclDesc()->getLocation(),
-                 diag::note_constexpr_destroy_out_of_lifetime)
-            << Ptr.toDiagnosticString(S.getASTContext());
-      }
-      return false;
-    }
-  }
-
-  S.Current->destroy(I);
-  return true;
-}
-
 inline bool InitScope(InterpState &S, CodePtr OpPC, uint32_t I) {
   S.Current->initScope(I);
   return true;
diff --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp
index 5fb6ccc92c7bf..ec98b62ec91b8 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -490,6 +490,19 @@ bool Pointer::isElementInitialized(unsigned Index) const {
   return isInitialized();
 }
 
+bool Pointer::isElementAlive(unsigned Index) const {
+  assert(getFieldDesc()->isPrimitiveArray());
+
+  InitMapPtr &IM = getInitMap();
+  if (!IM.hasInitMap())
+    return true;
+
+  if (IM.allInitialized())
+    return true;
+
+  return IM->isElementAlive(Index);
+}
+
 void Pointer::initialize() const {
   if (!isBlockPointer())
     return;
@@ -524,7 +537,6 @@ void Pointer::initializeElement(unsigned Index) const {
   assert(Index < getFieldDesc()->getNumElems());
 
   InitMapPtr &IM = getInitMap();
-
   if (IM.allInitialized())
     return;
 
@@ -562,6 +574,23 @@ bool Pointer::allElementsInitialized() const {
   return IM.allInitialized();
 }
 
+bool Pointer::allElementsAlive() const {
+  assert(getFieldDesc()->isPrimitiveArray());
+  assert(isArrayRoot());
+
+  if (isStatic() && BS.Base == 0)
+    return true;
+
+  if (isRoot() && BS.Base == sizeof(GlobalInlineDescriptor) &&
+      Offset == BS.Base) {
+    const auto &GD = block()->getBlockDesc<GlobalInlineDescriptor>();
+    return GD.InitState == GlobalInitState::Initialized;
+  }
+
+  InitMapPtr &IM = getInitMap();
+  return IM.allInitialized() || (IM.hasInitMap() && IM->allElementsAlive());
+}
+
 void Pointer::activate() const {
   // Field has its bit in an inline descriptor.
   assert(BS.Base != 0 && "Only composite fields can be activated");
diff --git a/clang/lib/AST/ByteCode/Pointer.h b/clang/lib/AST/ByteCode/Pointer.h
index 0978090ba8b19..073e71ca37757 100644
--- a/clang/lib/AST/ByteCode/Pointer.h
+++ b/clang/lib/AST/ByteCode/Pointer.h
@@ -15,6 +15,7 @@
 
 #include "Descriptor.h"
 #include "FunctionPointer.h"
+#include "InitMap.h"
 #include "InterpBlock.h"
 #include "clang/AST/ComparisonCategories.h"
 #include "clang/AST/Decl.h"
@@ -721,6 +722,9 @@ class Pointer {
   /// Like isInitialized(), but for primitive arrays.
   bool isElementInitialized(unsigned Index) const;
   bool allElementsInitialized() const;
+  bool allElementsAlive() const;
+  bool isElementAlive(unsigned Index) const;
+
   /// Activats a field.
   void activate() const;
   /// Deactivates an entire strurcutre.
@@ -731,6 +735,16 @@ class Pointer {
       return Lifetime::Started;
     if (BS.Base < sizeof(InlineDescriptor))
       return Lifetime::Started;
+
+    if (inArray() && !isArrayRoot()) {
+      InitMapPtr &IM = getInitMap();
+
+      if (!IM.hasInitMap())
+        return getArray().getLifetime();
+      return IM->isElementAlive(getIndex()) ? Lifetime::Started
+                                            : Lifetime::Ended;
+    }
+
     return getInlineDesc()->LifeState;
   }
 
@@ -739,6 +753,17 @@ class Pointer {
       return;
     if (BS.Base < sizeof(InlineDescriptor))
       return;
+
+    if (inArray()) {
+      const Descriptor *Desc = getFieldDesc();
+      InitMapPtr &IM = getInitMap();
+      if (!IM.hasInitMap())
+        IM.setInitMap(new InitMap(Desc->getNumElems(), IM.allInitialized()));
+
+      IM->endElementLifetime(getIndex());
+      return;
+    }
+
     getInlineDesc()->LifeState = Lifetime::Ended;
   }
 
@@ -747,6 +772,18 @@ class Pointer {
       return;
     if (BS.Base < sizeof(InlineDescriptor))
       return;
+
+    if (inArray()) {
+      InitMapPtr &IM = getInitMap();
+      if (!IM.hasInitMap()) {
+        const Descriptor *Desc = getFieldDesc();
+        IM.setInitMap(new InitMap(Desc->getNumElems(), IM.allInitialized()));
+      }
+
+      IM->startElementLifetime(getIndex());
+      return;
+    }
+
     getInlineDesc()->LifeState = Lifetime::Started;
   }
 
@@ -791,7 +828,6 @@ class Pointer {
   friend class DeadBlock;
   friend class MemberPointer;
   friend class InterpState;
-  friend struct InitMap;
   friend class DynamicAllocator;
   friend class Program;
 
@@ -838,6 +874,8 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Pointer &P) {
   OS << ' ';
   if (const Descriptor *D = P.getFieldDesc())
     D->dump(OS);
+  if (P.isArrayElement())
+    OS << " index " << P.getIndex();
   return OS;
 }
 
diff --git a/clang/lib/AST/CMakeLists.txt b/clang/lib/AST/CMakeLists.txt
index f9a5f4f0e7ecd..3534996af12dc 100644
--- a/clang/lib/AST/CMakeLists.txt
+++ b/clang/lib/AST/CMakeLists.txt
@@ -73,6 +73,7 @@ add_clang_library(clangAST
   ByteCode/Compiler.cpp
   ByteCode/Context.cpp
   ByteCode/Descriptor.cpp
+  ByteCode/InitMap.cpp
   ByteCode/Disasm.cpp
   ByteCode/EvalEmitter.cpp
   ByteCode/Function.cpp
diff --git a/clang/test/AST/ByteCode/builtin-functions.cpp b/clang/test/AST/ByteCode/builtin-functions.cpp
index 3076b5239ebbe..bdb66aeaad539 100644
--- a/clang/test/AST/ByteCode/builtin-functions.cpp
+++ b/clang/test/AST/ByteCode/builtin-functions.cpp
@@ -1848,11 +1848,9 @@ namespace WithinLifetime {
   } xstd; // both-error {{is not a constant expression}} \
           // both-note {{in call to}}
 
-  /// FIXME: We do not have per-element lifetime information for primitive arrays.
-  /// See https://github.com/llvm/llvm-project/issues/147528
   consteval bool test_dynamic(bool read_after_deallocate) {
     std::allocator<int> a;
-    int* p = a.allocate(1); // expected-note 2{{allocation performed here was not deallocated}}
+    int* p = a.allocate(1);
     // a.allocate starts the lifetime of an array,
     // the complete object of *p has started its lifetime
     if (__builtin_is_within_lifetime(p))
@@ -1865,12 +1863,12 @@ namespace WithinLifetime {
       return false;
     a.deallocate(p, 1);
     if (read_after_deallocate)
-      __builtin_is_within_lifetime(p); // ref-note {{read of heap allocated object that has been deleted}}
+      __builtin_is_within_lifetime(p); // both-note {{read of heap allocated object that has been deleted}}
     return true;
   }
-  static_assert(test_dynamic(false)); // expected-error {{not an integral constant expression}}
+  static_assert(test_dynamic(false));
   static_assert(test_dynamic(true)); // both-error {{not an integral constant expression}} \
-                                     // ref-note {{in call to}}
+                                     // both-note {{in call to}}
 }
 
 #ifdef __SIZEOF_INT128__
diff --git a/clang/test/AST/ByteCode/cxx23.cpp b/clang/test/AST/ByteCode/cxx23.cpp
index 819460628c1b7..427bdfdb6b3c5 100644
--- a/clang/test/AST/ByteCode/cxx23.cpp
+++ b/clang/test/AST/ByteCode/cxx23.cpp
@@ -12,6 +12,8 @@ inline constexpr void* operator new(__SIZE_TYPE__, void* p) noexcept { return p;
 namespace std {
 template<typename T, typename... Args>
 constexpr T* construct_at(T* p, Args&&... args) { return ::new((void*)p) T(static_cast<Args&&>(args)...); }
+template<typename T>
+constexpr void destroy_at(T *p) { p->~T(); } // #destroy
 }
 
 constexpr int f(int n) {  // all20-error {{constexpr function never produces a constant expression}}
@@ -495,4 +497,35 @@ namespace InactiveLocalsInConditionalOp {
   static_assert(test2(true));
 
 }
+
+namespace PrimitiveArrayLifetimes {
+  consteval int test1() {
+    int a[3];
+    assert_active(a[0]);
+    assert_active(a[1]);
+    assert_active(a[2]);
+    std::destroy_at(&a[0]);
+    assert_inactive(a[0]);
+
+    std::construct_at(&a[0]);
+    assert_active(a[0]);
+    return a[0];
+  }
+  static_assert(test1() == 0);
+
+  consteval int test2() {
+    int a[3] = {1,2,3};
+    assert_active(a[0]);
+    assert_active(a[1]);
+    assert_active(a[2]);
+    std::destroy_at(&a[0]);
+    assert_inactive(a[0]);
+
+    std::construct_at(&a[0]);
+    assert_active(a[0]);
+    return a[0];
+  }
+  static_assert(test2() == 0);
+}
+
 #endif
diff --git a/clang/test/AST/ByteCode/lifetimes26.cpp b/clang/test/AST/ByteCode/lifetimes26.cpp
index c3163f8a562bf..ced0af5ba2420 100644
--- a/clang/test/AST/ByteCode/lifetimes26.cpp
+++ b/clang/test/AST/ByteCode/lifetimes26.cpp
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -verify=expected,both -std=c++26 %s -fexperimental-new-constant-interpreter
 // RUN: %clang_cc1 -verify=ref,both      -std=c++26 %s
 
-// both-no-diagnostics
-
 namespace std {
   struct type_info;
   struct destroying_delete_t {
@@ -18,7 +16,7 @@ namespace std {
 constexpr void *operator new(std::size_t, void *p) { return p; }
 namespace std {
   template<typename T> constexpr T *construct_at(T *p) { return new (p) T; }
-  template<typename T> constexpr void destroy_at(T *p) { p->~T(); }
+  template<typename T> constexpr void destroy_at(T *p) { p->~T(); } // #destroy
 }
 
 constexpr bool foo() {
@@ -49,18 +47,47 @@ static_assert((destroy_pointer(), true));
 
 
 namespace DestroyArrayElem {
-  /// This is proof that std::destroy_at'ing an array element
-  /// ends the lifetime of the entire array.
-  /// See https://github.com/llvm/llvm-project/issues/147528
-  /// Using destroy_at on array elements is currently a no-op due to this.
-  constexpr int test() {
+
+  constexpr int test1() {
     int a[4] = {};
+    std::destroy_at(&a); // both-error@#destroy {{object expression of non-scalar type 'int[4]' cannot be used in a pseudo-destructor expression}} \
+                         // both-note {{in instantiation of function template specialization 'std::destroy_at<int[4]>' requested here}} \
+                         // both-note {{subexpression not valid}}
+    return 0;
+  }
+  static_assert(test1() == 0); // both-error {{not an integral constant expression}} \
+                               // both-note {{in call to}}
+
+  constexpr int test2() {
+
+    int a[4] = {};
+    std::destroy_at(&a[1]);
+    int r = a[1]; // both-note {{read of object outside its lifetime}}
+    std::construct_at(&a[1]);
+    return 0;
+  }
+  static_assert(test2() == 0); // both-error {{not an integral constant expression}} \
+                               // both-note {{in call to}}
 
-    std::destroy_at(&a[3]);
-    int r = a[1];
+  constexpr int test3() {
+    int a[4]; /// Array with no init map.
     std::construct_at(&a[3]);
+    return a[3]; // both-note {{read of uninitialized object}}
+  }
+  static_assert(test3() == 0); // both-error {{not an integral constant expression}} \
+                               // both-note {{in call to}}
 
-    return r;
+  constexpr int test4(bool b) {
+    int a[4];
+    a[0] = 12;
+    std::construct_at(&a[3]);
+    return b ? a[0] : a[3]; // both-note {{read of uninitialized object}}
   }
-  static_assert(test() == 0);
+  static_assert(test4(true) == 12);
+  static_assert(test4(false) == 0); // both-error {{not an integral constant expression}} \
+                                    // both-note {{in call to}}
+
+
+
+
 }

[github-actions]

🪟 Windows x64 Test Results

• 53142 tests passed
• 2094 tests skipped

✅ The build succeeded and all tests passed.

[github-actions]

🐧 Linux x64 Test Results

• 112232 tests passed
• 4533 tests skipped

✅ The build succeeded and all tests passed.