tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, cor3ntin, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
InitMaps are created for primitive arrays when the first element of the arrays
is initialized, and they are free'd when the last element is initialized,
leaving the array in a fully initialized state.
The initmap is also free'd in the `Descriptor`'s destructor function, which is
called at the end of a scope.
This works fine when the execution of the program is not interrupted. However,
when it is, we never destroy the scopes, leaving the initmaps behind.
To fix this, track the initmaps in `InterpState` and free them manually in
`~InterpState()`, so we don't leak the memory and fail on LSan enabled builders.
(Side note: There is a similar problem with the current handling of floating
point numbers, i.e. the `APFloat` might heap allocate some memory and we will
leak this when the execution is interrupted).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D154581
Files:
clang/lib/AST/Interp/Descriptor.cpp
clang/lib/AST/Interp/Descriptor.h
clang/lib/AST/Interp/Interp.h
clang/lib/AST/Interp/InterpBlock.h
clang/lib/AST/Interp/InterpState.cpp
clang/lib/AST/Interp/InterpState.h
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
@@ -350,3 +350,25 @@
static_assert(b.f[0] == 0.0, "");
static_assert(b.f[1] == 0.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()'}}
+}
Index: clang/lib/AST/Interp/Pointer.h
===================================================================
--- clang/lib/AST/Interp/Pointer.h
+++ clang/lib/AST/Interp/Pointer.h
@@ -353,7 +353,7 @@
}
/// Initializes a field.
- void initialize() const;
+ void initialize(InterpState &S) const;
/// Activats a field.
void activate() const;
/// Deactivates an entire strurcutre.
Index: clang/lib/AST/Interp/Pointer.cpp
===================================================================
--- clang/lib/AST/Interp/Pointer.cpp
+++ clang/lib/AST/Interp/Pointer.cpp
@@ -13,6 +13,7 @@
#include "Function.h"
#include "Integral.h"
#include "InterpBlock.h"
+#include "InterpState.h"
#include "PrimType.h"
#include "Record.h"
@@ -177,7 +178,7 @@
return Base == 0 || getInlineDesc()->IsInitialized;
}
-void Pointer::initialize() const {
+void Pointer::initialize(InterpState &S) const {
assert(Pointee && "Cannot initialize null pointer");
const Descriptor *Desc = getFieldDesc();
@@ -192,9 +193,9 @@
if (Map == (InitMap *)-1)
return;
if (Map == nullptr)
- Map = InitMap::allocate(Desc->getNumElems());
+ Map = S.allocateInitMap(Desc->getNumElems());
if (Map->initialize(getIndex())) {
- free(Map);
+ S.deallocateInitMap(Map);
Map = (InitMap *)-1;
}
}
Index: clang/lib/AST/Interp/InterpState.h
===================================================================
--- clang/lib/AST/Interp/InterpState.h
+++ clang/lib/AST/Interp/InterpState.h
@@ -91,6 +91,10 @@
Context &getContext() const { return Ctx; }
+ /// InitMap handling.
+ InitMap *allocateInitMap(unsigned N);
+ void deallocateInitMap(InitMap *IM);
+
private:
/// AST Walker state.
State &Parent;
@@ -98,6 +102,8 @@
DeadBlock *DeadBlocks = nullptr;
/// Reference to the offset-source mapping.
SourceMapper *M;
+ /// List of known InitMaps for primitive arrays.
+ std::vector<InitMap *> InitMaps;
public:
/// Reference to the module containing all bytecode.
Index: clang/lib/AST/Interp/InterpState.cpp
===================================================================
--- clang/lib/AST/Interp/InterpState.cpp
+++ clang/lib/AST/Interp/InterpState.cpp
@@ -31,6 +31,12 @@
free(DeadBlocks);
DeadBlocks = Next;
}
+
+ // If any InitMaps still exist at this point, the primitive array they belong
+ // to has not been fully initialized AND the execution of the program has been
+ // interrupted.
+ for (InitMap *IM : InitMaps)
+ std::free(IM);
}
Frame *InterpState::getCurrentFrame() {
@@ -67,6 +73,18 @@
} else {
// Free storage, if necessary.
if (Desc->DtorFn)
- Desc->DtorFn(B, B->data(), Desc);
+ Desc->DtorFn(B, B->data(), Desc, this);
}
}
+
+InitMap *InterpState::allocateInitMap(unsigned N) {
+ InitMap *IM = new (std::malloc(InitMap::byteSize(N))) InitMap(N);
+
+ InitMaps.push_back(IM);
+ return IM;
+}
+
+void InterpState::deallocateInitMap(InitMap *IM) {
+ llvm::erase_value(InitMaps, IM);
+ std::free(IM);
+}
Index: clang/lib/AST/Interp/InterpBlock.h
===================================================================
--- clang/lib/AST/Interp/InterpBlock.h
+++ clang/lib/AST/Interp/InterpBlock.h
@@ -107,7 +107,7 @@
/// Invokes the Destructor.
void invokeDtor() {
if (Desc->DtorFn)
- Desc->DtorFn(this, data(), Desc);
+ Desc->DtorFn(this, data(), Desc, nullptr);
}
protected:
Index: clang/lib/AST/Interp/Interp.h
===================================================================
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -901,7 +901,7 @@
const Pointer &Field = Obj.atField(I);
if (!CheckStore(S, OpPC, Field))
return false;
- Field.initialize();
+ Field.initialize(S);
Field.deref<T>() = Value;
return true;
}
@@ -1010,7 +1010,7 @@
return false;
const Pointer &Field = This.atField(I);
Field.deref<T>() = S.Stk.pop<T>();
- Field.initialize();
+ Field.initialize(S);
return true;
}
@@ -1024,7 +1024,7 @@
const Pointer &Field = This.atField(F->Offset);
const auto &Value = S.Stk.pop<T>();
Field.deref<T>() = Value.truncate(F->Decl->getBitWidthValue(S.getCtx()));
- Field.initialize();
+ Field.initialize(S);
return true;
}
@@ -1038,7 +1038,7 @@
const Pointer &Field = This.atField(I);
Field.deref<T>() = S.Stk.pop<T>();
Field.activate();
- Field.initialize();
+ Field.initialize(S);
return true;
}
@@ -1051,7 +1051,7 @@
const Pointer &Field = S.Stk.peek<Pointer>().atField(I);
Field.deref<T>() = Value;
Field.activate();
- Field.initialize();
+ Field.initialize(S);
return true;
}
@@ -1061,7 +1061,7 @@
const Pointer &Field = S.Stk.pop<Pointer>().atField(F->Offset);
Field.deref<T>() = Value.truncate(F->Decl->getBitWidthValue(S.getCtx()));
Field.activate();
- Field.initialize();
+ Field.initialize(S);
return true;
}
@@ -1072,7 +1072,7 @@
const Pointer &Field = Ptr.atField(I);
Field.deref<T>() = Value;
Field.activate();
- Field.initialize();
+ Field.initialize(S);
return true;
}
@@ -1251,7 +1251,7 @@
if (!CheckStore(S, OpPC, Ptr))
return false;
if (!Ptr.isRoot())
- Ptr.initialize();
+ Ptr.initialize(S);
Ptr.deref<T>() = Value;
return true;
}
@@ -1263,7 +1263,7 @@
if (!CheckStore(S, OpPC, Ptr))
return false;
if (!Ptr.isRoot())
- Ptr.initialize();
+ Ptr.initialize(S);
Ptr.deref<T>() = Value;
return true;
}
@@ -1275,7 +1275,7 @@
if (!CheckStore(S, OpPC, Ptr))
return false;
if (!Ptr.isRoot())
- Ptr.initialize();
+ Ptr.initialize(S);
if (auto *FD = Ptr.getField()) {
Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue(S.getCtx()));
} else {
@@ -1291,7 +1291,7 @@
if (!CheckStore(S, OpPC, Ptr))
return false;
if (!Ptr.isRoot())
- Ptr.initialize();
+ Ptr.initialize(S);
if (auto *FD = Ptr.getField()) {
Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue(S.getCtx()));
} else {
@@ -1306,7 +1306,7 @@
const Pointer &Ptr = S.Stk.pop<Pointer>();
if (!CheckInit(S, OpPC, Ptr))
return false;
- Ptr.initialize();
+ Ptr.initialize(S);
new (&Ptr.deref<T>()) T(Value);
return true;
}
@@ -1320,7 +1320,7 @@
const Pointer &Ptr = S.Stk.peek<Pointer>().atIndex(Idx);
if (!CheckInit(S, OpPC, Ptr))
return false;
- Ptr.initialize();
+ Ptr.initialize(S);
new (&Ptr.deref<T>()) T(Value);
return true;
}
@@ -1332,7 +1332,7 @@
const Pointer &Ptr = S.Stk.pop<Pointer>().atIndex(Idx);
if (!CheckInit(S, OpPC, Ptr))
return false;
- Ptr.initialize();
+ Ptr.initialize(S);
new (&Ptr.deref<T>()) T(Value);
return true;
}
Index: clang/lib/AST/Interp/Descriptor.h
===================================================================
--- clang/lib/AST/Interp/Descriptor.h
+++ clang/lib/AST/Interp/Descriptor.h
@@ -13,6 +13,7 @@
#ifndef LLVM_CLANG_AST_INTERP_DESCRIPTOR_H
#define LLVM_CLANG_AST_INTERP_DESCRIPTOR_H
+#include "PrimType.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
@@ -20,6 +21,7 @@
namespace interp {
class Block;
class Record;
+class InterpState;
struct Descriptor;
enum PrimType : unsigned;
@@ -35,7 +37,7 @@
/// Invoked when a block is destroyed. Invokes the destructors of all
/// non-trivial nested fields of arrays and records.
using BlockDtorFn = void (*)(Block *Storage, char *FieldPtr,
- const Descriptor *FieldDesc);
+ const Descriptor *FieldDesc, InterpState *S);
/// Invoked when a block with pointers referencing it goes out of scope. Such
/// blocks are persisted: the move function copies all inline descriptors and
@@ -215,10 +217,11 @@
/// Checks if an element was initialized.
bool isInitialized(unsigned I) const;
- /// Allocates a map holding N elements.
- static InitMap *allocate(unsigned N);
+ /// Returns the size of the InitMap in bytes, given \p N elements.
+ static size_t byteSize(unsigned N);
private:
+ friend class InterpState;
/// Number of fields initialized.
unsigned UninitFields;
};
Index: clang/lib/AST/Interp/Descriptor.cpp
===================================================================
--- clang/lib/AST/Interp/Descriptor.cpp
+++ clang/lib/AST/Interp/Descriptor.cpp
@@ -10,6 +10,7 @@
#include "Boolean.h"
#include "Floating.h"
#include "FunctionPointer.h"
+#include "InterpState.h"
#include "Pointer.h"
#include "PrimType.h"
#include "Record.h"
@@ -23,7 +24,7 @@
}
template <typename T>
-static void dtorTy(Block *, char *Ptr, const Descriptor *) {
+static void dtorTy(Block *, char *Ptr, const Descriptor *, InterpState *) {
reinterpret_cast<T *>(Ptr)->~T();
}
@@ -43,11 +44,11 @@
}
template <typename T>
-static void dtorArrayTy(Block *, char *Ptr, const Descriptor *D) {
- llvm::errs() << __PRETTY_FUNCTION__ << "\n";
+static void dtorArrayTy(Block *, char *Ptr, const Descriptor *D,
+ InterpState *S) {
InitMap *IM = *reinterpret_cast<InitMap **>(Ptr);
- if (IM != (InitMap *)-1)
- free(IM);
+ if (IM != (InitMap *)-1 && S)
+ S->deallocateInitMap(IM);
Ptr += sizeof(InitMap *);
for (unsigned I = 0, NE = D->getNumElems(); I < NE; ++I) {
@@ -91,7 +92,8 @@
}
}
-static void dtorArrayDesc(Block *B, char *Ptr, const Descriptor *D) {
+static void dtorArrayDesc(Block *B, char *Ptr, const Descriptor *D,
+ InterpState *S) {
const unsigned NumElems = D->getNumElems();
const unsigned ElemSize =
D->ElemDesc->getAllocSize() + sizeof(InlineDescriptor);
@@ -102,7 +104,7 @@
auto *Desc = reinterpret_cast<InlineDescriptor *>(ElemPtr);
auto *ElemLoc = reinterpret_cast<char *>(Desc + 1);
if (auto Fn = D->ElemDesc->DtorFn)
- Fn(B, ElemLoc, D->ElemDesc);
+ Fn(B, ElemLoc, D->ElemDesc, S);
}
}
@@ -152,17 +154,18 @@
CtorSub(V.Offset, V.Desc, /*isBase=*/true);
}
-static void dtorRecord(Block *B, char *Ptr, const Descriptor *D) {
- auto DtorSub = [=](unsigned SubOff, Descriptor *F) {
+static void dtorRecord(Block *B, char *Ptr, const Descriptor *D,
+ InterpState *S) {
+ auto DtorSub = [=](unsigned SubOff, Descriptor *F, InterpState *S) {
if (auto Fn = F->DtorFn)
- Fn(B, Ptr + SubOff, F);
+ Fn(B, Ptr + SubOff, F, S);
};
for (const auto &F : D->ElemRecord->bases())
- DtorSub(F.Offset, F.Desc);
+ DtorSub(F.Offset, F.Desc, S);
for (const auto &F : D->ElemRecord->fields())
- DtorSub(F.Offset, F.Desc);
+ DtorSub(F.Offset, F.Desc, S);
for (const auto &F : D->ElemRecord->virtual_bases())
- DtorSub(F.Offset, F.Desc);
+ DtorSub(F.Offset, F.Desc, S);
}
static void moveRecord(Block *B, const char *Src, char *Dst,
@@ -316,8 +319,8 @@
return data()[Bucket] & (T(1) << (I % PER_FIELD));
}
-InitMap *InitMap::allocate(unsigned N) {
+size_t InitMap::byteSize(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);
+ return Size;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
