[clang] 3fe2be7 - [clang][Interp] Call destructors of local variables
Timm Bäder via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 23 23:58:12 PDT 2023
Author: Timm Bäder
Date: 2023-10-24T08:57:59+02:00
New Revision: 3fe2be75c0707d3e386fdf96aec1deb69828ae74
URL: https://github.com/llvm/llvm-project/commit/3fe2be75c0707d3e386fdf96aec1deb69828ae74
DIFF: https://github.com/llvm/llvm-project/commit/3fe2be75c0707d3e386fdf96aec1deb69828ae74.diff
LOG: [clang][Interp] Call destructors of local variables
Differential Revision: https://reviews.llvm.org/D154581
Added:
Modified:
clang/lib/AST/Interp/Context.cpp
clang/lib/AST/Interp/Descriptor.cpp
clang/lib/AST/Interp/Descriptor.h
clang/lib/AST/Interp/EvalEmitter.cpp
clang/lib/AST/Interp/EvalEmitter.h
clang/lib/AST/Interp/InterpBlock.h
clang/lib/AST/Interp/InterpFrame.cpp
clang/lib/AST/Interp/InterpState.cpp
clang/lib/AST/Interp/Pointer.cpp
clang/lib/AST/Interp/Pointer.h
clang/test/AST/Interp/arrays.cpp
Removed:
################################################################################
diff --git a/clang/lib/AST/Interp/Context.cpp b/clang/lib/AST/Interp/Context.cpp
index 04710cd0f2c28ee..3ad63f40b0c72b7 100644
--- a/clang/lib/AST/Interp/Context.cpp
+++ b/clang/lib/AST/Interp/Context.cpp
@@ -160,8 +160,16 @@ const llvm::fltSemantics &Context::getFloatSemantics(QualType T) const {
bool Context::Run(State &Parent, const Function *Func, APValue &Result) {
InterpState State(Parent, *P, Stk, *this);
State.Current = new InterpFrame(State, Func, /*Caller=*/nullptr, {});
- if (Interpret(State, Result))
+ if (Interpret(State, Result)) {
+ assert(Stk.empty());
return true;
+ }
+
+ // We explicitly delete our state here, so the Stk.clear() call
+ // below doesn't violently free values the destructor would
+ // otherwise access.
+ State.~InterpState();
+
Stk.clear();
return false;
}
diff --git a/clang/lib/AST/Interp/Descriptor.cpp b/clang/lib/AST/Interp/Descriptor.cpp
index 3990282686fe3d6..abd75a8d67bbd3f 100644
--- a/clang/lib/AST/Interp/Descriptor.cpp
+++ b/clang/lib/AST/Interp/Descriptor.cpp
@@ -40,6 +40,9 @@ static void moveTy(Block *, const std::byte *Src, std::byte *Dst,
template <typename T>
static void ctorArrayTy(Block *, std::byte *Ptr, bool, bool, bool,
const Descriptor *D) {
+ new (Ptr) InitMapPtr(std::nullopt);
+
+ Ptr += sizeof(InitMapPtr);
for (unsigned I = 0, NE = D->getNumElems(); I < NE; ++I) {
new (&reinterpret_cast<T *>(Ptr)[I]) T();
}
@@ -47,11 +50,11 @@ static void ctorArrayTy(Block *, std::byte *Ptr, bool, bool, bool,
template <typename T>
static void dtorArrayTy(Block *, std::byte *Ptr, const Descriptor *D) {
- InitMap *IM = *reinterpret_cast<InitMap **>(Ptr);
- if (IM != (InitMap *)-1)
- free(IM);
+ InitMapPtr &IMP = *reinterpret_cast<InitMapPtr *>(Ptr);
- Ptr += sizeof(InitMap *);
+ if (IMP)
+ IMP = std::nullopt;
+ Ptr += sizeof(InitMapPtr);
for (unsigned I = 0, NE = D->getNumElems(); I < NE; ++I) {
reinterpret_cast<T *>(Ptr)[I].~T();
}
@@ -209,7 +212,8 @@ static BlockMoveFn getMovePrim(PrimType Type) {
}
static BlockCtorFn getCtorArrayPrim(PrimType Type) {
- COMPOSITE_TYPE_SWITCH(Type, return ctorArrayTy<T>, return nullptr);
+ TYPE_SWITCH(Type, return ctorArrayTy<T>);
+ llvm_unreachable("unknown Expr");
}
static BlockDtorFn getDtorArrayPrim(PrimType Type) {
@@ -218,7 +222,8 @@ static BlockDtorFn getDtorArrayPrim(PrimType Type) {
}
static BlockMoveFn getMoveArrayPrim(PrimType Type) {
- COMPOSITE_TYPE_SWITCH(Type, return moveArrayTy<T>, return nullptr);
+ TYPE_SWITCH(Type, return moveArrayTy<T>);
+ llvm_unreachable("unknown Expr");
}
/// Primitives.
@@ -238,7 +243,7 @@ Descriptor::Descriptor(const DeclTy &D, PrimType Type, MetadataSize MD,
bool IsMutable)
: Source(D), ElemSize(primSize(Type)), Size(ElemSize * NumElems),
MDSize(MD.value_or(0)),
- AllocSize(align(Size) + sizeof(InitMap *) + MDSize), IsConst(IsConst),
+ AllocSize(align(Size) + sizeof(InitMapPtr) + MDSize), IsConst(IsConst),
IsMutable(IsMutable), IsTemporary(IsTemporary), IsArray(true),
CtorFn(getCtorArrayPrim(Type)), DtorFn(getDtorArrayPrim(Type)),
MoveFn(getMoveArrayPrim(Type)) {
@@ -249,9 +254,10 @@ Descriptor::Descriptor(const DeclTy &D, PrimType Type, MetadataSize MD,
Descriptor::Descriptor(const DeclTy &D, PrimType Type, bool IsTemporary,
UnknownSize)
: Source(D), ElemSize(primSize(Type)), Size(UnknownSizeMark), MDSize(0),
- AllocSize(alignof(void *)), IsConst(true), IsMutable(false),
- IsTemporary(IsTemporary), IsArray(true), CtorFn(getCtorArrayPrim(Type)),
- DtorFn(getDtorArrayPrim(Type)), MoveFn(getMoveArrayPrim(Type)) {
+ AllocSize(alignof(void *) + sizeof(InitMapPtr)), IsConst(true),
+ IsMutable(false), IsTemporary(IsTemporary), IsArray(true),
+ CtorFn(getCtorArrayPrim(Type)), DtorFn(getDtorArrayPrim(Type)),
+ MoveFn(getMoveArrayPrim(Type)) {
assert(Source && "Missing source");
}
@@ -272,10 +278,10 @@ Descriptor::Descriptor(const DeclTy &D, Descriptor *Elem, MetadataSize MD,
Descriptor::Descriptor(const DeclTy &D, Descriptor *Elem, bool IsTemporary,
UnknownSize)
: Source(D), ElemSize(Elem->getAllocSize() + sizeof(InlineDescriptor)),
- Size(UnknownSizeMark), MDSize(0), AllocSize(alignof(void *)),
- ElemDesc(Elem), IsConst(true), IsMutable(false), IsTemporary(IsTemporary),
- IsArray(true), CtorFn(ctorArrayDesc), DtorFn(dtorArrayDesc),
- MoveFn(moveArrayDesc) {
+ Size(UnknownSizeMark), MDSize(0),
+ AllocSize(alignof(void *) + sizeof(InitMapPtr)), ElemDesc(Elem),
+ IsConst(true), IsMutable(false), IsTemporary(IsTemporary), IsArray(true),
+ CtorFn(ctorArrayDesc), DtorFn(dtorArrayDesc), MoveFn(moveArrayDesc) {
assert(Source && "Missing source");
}
@@ -314,21 +320,12 @@ SourceLocation Descriptor::getLocation() const {
llvm_unreachable("Invalid descriptor type");
}
-InitMap::InitMap(unsigned N) : UninitFields(N) {
- std::fill_n(data(), (N + PER_FIELD - 1) / PER_FIELD, 0);
+InitMap::InitMap(unsigned N)
+ : UninitFields(N), Data(std::make_unique<T[]>(numFields(N))) {
+ std::fill_n(data(), numFields(N), 0);
}
-InitMap::T *InitMap::data() {
- auto *Start = reinterpret_cast<char *>(this) + align(sizeof(InitMap));
- return reinterpret_cast<T *>(Start);
-}
-
-const InitMap::T *InitMap::data() const {
- auto *Start = reinterpret_cast<const char *>(this) + align(sizeof(InitMap));
- return reinterpret_cast<const T *>(Start);
-}
-
-bool InitMap::initialize(unsigned I) {
+bool InitMap::initializeElement(unsigned I) {
unsigned Bucket = I / PER_FIELD;
T Mask = T(1) << (I % PER_FIELD);
if (!(data()[Bucket] & Mask)) {
@@ -338,13 +335,7 @@ bool InitMap::initialize(unsigned I) {
return UninitFields == 0;
}
-bool InitMap::isInitialized(unsigned I) const {
+bool InitMap::isElementInitialized(unsigned I) const {
unsigned Bucket = I / PER_FIELD;
return data()[Bucket] & (T(1) << (I % PER_FIELD));
}
-
-InitMap *InitMap::allocate(unsigned N) {
- const size_t NumFields = ((N + PER_FIELD - 1) / PER_FIELD);
- const size_t Size = align(sizeof(InitMap)) + NumFields * PER_FIELD;
- return new (malloc(Size)) InitMap(N);
-}
diff --git a/clang/lib/AST/Interp/Descriptor.h b/clang/lib/AST/Interp/Descriptor.h
index 55a754c3505cce7..be9a380138a7b11 100644
--- a/clang/lib/AST/Interp/Descriptor.h
+++ b/clang/lib/AST/Interp/Descriptor.h
@@ -20,10 +20,12 @@ namespace clang {
namespace interp {
class Block;
class Record;
+struct InitMap;
struct Descriptor;
enum PrimType : unsigned;
using DeclTy = llvm::PointerUnion<const Decl *, const Expr *>;
+using InitMapPtr = std::optional<std::pair<bool, std::shared_ptr<InitMap>>>;
/// Invoked whenever a block is created. The constructor method fills in the
/// inline descriptors of all fields and array elements. It also initializes
@@ -193,9 +195,6 @@ struct Descriptor final {
};
/// Bitfield tracking the initialisation status of elements of primitive arrays.
-/// A pointer to this is embedded at the end of all primitive arrays.
-/// If the map was not yet created and nothing was initialized, the pointer to
-/// this structure is 0. If the object was fully initialized, the pointer is -1.
struct InitMap final {
private:
/// Type packing bits.
@@ -203,26 +202,29 @@ struct InitMap final {
/// Bits stored in a single field.
static constexpr uint64_t PER_FIELD = sizeof(T) * CHAR_BIT;
+public:
/// Initializes the map with no fields set.
- InitMap(unsigned N);
+ explicit InitMap(unsigned N);
+
+private:
+ friend class Pointer;
/// Returns a pointer to storage.
- T *data();
- const T *data() const;
+ T *data() { return Data.get(); }
+ const T *data() const { return Data.get(); }
-public:
/// Initializes an element. Returns true when object if fully initialized.
- bool initialize(unsigned I);
+ bool initializeElement(unsigned I);
/// Checks if an element was initialized.
- bool isInitialized(unsigned I) const;
+ bool isElementInitialized(unsigned I) const;
- /// Allocates a map holding N elements.
- static InitMap *allocate(unsigned N);
-
-private:
- /// Number of fields initialized.
+ static constexpr size_t numFields(unsigned N) {
+ return (N + PER_FIELD - 1) / PER_FIELD;
+ }
+ /// Number of fields not initialized.
unsigned UninitFields;
+ std::unique_ptr<T[]> Data;
};
} // namespace interp
diff --git a/clang/lib/AST/Interp/EvalEmitter.cpp b/clang/lib/AST/Interp/EvalEmitter.cpp
index f46ef1067cf52a0..f8942291b3b162d 100644
--- a/clang/lib/AST/Interp/EvalEmitter.cpp
+++ b/clang/lib/AST/Interp/EvalEmitter.cpp
@@ -25,6 +25,14 @@ EvalEmitter::EvalEmitter(Context &Ctx, Program &P, State &Parent,
new InterpFrame(S, /*Func=*/nullptr, /*Caller=*/nullptr, CodePtr());
}
+EvalEmitter::~EvalEmitter() {
+ for (auto &[K, V] : Locals) {
+ Block *B = reinterpret_cast<Block *>(V.get());
+ if (B->isInitialized())
+ B->invokeDtor();
+ }
+}
+
llvm::Expected<bool> EvalEmitter::interpretExpr(const Expr *E) {
if (this->visitExpr(E))
return true;
diff --git a/clang/lib/AST/Interp/EvalEmitter.h b/clang/lib/AST/Interp/EvalEmitter.h
index 1128e0fb09f0de8..5a9be18c34a03bc 100644
--- a/clang/lib/AST/Interp/EvalEmitter.h
+++ b/clang/lib/AST/Interp/EvalEmitter.h
@@ -40,7 +40,7 @@ class EvalEmitter : public SourceMapper {
EvalEmitter(Context &Ctx, Program &P, State &Parent, InterpStack &Stk,
APValue &Result);
- virtual ~EvalEmitter() {}
+ virtual ~EvalEmitter();
/// Define a label.
void emitLabel(LabelTy Label);
diff --git a/clang/lib/AST/Interp/InterpBlock.h b/clang/lib/AST/Interp/InterpBlock.h
index 5112108e6f010a6..89f937b588fb678 100644
--- a/clang/lib/AST/Interp/InterpBlock.h
+++ b/clang/lib/AST/Interp/InterpBlock.h
@@ -71,6 +71,7 @@ class Block final {
unsigned getSize() const { return Desc->getAllocSize(); }
/// Returns the declaration ID.
std::optional<unsigned> getDeclID() const { return DeclID; }
+ bool isInitialized() const { return IsInitialized; }
/// Returns a pointer to the stored data.
/// You are allowed to read Desc->getSize() bytes from this address.
@@ -104,12 +105,14 @@ class Block final {
if (Desc->CtorFn)
Desc->CtorFn(this, data(), Desc->IsConst, Desc->IsMutable,
/*isActive=*/true, Desc);
+ IsInitialized = true;
}
/// Invokes the Destructor.
void invokeDtor() {
if (Desc->DtorFn)
Desc->DtorFn(this, data(), Desc);
+ IsInitialized = false;
}
protected:
@@ -139,8 +142,12 @@ class Block final {
bool IsStatic = false;
/// Flag indicating if the block is an extern.
bool IsExtern = false;
- /// Flag indicating if the pointer is dead.
+ /// Flag indicating if the pointer is dead. This is only ever
+ /// set once, when converting the Block to a DeadBlock.
bool IsDead = false;
+ /// Flag indicating if the block contents have been initialized
+ /// via invokeCtor.
+ bool IsInitialized = false;
/// Pointer to the stack slot descriptor.
Descriptor *Desc;
};
diff --git a/clang/lib/AST/Interp/InterpFrame.cpp b/clang/lib/AST/Interp/InterpFrame.cpp
index 49db83a965ab001..b06923114c7a24e 100644
--- a/clang/lib/AST/Interp/InterpFrame.cpp
+++ b/clang/lib/AST/Interp/InterpFrame.cpp
@@ -72,6 +72,19 @@ InterpFrame::InterpFrame(InterpState &S, const Function *Func, CodePtr RetPC)
InterpFrame::~InterpFrame() {
for (auto &Param : Params)
S.deallocate(reinterpret_cast<Block *>(Param.second.get()));
+
+ // When destroying the InterpFrame, call the Dtor for all block
+ // that haven't been destroyed via a destroy() op yet.
+ // This happens when the execution is interruped midway-through.
+ if (Func) {
+ for (auto &Scope : Func->scopes()) {
+ for (auto &Local : Scope.locals()) {
+ Block *B = localBlock(Local.Offset);
+ if (B->isInitialized())
+ B->invokeDtor();
+ }
+ }
+ }
}
void InterpFrame::destroy(unsigned Idx) {
diff --git a/clang/lib/AST/Interp/InterpState.cpp b/clang/lib/AST/Interp/InterpState.cpp
index 4f050baea39e79a..2cb87ef07fe5882 100644
--- a/clang/lib/AST/Interp/InterpState.cpp
+++ b/clang/lib/AST/Interp/InterpState.cpp
@@ -65,9 +65,9 @@ void InterpState::deallocate(Block *B) {
std::memcpy(D->rawData(), B->rawData(), Desc->getMetadataSize());
}
+ // We moved the contents over to the DeadBlock.
+ B->IsInitialized = false;
} else {
- // Free storage, if necessary.
- if (Desc->DtorFn)
- Desc->DtorFn(B, B->data(), Desc);
+ B->invokeDtor();
}
}
diff --git a/clang/lib/AST/Interp/Pointer.cpp b/clang/lib/AST/Interp/Pointer.cpp
index e04d29b8bd81be4..d1af58203bec64d 100644
--- a/clang/lib/AST/Interp/Pointer.cpp
+++ b/clang/lib/AST/Interp/Pointer.cpp
@@ -164,13 +164,16 @@ bool Pointer::isInitialized() const {
if (Desc->isPrimitiveArray()) {
if (isStatic() && Base == 0)
return true;
- // Primitive array field are stored in a bitset.
- InitMap *Map = getInitMap();
- if (!Map)
+
+ InitMapPtr &IM = getInitMap();
+
+ if (!IM)
return false;
- if (Map == (InitMap *)-1)
+
+ if (IM->first)
return true;
- return Map->isInitialized(getIndex());
+
+ return IM->second->isElementInitialized(getIndex());
}
// Field has its bit in an inline descriptor.
@@ -187,15 +190,20 @@ void Pointer::initialize() const {
if (isStatic() && Base == 0)
return;
- // Primitive array initializer.
- InitMap *&Map = getInitMap();
- if (Map == (InitMap *)-1)
+ InitMapPtr &IM = getInitMap();
+ if (!IM)
+ IM =
+ std::make_pair(false, std::make_shared<InitMap>(Desc->getNumElems()));
+
+ assert(IM);
+
+ // All initialized.
+ if (IM->first)
return;
- if (Map == nullptr)
- Map = InitMap::allocate(Desc->getNumElems());
- if (Map->initialize(getIndex())) {
- free(Map);
- Map = (InitMap *)-1;
+
+ if (IM->second->initializeElement(getIndex())) {
+ IM->first = true;
+ IM->second.reset();
}
return;
}
diff --git a/clang/lib/AST/Interp/Pointer.h b/clang/lib/AST/Interp/Pointer.h
index d5279e757f04764..3b21290332a9d2f 100644
--- a/clang/lib/AST/Interp/Pointer.h
+++ b/clang/lib/AST/Interp/Pointer.h
@@ -108,7 +108,7 @@ class Pointer {
if (getFieldDesc()->ElemDesc)
Off += sizeof(InlineDescriptor);
else
- Off += sizeof(InitMap *);
+ Off += sizeof(InitMapPtr);
return Pointer(Pointee, Base, Base + Off);
}
@@ -147,7 +147,7 @@ class Pointer {
if (inPrimitiveArray()) {
if (Offset != Base)
return *this;
- return Pointer(Pointee, Base, Offset + sizeof(InitMap *));
+ return Pointer(Pointee, Base, Offset + sizeof(InitMapPtr));
}
// Pointer is to a field or array element - enter it.
@@ -168,7 +168,7 @@ class Pointer {
// Revert to an outer one-past-end pointer.
unsigned Adjust;
if (inPrimitiveArray())
- Adjust = sizeof(InitMap *);
+ Adjust = sizeof(InitMapPtr);
else
Adjust = sizeof(InlineDescriptor);
return Pointer(Pointee, Base, Base + getSize() + Adjust);
@@ -257,7 +257,7 @@ class Pointer {
if (getFieldDesc()->ElemDesc)
Adjust = sizeof(InlineDescriptor);
else
- Adjust = sizeof(InitMap *);
+ Adjust = sizeof(InitMapPtr);
}
return Offset - Base - Adjust;
}
@@ -359,7 +359,7 @@ class Pointer {
assert(isLive() && "Invalid pointer");
if (isArrayRoot())
return *reinterpret_cast<T *>(Pointee->rawData() + Base +
- sizeof(InitMap *));
+ sizeof(InitMapPtr));
return *reinterpret_cast<T *>(Pointee->rawData() + Offset);
}
@@ -367,7 +367,7 @@ class Pointer {
/// Dereferences a primitive element.
template <typename T> T &elem(unsigned I) const {
assert(I < getNumElems());
- return reinterpret_cast<T *>(Pointee->data() + sizeof(InitMap *))[I];
+ return reinterpret_cast<T *>(Pointee->data() + sizeof(InitMapPtr))[I];
}
/// Initializes a field.
@@ -418,6 +418,7 @@ class Pointer {
private:
friend class Block;
friend class DeadBlock;
+ friend struct InitMap;
Pointer(Block *Pointee, unsigned Base, unsigned Offset);
@@ -431,9 +432,9 @@ class Pointer {
1;
}
- /// Returns a reference to the pointer which stores the initialization map.
- InitMap *&getInitMap() const {
- return *reinterpret_cast<InitMap **>(Pointee->rawData() + Base);
+ /// Returns a reference to the InitMapPtr which stores the initialization map.
+ InitMapPtr &getInitMap() const {
+ return *reinterpret_cast<InitMapPtr *>(Pointee->rawData() + Base);
}
/// The block the pointer is pointing to.
diff --git a/clang/test/AST/Interp/arrays.cpp b/clang/test/AST/Interp/arrays.cpp
index ba29632e97c30a6..7110785ea4c662a 100644
--- a/clang/test/AST/Interp/arrays.cpp
+++ b/clang/test/AST/Interp/arrays.cpp
@@ -406,3 +406,52 @@ namespace StringZeroFill {
static_assert(b[4] == '\0', "");
static_assert(b[5] == '\0', "");
}
+
+namespace NoInitMapLeak {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdivision-by-zero"
+#pragma clang diagnostic ignored "-Wc++20-extensions"
+ constexpr int testLeak() { // expected-error {{never produces a constant expression}} \
+ // ref-error {{never produces a constant expression}}
+ int a[2];
+ a[0] = 1;
+ // interrupts interpretation.
+ (void)(1 / 0); // expected-note 2{{division by zero}} \
+ // ref-note 2{{division by zero}}
+
+
+ return 1;
+ }
+#pragma clang diagnostic pop
+ static_assert(testLeak() == 1, ""); // expected-error {{not an integral constant expression}} \
+ // expected-note {{in call to 'testLeak()'}} \
+ // ref-error {{not an integral constant expression}} \
+ // ref-note {{in call to 'testLeak()'}}
+
+
+ constexpr int a[] = {1,2,3,4/0,5}; // expected-error {{must be initialized by a constant expression}} \
+ // expected-note {{division by zero}} \
+ // ref-error {{must be initialized by a constant expression}} \
+ // ref-note {{division by zero}} \
+ // ref-note {{declared here}}
+
+ /// FIXME: This should fail in the new interpreter as well.
+ constexpr int b = a[0]; // ref-error {{must be initialized by a constant expression}} \
+ // ref-note {{is not a constant expression}} \
+ // ref-note {{declared here}}
+ static_assert(b == 1, ""); // ref-error {{not an integral constant expression}} \
+ // ref-note {{not a constant expression}}
+
+ constexpr int f() { // expected-error {{never produces a constant expression}} \
+ // ref-error {{never produces a constant expression}}
+ int a[] = {19,2,3/0,4}; // expected-note 2{{division by zero}} \
+ // expected-warning {{is undefined}} \
+ // ref-note 2{{division by zero}} \
+ // ref-warning {{is undefined}}
+ return 1;
+ }
+ static_assert(f() == 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}}
+}
More information about the cfe-commits
mailing list