[clang] Remove some false negatives in StackAddrEscapeChecker (PR #125638)
Michael Flanders via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 3 21:52:00 PST 2025
https://github.com/Flandini created https://github.com/llvm/llvm-project/pull/125638
Fixes https://github.com/llvm/llvm-project/issues/123459.
Previously, when the StackAddrEscapeChecker checked return values, it did not scan into the structure of the return SVal. Now it does, and we can catch some more false negatives that were already mocked out in the tests in addition to those mentioned in https://github.com/llvm/llvm-project/issues/123459.
The warning message at the moment for these newly caught leaks is not great. I think they would be better if they had a better trace of why and how the region leaks. If y'all are happy with these changes, I would try to improve these warnings and work on normalizing this SVal checking on the `checkEndFunction` side of the checker also.
Two of the stack address leak test cases now have two warnings, one warning from return expression checking and another from` checkEndFunction` `iterBindings` checking. For these two cases, I prefer the warnings from the return expression checking, but I couldn't figure out a way to drop the `checkEndFunction` without breaking other `checkEndFunction` warnings that we do want. Thoughts here?
Tagging @steakhal @NagyDonat @Xazax-hun @haoNoQ for review.
>From 0674909f03703a70c3e259acd0590f50cea4615f Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 27 Jan 2025 11:35:03 -0600
Subject: [PATCH 01/20] wip
---
.../Checkers/StackAddrEscapeChecker.cpp | 45 ++++++++++++++++++-
1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index f4de3b500499c4..59b47371df0d29 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -20,6 +20,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/Support/raw_ostream.h"
using namespace clang;
@@ -233,6 +234,43 @@ void StackAddrEscapeChecker::checkReturnedBlockCaptures(
}
}
+class FindEscapingStackAddrsVisitor : public FullSValVisitor<FindEscapingStackAddrsVisitor, bool>
+{
+ const StackFrameContext *Ctxt;
+
+ SmallVector<const MemRegion *> Escaped;
+
+ bool IsTopFrame;
+
+public:
+ explicit FindEscapingStackAddrsVisitor(CheckerContext &CC, bool IsTopFrame) :
+ Ctxt(CC.getStackFrame()), IsTopFrame(IsTopFrame) {}
+
+ bool CheckForEscapes(SVal V) { return Visit(V); }
+
+ bool VisitSymbolVal(nonloc::SymbolVal SymV) {
+ return Visit(SymV.getSymbol());
+ }
+
+ bool VisitLocAsInteger(nonloc::LocAsInteger LAI) {
+ return Visit(LAI.getAsRegion());
+ }
+
+ bool VisitMemRegionVal(loc::MemRegionVal MRVal) {
+ return Visit(MRVal.getRegion());
+ }
+
+ bool VisitMemRegion(const MemRegion *R) {
+ const MemSpaceRegion *MS = R->getMemorySpace();
+ const StackSpaceRegion *SSR = dyn_cast<StackSpaceRegion>(MS);
+ if (SSR && Ctxt == SSR->getStackFrame()) {
+ Escaped.push_back(R);
+ return true;
+ }
+ return false;
+ }
+};
+
void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
if (!ChecksEnabled[CK_StackAddrAsyncEscapeChecker])
@@ -257,7 +295,12 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
return;
RetE = RetE->IgnoreParens();
+ FindEscapingStackAddrsVisitor EscapeFinder(C, C.getPredecessor()->getLocationContext()->inTopFrame());
+
SVal V = C.getSVal(RetE);
+
+ bool AnyEscapes = EscapeFinder.CheckForEscapes(V);
+
const MemRegion *R = V.getAsRegion();
if (!R)
return;
@@ -402,7 +445,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
bool checkForDanglingStackVariable(const MemRegion *Referrer,
const MemRegion *Referred) {
const auto *ReferrerMemSpace = getStackOrGlobalSpaceRegion(Referrer);
- const auto *ReferredMemSpace =
+ const auto *ReferredMemSpace =
Referred->getMemorySpace()->getAs<StackSpaceRegion>();
if (!ReferrerMemSpace || !ReferredMemSpace)
>From 0a26f83cd5f4a42238ba4fadb3436fec83e4bfcb Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 29 Jan 2025 00:34:45 -0600
Subject: [PATCH 02/20] wip
---
.../Checkers/StackAddrEscapeChecker.cpp | 143 ++++++++++--------
clang/test/Analysis/stack-addr-ps.cpp | 73 ++++++---
clang/test/Analysis/stackaddrleak.cpp | 2 +-
3 files changed, 136 insertions(+), 82 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 59b47371df0d29..b6ef375936cccc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -22,6 +22,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h"
#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Debug.h"
#include "llvm/Support/raw_ostream.h"
using namespace clang;
using namespace ento;
@@ -210,6 +211,7 @@ void StackAddrEscapeChecker::checkAsyncExecutedBlockCaptures(
}
}
+
void StackAddrEscapeChecker::checkReturnedBlockCaptures(
const BlockDataRegion &B, CheckerContext &C) const {
for (const MemRegion *Region : getCapturedStackRegions(B, C)) {
@@ -234,43 +236,6 @@ void StackAddrEscapeChecker::checkReturnedBlockCaptures(
}
}
-class FindEscapingStackAddrsVisitor : public FullSValVisitor<FindEscapingStackAddrsVisitor, bool>
-{
- const StackFrameContext *Ctxt;
-
- SmallVector<const MemRegion *> Escaped;
-
- bool IsTopFrame;
-
-public:
- explicit FindEscapingStackAddrsVisitor(CheckerContext &CC, bool IsTopFrame) :
- Ctxt(CC.getStackFrame()), IsTopFrame(IsTopFrame) {}
-
- bool CheckForEscapes(SVal V) { return Visit(V); }
-
- bool VisitSymbolVal(nonloc::SymbolVal SymV) {
- return Visit(SymV.getSymbol());
- }
-
- bool VisitLocAsInteger(nonloc::LocAsInteger LAI) {
- return Visit(LAI.getAsRegion());
- }
-
- bool VisitMemRegionVal(loc::MemRegionVal MRVal) {
- return Visit(MRVal.getRegion());
- }
-
- bool VisitMemRegion(const MemRegion *R) {
- const MemSpaceRegion *MS = R->getMemorySpace();
- const StackSpaceRegion *SSR = dyn_cast<StackSpaceRegion>(MS);
- if (SSR && Ctxt == SSR->getStackFrame()) {
- Escaped.push_back(R);
- return true;
- }
- return false;
- }
-};
-
void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
if (!ChecksEnabled[CK_StackAddrAsyncEscapeChecker])
@@ -285,6 +250,33 @@ void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call,
}
}
+class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
+ const StackFrameContext *StackFrameContext;
+ SmallVector<const MemRegion *> &EscapingStackRegions;
+public:
+ explicit FindStackRegionsSymbolVisitor(CheckerContext &Ctxt, SmallVector<const MemRegion *> &StorageForStackRegions)
+ : StackFrameContext(Ctxt.getStackFrame()), EscapingStackRegions(StorageForStackRegions) {}
+
+ bool VisitSymbol(SymbolRef sym) override { return true; }
+
+ bool VisitMemRegion(const MemRegion *MR) override {
+ llvm::dbgs() << "Visiting region: ";
+ MR->dumpToStream(llvm::dbgs());
+ llvm::dbgs() << '\n';
+
+ const StackSpaceRegion *SSR = dyn_cast<StackSpaceRegion>(MR->getMemorySpace());
+ if (SSR && SSR->getStackFrame() == StackFrameContext)
+ EscapingStackRegions.push_back(MR);
+ return true;
+ }
+};
+
+static void FindEscapingStackRegions(CheckerContext &C, SmallVector<const MemRegion *> &EscapedRegionsStorage, SVal SVal) {
+ FindStackRegionsSymbolVisitor Finder(C, EscapedRegionsStorage);
+ ScanReachableSymbols Scanner(C.getState(), Finder);
+ Scanner.scan(SVal);
+}
+
void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
CheckerContext &C) const {
if (!ChecksEnabled[CK_StackAddrEscapeChecker])
@@ -295,40 +287,67 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
return;
RetE = RetE->IgnoreParens();
- FindEscapingStackAddrsVisitor EscapeFinder(C, C.getPredecessor()->getLocationContext()->inTopFrame());
-
SVal V = C.getSVal(RetE);
- bool AnyEscapes = EscapeFinder.CheckForEscapes(V);
+ SmallVector<const MemRegion *> EscapedRegions;
+ FindEscapingStackRegions(C, EscapedRegions, V);
+
+ for (const MemRegion *ER : EscapedRegions) {
+ llvm::dbgs() << "Escaped region: ";
+ ER->dumpToStream(llvm::dbgs());
+ llvm::dbgs() << '\n';
+
+ const MemRegion *R = ER;
+ while (true) {
+ switch (R->getKind()) {
+ case MemRegion::ElementRegionKind:
+ case MemRegion::FieldRegionKind:
+ case MemRegion::ObjCIvarRegionKind:
+ case MemRegion::CXXBaseObjectRegionKind:
+ case MemRegion::CXXDerivedObjectRegionKind: {
+ const SubRegion *SR = cast<SubRegion>(R);
+ R = SR->getSuperRegion();
+ llvm::dbgs() << "SuperRegion: ";
+ R->dumpToStream(llvm::dbgs());
+ llvm::dbgs() << '\n';
+ continue;
+ }
+ default:
+ break;
+ }
+ break;
+ }
- const MemRegion *R = V.getAsRegion();
- if (!R)
- return;
+ const VarRegion *Base = dyn_cast<VarRegion>(ER->getBaseRegion());
+ if (Base) {
+ Base->getDecl()->getBeginLoc().dump(C.getSourceManager());
+ }
- if (const BlockDataRegion *B = dyn_cast<BlockDataRegion>(R))
- checkReturnedBlockCaptures(*B, C);
+ EmitStackError(C, ER, RetE);
+ }
- if (!isa<StackSpaceRegion>(R->getMemorySpace()) || isNotInCurrentFrame(R, C))
- return;
+ // if (const BlockDataRegion *B = dyn_cast<BlockDataRegion>(R))
+ // checkReturnedBlockCaptures(*B, C);
// 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();
- 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.
- if (const auto *ICE = dyn_cast<ImplicitCastExpr>(RetE)) {
- if (isa<BlockDataRegion>(R) &&
- ICE->getCastKind() == CK_CopyAndAutoreleaseBlockObject) {
- return;
- }
- }
-
- EmitStackError(C, R, RetE);
+ // if (const ExprWithCleanups *Cleanup = dyn_cast<ExprWithCleanups>(RetE))
+ // RetE = Cleanup->getSubExpr();
+ // 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.
+ // if (const auto *ICE = dyn_cast<ImplicitCastExpr>(RetE)) {
+ // const MemRegion *R = V.getAsRegion();
+ // if (R && isa<BlockDataRegion>(R) &&
+ // ICE->getCastKind() == CK_CopyAndAutoreleaseBlockObject) {
+ // return;
+ // }
+ // }
+
+ // EmitStackError(C, R, RetE);
}
static const MemSpaceRegion *getStackOrGlobalSpaceRegion(const MemRegion *R) {
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index 73e9dbeca460f6..cfa887da025a80 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -410,16 +410,16 @@ void** returned_arr_of_ptr_top() {
int* p = &local;
void** arr = new void*[2];
arr[1] = p;
- return arr;
-} // no-warning False Negative
+ return arr; // expected-warning{{Address of stack memory associated with local variable 'local' returned to caller}}
+}
void** returned_arr_of_ptr_callee() {
int local = 42;
int* p = &local;
void** arr = new void*[2];
arr[1] = p;
- return arr;
-} // no-warning False Negative
+ return arr; // expected-warning{{Address of stack memory associated with local variable 'local' returned to caller}}
+}
void returned_arr_of_ptr_caller() {
void** arr = returned_arr_of_ptr_callee();
@@ -466,16 +466,16 @@ void** returned_arr_of_ptr_top(int idx) {
int* p = &local;
void** arr = new void*[2];
arr[idx] = p;
- return arr;
-} // no-warning False Negative
+ return arr; // expected-warning{{Address of stack memory associated with local variable 'local' returned to caller}}
+}
void** returned_arr_of_ptr_callee(int idx) {
int local = 42;
int* p = &local;
void** arr = new void*[2];
arr[idx] = p;
- return arr;
-} // no-warning False Negative
+ return arr; // expected-warning{{Address of stack memory associated with local variable 'local' returned to caller}}
+}
void returned_arr_of_ptr_caller(int idx) {
void** arr = returned_arr_of_ptr_callee(idx);
@@ -525,14 +525,25 @@ S returned_struct_with_ptr_top() {
int local = 42;
S s;
s.p = &local;
- return s;
-} // no-warning False Negative, requires traversing returned LazyCompoundVals
+ return s; // expected-warning{{Address of stack memory associated with local variable 'local' returned to caller}}
+}
S returned_struct_with_ptr_callee() {
int local = 42;
S s;
s.p = &local;
- return s; // expected-warning{{'local' is still referred to by the caller variable 's'}}
+ return s; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}}
+}
+
+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}}
}
void returned_struct_with_ptr_caller() {
@@ -555,6 +566,30 @@ 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;
@@ -613,15 +648,15 @@ S* returned_ptr_to_struct_with_ptr_top() {
int local = 42;
S* s = new S;
s->p = &local;
- return s;
-} // no-warning False Negative
+ return s; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}}
+}
S* returned_ptr_to_struct_with_ptr_callee() {
int local = 42;
S* s = new S;
s->p = &local;
- return s;
-} // no-warning False Negative
+ return s; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}}
+}
void returned_ptr_to_struct_with_ptr_caller() {
S* s = returned_ptr_to_struct_with_ptr_callee();
@@ -676,15 +711,15 @@ S* returned_ptr_to_struct_with_ptr_top() {
int local = 42;
S* s = new S[2];
s[1].p = &local;
- return s;
-} // no-warning False Negative
+ return s; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}}
+}
S* returned_ptr_to_struct_with_ptr_callee() {
int local = 42;
S* s = new S[2];
s[1].p = &local;
- return s;
-} // no-warning False Negative
+ return s; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}}
+}
void returned_ptr_to_struct_with_ptr_caller() {
S* s = returned_ptr_to_struct_with_ptr_callee();
diff --git a/clang/test/Analysis/stackaddrleak.cpp b/clang/test/Analysis/stackaddrleak.cpp
index 3daffb35a6cd9a..82697d94a7f60d 100644
--- a/clang/test/Analysis/stackaddrleak.cpp
+++ b/clang/test/Analysis/stackaddrleak.cpp
@@ -22,4 +22,4 @@ myfunction create_func() {
}
void gh_66221() {
create_func()();
-}
+}
\ No newline at end of file
>From 3ebf2221ec153c384db450890c7b65996df9d19b Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 29 Jan 2025 20:49:26 -0600
Subject: [PATCH 03/20] Changes to checkPreStmt
---
.../Checkers/StackAddrEscapeChecker.cpp | 102 ++++++++----------
clang/test/Analysis/stackaddrleak.cpp | 2 +-
2 files changed, 44 insertions(+), 60 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index b6ef375936cccc..ed9fa80a10de6c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -29,7 +29,8 @@ using namespace ento;
namespace {
class StackAddrEscapeChecker
- : public Checker<check::PreCall, check::PreStmt<ReturnStmt>,
+ : public Checker<check::PreCall,
+ check::PreStmt<ReturnStmt>,
check::EndFunction> {
mutable IdentifierInfo *dispatch_semaphore_tII = nullptr;
mutable std::unique_ptr<BugType> BT_stackleak;
@@ -260,9 +261,7 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
bool VisitSymbol(SymbolRef sym) override { return true; }
bool VisitMemRegion(const MemRegion *MR) override {
- llvm::dbgs() << "Visiting region: ";
- MR->dumpToStream(llvm::dbgs());
- llvm::dbgs() << '\n';
+ llvm::dbgs() << "Visiting region: " << MR << '\n';
const StackSpaceRegion *SSR = dyn_cast<StackSpaceRegion>(MR->getMemorySpace());
if (SSR && SSR->getStackFrame() == StackFrameContext)
@@ -277,8 +276,44 @@ static void FindEscapingStackRegions(CheckerContext &C, SmallVector<const MemReg
Scanner.scan(SVal);
}
-void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
- CheckerContext &C) const {
+static void FilterReturnExpressionLeaks(SmallVector<const MemRegion *> &MaybeEscaped,
+ CheckerContext &C,
+ const Expr *RetE,
+ SVal &RetVal) {
+
+ // 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();
+
+ const MemRegion *RetRegion = RetVal.getAsRegion();
+
+ // 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 = RetRegion &&
+ isa<BlockDataRegion>(RetRegion) &&
+ ICE->getCastKind() == CK_CopyAndAutoreleaseBlockObject;
+ }
+
+ // Assuming MR is never nullptr
+ auto ShouldNotEmitError = [=](const MemRegion *MR) -> bool {
+ // If this particular MR is one of these special cases, then
+ // don't emit an error for this MR, but this still allows emitting these
+ // errors for MRs captured by, e.g., the temporary object.
+ if (RetRegion == MR) {
+ return IsConstructExpr || IsCopyAndAutoreleaseBlockObj;;
+ }
+ return false;
+ };
+
+ std::ignore = std::remove_if(MaybeEscaped.begin(), MaybeEscaped.end(), ShouldNotEmitError);
+}
+
+void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const {
if (!ChecksEnabled[CK_StackAddrEscapeChecker])
return;
@@ -290,64 +325,13 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
SVal V = C.getSVal(RetE);
SmallVector<const MemRegion *> EscapedRegions;
+
FindEscapingStackRegions(C, EscapedRegions, V);
+ FilterReturnExpressionLeaks(EscapedRegions, C, RetE, V);
for (const MemRegion *ER : EscapedRegions) {
- llvm::dbgs() << "Escaped region: ";
- ER->dumpToStream(llvm::dbgs());
- llvm::dbgs() << '\n';
-
- const MemRegion *R = ER;
- while (true) {
- switch (R->getKind()) {
- case MemRegion::ElementRegionKind:
- case MemRegion::FieldRegionKind:
- case MemRegion::ObjCIvarRegionKind:
- case MemRegion::CXXBaseObjectRegionKind:
- case MemRegion::CXXDerivedObjectRegionKind: {
- const SubRegion *SR = cast<SubRegion>(R);
- R = SR->getSuperRegion();
- llvm::dbgs() << "SuperRegion: ";
- R->dumpToStream(llvm::dbgs());
- llvm::dbgs() << '\n';
- continue;
- }
- default:
- break;
- }
- break;
- }
-
- const VarRegion *Base = dyn_cast<VarRegion>(ER->getBaseRegion());
- if (Base) {
- Base->getDecl()->getBeginLoc().dump(C.getSourceManager());
- }
-
EmitStackError(C, ER, RetE);
}
-
- // if (const BlockDataRegion *B = dyn_cast<BlockDataRegion>(R))
- // checkReturnedBlockCaptures(*B, C);
-
- // 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();
- // 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.
- // if (const auto *ICE = dyn_cast<ImplicitCastExpr>(RetE)) {
- // const MemRegion *R = V.getAsRegion();
- // if (R && isa<BlockDataRegion>(R) &&
- // ICE->getCastKind() == CK_CopyAndAutoreleaseBlockObject) {
- // return;
- // }
- // }
-
- // EmitStackError(C, R, RetE);
}
static const MemSpaceRegion *getStackOrGlobalSpaceRegion(const MemRegion *R) {
diff --git a/clang/test/Analysis/stackaddrleak.cpp b/clang/test/Analysis/stackaddrleak.cpp
index 82697d94a7f60d..733a86be1091a0 100644
--- a/clang/test/Analysis/stackaddrleak.cpp
+++ b/clang/test/Analysis/stackaddrleak.cpp
@@ -18,7 +18,7 @@ struct myfunction {
myfunction create_func() {
int n;
auto c = [&n] {};
- return c; // expected-warning {{Address of stack memory associated with local variable 'n' is still referred to by a temporary object on the stack upon returning to the caller. This will be a dangling reference}}
+ return c; // expected-warning {{Address of stack memory associated with local variable 'n' returned to caller}}
}
void gh_66221() {
create_func()();
>From 17bf25036cfa2ba9e93b15298c6141cbdd5dea35 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 29 Jan 2025 20:49:56 -0600
Subject: [PATCH 04/20] start separating checkEndFunction and checkPreStmt
---
.../Checkers/StackAddrEscapeChecker.cpp | 37 +++++++++++++------
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index ed9fa80a10de6c..5234f2b27806eb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -434,8 +434,8 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
Node = Node->getFirstPred();
}
- // Iterate over all bindings to global variables and see if it contains
- // a memory region in the stack space.
+ // Iterate over all bindings to global variables and stack arguments
+ // see if they contain a memory region in the current stack frame.
class CallBack : public StoreManager::BindingsHandler {
private:
CheckerContext &Ctx;
@@ -503,20 +503,33 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Region,
SVal Val) override {
recordInInvalidatedRegions(Region);
- const MemRegion *VR = Val.getAsRegion();
- if (!VR)
- return true;
- if (checkForDanglingStackVariable(Region, VR))
- return true;
+ const MemRegion *ReferrerMR = getStackOrGlobalSpaceRegion(Region);
- // Check the globals for the same.
- if (!isa_and_nonnull<GlobalsSpaceRegion>(
- getStackOrGlobalSpaceRegion(Region)))
+ if (!isa_and_nonnull<GlobalsSpaceRegion, StackArgumentsSpaceRegion>(ReferrerMR))
return true;
- if (VR && VR->hasStackStorage() && !isNotInCurrentFrame(VR, Ctx))
- V.emplace_back(Region, VR);
+
+ SmallVector<const MemRegion *> EscapingStackAddrs;
+ FindEscapingStackRegions(Ctx, EscapingStackAddrs, Val);
+
+ for (const MemRegion *Escapee : EscapingStackAddrs) {
+ V.emplace_back(Region, Escapee);
+ }
+
return true;
+ // const MemRegion *VR = Val.getAsRegion();
+ // if (!VR)
+ // return true;
+
+ // if (checkForDanglingStackVariable(Region, VR))
+ // return true;
+
+ // // Check the globals for the same.
+ // if (!isa_and_nonnull<GlobalsSpaceRegion>())
+ // return true;
+ // if (VR && VR->hasStackStorage() && !isNotInCurrentFrame(VR, Ctx))
+ // V.emplace_back(Region, VR);
+ // return true;
}
};
>From 89faf662ea20bfe8d4e24a50671e4ff5dab0b4bb Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Wed, 29 Jan 2025 23:00:43 -0600
Subject: [PATCH 05/20] changes to checkEndFunction
---
.../Checkers/StackAddrEscapeChecker.cpp | 66 ++++++++-----------
1 file changed, 27 insertions(+), 39 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 5234f2b27806eb..7bc99d681802e6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -261,7 +261,7 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
bool VisitSymbol(SymbolRef sym) override { return true; }
bool VisitMemRegion(const MemRegion *MR) override {
- llvm::dbgs() << "Visiting region: " << MR << '\n';
+ // llvm::dbgs() << "Visiting region: " << MR << '\n';
const StackSpaceRegion *SSR = dyn_cast<StackSpaceRegion>(MR->getMemorySpace());
if (SSR && SSR->getStackFrame() == StackFrameContext)
@@ -445,38 +445,39 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
/// Look for stack variables referring to popped stack variables.
/// Returns true only if it found some dangling stack variables
/// referred by an other stack variable from different stack frame.
- bool checkForDanglingStackVariable(const MemRegion *Referrer,
- const MemRegion *Referred) {
- const auto *ReferrerMemSpace = getStackOrGlobalSpaceRegion(Referrer);
- const auto *ReferredMemSpace =
- Referred->getMemorySpace()->getAs<StackSpaceRegion>();
+ /// Here, ReferrerMS is either a StackSpaceRegion or GlobalSpaceRegion,
+ /// and Referred has StackSpaceRegion that is the same as PoppedFrame.
+ bool isDanglingStackVariable(const MemRegion *Referrer, const MemSpaceRegion *ReferrerMemSpace,
+ const MemRegion *Referred) {
+ // Case 1: The referrer is a global and Referred is in the current stack context
+ if (isa<GlobalsSpaceRegion>(ReferrerMemSpace))
+ return true;
- if (!ReferrerMemSpace || !ReferredMemSpace)
+ const auto *ReferredMemSpace = Referred->getMemorySpace()->getAs<StackSpaceRegion>();
+ if (!ReferredMemSpace)
return false;
- const auto *ReferrerStackSpace =
- ReferrerMemSpace->getAs<StackSpaceRegion>();
-
+ const auto *ReferrerStackSpace = ReferrerMemSpace->getAs<StackSpaceRegion>();
if (!ReferrerStackSpace)
return false;
- if (const auto *ReferredFrame = ReferredMemSpace->getStackFrame();
- ReferredFrame != PoppedFrame) {
- return false;
- }
-
+ // Case 2: The referrer stack space is a parent of the referred stack space, e.g.,
+ // in case of leaking a local in a lambda to the outer scope
if (ReferrerStackSpace->getStackFrame()->isParentOf(PoppedFrame)) {
- V.emplace_back(Referrer, Referred);
return true;
}
+
+ // Case 3: The referrer is a memregion of a stack argument, e.g., an out
+ // argument, this is a top-level function, and referred is this top level
+ // stack space
if (isa<StackArgumentsSpaceRegion>(ReferrerMemSpace) &&
// Not a simple ptr (int*) but something deeper, e.g. int**
isa<SymbolicRegion>(Referrer->getBaseRegion()) &&
ReferrerStackSpace->getStackFrame() == PoppedFrame && TopFrame) {
// Output parameter of a top-level function
- V.emplace_back(Referrer, Referred);
return true;
}
+
return false;
}
@@ -500,36 +501,23 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
CallBack(CheckerContext &CC, bool TopFrame)
: Ctx(CC), PoppedFrame(CC.getStackFrame()), TopFrame(TopFrame) {}
- bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Region,
+ bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Referrer,
SVal Val) override {
- recordInInvalidatedRegions(Region);
-
- const MemRegion *ReferrerMR = getStackOrGlobalSpaceRegion(Region);
+ recordInInvalidatedRegions(Referrer);
- if (!isa_and_nonnull<GlobalsSpaceRegion, StackArgumentsSpaceRegion>(ReferrerMR))
+ const MemSpaceRegion *ReferrerMS = getStackOrGlobalSpaceRegion(Referrer);
+ if (!ReferrerMS)
return true;
- SmallVector<const MemRegion *> EscapingStackAddrs;
- FindEscapingStackRegions(Ctx, EscapingStackAddrs, Val);
+ SmallVector<const MemRegion *> PotentialEscapingAddrs;
+ FindEscapingStackRegions(Ctx, PotentialEscapingAddrs, Val);
- for (const MemRegion *Escapee : EscapingStackAddrs) {
- V.emplace_back(Region, Escapee);
+ for (const MemRegion *Escapee : PotentialEscapingAddrs) {
+ if (isDanglingStackVariable(Referrer, ReferrerMS, Escapee))
+ V.emplace_back(Referrer, Escapee);
}
return true;
- // const MemRegion *VR = Val.getAsRegion();
- // if (!VR)
- // return true;
-
- // if (checkForDanglingStackVariable(Region, VR))
- // return true;
-
- // // Check the globals for the same.
- // if (!isa_and_nonnull<GlobalsSpaceRegion>())
- // return true;
- // if (VR && VR->hasStackStorage() && !isNotInCurrentFrame(VR, Ctx))
- // V.emplace_back(Region, VR);
- // return true;
}
};
>From f225a2d98683fc078906dcbf9de49f000634409e Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Thu, 30 Jan 2025 00:10:20 -0600
Subject: [PATCH 06/20] wip
---
.../Checkers/StackAddrEscapeChecker.cpp | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 7bc99d681802e6..439963d744ba98 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -261,8 +261,6 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
bool VisitSymbol(SymbolRef sym) override { return true; }
bool VisitMemRegion(const MemRegion *MR) override {
- // llvm::dbgs() << "Visiting region: " << MR << '\n';
-
const StackSpaceRegion *SSR = dyn_cast<StackSpaceRegion>(MR->getMemorySpace());
if (SSR && SSR->getStackFrame() == StackFrameContext)
EscapingStackRegions.push_back(MR);
@@ -270,7 +268,7 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
}
};
-static void FindEscapingStackRegions(CheckerContext &C, SmallVector<const MemRegion *> &EscapedRegionsStorage, SVal SVal) {
+static void FindEscapingStackRegions(SmallVector<const MemRegion *> &EscapedRegionsStorage, CheckerContext &C, SVal SVal) {
FindStackRegionsSymbolVisitor Finder(C, EscapedRegionsStorage);
ScanReachableSymbols Scanner(C.getState(), Finder);
Scanner.scan(SVal);
@@ -326,7 +324,7 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &
SmallVector<const MemRegion *> EscapedRegions;
- FindEscapingStackRegions(C, EscapedRegions, V);
+ FindEscapingStackRegions(EscapedRegions, C, V);
FilterReturnExpressionLeaks(EscapedRegions, C, RetE, V);
for (const MemRegion *ER : EscapedRegions) {
@@ -463,9 +461,8 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
// Case 2: The referrer stack space is a parent of the referred stack space, e.g.,
// in case of leaking a local in a lambda to the outer scope
- if (ReferrerStackSpace->getStackFrame()->isParentOf(PoppedFrame)) {
+ if (ReferrerStackSpace->getStackFrame()->isParentOf(PoppedFrame))
return true;
- }
// Case 3: The referrer is a memregion of a stack argument, e.g., an out
// argument, this is a top-level function, and referred is this top level
@@ -503,6 +500,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Referrer,
SVal Val) override {
+ llvm::dbgs() << "HandleBinding: (" << Referrer << ", " << Val << ")\n";
recordInInvalidatedRegions(Referrer);
const MemSpaceRegion *ReferrerMS = getStackOrGlobalSpaceRegion(Referrer);
@@ -510,7 +508,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
return true;
SmallVector<const MemRegion *> PotentialEscapingAddrs;
- FindEscapingStackRegions(Ctx, PotentialEscapingAddrs, Val);
+ FindEscapingStackRegions(PotentialEscapingAddrs, Ctx, Val);
for (const MemRegion *Escapee : PotentialEscapingAddrs) {
if (isDanglingStackVariable(Referrer, ReferrerMS, Escapee))
>From 5816e91e1736f234a5f6099f9984b634c77c2621 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Sun, 2 Feb 2025 18:27:08 -0600
Subject: [PATCH 07/20] wip, fixed lambdas test failures
---
.../Checkers/StackAddrEscapeChecker.cpp | 135 ++++++++++++------
1 file changed, 89 insertions(+), 46 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 439963d744ba98..032caf0a19f540 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -213,29 +213,29 @@ 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::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 {
@@ -264,21 +264,27 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
const StackSpaceRegion *SSR = dyn_cast<StackSpaceRegion>(MR->getMemorySpace());
if (SSR && SSR->getStackFrame() == StackFrameContext)
EscapingStackRegions.push_back(MR);
- return true;
+ return true;
}
};
-static void FindEscapingStackRegions(SmallVector<const MemRegion *> &EscapedRegionsStorage, CheckerContext &C, SVal SVal) {
- FindStackRegionsSymbolVisitor Finder(C, EscapedRegionsStorage);
- ScanReachableSymbols Scanner(C.getState(), Finder);
+static SmallVector<const MemRegion *> FindEscapingStackRegions(CheckerContext &C, SVal SVal) {
+ SmallVector<const MemRegion *> FoundStackRegions;
+
+ FindStackRegionsSymbolVisitor Finder(C, FoundStackRegions);
+ ScanReachableSymbols Scanner(C.getState(), Finder);
Scanner.scan(SVal);
+
+ return FoundStackRegions;
}
-static void FilterReturnExpressionLeaks(SmallVector<const MemRegion *> &MaybeEscaped,
+static SmallVector<const MemRegion *> FilterReturnExpressionLeaks(const SmallVector<const MemRegion *> &MaybeEscaped,
CheckerContext &C,
const Expr *RetE,
SVal &RetVal) {
+ SmallVector<const MemRegion *> WillEscape;
+
// Returning a record by value is fine. (In this case, the returned
// expression will be a copy-constructor, possibly wrapped in an
// ExprWithCleanups node.)
@@ -297,18 +303,59 @@ static void FilterReturnExpressionLeaks(SmallVector<const MemRegion *> &MaybeEsc
ICE->getCastKind() == CK_CopyAndAutoreleaseBlockObject;
}
- // Assuming MR is never nullptr
- auto ShouldNotEmitError = [=](const MemRegion *MR) -> bool {
- // If this particular MR is one of these special cases, then
- // don't emit an error for this MR, but this still allows emitting these
- // errors for MRs captured by, e.g., the temporary object.
- if (RetRegion == MR) {
- return IsConstructExpr || IsCopyAndAutoreleaseBlockObj;;
+ for (const MemRegion *MR : MaybeEscaped) {
+ if (IsCopyAndAutoreleaseBlockObj) {
+ if (MR == RetRegion)
+ continue;
+
+ const SubRegion *SR = dyn_cast<SubRegion>(MR);
+ if (SR && SR->isSubRegionOf(RetRegion))
+ continue;
}
- return false;
- };
- std::ignore = std::remove_if(MaybeEscaped.begin(), MaybeEscaped.end(), ShouldNotEmitError);
+ if (RetRegion == MR && (IsConstructExpr || isa<CXXTempObjectRegion>(MR)))
+ continue;
+
+ if (isa<CXXTempObjectRegion>(MR->getBaseRegion()))
+ continue;
+
+ WillEscape.push_back(MR);
+ }
+
+ return WillEscape;
+
+ // // Assuming MR is never nullptr
+ // auto ShouldNotEmitError = [=](const MemRegion *MR) -> bool {
+ // // If we are checking the region that is returned (compared to
+ // // a region stored somewhere deeper in the return value), then
+ // // there are some potential false positives with reporting this
+ // // returned region:
+ // // 1. If we are returning a temp obj in an inlined call, back to this
+ // // same stack frame context:
+ // if (IsCopyAndAutoreleaseBlockObj) {
+ // if (MR == RetRegion)
+ // return true;
+
+ // const SubRegion *SR = dyn_cast<SubRegion>(MR);
+ // if (SR && SR->isSubRegionOf(RetRegion))
+ // return true;
+ // }
+
+ // if (RetRegion == MR) {
+ // return IsConstructExpr || isa<CXXTempObjectRegion>(MR);
+ // }
+
+ // if (isa<CXXTempObjectRegion>(MR->getBaseRegion()))
+ // return true;
+
+ // return false;
+ // };
+
+ // const auto NewEndIter = std::remove_if(MaybeEscaped.begin(), MaybeEscaped.end(), ShouldNotEmitError);
+
+ // llvm::dbgs() << "New num escaped regions: " << std::distance(MaybeEscaped.begin(), NewEndIter) << '\n';
+
+ // return SmallVector<const MemRegion *>()
}
void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const {
@@ -322,10 +369,9 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &
SVal V = C.getSVal(RetE);
- SmallVector<const MemRegion *> EscapedRegions;
+ SmallVector<const MemRegion *> MaybeEscapedRegions = FindEscapingStackRegions(C, V);
- FindEscapingStackRegions(EscapedRegions, C, V);
- FilterReturnExpressionLeaks(EscapedRegions, C, RetE, V);
+ SmallVector<const MemRegion *> EscapedRegions = FilterReturnExpressionLeaks(MaybeEscapedRegions, C, RetE, V);
for (const MemRegion *ER : EscapedRegions) {
EmitStackError(C, ER, RetE);
@@ -498,17 +544,14 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
CallBack(CheckerContext &CC, bool TopFrame)
: Ctx(CC), PoppedFrame(CC.getStackFrame()), TopFrame(TopFrame) {}
- bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Referrer,
- SVal Val) override {
- llvm::dbgs() << "HandleBinding: (" << Referrer << ", " << Val << ")\n";
+ bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Referrer, SVal Val) override {
recordInInvalidatedRegions(Referrer);
const MemSpaceRegion *ReferrerMS = getStackOrGlobalSpaceRegion(Referrer);
if (!ReferrerMS)
return true;
- SmallVector<const MemRegion *> PotentialEscapingAddrs;
- FindEscapingStackRegions(PotentialEscapingAddrs, Ctx, Val);
+ SmallVector<const MemRegion *> PotentialEscapingAddrs = FindEscapingStackRegions(Ctx, Val);
for (const MemRegion *Escapee : PotentialEscapingAddrs) {
if (isDanglingStackVariable(Referrer, ReferrerMS, Escapee))
>From 27c6cf8a3f9d8ef254ab23f44558d6878edd20e6 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Sun, 2 Feb 2025 18:27:22 -0600
Subject: [PATCH 08/20] formatting
---
.../Checkers/StackAddrEscapeChecker.cpp | 34 +++++++++++--------
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 032caf0a19f540..1e2801faefe0bd 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -212,7 +212,6 @@ void StackAddrEscapeChecker::checkAsyncExecutedBlockCaptures(
}
}
-
// void StackAddrEscapeChecker::checkReturnedBlockCaptures(
// const BlockDataRegion &B, CheckerContext &C) const {
// for (const MemRegion *Region : getCapturedStackRegions(B, C)) {
@@ -229,7 +228,8 @@ void StackAddrEscapeChecker::checkAsyncExecutedBlockCaptures(
// 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,
+// auto Report =
+// std::make_unique<PathSensitiveBugReport>(*BT_capturedstackret,
// Out.str(), N);
// if (Range.isValid())
// Report->addRange(Range);
@@ -268,20 +268,20 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
}
};
-static SmallVector<const MemRegion *> FindEscapingStackRegions(CheckerContext &C, SVal SVal) {
+static SmallVector<const MemRegion *>
+FindEscapingStackRegions(CheckerContext &C, SVal SVal) {
SmallVector<const MemRegion *> FoundStackRegions;
FindStackRegionsSymbolVisitor Finder(C, FoundStackRegions);
- ScanReachableSymbols Scanner(C.getState(), Finder);
+ ScanReachableSymbols Scanner(C.getState(), Finder);
Scanner.scan(SVal);
return FoundStackRegions;
}
-static SmallVector<const MemRegion *> FilterReturnExpressionLeaks(const SmallVector<const MemRegion *> &MaybeEscaped,
- CheckerContext &C,
- const Expr *RetE,
- SVal &RetVal) {
+static SmallVector<const MemRegion *>
+FilterReturnExpressionLeaks(const SmallVector<const MemRegion *> &MaybeEscaped,
+ CheckerContext &C, const Expr *RetE, SVal &RetVal) {
SmallVector<const MemRegion *> WillEscape;
@@ -351,9 +351,11 @@ static SmallVector<const MemRegion *> FilterReturnExpressionLeaks(const SmallVec
// return false;
// };
- // const auto NewEndIter = std::remove_if(MaybeEscaped.begin(), MaybeEscaped.end(), ShouldNotEmitError);
+ // const auto NewEndIter = std::remove_if(MaybeEscaped.begin(),
+ // MaybeEscaped.end(), ShouldNotEmitError);
- // llvm::dbgs() << "New num escaped regions: " << std::distance(MaybeEscaped.begin(), NewEndIter) << '\n';
+ // llvm::dbgs() << "New num escaped regions: " <<
+ // std::distance(MaybeEscaped.begin(), NewEndIter) << '\n';
// return SmallVector<const MemRegion *>()
}
@@ -369,9 +371,11 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &
SVal V = C.getSVal(RetE);
- SmallVector<const MemRegion *> MaybeEscapedRegions = FindEscapingStackRegions(C, V);
+ SmallVector<const MemRegion *> MaybeEscapedRegions =
+ FindEscapingStackRegions(C, V);
- SmallVector<const MemRegion *> EscapedRegions = FilterReturnExpressionLeaks(MaybeEscapedRegions, C, RetE, V);
+ SmallVector<const MemRegion *> EscapedRegions =
+ FilterReturnExpressionLeaks(MaybeEscapedRegions, C, RetE, V);
for (const MemRegion *ER : EscapedRegions) {
EmitStackError(C, ER, RetE);
@@ -544,14 +548,16 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
CallBack(CheckerContext &CC, bool TopFrame)
: Ctx(CC), PoppedFrame(CC.getStackFrame()), TopFrame(TopFrame) {}
- bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Referrer, SVal Val) override {
+ bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Referrer,
+ SVal Val) override {
recordInInvalidatedRegions(Referrer);
const MemSpaceRegion *ReferrerMS = getStackOrGlobalSpaceRegion(Referrer);
if (!ReferrerMS)
return true;
- SmallVector<const MemRegion *> PotentialEscapingAddrs = FindEscapingStackRegions(Ctx, Val);
+ SmallVector<const MemRegion *> PotentialEscapingAddrs =
+ FindEscapingStackRegions(Ctx, Val);
for (const MemRegion *Escapee : PotentialEscapingAddrs) {
if (isDanglingStackVariable(Referrer, ReferrerMS, Escapee))
>From b73ad74338acf64bcb0a6854b906a6a35024a6f6 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Sun, 2 Feb 2025 18:47:26 -0600
Subject: [PATCH 09/20] some cleanup
---
.../Checkers/StackAddrEscapeChecker.cpp | 45 +++----------------
1 file changed, 5 insertions(+), 40 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 1e2801faefe0bd..b8879ae5ab526c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -313,51 +313,16 @@ FilterReturnExpressionLeaks(const SmallVector<const MemRegion *> &MaybeEscaped,
continue;
}
- if (RetRegion == MR && (IsConstructExpr || isa<CXXTempObjectRegion>(MR)))
- continue;
+ // if (RetRegion == MR && (IsConstructExpr || isa<CXXTempObjectRegion>(MR)))
+ // continue;
- if (isa<CXXTempObjectRegion>(MR->getBaseRegion()))
- continue;
+ // if (isa<CXXTempObjectRegion>(MR->getBaseRegion()))
+ // continue;
WillEscape.push_back(MR);
}
return WillEscape;
-
- // // Assuming MR is never nullptr
- // auto ShouldNotEmitError = [=](const MemRegion *MR) -> bool {
- // // If we are checking the region that is returned (compared to
- // // a region stored somewhere deeper in the return value), then
- // // there are some potential false positives with reporting this
- // // returned region:
- // // 1. If we are returning a temp obj in an inlined call, back to this
- // // same stack frame context:
- // if (IsCopyAndAutoreleaseBlockObj) {
- // if (MR == RetRegion)
- // return true;
-
- // const SubRegion *SR = dyn_cast<SubRegion>(MR);
- // if (SR && SR->isSubRegionOf(RetRegion))
- // return true;
- // }
-
- // if (RetRegion == MR) {
- // return IsConstructExpr || isa<CXXTempObjectRegion>(MR);
- // }
-
- // if (isa<CXXTempObjectRegion>(MR->getBaseRegion()))
- // return true;
-
- // return false;
- // };
-
- // const auto NewEndIter = std::remove_if(MaybeEscaped.begin(),
- // MaybeEscaped.end(), ShouldNotEmitError);
-
- // llvm::dbgs() << "New num escaped regions: " <<
- // std::distance(MaybeEscaped.begin(), NewEndIter) << '\n';
-
- // return SmallVector<const MemRegion *>()
}
void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const {
@@ -595,7 +560,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
// Generate a report for this bug.
const StringRef CommonSuffix =
- " upon returning to the caller. This will be a dangling reference";
+ " upon returning to the caller. This will be a dangling reference";
SmallString<128> Buf;
llvm::raw_svector_ostream Out(Buf);
const SourceRange Range = genName(Out, Referred, Ctx.getASTContext());
>From 13eac11477ccc136f58ddd226c446c011f068884 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Sun, 2 Feb 2025 19:16:11 -0600
Subject: [PATCH 10/20] emit returned by part added
---
.../Checkers/StackAddrEscapeChecker.cpp | 119 ++++++++----------
1 file changed, 54 insertions(+), 65 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index b8879ae5ab526c..5d31c4410fe3d1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -57,8 +57,7 @@ class StackAddrEscapeChecker
CheckerContext &C) const;
void checkAsyncExecutedBlockCaptures(const BlockDataRegion &B,
CheckerContext &C) const;
- void EmitStackError(CheckerContext &C, const MemRegion *R,
- const Expr *RetE) const;
+ void EmitReturnLeakError(CheckerContext &C, const MemRegion *LeakedRegion, const Expr *RetE) const;
bool isSemaphoreCaptured(const BlockDecl &B) const;
static SourceRange genName(raw_ostream &os, const MemRegion *R,
ASTContext &Ctx);
@@ -150,7 +149,19 @@ StackAddrEscapeChecker::getCapturedStackRegions(const BlockDataRegion &B,
return Regions;
}
-void StackAddrEscapeChecker::EmitStackError(CheckerContext &C,
+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";
+ } else {
+ // Generic message
+ OS << " returned to caller";
+ }
+}
+
+void StackAddrEscapeChecker::EmitReturnLeakError(CheckerContext &C,
const MemRegion *R,
const Expr *RetE) const {
ExplodedNode *N = C.generateNonFatalErrorNode();
@@ -160,11 +171,15 @@ void StackAddrEscapeChecker::EmitStackError(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());
- os << " returned to caller";
+ EmitReturnedAsPartOfError(os, C.getSVal(RetE), R);
+
auto report =
std::make_unique<PathSensitiveBugReport>(*BT_returnstack, os.str(), N);
report->addRange(RetE->getSourceRange());
@@ -212,31 +227,6 @@ 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])
@@ -342,9 +332,8 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &
SmallVector<const MemRegion *> EscapedRegions =
FilterReturnExpressionLeaks(MaybeEscapedRegions, C, RetE, V);
- for (const MemRegion *ER : EscapedRegions) {
- EmitStackError(C, ER, RetE);
- }
+ for (const MemRegion *ER : EscapedRegions)
+ EmitReturnLeakError(C, ER, RetE);
}
static const MemSpaceRegion *getStackOrGlobalSpaceRegion(const MemRegion *R) {
@@ -447,8 +436,8 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
Node = Node->getFirstPred();
}
- // Iterate over all bindings to global variables and stack arguments
- // see if they contain a memory region in the current stack frame.
+ // Iterate over all bindings to global variables and see if it contains
+ // a memory region in the stack space.
class CallBack : public StoreManager::BindingsHandler {
private:
CheckerContext &Ctx;
@@ -458,38 +447,38 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
/// Look for stack variables referring to popped stack variables.
/// Returns true only if it found some dangling stack variables
/// referred by an other stack variable from different stack frame.
- /// Here, ReferrerMS is either a StackSpaceRegion or GlobalSpaceRegion,
- /// and Referred has StackSpaceRegion that is the same as PoppedFrame.
- bool isDanglingStackVariable(const MemRegion *Referrer, const MemSpaceRegion *ReferrerMemSpace,
- const MemRegion *Referred) {
- // Case 1: The referrer is a global and Referred is in the current stack context
- if (isa<GlobalsSpaceRegion>(ReferrerMemSpace))
- return true;
+ bool checkForDanglingStackVariable(const MemRegion *Referrer,
+ const MemRegion *Referred) {
+ const auto *ReferrerMemSpace = getStackOrGlobalSpaceRegion(Referrer);
+ const auto *ReferredMemSpace =
+ Referred->getMemorySpace()->getAs<StackSpaceRegion>();
- const auto *ReferredMemSpace = Referred->getMemorySpace()->getAs<StackSpaceRegion>();
- if (!ReferredMemSpace)
+ if (!ReferrerMemSpace || !ReferredMemSpace)
return false;
- const auto *ReferrerStackSpace = ReferrerMemSpace->getAs<StackSpaceRegion>();
+ const auto *ReferrerStackSpace =
+ ReferrerMemSpace->getAs<StackSpaceRegion>();
+
if (!ReferrerStackSpace)
return false;
- // Case 2: The referrer stack space is a parent of the referred stack space, e.g.,
- // in case of leaking a local in a lambda to the outer scope
- if (ReferrerStackSpace->getStackFrame()->isParentOf(PoppedFrame))
- return true;
+ if (const auto *ReferredFrame = ReferredMemSpace->getStackFrame();
+ ReferredFrame != PoppedFrame) {
+ return false;
+ }
- // Case 3: The referrer is a memregion of a stack argument, e.g., an out
- // argument, this is a top-level function, and referred is this top level
- // stack space
+ if (ReferrerStackSpace->getStackFrame()->isParentOf(PoppedFrame)) {
+ V.emplace_back(Referrer, Referred);
+ return true;
+ }
if (isa<StackArgumentsSpaceRegion>(ReferrerMemSpace) &&
// Not a simple ptr (int*) but something deeper, e.g. int**
isa<SymbolicRegion>(Referrer->getBaseRegion()) &&
ReferrerStackSpace->getStackFrame() == PoppedFrame && TopFrame) {
// Output parameter of a top-level function
+ V.emplace_back(Referrer, Referred);
return true;
}
-
return false;
}
@@ -513,22 +502,22 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
CallBack(CheckerContext &CC, bool TopFrame)
: Ctx(CC), PoppedFrame(CC.getStackFrame()), TopFrame(TopFrame) {}
- bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Referrer,
+ bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Region,
SVal Val) override {
- recordInInvalidatedRegions(Referrer);
-
- const MemSpaceRegion *ReferrerMS = getStackOrGlobalSpaceRegion(Referrer);
- if (!ReferrerMS)
+ recordInInvalidatedRegions(Region);
+ const MemRegion *VR = Val.getAsRegion();
+ if (!VR)
return true;
- SmallVector<const MemRegion *> PotentialEscapingAddrs =
- FindEscapingStackRegions(Ctx, Val);
-
- for (const MemRegion *Escapee : PotentialEscapingAddrs) {
- if (isDanglingStackVariable(Referrer, ReferrerMS, Escapee))
- V.emplace_back(Referrer, Escapee);
- }
+ if (checkForDanglingStackVariable(Region, VR))
+ return true;
+ // Check the globals for the same.
+ if (!isa_and_nonnull<GlobalsSpaceRegion>(
+ getStackOrGlobalSpaceRegion(Region)))
+ return true;
+ if (VR && VR->hasStackStorage() && !isNotInCurrentFrame(VR, Ctx))
+ V.emplace_back(Region, VR);
return true;
}
};
@@ -560,7 +549,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
// Generate a report for this bug.
const StringRef CommonSuffix =
- " upon returning to the caller. This will be a dangling reference";
+ " upon returning to the caller. This will be a dangling reference";
SmallString<128> Buf;
llvm::raw_svector_ostream Out(Buf);
const SourceRange Range = genName(Out, Referred, Ctx.getASTContext());
>From 129777e578c5ebd5601e3694cadc4721a4ad5d89 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Sun, 2 Feb 2025 19:49:15 -0600
Subject: [PATCH 11/20] add back IsConstructExpr bool in filtering
---
.../Checkers/StackAddrEscapeChecker.cpp | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 5d31c4410fe3d1..6983e5baefa543 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -153,12 +153,14 @@ static void EmitReturnedAsPartOfError(llvm::raw_ostream &OS,
SVal ReturnedVal,
const MemRegion *LeakedRegion) {
if (const MemRegion *ReturnedRegion = ReturnedVal.getAsRegion()) {
- if (isa<BlockDataRegion>(ReturnedRegion))
+ if (isa<BlockDataRegion>(ReturnedRegion)) {
OS << " is captured by a returned block";
- } else {
- // Generic message
- OS << " returned to caller";
+ return;
+ }
}
+
+ // Generic message
+ OS << " returned to caller";
}
void StackAddrEscapeChecker::EmitReturnLeakError(CheckerContext &C,
@@ -294,6 +296,8 @@ FilterReturnExpressionLeaks(const SmallVector<const MemRegion *> &MaybeEscaped,
}
for (const MemRegion *MR : MaybeEscaped) {
+ // In this case, the block_data region being returned isn't a leak,
+ // and all of its subregions are not leaks
if (IsCopyAndAutoreleaseBlockObj) {
if (MR == RetRegion)
continue;
@@ -303,11 +307,8 @@ FilterReturnExpressionLeaks(const SmallVector<const MemRegion *> &MaybeEscaped,
continue;
}
- // if (RetRegion == MR && (IsConstructExpr || isa<CXXTempObjectRegion>(MR)))
- // continue;
-
- // if (isa<CXXTempObjectRegion>(MR->getBaseRegion()))
- // continue;
+ if (RetRegion == MR && IsConstructExpr)
+ continue;
WillEscape.push_back(MR);
}
>From adb78d192ed956778b7d0b7f6ddedfbe38dd2a3d Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Sun, 2 Feb 2025 22:33:29 -0600
Subject: [PATCH 12/20] wip
---
.../Checkers/StackAddrEscapeChecker.cpp | 21 ++++++++++++++-----
clang/test/Analysis/stack-addr-ps.cpp | 2 +-
2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 6983e5baefa543..dab14aa88e9756 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -244,16 +244,27 @@ void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call,
}
class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
+ CheckerContext &Ctxt;
const StackFrameContext *StackFrameContext;
SmallVector<const MemRegion *> &EscapingStackRegions;
public:
explicit FindStackRegionsSymbolVisitor(CheckerContext &Ctxt, SmallVector<const MemRegion *> &StorageForStackRegions)
- : StackFrameContext(Ctxt.getStackFrame()), EscapingStackRegions(StorageForStackRegions) {}
+ : Ctxt(Ctxt), StackFrameContext(Ctxt.getStackFrame()), EscapingStackRegions(StorageForStackRegions) {}
bool VisitSymbol(SymbolRef sym) override { return true; }
bool VisitMemRegion(const MemRegion *MR) override {
- const StackSpaceRegion *SSR = dyn_cast<StackSpaceRegion>(MR->getMemorySpace());
+ if (const BlockDataRegion *BDR = MR->getAs<BlockDataRegion>()) {
+ for (auto Var : BDR->referenced_vars()) {
+ SVal Val = Ctxt.getState()->getSVal(Var.getCapturedRegion());
+ const MemRegion *Region = Val.getAsRegion();
+ if (Region && isa<StackSpaceRegion>(Region->getMemorySpace()))
+ EscapingStackRegions.push_back(Region);
+ }
+ return false;
+ }
+
+ const StackSpaceRegion *SSR = MR->getMemorySpace()->getAs<StackSpaceRegion>();
if (SSR && SSR->getStackFrame() == StackFrameContext)
EscapingStackRegions.push_back(MR);
return true;
@@ -302,9 +313,9 @@ FilterReturnExpressionLeaks(const SmallVector<const MemRegion *> &MaybeEscaped,
if (MR == RetRegion)
continue;
- const SubRegion *SR = dyn_cast<SubRegion>(MR);
- if (SR && SR->isSubRegionOf(RetRegion))
- continue;
+ // const SubRegion *SR = dyn_cast<SubRegion>(MR);
+ // if (SR && SR->isSubRegionOf(RetRegion))
+ // continue;
}
if (RetRegion == MR && IsConstructExpr)
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index cfa887da025a80..8413757b57a92b 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -251,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);
+ return new MyFunction(&lambda); // expected-warning{{Address of stack memory associated with local variable 'lambda' returned to caller}}
}
void lambda_to_context_direct_pointer_lifetime_extended() {
>From 2c629607662325109a6cda773ed562954c5f45b5 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 3 Feb 2025 12:45:06 -0600
Subject: [PATCH 13/20] cleanup
---
.../Checkers/StackAddrEscapeChecker.cpp | 56 ++++++++-----------
1 file changed, 22 insertions(+), 34 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index dab14aa88e9756..cb8dfcad21cbcb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -258,8 +258,10 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
for (auto Var : BDR->referenced_vars()) {
SVal Val = Ctxt.getState()->getSVal(Var.getCapturedRegion());
const MemRegion *Region = Val.getAsRegion();
- if (Region && isa<StackSpaceRegion>(Region->getMemorySpace()))
+ if (Region && isa<StackSpaceRegion>(Region->getMemorySpace())) {
EscapingStackRegions.push_back(Region);
+ VisitMemRegion(Region);
+ }
}
return false;
}
@@ -271,23 +273,14 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
}
};
-static SmallVector<const MemRegion *>
-FindEscapingStackRegions(CheckerContext &C, SVal SVal) {
- SmallVector<const MemRegion *> FoundStackRegions;
-
- FindStackRegionsSymbolVisitor Finder(C, FoundStackRegions);
- ScanReachableSymbols Scanner(C.getState(), Finder);
- Scanner.scan(SVal);
-
- return FoundStackRegions;
-}
-
static SmallVector<const MemRegion *>
FilterReturnExpressionLeaks(const SmallVector<const MemRegion *> &MaybeEscaped,
CheckerContext &C, const Expr *RetE, SVal &RetVal) {
SmallVector<const MemRegion *> WillEscape;
+ const MemRegion *RetRegion = RetVal.getAsRegion();
+
// Returning a record by value is fine. (In this case, the returned
// expression will be a copy-constructor, possibly wrapped in an
// ExprWithCleanups node.)
@@ -295,30 +288,17 @@ FilterReturnExpressionLeaks(const SmallVector<const MemRegion *> &MaybeEscaped,
RetE = Cleanup->getSubExpr();
bool IsConstructExpr = isa<CXXConstructExpr>(RetE) && RetE->getType()->isRecordType();
- const MemRegion *RetRegion = RetVal.getAsRegion();
-
// 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 = RetRegion &&
- isa<BlockDataRegion>(RetRegion) &&
+ IsCopyAndAutoreleaseBlockObj =
+ isa_and_nonnull<BlockDataRegion>(RetRegion) &&
ICE->getCastKind() == CK_CopyAndAutoreleaseBlockObject;
}
for (const MemRegion *MR : MaybeEscaped) {
- // In this case, the block_data region being returned isn't a leak,
- // and all of its subregions are not leaks
- if (IsCopyAndAutoreleaseBlockObj) {
- if (MR == RetRegion)
- continue;
-
- // const SubRegion *SR = dyn_cast<SubRegion>(MR);
- // if (SR && SR->isSubRegionOf(RetRegion))
- // continue;
- }
-
- if (RetRegion == MR && IsConstructExpr)
+ if (RetRegion == MR && (IsCopyAndAutoreleaseBlockObj || IsConstructExpr))
continue;
WillEscape.push_back(MR);
@@ -327,6 +307,17 @@ FilterReturnExpressionLeaks(const SmallVector<const MemRegion *> &MaybeEscaped,
return WillEscape;
}
+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;
@@ -338,13 +329,10 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &
SVal V = C.getSVal(RetE);
- SmallVector<const MemRegion *> MaybeEscapedRegions =
- FindEscapingStackRegions(C, V);
-
- SmallVector<const MemRegion *> EscapedRegions =
- FilterReturnExpressionLeaks(MaybeEscapedRegions, C, RetE, V);
+ SmallVector<const MemRegion *> EscapedStackRegions =
+ FindEscapingStackRegions(C, RetE, V);
- for (const MemRegion *ER : EscapedRegions)
+ for (const MemRegion *ER : EscapedStackRegions)
EmitReturnLeakError(C, ER, RetE);
}
>From e43fb34395db988c9621507915def609f0265122 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 3 Feb 2025 17:17:31 -0600
Subject: [PATCH 14/20] fix objc stack-allocated-capturing block return
---
.../Checkers/StackAddrEscapeChecker.cpp | 27 +++++++++++--------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index cb8dfcad21cbcb..4d0f5dbc896ab9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -253,22 +253,27 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
bool VisitSymbol(SymbolRef sym) override { return true; }
- bool VisitMemRegion(const MemRegion *MR) override {
- if (const BlockDataRegion *BDR = MR->getAs<BlockDataRegion>()) {
- for (auto Var : BDR->referenced_vars()) {
- SVal Val = Ctxt.getState()->getSVal(Var.getCapturedRegion());
- const MemRegion *Region = Val.getAsRegion();
- if (Region && isa<StackSpaceRegion>(Region->getMemorySpace())) {
- EscapingStackRegions.push_back(Region);
- VisitMemRegion(Region);
- }
+ bool VisitBlockDataRegion(const BlockDataRegion *BDR) {
+ for (auto Var : BDR->referenced_vars()) {
+ SVal Val = Ctxt.getState()->getSVal(Var.getCapturedRegion());
+ const MemRegion *Region = Val.getAsRegion();
+ if (Region && isa<StackSpaceRegion>(Region->getMemorySpace())) {
+ EscapingStackRegions.push_back(Region);
+ VisitMemRegion(Region);
}
- return false;
}
+ return false;
+ }
+
+ bool VisitMemRegion(const MemRegion *MR) override {
const StackSpaceRegion *SSR = MR->getMemorySpace()->getAs<StackSpaceRegion>();
if (SSR && SSR->getStackFrame() == StackFrameContext)
EscapingStackRegions.push_back(MR);
+
+ if (const BlockDataRegion *BDR = MR->getAs<BlockDataRegion>())
+ return VisitBlockDataRegion(BDR);
+
return true;
}
};
@@ -330,7 +335,7 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &
SVal V = C.getSVal(RetE);
SmallVector<const MemRegion *> EscapedStackRegions =
- FindEscapingStackRegions(C, RetE, V);
+ FindEscapingStackRegions(C, RetE, V);
for (const MemRegion *ER : EscapedStackRegions)
EmitReturnLeakError(C, ER, RetE);
>From 8c39d666caeeb27d7d3f7a425a6b93351d6364af Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 3 Feb 2025 18:12:28 -0600
Subject: [PATCH 15/20] why does this work
---
.../StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 4d0f5dbc896ab9..b64a5b4214d02a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -253,7 +253,8 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
bool VisitSymbol(SymbolRef sym) override { return true; }
- bool VisitBlockDataRegion(const BlockDataRegion *BDR) {
+ /// Visits the captured region values
+ bool VisitBlockDataRegionCaptures(const BlockDataRegion *BDR) {
for (auto Var : BDR->referenced_vars()) {
SVal Val = Ctxt.getState()->getSVal(Var.getCapturedRegion());
const MemRegion *Region = Val.getAsRegion();
@@ -272,7 +273,7 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
EscapingStackRegions.push_back(MR);
if (const BlockDataRegion *BDR = MR->getAs<BlockDataRegion>())
- return VisitBlockDataRegion(BDR);
+ return VisitBlockDataRegionCaptures(BDR);
return true;
}
@@ -306,6 +307,10 @@ FilterReturnExpressionLeaks(const SmallVector<const MemRegion *> &MaybeEscaped,
if (RetRegion == MR && (IsCopyAndAutoreleaseBlockObj || IsConstructExpr))
continue;
+ if (const StackSpaceRegion *SSR = MR->getMemorySpace()->getAs<StackSpaceRegion>())
+ if (SSR->getStackFrame() != C.getStackFrame())
+ continue;
+
WillEscape.push_back(MR);
}
>From 76493d1262eb53b8751a2b5a658647b504560aa9 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 3 Feb 2025 18:31:59 -0600
Subject: [PATCH 16/20] double up these test expectations
---
clang/test/Analysis/stack-addr-ps.cpp | 2 +-
clang/test/Analysis/stackaddrleak.cpp | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index 8413757b57a92b..392982d92a3f14 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -532,7 +532,7 @@ 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}}
+ 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() {
diff --git a/clang/test/Analysis/stackaddrleak.cpp b/clang/test/Analysis/stackaddrleak.cpp
index 733a86be1091a0..a44fb1f7d2dd12 100644
--- a/clang/test/Analysis/stackaddrleak.cpp
+++ b/clang/test/Analysis/stackaddrleak.cpp
@@ -18,7 +18,7 @@ struct myfunction {
myfunction create_func() {
int n;
auto c = [&n] {};
- return c; // expected-warning {{Address of stack memory associated with local variable 'n' returned to caller}}
+ return c; // expected-warning {{Address of stack memory associated with local variable 'n' returned to caller}} expected-warning{{Address of stack memory associated with local variable 'n' is still referred to by a temporary object on the stack upon returning to the caller. This will be a dangling reference}}
}
void gh_66221() {
create_func()();
>From 82400a79d81291d07d79d7916216f27f922a0217 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 3 Feb 2025 20:24:37 -0600
Subject: [PATCH 17/20] check everytime visitor adds to escaped region
---
.../Checkers/StackAddrEscapeChecker.cpp | 32 +++++++++++--------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index b64a5b4214d02a..d18e551d6497fc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -253,30 +253,34 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
bool VisitSymbol(SymbolRef sym) override { return true; }
- /// Visits the captured region values
+ 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 && isa<StackSpaceRegion>(Region->getMemorySpace())) {
- EscapingStackRegions.push_back(Region);
+ if (Region) {
+ SaveIfEscapes(Region);
VisitMemRegion(Region);
}
}
return false;
}
-
- bool VisitMemRegion(const MemRegion *MR) override {
- const StackSpaceRegion *SSR = MR->getMemorySpace()->getAs<StackSpaceRegion>();
- if (SSR && SSR->getStackFrame() == StackFrameContext)
- EscapingStackRegions.push_back(MR);
-
- if (const BlockDataRegion *BDR = MR->getAs<BlockDataRegion>())
- return VisitBlockDataRegionCaptures(BDR);
-
- return true;
- }
};
static SmallVector<const MemRegion *>
>From 39df282dfa3553aaa4617004353942f00adb009d Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 3 Feb 2025 22:57:45 -0600
Subject: [PATCH 18/20] fix comments
---
.../Checkers/StackAddrEscapeChecker.cpp | 21 ++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index d18e551d6497fc..919ef932b40c29 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -243,6 +243,13 @@ 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;
@@ -283,6 +290,11 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
}
};
+/// 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 SmallVector<const MemRegion *> &MaybeEscaped,
CheckerContext &C, const Expr *RetE, SVal &RetVal) {
@@ -311,9 +323,10 @@ FilterReturnExpressionLeaks(const SmallVector<const MemRegion *> &MaybeEscaped,
if (RetRegion == MR && (IsCopyAndAutoreleaseBlockObj || IsConstructExpr))
continue;
- if (const StackSpaceRegion *SSR = MR->getMemorySpace()->getAs<StackSpaceRegion>())
- if (SSR->getStackFrame() != C.getStackFrame())
- continue;
+ // If this is a construct expr of an unelided return value copy, then don't
+ // warn about returning a region that currently lives on the stack.
+ if (IsConstructExpr && RetVal.getAs<nonloc::LazyCompoundVal>() && isa<CXXTempObjectRegion>(MR))
+ continue;
WillEscape.push_back(MR);
}
@@ -321,6 +334,8 @@ FilterReturnExpressionLeaks(const SmallVector<const MemRegion *> &MaybeEscaped,
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;
>From 65bee7a910a7fa02981fca35b0619ac4c64ca233 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 3 Feb 2025 23:05:13 -0600
Subject: [PATCH 19/20] formatting
---
.../Checkers/StackAddrEscapeChecker.cpp | 45 +++++++++++--------
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 919ef932b40c29..2b1bd97c78743f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -29,8 +29,7 @@ using namespace ento;
namespace {
class StackAddrEscapeChecker
- : public Checker<check::PreCall,
- check::PreStmt<ReturnStmt>,
+ : public Checker<check::PreCall, check::PreStmt<ReturnStmt>,
check::EndFunction> {
mutable IdentifierInfo *dispatch_semaphore_tII = nullptr;
mutable std::unique_ptr<BugType> BT_stackleak;
@@ -57,7 +56,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 EmitReturnLeakError(CheckerContext &C, const MemRegion *LeakedRegion,
+ const Expr *RetE) const;
bool isSemaphoreCaptured(const BlockDecl &B) const;
static SourceRange genName(raw_ostream &os, const MemRegion *R,
ASTContext &Ctx);
@@ -149,8 +149,7 @@ StackAddrEscapeChecker::getCapturedStackRegions(const BlockDataRegion &B,
return Regions;
}
-static void EmitReturnedAsPartOfError(llvm::raw_ostream &OS,
- SVal ReturnedVal,
+static void EmitReturnedAsPartOfError(llvm::raw_ostream &OS, SVal ReturnedVal,
const MemRegion *LeakedRegion) {
if (const MemRegion *ReturnedRegion = ReturnedVal.getAsRegion()) {
if (isa<BlockDataRegion>(ReturnedRegion)) {
@@ -164,8 +163,8 @@ static void EmitReturnedAsPartOfError(llvm::raw_ostream &OS,
}
void StackAddrEscapeChecker::EmitReturnLeakError(CheckerContext &C,
- const MemRegion *R,
- const Expr *RetE) const {
+ const MemRegion *R,
+ const Expr *RetE) const {
ExplodedNode *N = C.generateNonFatalErrorNode();
if (!N)
return;
@@ -243,26 +242,30 @@ void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call,
}
}
-/// A visitor made for use with a ScanReachableSymbols scanner, used
+/// 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.
+/// that would leak.
class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
CheckerContext &Ctxt;
const StackFrameContext *StackFrameContext;
SmallVector<const MemRegion *> &EscapingStackRegions;
+
public:
- explicit FindStackRegionsSymbolVisitor(CheckerContext &Ctxt, SmallVector<const MemRegion *> &StorageForStackRegions)
- : Ctxt(Ctxt), StackFrameContext(Ctxt.getStackFrame()), EscapingStackRegions(StorageForStackRegions) {}
+ explicit FindStackRegionsSymbolVisitor(
+ CheckerContext &Ctxt,
+ SmallVector<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);
@@ -271,7 +274,8 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
private:
void SaveIfEscapes(const MemRegion *MR) {
- const StackSpaceRegion *SSR = MR->getMemorySpace()->getAs<StackSpaceRegion>();
+ const StackSpaceRegion *SSR =
+ MR->getMemorySpace()->getAs<StackSpaceRegion>();
if (SSR && SSR->getStackFrame() == StackFrameContext)
EscapingStackRegions.push_back(MR);
}
@@ -308,15 +312,16 @@ FilterReturnExpressionLeaks(const SmallVector<const MemRegion *> &MaybeEscaped,
// ExprWithCleanups node.)
if (const ExprWithCleanups *Cleanup = dyn_cast<ExprWithCleanups>(RetE))
RetE = Cleanup->getSubExpr();
- bool IsConstructExpr = isa<CXXConstructExpr>(RetE) && RetE->getType()->isRecordType();
+ bool IsConstructExpr =
+ isa<CXXConstructExpr>(RetE) && RetE->getType()->isRecordType();
// 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;
+ isa_and_nonnull<BlockDataRegion>(RetRegion) &&
+ ICE->getCastKind() == CK_CopyAndAutoreleaseBlockObject;
}
for (const MemRegion *MR : MaybeEscaped) {
@@ -325,7 +330,8 @@ FilterReturnExpressionLeaks(const SmallVector<const MemRegion *> &MaybeEscaped,
// If this is a construct expr of an unelided return value copy, then don't
// warn about returning a region that currently lives on the stack.
- if (IsConstructExpr && RetVal.getAs<nonloc::LazyCompoundVal>() && isa<CXXTempObjectRegion>(MR))
+ if (IsConstructExpr && RetVal.getAs<nonloc::LazyCompoundVal>() &&
+ isa<CXXTempObjectRegion>(MR))
continue;
WillEscape.push_back(MR);
@@ -347,7 +353,8 @@ FindEscapingStackRegions(CheckerContext &C, const Expr *RetE, SVal RetVal) {
return FilterReturnExpressionLeaks(FoundStackRegions, C, RetE, RetVal);
}
-void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const {
+void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
+ CheckerContext &C) const {
if (!ChecksEnabled[CK_StackAddrEscapeChecker])
return;
@@ -359,7 +366,7 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &
SVal V = C.getSVal(RetE);
SmallVector<const MemRegion *> EscapedStackRegions =
- FindEscapingStackRegions(C, RetE, V);
+ FindEscapingStackRegions(C, RetE, V);
for (const MemRegion *ER : EscapedStackRegions)
EmitReturnLeakError(C, ER, RetE);
>From 47f3b408eff9f92978dfa28c2444bdd141df22f9 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 3 Feb 2025 23:06:23 -0600
Subject: [PATCH 20/20] remove old includes
---
clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp | 2 --
1 file changed, 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 2b1bd97c78743f..86f0949994cf6b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -20,9 +20,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h"
#include "llvm/ADT/SmallString.h"
-#include "llvm/Support/Debug.h"
#include "llvm/Support/raw_ostream.h"
using namespace clang;
using namespace ento;
More information about the cfe-commits
mailing list