[clang][bytecode] Recursively start lifetimes as well

[tbaederr]

The constructor starts the lifetime of all the subobjects.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

The constructor starts the lifetime of all the subobjects.

---

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

3 Files Affected:

• (modified) clang/lib/AST/ByteCode/DynamicAllocator.cpp (+5-3)
• (modified) clang/lib/AST/ByteCode/Interp.cpp (+30-7)
• (modified) clang/test/AST/ByteCode/placement-new.cpp (+42)

diff --git a/clang/lib/AST/ByteCode/DynamicAllocator.cpp b/clang/lib/AST/ByteCode/DynamicAllocator.cpp
index 733984508ed79..4f0f511fb6164 100644
--- a/clang/lib/AST/ByteCode/DynamicAllocator.cpp
+++ b/clang/lib/AST/ByteCode/DynamicAllocator.cpp
@@ -86,9 +86,11 @@ Block *DynamicAllocator::allocate(const Descriptor *D, unsigned EvalID,
   ID->IsInitialized = false;
   ID->IsVolatile = false;
 
-  ID->LifeState =
-      AllocForm == Form::Operator ? Lifetime::Ended : Lifetime::Started;
-  ;
+  if (D->isCompositeArray())
+    ID->LifeState = Lifetime::Started;
+  else
+    ID->LifeState =
+        AllocForm == Form::Operator ? Lifetime::Ended : Lifetime::Started;
 
   B->IsDynamic = true;
 
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index e454d9e3bc218..a8286dd75f09a 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -1699,20 +1699,38 @@ bool CallPtr(InterpState &S, CodePtr OpPC, uint32_t ArgSize,
   return Call(S, OpPC, F, VarArgSize);
 }
 
+static void startLifetimeRecurse(const Pointer &Ptr) {
+  if (const Record *R = Ptr.getRecord()) {
+    Ptr.startLifetime();
+    for (const Record::Field &Fi : R->fields())
+      startLifetimeRecurse(Ptr.atField(Fi.Offset));
+    return;
+  }
+
+  if (const Descriptor *FieldDesc = Ptr.getFieldDesc();
+      FieldDesc->isCompositeArray()) {
+    assert(Ptr.getLifetime() == Lifetime::Started);
+    for (unsigned I = 0; I != FieldDesc->getNumElems(); ++I)
+      startLifetimeRecurse(Ptr.atIndex(I).narrow());
+    return;
+  }
+
+  Ptr.startLifetime();
+}
+
 bool StartLifetime(InterpState &S, CodePtr OpPC) {
   const auto &Ptr = S.Stk.peek<Pointer>();
   if (!CheckDummy(S, OpPC, Ptr, AK_Destroy))
     return false;
-
-  Ptr.startLifetime();
+  startLifetimeRecurse(Ptr.narrow());
   return true;
 }
 
 // FIXME: It might be better to the recursing as part of the generated code for
 // a destructor?
 static void endLifetimeRecurse(const Pointer &Ptr) {
-  Ptr.endLifetime();
   if (const Record *R = Ptr.getRecord()) {
+    Ptr.endLifetime();
     for (const Record::Field &Fi : R->fields())
       endLifetimeRecurse(Ptr.atField(Fi.Offset));
     return;
@@ -1720,9 +1738,14 @@ static void endLifetimeRecurse(const Pointer &Ptr) {
 
   if (const Descriptor *FieldDesc = Ptr.getFieldDesc();
       FieldDesc->isCompositeArray()) {
+    // No endLifetime() for array roots.
+    assert(Ptr.getLifetime() == Lifetime::Started);
     for (unsigned I = 0; I != FieldDesc->getNumElems(); ++I)
       endLifetimeRecurse(Ptr.atIndex(I).narrow());
+    return;
   }
+
+  Ptr.endLifetime();
 }
 
 /// Ends the lifetime of the peek'd pointer.
@@ -1730,7 +1753,7 @@ bool EndLifetime(InterpState &S, CodePtr OpPC) {
   const auto &Ptr = S.Stk.peek<Pointer>();
   if (!CheckDummy(S, OpPC, Ptr, AK_Destroy))
     return false;
-  endLifetimeRecurse(Ptr);
+  endLifetimeRecurse(Ptr.narrow());
   return true;
 }
 
@@ -1739,7 +1762,7 @@ bool EndLifetimePop(InterpState &S, CodePtr OpPC) {
   const auto &Ptr = S.Stk.pop<Pointer>();
   if (!CheckDummy(S, OpPC, Ptr, AK_Destroy))
     return false;
-  endLifetimeRecurse(Ptr);
+  endLifetimeRecurse(Ptr.narrow());
   return true;
 }
 
@@ -1758,9 +1781,9 @@ bool CheckNewTypeMismatch(InterpState &S, CodePtr OpPC, const Expr *E,
 
   // CheckLifetime for this and all base pointers.
   for (Pointer P = Ptr;;) {
-    if (!CheckLifetime(S, OpPC, P, AK_Construct)) {
+    if (!CheckLifetime(S, OpPC, P, AK_Construct))
       return false;
-    }
+
     if (P.isRoot())
       break;
     P = P.getBase();
diff --git a/clang/test/AST/ByteCode/placement-new.cpp b/clang/test/AST/ByteCode/placement-new.cpp
index a301c96739c83..f23d71510602c 100644
--- a/clang/test/AST/ByteCode/placement-new.cpp
+++ b/clang/test/AST/ByteCode/placement-new.cpp
@@ -423,3 +423,45 @@ namespace SubObj {
   }
   static_assert(construct_after_lifetime_2()); // both-error {{}} both-note {{in call}}
 }
+
+namespace RecursiveLifetimeStart {
+  struct B {
+    int b;
+  };
+
+  struct A {
+    B b;
+    int a;
+  };
+
+  constexpr int foo() {
+    A a;
+    a.~A();
+
+    new (&a) A();
+    a.a = 10;
+    a.b.b = 12;
+    return a.a;
+  }
+  static_assert(foo() == 10);
+}
+
+namespace ArrayRoot {
+  struct S {
+    int a;
+  };
+  constexpr int foo() {
+    S* ss = std::allocator<S>().allocate(2);
+    new (ss) S{};
+    new (ss + 1) S{};
+
+    S* ps = &ss[2];
+    ps = ss;
+    ps->~S();
+
+    std::allocator<S>().deallocate(ss);
+    return 0;
+  }
+
+  static_assert(foo() == 0);
+}