This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3671a69470f3: [clang][Interp] Call destructors of local
variables (authored by tbaeder).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154581/new/
https://reviews.llvm.org/D154581
Files:
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
Index: clang/test/AST/Interp/arrays.cpp
===================================================================
--- clang/test/AST/Interp/arrays.cpp
+++ clang/test/AST/Interp/arrays.cpp
@@ -406,3 +406,52 @@
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}}
+}
Index: clang/lib/AST/Interp/Pointer.h
===================================================================
--- clang/lib/AST/Interp/Pointer.h
+++ clang/lib/AST/Interp/Pointer.h
@@ -108,7 +108,7 @@
if (getFieldDesc()->ElemDesc)
Off += sizeof(InlineDescriptor);
else
- Off += sizeof(InitMap *);
+ Off += sizeof(InitMapPtr);
return Pointer(Pointee, Base, Base + Off);
}
@@ -147,7 +147,7 @@
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 @@
// 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 @@
if (getFieldDesc()->ElemDesc)
Adjust = sizeof(InlineDescriptor);
else
- Adjust = sizeof(InitMap *);
+ Adjust = sizeof(InitMapPtr);
}
return Offset - Base - Adjust;
}
@@ -359,7 +359,7 @@
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 @@
/// 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 @@
private:
friend class Block;
friend class DeadBlock;
+ friend struct InitMap;
Pointer(Block *Pointee, unsigned Base, unsigned Offset);
@@ -431,9 +432,9 @@
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.
Index: clang/lib/AST/Interp/Pointer.cpp
===================================================================
--- clang/lib/AST/Interp/Pointer.cpp
+++ clang/lib/AST/Interp/Pointer.cpp
@@ -164,13 +164,16 @@
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 @@
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;
}
Index: clang/lib/AST/Interp/InterpState.cpp
===================================================================
--- clang/lib/AST/Interp/InterpState.cpp
+++ clang/lib/AST/Interp/InterpState.cpp
@@ -65,9 +65,9 @@
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();
}
}
Index: clang/lib/AST/Interp/InterpFrame.cpp
===================================================================
--- clang/lib/AST/Interp/InterpFrame.cpp
+++ clang/lib/AST/Interp/InterpFrame.cpp
@@ -72,6 +72,19 @@
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) {
Index: clang/lib/AST/Interp/InterpBlock.h
===================================================================
--- clang/lib/AST/Interp/InterpBlock.h
+++ clang/lib/AST/Interp/InterpBlock.h
@@ -71,6 +71,7 @@
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 @@
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 @@
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;
};
Index: clang/lib/AST/Interp/EvalEmitter.h
===================================================================
--- clang/lib/AST/Interp/EvalEmitter.h
+++ clang/lib/AST/Interp/EvalEmitter.h
@@ -40,7 +40,7 @@
EvalEmitter(Context &Ctx, Program &P, State &Parent, InterpStack &Stk,
APValue &Result);
- virtual ~EvalEmitter() {}
+ virtual ~EvalEmitter();
/// Define a label.
void emitLabel(LabelTy Label);
Index: clang/lib/AST/Interp/EvalEmitter.cpp
===================================================================
--- clang/lib/AST/Interp/EvalEmitter.cpp
+++ clang/lib/AST/Interp/EvalEmitter.cpp
@@ -25,6 +25,14 @@
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;
Index: clang/lib/AST/Interp/Descriptor.h
===================================================================
--- clang/lib/AST/Interp/Descriptor.h
+++ clang/lib/AST/Interp/Descriptor.h
@@ -20,10 +20,12 @@
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 @@
};
/// 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 @@
/// 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
Index: clang/lib/AST/Interp/Descriptor.cpp
===================================================================
--- clang/lib/AST/Interp/Descriptor.cpp
+++ clang/lib/AST/Interp/Descriptor.cpp
@@ -40,6 +40,9 @@
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 @@
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 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 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 @@
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, 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, 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 @@
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 @@
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);
-}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
