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); }
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); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits