[clang] Flanders/123459 stack addr escape checker with buildbot fix (PR #126618)
Michael Flanders via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 10 14:56:53 PST 2025
https://github.com/Flandini created https://github.com/llvm/llvm-project/pull/126618
Reopening https://github.com/llvm/llvm-project/pull/125638 after buildbot failures.
Buildbot failures fixed in f73a7f52f93050ab98f9b47cdac1c801fc807781, latest commit on this PR. It was a problem with a declared class member with same name as its type. Sorry!
>From 5bf5270d9237715114f560c4fb4b17ff3e29a7f0 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/40] 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 f4de3b500499c48..59b47371df0d29e 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 ff3478d8a1df07611d6e8d5aa00eed280db492a7 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/40] 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 59b47371df0d29e..b6ef375936cccc8 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 73e9dbeca460f60..cfa887da025a804 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 3daffb35a6cd9a6..82697d94a7f60d9 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 9d58c46b7754dd2a12f6d48a2ae76220d8080ea9 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/40] 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 b6ef375936cccc8..ed9fa80a10de6c8 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 82697d94a7f60d9..733a86be1091a0a 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 e9e0ae889368e7be77e3ac5804b5a45f6e28b319 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/40] 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 ed9fa80a10de6c8..5234f2b27806eb7 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 4edaa02c00546429b84cc4837cacafd8d7cf26d1 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/40] 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 5234f2b27806eb7..7bc99d681802e61 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 78be69dfbccd5d03c0ed5b5128aaa337e3621587 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/40] 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 7bc99d681802e61..439963d744ba98c 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 21e5c9d27cf1c6048dafde2c78feae6a53d4ab7a 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/40] 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 439963d744ba98c..032caf0a19f5400 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 a0bcc4d00e38c4668388d73b741afbe8f2e51de7 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/40] 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 032caf0a19f5400..1e2801faefe0bd3 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 a94ccd60d08c40a4c4a3dd54f64c44db63d59d9e 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/40] 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 1e2801faefe0bd3..b8879ae5ab526c5 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 b72b98cd31a9a121c48098ef1f27a0eab0f02bf1 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/40] 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 b8879ae5ab526c5..5d31c4410fe3d1c 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 b78b96cb172b3b7faadff6043f276a1deab780ca 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/40] 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 5d31c4410fe3d1c..6983e5baefa543b 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 a078753487d3366f5c0a83e4c6b919a24191e252 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/40] 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 6983e5baefa543b..dab14aa88e97564 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 cfa887da025a804..8413757b57a92be 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 891f5fd0bf9a8bc2168e9ca60b7461beb676d960 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/40] 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 dab14aa88e97564..cb8dfcad21cbcbf 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 6a7cb9641ef1de7e65e004adf83c1a4b0a80c69c 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/40] 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 cb8dfcad21cbcbf..4d0f5dbc896ab9a 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 6d29efbee22f6b788906aa424816c6d43e6bd142 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/40] 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 4d0f5dbc896ab9a..b64a5b4214d02a3 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 296555e3ef2430fc9b71ef67d3c220e69ea0c22f 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/40] 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 8413757b57a92be..392982d92a3f14c 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 733a86be1091a0a..a44fb1f7d2dd121 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 932c14fc3537988771ee8b86e213b3c21da93e2e 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/40] 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 b64a5b4214d02a3..d18e551d6497fc6 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 9d8fc1a2a588ca1491d0601c54904b51306fbf78 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/40] 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 d18e551d6497fc6..919ef932b40c29a 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 f909ae127d2814a295980d0e03eb6a014ca6d68e 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/40] 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 919ef932b40c29a..2b1bd97c78743f8 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 ef7949263cd8c47d1ec4b3ed38bec3517a53c532 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/40] 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 2b1bd97c78743f8..86f0949994cf6b9 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;
>From cb702c92b06c9a9cfb68ecd9b6d81a8520d6f08b Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Tue, 4 Feb 2025 14:17:27 -0600
Subject: [PATCH 21/40] restore whitespace at end of file
---
clang/test/Analysis/stackaddrleak.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/Analysis/stackaddrleak.cpp b/clang/test/Analysis/stackaddrleak.cpp
index a44fb1f7d2dd121..df202a2fee77628 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 d5c123d0a98fbf769c5400bc4de9345f29894606 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Tue, 4 Feb 2025 14:35:25 -0600
Subject: [PATCH 22/40] add basic int* and int return assign expr tests
---
clang/test/Analysis/stack-addr-ps.cpp | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index 392982d92a3f14c..cdd6e08f579b6ef 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -90,6 +90,12 @@ 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}}
+}
+
void *lf() {
label:
void *const &x = &&label; // expected-note {{binding reference variable 'x' here}}
@@ -912,3 +918,14 @@ void top_malloc_no_crash_fn() {
free(pptr);
}
} // namespace alloca_region_pointer
+
+namespace true_negatives_return_expressions {
+void return_void() {
+ return void(); // no-warning
+}
+
+int return_safe_assign_expr() {
+ int y, x = 14;
+ return y = x; // no-warning
+}
+}
\ No newline at end of file
>From 19cdb1bbe3977b8d48ed80a263e4ded3cff00f50 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Tue, 4 Feb 2025 16:01:55 -0600
Subject: [PATCH 23/40] return comma-separated expression
---
clang/test/Analysis/stack-addr-ps.cpp | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index cdd6e08f579b6ef..edaec8ec4be48db 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -96,6 +96,12 @@ int *return_assign_expr_leak() {
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}}
@@ -920,12 +926,26 @@ void top_malloc_no_crash_fn() {
} // namespace alloca_region_pointer
namespace true_negatives_return_expressions {
+struct Container { int *x; };
+
void return_void() {
return void(); // no-warning
}
-int return_safe_assign_expr() {
+int return_assign_expr_safe() {
int y, x = 14;
return y = x; // no-warning
}
+
+int return_comma_separated_expressions_safe() {
+ int x = 1;
+ int *y;
+ return (y=&x), (x=15); // no-warning
+}
+
+int return_comma_separated_expressions_container_safe() {
+ int x = 1;
+ Container Other;
+ return Other = Container{&x}, x = 14; // no-warning
+}
}
\ No newline at end of file
>From cf0a5f94fb7ee3f5f6ed80054bd7df721a0005d8 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Thu, 6 Feb 2025 19:07:36 -0600
Subject: [PATCH 24/40] Add expected warning to copy-elision.cpp no-elide tests
---
clang/test/Analysis/copy-elision.cpp | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/clang/test/Analysis/copy-elision.cpp b/clang/test/Analysis/copy-elision.cpp
index cd941854fc7f196..e69f52f351f1bc0 100644
--- a/clang/test/Analysis/copy-elision.cpp
+++ b/clang/test/Analysis/copy-elision.cpp
@@ -154,20 +154,26 @@ 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);
+ return ClassWithoutDestructor(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithoutDestructor' returned to caller}}
// no-elide-warning at -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);
+ return make1(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithoutDestructor' returned to caller}}
// no-elide-warning at -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);
+ return make2(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithoutDestructor' returned to caller}}
// no-elide-warning at -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}}
@@ -298,21 +304,26 @@ 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);
+ return ClassWithDestructor(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithDestructor' returned to caller}}
// no-elide-warning at -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);
+ return make1(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithDestructor' returned to caller}}
// no-elide-warning at -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);
+ return make2(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithDestructor' returned to caller}}
// no-elide-warning at -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}}
>From 02b24e8fa42b23b39c346688772cb8169c12275d Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Thu, 6 Feb 2025 19:08:58 -0600
Subject: [PATCH 25/40] Add simple true negative test case from code review
---
clang/test/Analysis/stack-addr-ps.cpp | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index edaec8ec4be48db..c691b4960207716 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -928,6 +928,11 @@ void top_malloc_no_crash_fn() {
namespace true_negatives_return_expressions {
struct Container { int *x; };
+int test2() {
+ int x = 14;
+ return x; // no-warning
+}
+
void return_void() {
return void(); // no-warning
}
>From e3b9ec94a183c626bfc6b0c9f085b3d4b8758303 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Thu, 6 Feb 2025 19:09:33 -0600
Subject: [PATCH 26/40] Remove temp object check in return expr checking
---
.../lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp | 6 ------
1 file changed, 6 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 86f0949994cf6b9..4f8099e1a6f8564 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -326,12 +326,6 @@ FilterReturnExpressionLeaks(const SmallVector<const MemRegion *> &MaybeEscaped,
if (RetRegion == MR && (IsCopyAndAutoreleaseBlockObj || IsConstructExpr))
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);
}
>From 5b67313529ea745bd142a94c37f0f1e5c145a565 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Thu, 6 Feb 2025 19:10:30 -0600
Subject: [PATCH 27/40] Add newline end of file
---
clang/test/Analysis/stack-addr-ps.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index c691b4960207716..7e5e6842cf22cbe 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -953,4 +953,4 @@ int return_comma_separated_expressions_container_safe() {
Container Other;
return Other = Container{&x}, x = 14; // no-warning
}
-}
\ No newline at end of file
+}
>From 6c2425ff430e7d2c863b9e3a795b2e8b52ce5f85 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Fri, 7 Feb 2025 11:45:57 -0600
Subject: [PATCH 28/40] Add testcase: return of unknown symbol on stack
---
clang/test/Analysis/stack-addr-ps.cpp | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index 7e5e6842cf22cbe..1160b40b1982c0d 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -953,4 +953,11 @@ int return_comma_separated_expressions_container_safe() {
Container Other;
return Other = Container{&x}, x = 14; // no-warning
}
+
+int make_x();
+int return_symbol_safe() {
+ int x = make_x();
+ clang_analyzer_dump(x); // expected-warning-re {{conj_$2{int, {{.+}}}}}
+ return x; // no-warning
+}
}
>From 84ce3a12e20a0acc429894e4bd6478e5f36403a1 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Fri, 7 Feb 2025 11:47:38 -0600
Subject: [PATCH 29/40] Add testcase of false negative returning symbolic arg
---
clang/test/Analysis/stack-addr-ps.cpp | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index 1160b40b1982c0d..f2d6e762c150df6 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -352,6 +352,13 @@ 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 at -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);
}
@@ -960,4 +967,8 @@ int return_symbol_safe() {
clang_analyzer_dump(x); // expected-warning-re {{conj_$2{int, {{.+}}}}}
return x; // no-warning
}
+
+int *return_symbolic_region_safe(int **ptr) {
+ return *ptr; // no-warning
+}
}
>From d1870073b0c23290807bdbd906f53a2b3f68b909 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Fri, 7 Feb 2025 22:00:00 -0600
Subject: [PATCH 30/40] Add testcase: bind rval ref to temporary
---
clang/test/Analysis/stack-addr-ps.cpp | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index f2d6e762c150df6..3e839478e6e50ef 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -190,6 +190,11 @@ 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 at -1{{returning reference to local temporary object}} -Wreturn-stack-address
+}
+
namespace leaking_via_direct_pointer {
void* returned_direct_pointer_top() {
int local = 42;
>From abac2ed17731ae9542aed46d2ee3b63f23bcc07e Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Sat, 8 Feb 2025 20:05:21 -0600
Subject: [PATCH 31/40] Add testcases: return argptr, conditional expressions,
comparison expressions
---
clang/test/Analysis/stack-addr-ps.cpp | 32 +++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index 3e839478e6e50ef..ab8c91b217d6d04 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -976,4 +976,36 @@ int return_symbol_safe() {
int *return_symbolic_region_safe(int **ptr) {
return *ptr; // no-warning
}
+
+int *return_arg_ptr(int *arg) {
+ return arg; // no-warning
+}
+
+// This has a false positive with -Wreturn-stack-address, but CSA should not
+// warn on this
+int *return_conditional_never_false(int *arg) {
+ int x = 14;
+ return true ? arg : &x; // expected-warning {{address of stack memory associated with local variable 'x' returned}}
+}
+
+// This has a false positive with -Wreturn-stack-address, but CSA should not
+// warn on this
+int *return_conditional_never_true(int *arg) {
+ int x = 14;
+ return false ? &x : arg; // expected-warning {{address of stack memory associated with local variable 'x' returned}}
+}
+
+int *return_conditional_path_split(int *arg, bool b) {
+ int x = 14;
+ return b ? arg : &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}} -Wreturn-stack-address
+}
+
+// compare of two symbolic regions
+// maybe in some implementation, make_ptr returns interior pointers that are comparable
+int *make_ptr();
+bool return_symbolic_exxpression() {
+ int *a = make_ptr();
+ int *b = make_ptr();
+ return a < b; // no-warning
+}
}
>From e63d78a812eee50e05120f2d43bf58f35f04af7d Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 10 Feb 2025 10:48:59 -0600
Subject: [PATCH 32/40] Add testscases from SemaCXX/return-stack-addr.cpp
---
clang/test/Analysis/stack-addr-ps.cpp | 54 +++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index ab8c91b217d6d04..c442fc4b96ae356 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -164,6 +164,18 @@ 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;
@@ -937,6 +949,39 @@ void top_malloc_no_crash_fn() {
}
} // namespace alloca_region_pointer
+// These all warn with -Wreturn-stack-address also
+namespace return_address_of_true_positives {
+int* ret_local_addrOf() {
+ int x = 1;
+ return &*&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}}
+}
+
+int* ret_local_addrOf_paren() {
+ int x = 1;
+ return (&(*(&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}}
+}
+
+int* ret_local_addrOf_ptr_arith() {
+ int x = 1;
+ return &*(&x+1); // 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}}
+}
+
+int* ret_local_addrOf_ptr_arith2() {
+ int x = 1;
+ return &*(&x+1); // 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}}
+}
+
+int* ret_local_field() {
+ struct { int x; } a;
+ return &a.x; // expected-warning {{Address of stack memory associated with local variable 'a' returned to caller}} expected-warning {{address of stack memory associated with local variable 'a' returned}}
+}
+
+int& ret_local_field_ref() {
+ struct { int x; } a;
+ return a.x; // expected-warning {{Address of stack memory associated with local variable 'a' returned to caller}} expected-warning {{reference to stack memory associated with local variable 'a' returned}}
+}
+} //namespace return_address_of_true_positives
+
namespace true_negatives_return_expressions {
struct Container { int *x; };
@@ -1008,4 +1053,13 @@ bool return_symbolic_exxpression() {
int *b = make_ptr();
return a < b; // no-warning
}
+
+int *return_static() {
+ static int x = 0;
+ return &x; // no-warning
+}
+
+int* return_cpp_reinterpret_cast_no_warning(long x) {
+ return reinterpret_cast<int*>(x); // no-warning
+}
}
>From 628fd5252bd6637d413b6995d54e3698ceac8cfe Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 10 Feb 2025 11:07:18 -0600
Subject: [PATCH 33/40] Add lifetimebound attribute test case
---
clang/test/Analysis/stackaddrleak.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/clang/test/Analysis/stackaddrleak.c b/clang/test/Analysis/stackaddrleak.c
index 5f508275ba9c8d4..f580955b0b328dc 100644
--- a/clang/test/Analysis/stackaddrleak.c
+++ b/clang/test/Analysis/stackaddrleak.c
@@ -56,3 +56,15 @@ void assignAsBool(void) {
int x;
b = &x;
} // no-warning
+
+int *f(int* p __attribute__((lifetimebound)));
+int *g() {
+ int i;
+ return f(&i); // expected-warning {{address of stack memory associated with local variable 'i' returned}}
+}
+
+int *f_no_lifetime_bound(int *p);
+int *g_no_lifetime_bound() {
+ int i = 0;
+ return f_no_lifetime_bound(&i); // no-warning
+}
\ No newline at end of file
>From f938dd59edb52603ed936823a59d9eee0618015b Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 10 Feb 2025 11:07:44 -0600
Subject: [PATCH 34/40] Add TODO on testcase for musttail attribute
---
clang/test/Analysis/stack-addr-ps.cpp | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index c442fc4b96ae356..aa51e5954a2ad9f 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -1063,3 +1063,22 @@ int* return_cpp_reinterpret_cast_no_warning(long x) {
return reinterpret_cast<int*>(x); // no-warning
}
}
+
+// TODO: The tail call expression tree must not hold a reference to an arg or stack local:
+// "The lifetimes of all local variables and function parameters end immediately before the
+// [tail] call to the function. This means that it is undefined behaviour to pass a pointer or
+// reference to a local variable to the called function, which is not the case without the
+// attribute."
+// These only warn from -Wreturn-stack-address for now
+namespace with_attr_musttail {
+void TakesIntAndPtr(int, int *);
+void PassAddressOfLocal(int a, int *b) {
+ int c;
+ [[clang::musttail]] return TakesIntAndPtr(0, &c); // expected-warning {{address of stack memory associated with local variable 'c' pass\
+ed to musttail function}}
+}
+void PassAddressOfParam(int a, int *b) {
+ [[clang::musttail]] return TakesIntAndPtr(0, &a); // expected-warning {{address of stack memory associated with parameter 'a' passed to\
+ musttail function}}
+}
+} // namespace with_attr_musttail
\ No newline at end of file
>From a778a1949829e8815299505d672a9f8b89c720d8 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 10 Feb 2025 11:08:22 -0600
Subject: [PATCH 35/40] Add false negative comments
---
clang/test/Analysis/stack-addr-ps.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index aa51e5954a2ad9f..ddbf678adc1c021 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -1075,10 +1075,10 @@ void TakesIntAndPtr(int, int *);
void PassAddressOfLocal(int a, int *b) {
int c;
[[clang::musttail]] return TakesIntAndPtr(0, &c); // expected-warning {{address of stack memory associated with local variable 'c' pass\
-ed to musttail function}}
+ed to musttail function}} False-negative on CSA
}
void PassAddressOfParam(int a, int *b) {
[[clang::musttail]] return TakesIntAndPtr(0, &a); // expected-warning {{address of stack memory associated with parameter 'a' passed to\
- musttail function}}
+ musttail function}} False-negative on CSA
}
} // namespace with_attr_musttail
\ No newline at end of file
>From 8bf192dca4d4bd6b159a0dd6f01d8b4a522818af Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 10 Feb 2025 11:20:05 -0600
Subject: [PATCH 36/40] Take references to SmallVectorImpl rather than
SmallVector
---
.../lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 4f8099e1a6f8564..dcd2ad327c8d89b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -250,12 +250,12 @@ void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call,
class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
CheckerContext &Ctxt;
const StackFrameContext *StackFrameContext;
- SmallVector<const MemRegion *> &EscapingStackRegions;
+ SmallVectorImpl<const MemRegion *> &EscapingStackRegions;
public:
explicit FindStackRegionsSymbolVisitor(
CheckerContext &Ctxt,
- SmallVector<const MemRegion *> &StorageForStackRegions)
+ SmallVectorImpl<const MemRegion *> &StorageForStackRegions)
: Ctxt(Ctxt), StackFrameContext(Ctxt.getStackFrame()),
EscapingStackRegions(StorageForStackRegions) {}
@@ -298,7 +298,7 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
/// 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,
+FilterReturnExpressionLeaks(const SmallVectorImpl<const MemRegion *> &MaybeEscaped,
CheckerContext &C, const Expr *RetE, SVal &RetVal) {
SmallVector<const MemRegion *> WillEscape;
>From f911a26c0c2096843be67278208c200140899069 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 10 Feb 2025 11:20:37 -0600
Subject: [PATCH 37/40] formatting
---
.../lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index dcd2ad327c8d89b..826dc42e4f59948 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -297,9 +297,9 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
/// 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) {
+static SmallVector<const MemRegion *> FilterReturnExpressionLeaks(
+ const SmallVectorImpl<const MemRegion *> &MaybeEscaped, CheckerContext &C,
+ const Expr *RetE, SVal &RetVal) {
SmallVector<const MemRegion *> WillEscape;
>From 4dca6141d6a036a3f67276a8ea48d494b0c6a793 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 10 Feb 2025 12:00:26 -0600
Subject: [PATCH 38/40] Add newline at end of file
---
clang/test/Analysis/stack-addr-ps.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index ddbf678adc1c021..bf988d0a1695947 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -1081,4 +1081,4 @@ void PassAddressOfParam(int a, int *b) {
[[clang::musttail]] return TakesIntAndPtr(0, &a); // expected-warning {{address of stack memory associated with parameter 'a' passed to\
musttail function}} False-negative on CSA
}
-} // namespace with_attr_musttail
\ No newline at end of file
+} // namespace with_attr_musttail
>From 9ac7689edb48b317708d92e0588b231bb314b8d8 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 10 Feb 2025 12:01:14 -0600
Subject: [PATCH 39/40] Add newline at end of file
---
clang/test/Analysis/stackaddrleak.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/Analysis/stackaddrleak.c b/clang/test/Analysis/stackaddrleak.c
index f580955b0b328dc..f8101525401b0ff 100644
--- a/clang/test/Analysis/stackaddrleak.c
+++ b/clang/test/Analysis/stackaddrleak.c
@@ -67,4 +67,4 @@ int *f_no_lifetime_bound(int *p);
int *g_no_lifetime_bound() {
int i = 0;
return f_no_lifetime_bound(&i); // no-warning
-}
\ No newline at end of file
+}
>From f73a7f52f93050ab98f9b47cdac1c801fc807781 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.michaelk at gmail.com>
Date: Mon, 10 Feb 2025 16:37:03 -0600
Subject: [PATCH 40/40] [analyzer] Fix StackFrameContext define in stack addr
escape checker
---
.../lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 826dc42e4f59948..c9df15ceb3b4092 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -249,14 +249,14 @@ void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call,
/// that would leak.
class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
CheckerContext &Ctxt;
- const StackFrameContext *StackFrameContext;
+ const StackFrameContext *PoppedStackFrame;
SmallVectorImpl<const MemRegion *> &EscapingStackRegions;
public:
explicit FindStackRegionsSymbolVisitor(
CheckerContext &Ctxt,
SmallVectorImpl<const MemRegion *> &StorageForStackRegions)
- : Ctxt(Ctxt), StackFrameContext(Ctxt.getStackFrame()),
+ : Ctxt(Ctxt), PoppedStackFrame(Ctxt.getStackFrame()),
EscapingStackRegions(StorageForStackRegions) {}
bool VisitSymbol(SymbolRef sym) override { return true; }
@@ -274,7 +274,7 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
void SaveIfEscapes(const MemRegion *MR) {
const StackSpaceRegion *SSR =
MR->getMemorySpace()->getAs<StackSpaceRegion>();
- if (SSR && SSR->getStackFrame() == StackFrameContext)
+ if (SSR && SSR->getStackFrame() == PoppedStackFrame)
EscapingStackRegions.push_back(MR);
}
More information about the cfe-commits
mailing list