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