https://github.com/Flandini updated
https://github.com/llvm/llvm-project/pull/125638
>From 0674909f03703a70c3e259acd0590f50cea4615f Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Mon, 27 Jan 2025 11:35:03 -0600
Subject: [PATCH 01/27] 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 0a26f83cd5f4a42238ba4fadb3436fec83e4bfcb Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Wed, 29 Jan 2025 00:34:45 -0600
Subject: [PATCH 02/27] 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 3ebf2221ec153c384db450890c7b65996df9d19b Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Wed, 29 Jan 2025 20:49:26 -0600
Subject: [PATCH 03/27] 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 17bf25036cfa2ba9e93b15298c6141cbdd5dea35 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Wed, 29 Jan 2025 20:49:56 -0600
Subject: [PATCH 04/27] 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 89faf662ea20bfe8d4e24a50671e4ff5dab0b4bb Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Wed, 29 Jan 2025 23:00:43 -0600
Subject: [PATCH 05/27] 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 f225a2d98683fc078906dcbf9de49f000634409e Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Thu, 30 Jan 2025 00:10:20 -0600
Subject: [PATCH 06/27] 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 5816e91e1736f234a5f6099f9984b634c77c2621 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Sun, 2 Feb 2025 18:27:08 -0600
Subject: [PATCH 07/27] 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 27c6cf8a3f9d8ef254ab23f44558d6878edd20e6 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Sun, 2 Feb 2025 18:27:22 -0600
Subject: [PATCH 08/27] 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 b73ad74338acf64bcb0a6854b906a6a35024a6f6 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Sun, 2 Feb 2025 18:47:26 -0600
Subject: [PATCH 09/27] 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 13eac11477ccc136f58ddd226c446c011f068884 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Sun, 2 Feb 2025 19:16:11 -0600
Subject: [PATCH 10/27] 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 129777e578c5ebd5601e3694cadc4721a4ad5d89 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Sun, 2 Feb 2025 19:49:15 -0600
Subject: [PATCH 11/27] 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 adb78d192ed956778b7d0b7f6ddedfbe38dd2a3d Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Sun, 2 Feb 2025 22:33:29 -0600
Subject: [PATCH 12/27] 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 2c629607662325109a6cda773ed562954c5f45b5 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Mon, 3 Feb 2025 12:45:06 -0600
Subject: [PATCH 13/27] 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 e43fb34395db988c9621507915def609f0265122 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Mon, 3 Feb 2025 17:17:31 -0600
Subject: [PATCH 14/27] 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 8c39d666caeeb27d7d3f7a425a6b93351d6364af Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Mon, 3 Feb 2025 18:12:28 -0600
Subject: [PATCH 15/27] 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 76493d1262eb53b8751a2b5a658647b504560aa9 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Mon, 3 Feb 2025 18:31:59 -0600
Subject: [PATCH 16/27] 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 82400a79d81291d07d79d7916216f27f922a0217 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Mon, 3 Feb 2025 20:24:37 -0600
Subject: [PATCH 17/27] 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 39df282dfa3553aaa4617004353942f00adb009d Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Mon, 3 Feb 2025 22:57:45 -0600
Subject: [PATCH 18/27] 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 65bee7a910a7fa02981fca35b0619ac4c64ca233 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Mon, 3 Feb 2025 23:05:13 -0600
Subject: [PATCH 19/27] 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 47f3b408eff9f92978dfa28c2444bdd141df22f9 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Mon, 3 Feb 2025 23:06:23 -0600
Subject: [PATCH 20/27] 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 6a2a01f31ef86d7882c92227bb034657171bc7ba Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Tue, 4 Feb 2025 14:17:27 -0600
Subject: [PATCH 21/27] 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 30ea259896668d199c7e8545f77e5019835843da Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Tue, 4 Feb 2025 14:35:25 -0600
Subject: [PATCH 22/27] 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 567c8e6e1fbd7386822b89da2118b3bcb708835a Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Tue, 4 Feb 2025 16:01:55 -0600
Subject: [PATCH 23/27] 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 4d4d537297954216ec9d299fcb1572909daffcdf Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Thu, 6 Feb 2025 19:07:36 -0600
Subject: [PATCH 24/27] 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@-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@-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@-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@-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@-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@-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 a4ad3cb7371477178418af429a46f2a69bf35409 Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Thu, 6 Feb 2025 19:08:58 -0600
Subject: [PATCH 25/27] 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 8603e15ae8f664ab99a72fc58a45e89e4ca1218d Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Thu, 6 Feb 2025 19:09:33 -0600
Subject: [PATCH 26/27] 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 7953dba62dfdf960629d9e7b2ea903c4bde81b7a Mon Sep 17 00:00:00 2001
From: Michael Flanders <flanders.micha...@gmail.com>
Date: Thu, 6 Feb 2025 19:10:30 -0600
Subject: [PATCH 27/27] 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
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
