llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Gábor Horváth (Xazax-hun) <details> <summary>Changes</summary> …ker (#<!-- -->125638)" This reverts commit 7ba3c55d91dcd7da5a5eb1c58225f648fb38b740. --- Patch is 27.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/126614.diff 5 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp (+53-129) - (modified) clang/test/Analysis/copy-elision.cpp (+7-18) - (modified) clang/test/Analysis/stack-addr-ps.cpp (+20-225) - (modified) clang/test/Analysis/stackaddrleak.c (-12) - (modified) clang/test/Analysis/stackaddrleak.cpp (+1-1) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp index 826dc42e4f59948..f4de3b500499c48 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp @@ -54,8 +54,8 @@ class StackAddrEscapeChecker CheckerContext &C) const; void checkAsyncExecutedBlockCaptures(const BlockDataRegion &B, CheckerContext &C) const; - void EmitReturnLeakError(CheckerContext &C, const MemRegion *LeakedRegion, - const Expr *RetE) const; + void EmitStackError(CheckerContext &C, const MemRegion *R, + const Expr *RetE) const; bool isSemaphoreCaptured(const BlockDecl &B) const; static SourceRange genName(raw_ostream &os, const MemRegion *R, ASTContext &Ctx); @@ -147,22 +147,9 @@ StackAddrEscapeChecker::getCapturedStackRegions(const BlockDataRegion &B, return Regions; } -static void EmitReturnedAsPartOfError(llvm::raw_ostream &OS, SVal ReturnedVal, - const MemRegion *LeakedRegion) { - if (const MemRegion *ReturnedRegion = ReturnedVal.getAsRegion()) { - if (isa<BlockDataRegion>(ReturnedRegion)) { - OS << " is captured by a returned block"; - return; - } - } - - // Generic message - OS << " returned to caller"; -} - -void StackAddrEscapeChecker::EmitReturnLeakError(CheckerContext &C, - const MemRegion *R, - const Expr *RetE) const { +void StackAddrEscapeChecker::EmitStackError(CheckerContext &C, + const MemRegion *R, + const Expr *RetE) const { ExplodedNode *N = C.generateNonFatalErrorNode(); if (!N) return; @@ -170,15 +157,11 @@ void StackAddrEscapeChecker::EmitReturnLeakError(CheckerContext &C, BT_returnstack = std::make_unique<BugType>( CheckNames[CK_StackAddrEscapeChecker], "Return of address to stack-allocated memory"); - // Generate a report for this bug. SmallString<128> buf; llvm::raw_svector_ostream os(buf); - - // Error message formatting SourceRange range = genName(os, R, C.getASTContext()); - EmitReturnedAsPartOfError(os, C.getSVal(RetE), R); - + os << " returned to caller"; auto report = std::make_unique<PathSensitiveBugReport>(*BT_returnstack, os.str(), N); report->addRange(RetE->getSourceRange()); @@ -226,6 +209,30 @@ void StackAddrEscapeChecker::checkAsyncExecutedBlockCaptures( } } +void StackAddrEscapeChecker::checkReturnedBlockCaptures( + const BlockDataRegion &B, CheckerContext &C) const { + for (const MemRegion *Region : getCapturedStackRegions(B, C)) { + if (isNotInCurrentFrame(Region, C)) + continue; + ExplodedNode *N = C.generateNonFatalErrorNode(); + if (!N) + continue; + if (!BT_capturedstackret) + BT_capturedstackret = std::make_unique<BugType>( + CheckNames[CK_StackAddrEscapeChecker], + "Address of stack-allocated memory is captured"); + SmallString<128> Buf; + llvm::raw_svector_ostream Out(Buf); + SourceRange Range = genName(Out, Region, C.getASTContext()); + Out << " is captured by a returned block"; + auto Report = std::make_unique<PathSensitiveBugReport>(*BT_capturedstackret, + Out.str(), N); + if (Range.isValid()) + Report->addRange(Range); + C.emitReport(std::move(Report)); + } +} + void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { if (!ChecksEnabled[CK_StackAddrAsyncEscapeChecker]) @@ -240,128 +247,45 @@ void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call, } } -/// A visitor made for use with a ScanReachableSymbols scanner, used -/// for finding stack regions within an SVal that live on the current -/// stack frame of the given checker context. This visitor excludes -/// NonParamVarRegion that data is bound to in a BlockDataRegion's -/// bindings, since these are likely uninteresting, e.g., in case a -/// temporary is constructed on the stack, but it captures values -/// that would leak. -class FindStackRegionsSymbolVisitor final : public SymbolVisitor { - CheckerContext &Ctxt; - const StackFrameContext *StackFrameContext; - SmallVectorImpl<const MemRegion *> &EscapingStackRegions; - -public: - explicit FindStackRegionsSymbolVisitor( - CheckerContext &Ctxt, - SmallVectorImpl<const MemRegion *> &StorageForStackRegions) - : Ctxt(Ctxt), StackFrameContext(Ctxt.getStackFrame()), - EscapingStackRegions(StorageForStackRegions) {} - - bool VisitSymbol(SymbolRef sym) override { return true; } - - bool VisitMemRegion(const MemRegion *MR) override { - SaveIfEscapes(MR); - - if (const BlockDataRegion *BDR = MR->getAs<BlockDataRegion>()) - return VisitBlockDataRegionCaptures(BDR); - - return true; - } - -private: - void SaveIfEscapes(const MemRegion *MR) { - const StackSpaceRegion *SSR = - MR->getMemorySpace()->getAs<StackSpaceRegion>(); - if (SSR && SSR->getStackFrame() == StackFrameContext) - EscapingStackRegions.push_back(MR); - } - - bool VisitBlockDataRegionCaptures(const BlockDataRegion *BDR) { - for (auto Var : BDR->referenced_vars()) { - SVal Val = Ctxt.getState()->getSVal(Var.getCapturedRegion()); - const MemRegion *Region = Val.getAsRegion(); - if (Region) { - SaveIfEscapes(Region); - VisitMemRegion(Region); - } - } +void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, + CheckerContext &C) const { + if (!ChecksEnabled[CK_StackAddrEscapeChecker]) + return; - return false; - } -}; + const Expr *RetE = RS->getRetValue(); + if (!RetE) + return; + RetE = RetE->IgnoreParens(); -/// Given some memory regions that are flagged by FindStackRegionsSymbolVisitor, -/// this function filters out memory regions that are being returned that are -/// likely not true leaks: -/// 1. If returning a block data region that has stack memory space -/// 2. If returning a constructed object that has stack memory space -static SmallVector<const MemRegion *> FilterReturnExpressionLeaks( - const SmallVectorImpl<const MemRegion *> &MaybeEscaped, CheckerContext &C, - const Expr *RetE, SVal &RetVal) { + SVal V = C.getSVal(RetE); + const MemRegion *R = V.getAsRegion(); + if (!R) + return; - SmallVector<const MemRegion *> WillEscape; + if (const BlockDataRegion *B = dyn_cast<BlockDataRegion>(R)) + checkReturnedBlockCaptures(*B, C); - const MemRegion *RetRegion = RetVal.getAsRegion(); + if (!isa<StackSpaceRegion>(R->getMemorySpace()) || isNotInCurrentFrame(R, C)) + return; // Returning a record by value is fine. (In this case, the returned // expression will be a copy-constructor, possibly wrapped in an // ExprWithCleanups node.) if (const ExprWithCleanups *Cleanup = dyn_cast<ExprWithCleanups>(RetE)) RetE = Cleanup->getSubExpr(); - bool IsConstructExpr = - isa<CXXConstructExpr>(RetE) && RetE->getType()->isRecordType(); + if (isa<CXXConstructExpr>(RetE) && RetE->getType()->isRecordType()) + return; // The CK_CopyAndAutoreleaseBlockObject cast causes the block to be copied // so the stack address is not escaping here. - bool IsCopyAndAutoreleaseBlockObj = false; if (const auto *ICE = dyn_cast<ImplicitCastExpr>(RetE)) { - IsCopyAndAutoreleaseBlockObj = - isa_and_nonnull<BlockDataRegion>(RetRegion) && - ICE->getCastKind() == CK_CopyAndAutoreleaseBlockObject; - } - - for (const MemRegion *MR : MaybeEscaped) { - if (RetRegion == MR && (IsCopyAndAutoreleaseBlockObj || IsConstructExpr)) - continue; - - WillEscape.push_back(MR); + if (isa<BlockDataRegion>(R) && + ICE->getCastKind() == CK_CopyAndAutoreleaseBlockObject) { + return; + } } - return WillEscape; -} - -/// For use in finding regions that live on the checker context's current -/// stack frame, deep in the SVal representing the return value. -static SmallVector<const MemRegion *> -FindEscapingStackRegions(CheckerContext &C, const Expr *RetE, SVal RetVal) { - SmallVector<const MemRegion *> FoundStackRegions; - - FindStackRegionsSymbolVisitor Finder(C, FoundStackRegions); - ScanReachableSymbols Scanner(C.getState(), Finder); - Scanner.scan(RetVal); - - return FilterReturnExpressionLeaks(FoundStackRegions, C, RetE, RetVal); -} - -void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, - CheckerContext &C) const { - if (!ChecksEnabled[CK_StackAddrEscapeChecker]) - return; - - const Expr *RetE = RS->getRetValue(); - if (!RetE) - return; - RetE = RetE->IgnoreParens(); - - SVal V = C.getSVal(RetE); - - SmallVector<const MemRegion *> EscapedStackRegions = - FindEscapingStackRegions(C, RetE, V); - - for (const MemRegion *ER : EscapedStackRegions) - EmitReturnLeakError(C, ER, RetE); + EmitStackError(C, R, RetE); } static const MemSpaceRegion *getStackOrGlobalSpaceRegion(const MemRegion *R) { diff --git a/clang/test/Analysis/copy-elision.cpp b/clang/test/Analysis/copy-elision.cpp index e69f52f351f1bc0..cd941854fc7f196 100644 --- a/clang/test/Analysis/copy-elision.cpp +++ b/clang/test/Analysis/copy-elision.cpp @@ -154,26 +154,20 @@ class ClassWithoutDestructor { void push() { v.push(this); } }; -// Two warnings on no-elide: arg v holds the address of the temporary, and we -// are returning an object which holds v which holds the address of the temporary ClassWithoutDestructor make1(AddressVector<ClassWithoutDestructor> &v) { - return ClassWithoutDestructor(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithoutDestructor' returned to caller}} + return ClassWithoutDestructor(v); // no-elide-warning@-1 {{Address of stack memory associated with temporary \ object of type 'ClassWithoutDestructor' is still \ referred to by the caller variable 'v' upon returning to the caller}} } -// Two warnings on no-elide: arg v holds the address of the temporary, and we -// are returning an object which holds v which holds the address of the temporary ClassWithoutDestructor make2(AddressVector<ClassWithoutDestructor> &v) { - return make1(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithoutDestructor' returned to caller}} + return make1(v); // no-elide-warning@-1 {{Address of stack memory associated with temporary \ object of type 'ClassWithoutDestructor' is still \ referred to by the caller variable 'v' upon returning to the caller}} } -// Two warnings on no-elide: arg v holds the address of the temporary, and we -// are returning an object which holds v which holds the address of the temporary ClassWithoutDestructor make3(AddressVector<ClassWithoutDestructor> &v) { - return make2(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithoutDestructor' returned to caller}} + return make2(v); // no-elide-warning@-1 {{Address of stack memory associated with temporary \ object of type 'ClassWithoutDestructor' is still \ referred to by the caller variable 'v' upon returning to the caller}} @@ -304,26 +298,21 @@ to by the caller variable 'v' upon returning to the caller}} #endif } -// Two warnings on no-elide: arg v holds the address of the temporary, and we -// are returning an object which holds v which holds the address of the temporary + ClassWithDestructor make1(AddressVector<ClassWithDestructor> &v) { - return ClassWithDestructor(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithDestructor' returned to caller}} + return ClassWithDestructor(v); // no-elide-warning@-1 {{Address of stack memory associated with temporary \ object of type 'ClassWithDestructor' is still referred \ to by the caller variable 'v' upon returning to the caller}} } -// Two warnings on no-elide: arg v holds the address of the temporary, and we -// are returning an object which holds v which holds the address of the temporary ClassWithDestructor make2(AddressVector<ClassWithDestructor> &v) { - return make1(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithDestructor' returned to caller}} + return make1(v); // no-elide-warning@-1 {{Address of stack memory associated with temporary \ object of type 'ClassWithDestructor' is still referred \ to by the caller variable 'v' upon returning to the caller}} } -// Two warnings on no-elide: arg v holds the address of the temporary, and we -// are returning an object which holds v which holds the address of the temporary ClassWithDestructor make3(AddressVector<ClassWithDestructor> &v) { - return make2(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithDestructor' returned to caller}} + return make2(v); // no-elide-warning@-1 {{Address of stack memory associated with temporary \ object of type 'ClassWithDestructor' is still referred \ to by the caller variable 'v' upon returning to the caller}} diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp index bf988d0a1695947..73e9dbeca460f60 100644 --- a/clang/test/Analysis/stack-addr-ps.cpp +++ b/clang/test/Analysis/stack-addr-ps.cpp @@ -90,18 +90,6 @@ int *mf() { return &x; // expected-warning{{Address of stack memory associated with local variable 's1' returned}} expected-warning {{address of stack memory associated with local variable 's1' returned}} } -int *return_assign_expr_leak() { - int x = 1; - int *y; - return y = &x; // expected-warning{{Address of stack memory associated with local variable 'x' returned}} -} - -// Additional diagnostic from -Wreturn-stack-address in the simple case? -int *return_comma_separated_expressions_leak() { - int x = 1; - return (x=14), &x; // expected-warning{{Address of stack memory associated with local variable 'x' returned to caller}} expected-warning{{address of stack memory associated with local variable 'x' returned}} -} - void *lf() { label: void *const &x = &&label; // expected-note {{binding reference variable 'x' here}} @@ -164,18 +152,6 @@ namespace rdar13296133 { } } // namespace rdar13296133 -void* ret_cpp_static_cast(short x) { - return static_cast<void*>(&x); // expected-warning {{Address of stack memory associated with local variable 'x' returned to caller}} expected-warning {{address of stack memory}} -} - -int* ret_cpp_reinterpret_cast(double x) { - return reinterpret_cast<int*>(&x); // expected-warning {{Address of stack memory associated with local variable 'x' returned to caller}} expected-warning {{address of stack me}} -} - -int* ret_cpp_const_cast(const int x) { - return const_cast<int*>(&x); // expected-warning {{Address of stack memory associated with local variable 'x' returned to caller}} expected-warning {{address of stack memory}} -} - void write_stack_address_to(char **q) { char local; *q = &local; @@ -202,11 +178,6 @@ void test_copy_elision() { C c1 = make1(); } -C&& return_bind_rvalue_reference_to_temporary() { - return C(); // expected-warning{{Address of stack memory associated with temporary object of type 'C' returned to caller}} - // expected-warning@-1{{returning reference to local temporary object}} -Wreturn-stack-address -} - namespace leaking_via_direct_pointer { void* returned_direct_pointer_top() { int local = 42; @@ -280,7 +251,7 @@ void* lambda_to_context_direct_pointer_uncalled() { int local = 42; p = &local; // no-warning: analyzed only as top-level, ignored explicitly by the checker }; - return new MyFunction(&lambda); // expected-warning{{Address of stack memory associated with local variable 'lambda' returned to caller}} + return new MyFunction(&lambda); } void lambda_to_context_direct_pointer_lifetime_extended() { @@ -369,13 +340,6 @@ void param_ptr_to_ptr_to_ptr_callee(void*** ppp) { **ppp = &local; // expected-warning{{local variable 'local' is still referred to by the caller variable 'pp'}} } -void ***param_ptr_to_ptr_to_ptr_return(void ***ppp) { - int local = 0; - **ppp = &local; - return ppp; - // expected-warning@-1 {{Address of stack memory associated with local variable 'local' is still referred to by the caller variable 'ppp' upon returning to the caller. This will be a dangling reference}} -} - void param_ptr_to_ptr_to_ptr_caller(void** pp) { param_ptr_to_ptr_to_ptr_callee(&pp); } @@ -446,16 +410,16 @@ void** returned_arr_of_ptr_top() { int* p = &local; void** arr = new void*[2]; arr[1] = p; - return arr; // expected-warning{{Address of stack memory associated with local variable 'local' returned to caller}} -} + return arr; +} // no-warning False Negative void** returned_arr_of_ptr_callee() { int local = 42; int* p = &local; void** arr = new void*[2]; arr[1] = p; - return arr; // expected-warning{{Address of stack memory associated with local variable 'local' returned to caller}} -} + return arr; +} // no-warning False Negative void returned_arr_of_ptr_caller() { void** arr = returned_arr_of_ptr_callee(); @@ -502,16 +466,16 @@ void** returned_arr_of_ptr_top(int idx) { int* p = &local; void** arr = new void*[2]; arr[idx] = p; - return arr; // expected-warning{{Address of stack memory associated with local variable 'local' returned to caller}} -} + return arr; +} // no-warning False Negative void** returned_arr_of_ptr_callee(int idx) { int local = 42; int* p = &local; void** arr = new void*[2]; arr[idx] = p; - return arr; // expected-warning{{Address of stack memory associated with local variable 'local' returned to caller}} -} + return arr; +} // no-warning False Negative void returned_arr_of_ptr_caller(int idx) { void** arr = returned_arr_of_ptr_callee(idx); @@ -561,25 +525,14 @@ S returned_struct_with_ptr_top() { int local = 42; S s; s.p = &local; - return s; // expected-warning{{Address of stack memory associated with local variable 'local' returned to caller}} -} + return s; +} // no-warning False Negative, requires traversing returned LazyCompoundVals S returned_struct_with_ptr_callee() { int local = 42; S s; s.p = &local; - return s; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}} expected-warning{{Address of stack memory associated with local variable 'local' is still referred to by the caller variable 's' upon returning to the caller. This will be a dangling reference}} -} - -S leak_through_field_of_returned_object() { - int local = 14; - S s{&local}; - return s; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}} -} - -S leak_through_compound_literal() { - int local = 0; - return (S) { &local }; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}} + return s; // expected-warning{{'local' is still referred to by the caller variable 's'}} } void returned_struct_with_ptr_caller() { @@ -602,30 +555,6 @@ void static_struct_with_ptr() { } } // namespace leaking_via_struct_with_ptr -namespace leaking_via_nested_structs_with_ptr { -struct Inner { - int *ptr; -}; - -struct Outer { - Inner I; -}; - -struct Deriving : public Outer {}; - -Outer leaks_through_nested_objects() { - int local = 0; - Outer O{&local}; - return O; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}} -} - -Deriving leaks_through_base_objects() { - int local = 0; - Deriving D{&local}; - return D; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}} -} -} // namespace leaking_via_nested_structs_with_ptr - namespace leaking_via_ref_to_struct_with_ptr { struct S { int* p; @@ -684,15 +613,15 @@ S* returned_ptr_to_struct_with_ptr_top() { int local = 42; S* s = new S; s->p = &local; - r... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/126614 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
