[clang] 3eaf4a9 - [clang][bytecode] Check for memory leaks after destroying global scope (#112868)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 18 04:03:16 PDT 2024
Author: Timm Baeder
Date: 2024-10-18T13:03:13+02:00
New Revision: 3eaf4a9d1a847a4e03a21365682b3a73d7e2e6d0
URL: https://github.com/llvm/llvm-project/commit/3eaf4a9d1a847a4e03a21365682b3a73d7e2e6d0
DIFF: https://github.com/llvm/llvm-project/commit/3eaf4a9d1a847a4e03a21365682b3a73d7e2e6d0.diff
LOG: [clang][bytecode] Check for memory leaks after destroying global scope (#112868)
The global scope we create when evaluating expressions might free some
of the dynamic memory allocations, so we can't check for memory leaks
before destroying it.
Added:
Modified:
clang/lib/AST/ByteCode/Compiler.cpp
clang/lib/AST/ByteCode/Context.cpp
clang/lib/AST/ByteCode/EvalEmitter.cpp
clang/lib/AST/ByteCode/Interp.h
clang/lib/AST/ByteCode/Opcodes.td
clang/test/AST/ByteCode/new-delete.cpp
Removed:
################################################################################
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index a71c0dcc9381e8..672fa7fc25d6d0 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -4132,10 +4132,16 @@ template <class Emitter>
bool Compiler<Emitter>::visitExpr(const Expr *E, bool DestroyToplevelScope) {
LocalScope<Emitter> RootScope(this);
+ // If we won't destroy the toplevel scope, check for memory leaks first.
+ if (!DestroyToplevelScope) {
+ if (!this->emitCheckAllocations(E))
+ return false;
+ }
+
auto maybeDestroyLocals = [&]() -> bool {
if (DestroyToplevelScope)
- return RootScope.destroyLocals();
- return true;
+ return RootScope.destroyLocals() && this->emitCheckAllocations(E);
+ return this->emitCheckAllocations(E);
};
// Void expressions.
@@ -4171,8 +4177,7 @@ bool Compiler<Emitter>::visitExpr(const Expr *E, bool DestroyToplevelScope) {
return this->emitRetValue(E) && maybeDestroyLocals();
}
- (void)maybeDestroyLocals();
- return false;
+ return maybeDestroyLocals() && this->emitCheckAllocations(E) && false;
}
template <class Emitter>
@@ -4214,7 +4219,8 @@ bool Compiler<Emitter>::visitDeclAndReturn(const VarDecl *VD,
DeclScope<Emitter> LS(this, VD);
if (!this->visit(VD->getAnyInitializer()))
return false;
- return this->emitRet(VarT.value_or(PT_Ptr), VD) && LS.destroyLocals();
+ return this->emitRet(VarT.value_or(PT_Ptr), VD) && LS.destroyLocals() &&
+ this->emitCheckAllocations(VD);
}
LocalScope<Emitter> VDScope(this, VD);
@@ -4260,7 +4266,7 @@ bool Compiler<Emitter>::visitDeclAndReturn(const VarDecl *VD,
return false;
}
- return VDScope.destroyLocals();
+ return VDScope.destroyLocals() && this->emitCheckAllocations(VD);
}
template <class Emitter>
diff --git a/clang/lib/AST/ByteCode/Context.cpp b/clang/lib/AST/ByteCode/Context.cpp
index 9bca8138cd9f6f..7088cf02901c63 100644
--- a/clang/lib/AST/ByteCode/Context.cpp
+++ b/clang/lib/AST/ByteCode/Context.cpp
@@ -78,8 +78,7 @@ bool Context::evaluate(State &Parent, const Expr *E, APValue &Result,
Compiler<EvalEmitter> C(*this, *P, Parent, Stk);
auto Res = C.interpretExpr(E, /*ConvertResultToRValue=*/false,
- /*DestroyToplevelScope=*/Kind ==
- ConstantExprKind::ClassTemplateArgument);
+ /*DestroyToplevelScope=*/true);
if (Res.isInvalid()) {
C.cleanup();
Stk.clearTo(StackSizeBefore);
diff --git a/clang/lib/AST/ByteCode/EvalEmitter.cpp b/clang/lib/AST/ByteCode/EvalEmitter.cpp
index 7eecee25bb3c1e..65ad960cfa8d21 100644
--- a/clang/lib/AST/ByteCode/EvalEmitter.cpp
+++ b/clang/lib/AST/ByteCode/EvalEmitter.cpp
@@ -132,17 +132,10 @@ bool EvalEmitter::fallthrough(const LabelTy &Label) {
return true;
}
-static bool checkReturnState(InterpState &S) {
- return S.maybeDiagnoseDanglingAllocations();
-}
-
template <PrimType OpType> bool EvalEmitter::emitRet(const SourceInfo &Info) {
if (!isActive())
return true;
- if (!checkReturnState(S))
- return false;
-
using T = typename PrimConv<OpType>::T;
EvalResult.setValue(S.Stk.pop<T>().toAPValue(Ctx.getASTContext()));
return true;
@@ -159,9 +152,6 @@ template <> bool EvalEmitter::emitRet<PT_Ptr>(const SourceInfo &Info) {
if (CheckFullyInitialized && !EvalResult.checkFullyInitialized(S, Ptr))
return false;
- if (!checkReturnState(S))
- return false;
-
// Implicitly convert lvalue to rvalue, if requested.
if (ConvertResultToRValue) {
if (!Ptr.isZero() && !Ptr.isDereferencable())
@@ -194,16 +184,12 @@ template <> bool EvalEmitter::emitRet<PT_FnPtr>(const SourceInfo &Info) {
if (!isActive())
return true;
- if (!checkReturnState(S))
- return false;
// Function pointers cannot be converted to rvalues.
EvalResult.setFunctionPointer(S.Stk.pop<FunctionPointer>());
return true;
}
bool EvalEmitter::emitRetVoid(const SourceInfo &Info) {
- if (!checkReturnState(S))
- return false;
EvalResult.setValid();
return true;
}
@@ -216,9 +202,6 @@ bool EvalEmitter::emitRetValue(const SourceInfo &Info) {
if (CheckFullyInitialized && !EvalResult.checkFullyInitialized(S, Ptr))
return false;
- if (!checkReturnState(S))
- return false;
-
if (std::optional<APValue> APV =
Ptr.toRValue(S.getASTContext(), EvalResult.getSourceType())) {
EvalResult.setValue(*APV);
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index f034bde309035f..aafc848a9c53f3 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -3007,6 +3007,10 @@ static inline bool IsConstantContext(InterpState &S, CodePtr OpPC) {
return true;
}
+static inline bool CheckAllocations(InterpState &S, CodePtr OpPC) {
+ return S.maybeDiagnoseDanglingAllocations();
+}
+
/// Check if the initializer and storage types of a placement-new expression
/// match.
bool CheckNewTypeMismatch(InterpState &S, CodePtr OpPC, const Expr *E,
diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td
index 4fa9b6d61d5ab9..a1970f25ca977f 100644
--- a/clang/lib/AST/ByteCode/Opcodes.td
+++ b/clang/lib/AST/ByteCode/Opcodes.td
@@ -836,3 +836,4 @@ def CheckNewTypeMismatchArray : Opcode {
}
def IsConstantContext: Opcode;
+def CheckAllocations : Opcode;
diff --git a/clang/test/AST/ByteCode/new-delete.cpp b/clang/test/AST/ByteCode/new-delete.cpp
index 8bcbed1aba21e9..94fe2d4497df6a 100644
--- a/clang/test/AST/ByteCode/new-delete.cpp
+++ b/clang/test/AST/ByteCode/new-delete.cpp
@@ -796,6 +796,28 @@ static_assert(virt_delete(false)); // both-error {{not an integral constant expr
// both-note {{in call to}}
+namespace ToplevelScopeInTemplateArg {
+ class string {
+ public:
+ char *mem;
+ constexpr string() {
+ this->mem = new char(1);
+ }
+ constexpr ~string() {
+ delete this->mem;
+ }
+ constexpr unsigned size() const { return 4; }
+ };
+
+
+ template <unsigned N>
+ void test() {};
+
+ void f() {
+ test<string().size()>();
+ static_assert(string().size() == 4);
+ }
+}
#else
/// Make sure we reject this prior to C++20
More information about the cfe-commits
mailing list