[PATCH] D152132: [clang][Inter] Fix lifetime diagnostics for dead records

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

  This used to crash the interpreter, either because we ran into the
  assertion in CheckMutable() or because we accessed a Descriptor* pointer
  preceding the field of a record. Those are preceded by an
  InlineDescriptor though.

Also, we forgot to handle the metadata when moving the `Block` over to a `DeadBlock`.

(Regarding the InlineDescriptor stuff from above... I didn't add it in this patch because the problem does not require it, but I'll do fix that later).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152132

Files:
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/InterpBlock.h
  clang/lib/AST/Interp/InterpState.cpp
  clang/test/AST/Interp/lifetimes.cpp


Index: clang/test/AST/Interp/lifetimes.cpp
===================================================================
--- /dev/null
+++ clang/test/AST/Interp/lifetimes.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -verify=ref %s
+
+struct Foo {
+  int a;
+};
+
+constexpr int dead1() { // expected-error {{never produces a constant expression}}
+
+  Foo *F2 = nullptr;
+  {
+    Foo F{12}; // expected-note 2{{declared here}}
+    F2 = &F;
+  } // Ends lifetime of F.
+
+  return F2->a; // expected-note 2{{read of variable whose lifetime has ended}} \
+                // ref-note {{read of object outside its lifetime is not allowed in a constant expression}}
+}
+static_assert(dead1() == 1, ""); // expected-error {{not an integral constant expression}} \
+                                 // expected-note {{in call to}} \
+                                 // ref-error {{not an integral constant expression}} \
+                                 // ref-note {{in call to}} \
+
+
Index: clang/lib/AST/Interp/InterpState.cpp
===================================================================
--- clang/lib/AST/Interp/InterpState.cpp
+++ clang/lib/AST/Interp/InterpState.cpp
@@ -54,9 +54,13 @@
     char *Memory = reinterpret_cast<char *>(malloc(sizeof(DeadBlock) + Size));
     auto *D = new (Memory) DeadBlock(DeadBlocks, B);
 
-    // Move data from one block to another.
-    if (Desc->MoveFn)
+    // Move data and metadata from the old block to the new (dead)block.
+    if (Desc->MoveFn) {
       Desc->MoveFn(B, B->data(), D->data(), Desc);
+      if (Desc->getMetadataSize() > 0)
+        std::memcpy(D->rawData(), B->rawData(), Desc->getMetadataSize());
+    }
+
   } else {
     // Free storage, if necessary.
     if (Desc->DtorFn)
Index: clang/lib/AST/Interp/InterpBlock.h
===================================================================
--- clang/lib/AST/Interp/InterpBlock.h
+++ clang/lib/AST/Interp/InterpBlock.h
@@ -151,6 +151,7 @@
 
   /// Returns a pointer to the stored data.
   char *data() { return B.data(); }
+  char *rawData() { return B.rawData(); }
 
 private:
   friend class Block;
Index: clang/lib/AST/Interp/Interp.cpp
===================================================================
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -238,7 +238,6 @@
 }
 
 bool CheckMutable(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
-  assert(Ptr.isLive() && "Pointer is not live");
   if (!Ptr.isMutable()) {
     return true;
   }
Index: clang/lib/AST/Interp/Descriptor.cpp
===================================================================
--- clang/lib/AST/Interp/Descriptor.cpp
+++ clang/lib/AST/Interp/Descriptor.cpp
@@ -165,9 +165,8 @@
 static void moveRecord(Block *B, char *Src, char *Dst, const Descriptor *D) {
   for (const auto &F : D->ElemRecord->fields()) {
     auto FieldOff = F.Offset;
-    auto FieldDesc = F.Desc;
+    auto *FieldDesc = F.Desc;
 
-    *(reinterpret_cast<Descriptor **>(Dst + FieldOff) - 1) = FieldDesc;
     if (auto Fn = FieldDesc->MoveFn)
       Fn(B, Src + FieldOff, Dst + FieldOff, FieldDesc);
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D152132.528332.patch
Type: text/x-patch
Size: 3158 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230605/525f2bf0/attachment-0001.bin>