[clang][Interp] Fix lifetime diagnostics for dead records

[tbaeder]

Summary

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).

[tbaeder]

Ping

[tbaeder]

Ping

[tbaeder]

Ping

[tbaeder]

Ping

[tbaeder]

Ping

[aaron.ballman]

I think this is mostly good, but I did run into a question about removing the assertion so I might still be missing something about the changes.

clang/lib/AST/Interp/Interp.cpp

241

I understand that we were hitting this assertion, but why is removing the assertion the correct approach? I thought it made sense to check for pointer liveness here.

[tbaeder]

clang/lib/AST/Interp/Interp.cpp

241

I re-checked and I think I removed this only for testing and it should stay in. The test succeeds like this.

[aaron.ballman]

LGTM!

clang/lib/AST/Interp/Descriptor.cpp

for (const auto &F : D->ElemRecord->fields())
     DtorSub(F.Offset, F.Desc);
   for (const auto &F : D->ElemRecord->virtual_bases())
     DtorSub(F.Offset, F.Desc);
 }
 
 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);
   }
 }
 
 static BlockCtorFn getCtorPrim(PrimType Type) {
   // Floating types are special. They are primitives, but need their
   // constructor called.

clang/lib/AST/Interp/Interp.cpp

bool CheckConst(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
 
   const QualType Ty = Ptr.getType();
   const SourceInfo &Loc = S.Current->getSource(OpPC);
   S.FFDiag(Loc, diag::note_constexpr_modify_const_type) << Ty;
   return false;
 }
 
 bool CheckMutable(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
-  assert(Ptr.isLive() && "Pointer is not live");
   if (!Ptr.isMutable()) {
     return true;
   }
 
   // In C++14 onwards, it is permitted to read a mutable member whose
   // lifetime began within the evaluation.
   if (S.getLangOpts().CPlusPlus14)
     return true;

clang/lib/AST/Interp/InterpBlock.h

 /// whenever pointers become dead.
 class DeadBlock final {
 public:
   /// Copies the block.
   DeadBlock(DeadBlock *&Root, Block *Blk);
 
   /// Returns a pointer to the stored data.
   char *data() { return B.data(); }
+  char *rawData() { return B.rawData(); }
 
 private:
   friend class Block;
   friend class InterpState;
 
   void free();
 
   /// Root pointer of the list.

clang/lib/AST/Interp/InterpState.cpp

void InterpState::deallocate(Block *B) {
   Descriptor *Desc = B->getDescriptor();
   if (B->hasPointers()) {
     size_t Size = B->getSize();
 
     // Allocate a new block, transferring over pointers.
     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)
       Desc->DtorFn(B, B->data(), Desc);
   }
 }

clang/test/AST/Interp/lifetimes.cpp

+// 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}} \
+
+