[clang] [analyzer] Detect leaks of stack addresses via output params, indirect globals 3/3 (PR #105648)
Arseniy Zaostrovnykh via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 27 06:14:49 PDT 2024
https://github.com/necto updated https://github.com/llvm/llvm-project/pull/105648
>From 991f176c5545fedae2ba8b5c1b357734abe68ac7 Mon Sep 17 00:00:00 2001
From: Arseniy Zaostrovnykh <necto.ne at gmail.com>
Date: Tue, 20 Aug 2024 11:16:10 +0200
Subject: [PATCH 1/4] [analyzer] Detect leaks on top-level via output params,
indirect globals
Fix some false negatives of StackAddrEscapeChecker:
- Output parameters
```
void top(int **out) {
int local = 42;
*out = &local; // Noncompliant
}
```
- Indirect global pointers
```
int **global;
void top() {
int local = 42;
*global = &local; // Noncompliant
}
```
Note that now StackAddrEscapeChecker produces a diagnostic if a function
with an output parameter is analyzed as top-level or as a callee. I took
special care to make sure the reports point to the same primary location
and, in many cases, feature the same primary message. That is the
motivation to modify Core/BugReporter.cpp and Core/ExplodedGraph.cpp
To avoid false positive reports when a global indirect pointer is
assigned a local address, invalidated, and then reset, I rely on the
fact that the invalidation symbol will be a DerivedSymbol of a
ConjuredSymbol that refers to the same memory region.
The checker still has a false negative for non-trivial escaping via a
returned value. It requires a more sophisticated traversal akin to
scanReachableSymbols, which out of the scope of this change.
CPP-4734
---
.../Checkers/StackAddrEscapeChecker.cpp | 64 +++++++++-
clang/lib/StaticAnalyzer/Core/BugReporter.cpp | 13 +-
.../lib/StaticAnalyzer/Core/ExplodedGraph.cpp | 2 +-
clang/test/Analysis/copy-elision.cpp | 20 +--
.../test/Analysis/incorrect-checker-names.cpp | 4 +-
clang/test/Analysis/loop-block-counts.c | 2 +-
clang/test/Analysis/stack-addr-ps.c | 6 +-
clang/test/Analysis/stack-addr-ps.cpp | 117 ++++++++++--------
.../Analysis/stack-capture-leak-no-arc.mm | 4 +-
clang/test/Analysis/stackaddrleak.c | 8 +-
10 files changed, 157 insertions(+), 83 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index dcf6801a73de2d..82f03e5f7d09d3 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -305,6 +305,14 @@ static const MemSpaceRegion *getStackOrGlobalSpaceRegion(const MemRegion *R) {
return nullptr;
}
+const MemRegion *getOriginBaseRegion(const MemRegion *Referrer) {
+ Referrer = Referrer->getBaseRegion();
+ while (const auto *SymReg = dyn_cast<SymbolicRegion>(Referrer)) {
+ Referrer = SymReg->getSymbol()->getOriginRegion()->getBaseRegion();
+ }
+ return Referrer;
+}
+
std::optional<std::string> printReferrer(const MemRegion *Referrer) {
assert(Referrer);
const StringRef ReferrerMemorySpace = [](const MemSpaceRegion *Space) {
@@ -313,7 +321,8 @@ std::optional<std::string> printReferrer(const MemRegion *Referrer) {
if (isa<GlobalsSpaceRegion>(Space))
return "global";
assert(isa<StackSpaceRegion>(Space));
- return "stack";
+ // This case covers top-level and inlined analyses.
+ return "caller";
}(getStackOrGlobalSpaceRegion(Referrer));
while (!Referrer->canPrintPretty()) {
@@ -348,12 +357,27 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
ExplodedNode *Node = Ctx.getPredecessor();
+ bool ExitingTopFrame =
+ Ctx.getPredecessor()->getLocationContext()->inTopFrame();
+
+ if (ExitingTopFrame && Node->getLocation().getTag() &&
+ Node->getLocation().getTag()->getTagDescription() ==
+ "ExprEngine : Clean Node" &&
+ Node->getFirstPred()) {
+ // When finishing analysis of a top-level function, engine proactively
+ // removes dead symbols thus preventing this checker from looking through
+ // the output parameters. Take 1 step back, to the node where these symbols
+ // and their bindings are still present
+ Node = Node->getFirstPred();
+ }
+
// 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;
const StackFrameContext *PoppedFrame;
+ const bool TopFrame;
/// Look for stack variables referring to popped stack variables.
/// Returns true only if it found some dangling stack variables
@@ -369,24 +393,48 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
const auto *ReferrerStackSpace =
ReferrerMemSpace->getAs<StackSpaceRegion>();
+
if (!ReferrerStackSpace)
return false;
- if (ReferredMemSpace->getStackFrame() == PoppedFrame &&
- ReferrerStackSpace->getStackFrame()->isParentOf(PoppedFrame)) {
+ if (const auto *ReferredFrame = ReferredMemSpace->getStackFrame();
+ ReferredFrame != PoppedFrame) {
+ return false;
+ }
+
+ if (ReferrerStackSpace->getStackFrame()->isParentOf(PoppedFrame)) {
+ V.emplace_back(Referrer, Referred);
+ return true;
+ }
+ if (isa<StackArgumentsSpaceRegion>(ReferrerMemSpace) &&
+ ReferrerStackSpace->getStackFrame() == PoppedFrame && TopFrame) {
+ // Output parameter of a top-level function
V.emplace_back(Referrer, Referred);
return true;
}
return false;
}
+ void updateInvalidatedRegions(const MemRegion *Region) {
+ if (const auto *SymReg = Region->getAs<SymbolicRegion>()) {
+ SymbolRef Symbol = SymReg->getSymbol();
+ if (const auto *DerS = dyn_cast<SymbolDerived>(Symbol);
+ DerS && isa_and_nonnull<SymbolConjured>(DerS->getParentSymbol())) {
+ InvalidatedRegions.insert(Symbol->getOriginRegion()->getBaseRegion());
+ }
+ }
+ }
+
public:
SmallVector<std::pair<const MemRegion *, const MemRegion *>, 10> V;
+ llvm::SmallPtrSet<const MemRegion *, 4> InvalidatedRegions;
- CallBack(CheckerContext &CC) : Ctx(CC), PoppedFrame(CC.getStackFrame()) {}
+ CallBack(CheckerContext &CC, bool TopFrame)
+ : Ctx(CC), PoppedFrame(CC.getStackFrame()), TopFrame(TopFrame) {}
bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Region,
SVal Val) override {
+ updateInvalidatedRegions(Region);
const MemRegion *VR = Val.getAsRegion();
if (!VR)
return true;
@@ -395,7 +443,8 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
return true;
// Check the globals for the same.
- if (!isa<GlobalsSpaceRegion>(Region->getMemorySpace()))
+ if (!isa_and_nonnull<GlobalsSpaceRegion>(
+ getStackOrGlobalSpaceRegion(Region)))
return true;
if (VR && VR->hasStackStorage() && !isNotInCurrentFrame(VR, Ctx))
V.emplace_back(Region, VR);
@@ -403,7 +452,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
}
};
- CallBack Cb(Ctx);
+ CallBack Cb(Ctx, ExitingTopFrame);
ProgramStateRef State = Node->getState();
State->getStateManager().getStoreManager().iterBindings(State->getStore(),
Cb);
@@ -424,6 +473,9 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
for (const auto &P : Cb.V) {
const MemRegion *Referrer = P.first->getBaseRegion();
const MemRegion *Referred = P.second;
+ if (Cb.InvalidatedRegions.contains(getOriginBaseRegion(Referrer))) {
+ continue;
+ }
// Generate a report for this bug.
const StringRef CommonSuffix =
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index d73dc40cf03fbb..ea916c3585cadc 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2436,8 +2436,19 @@ PathSensitiveBugReport::getLocation() const {
if (auto FE = P.getAs<FunctionExitPoint>()) {
if (const ReturnStmt *RS = FE->getStmt())
return PathDiagnosticLocation::createBegin(RS, SM, LC);
+
+ // If we are exiting a destructor call, it is more useful to point to the
+ // next stmt which is usually the temporary declaration.
+ // For non-destructor and non-top-level calls, the next stmt will still
+ // refer to the last executed stmt of the body.
+ S = ErrorNode->getNextStmtForDiagnostics();
+ // If next stmt is not found, it is likely the end of a top-level function
+ // analysis. find the last execution statement then.
+ if (!S)
+ S = ErrorNode->getPreviousStmtForDiagnostics();
}
- S = ErrorNode->getNextStmtForDiagnostics();
+ if (!S)
+ S = ErrorNode->getNextStmtForDiagnostics();
}
if (S) {
diff --git a/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp b/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
index f84da769d182f8..11cef00ada330b 100644
--- a/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
@@ -376,7 +376,7 @@ const Stmt *ExplodedNode::getNextStmtForDiagnostics() const {
const Stmt *ExplodedNode::getPreviousStmtForDiagnostics() const {
for (const ExplodedNode *N = getFirstPred(); N; N = N->getFirstPred())
- if (const Stmt *S = N->getStmtForDiagnostics())
+ if (const Stmt *S = N->getStmtForDiagnostics(); S && !isa<CompoundStmt>(S))
return S;
return nullptr;
diff --git a/clang/test/Analysis/copy-elision.cpp b/clang/test/Analysis/copy-elision.cpp
index 991f325c05853d..423c4519f5bfc6 100644
--- a/clang/test/Analysis/copy-elision.cpp
+++ b/clang/test/Analysis/copy-elision.cpp
@@ -158,19 +158,19 @@ ClassWithoutDestructor make1(AddressVector<ClassWithoutDestructor> &v) {
return ClassWithoutDestructor(v);
// no-elide-warning at -1 {{Address of stack memory associated with temporary \
object of type 'ClassWithoutDestructor' is still \
-referred to by the stack variable 'v' upon returning to the caller}}
+referred to by the caller variable 'v' upon returning to the caller}}
}
ClassWithoutDestructor make2(AddressVector<ClassWithoutDestructor> &v) {
return make1(v);
// no-elide-warning at -1 {{Address of stack memory associated with temporary \
object of type 'ClassWithoutDestructor' is still \
-referred to by the stack variable 'v' upon returning to the caller}}
+referred to by the caller variable 'v' upon returning to the caller}}
}
ClassWithoutDestructor make3(AddressVector<ClassWithoutDestructor> &v) {
return make2(v);
// no-elide-warning at -1 {{Address of stack memory associated with temporary \
object of type 'ClassWithoutDestructor' is still \
-referred to by the stack variable 'v' upon returning to the caller}}
+referred to by the caller variable 'v' upon returning to the caller}}
}
void testMultipleReturns() {
@@ -193,7 +193,7 @@ void testMultipleReturns() {
void consume(ClassWithoutDestructor c) {
c.push();
// expected-warning at -1 {{Address of stack memory associated with local \
-variable 'c' is still referred to by the stack variable 'v' upon returning \
+variable 'c' is still referred to by the caller variable 'v' upon returning \
to the caller}}
}
@@ -267,7 +267,7 @@ struct TestCtorInitializer {
: c(ClassWithDestructor(v)) {}
// no-elide-warning at -1 {{Address of stack memory associated with temporary \
object of type 'ClassWithDestructor' is still referred \
-to by the stack variable 'v' upon returning to the caller}}
+to by the caller variable 'v' upon returning to the caller}}
};
void testCtorInitializer() {
@@ -303,19 +303,19 @@ ClassWithDestructor make1(AddressVector<ClassWithDestructor> &v) {
return ClassWithDestructor(v);
// no-elide-warning at -1 {{Address of stack memory associated with temporary \
object of type 'ClassWithDestructor' is still referred \
-to by the stack variable 'v' upon returning to the caller}}
+to by the caller variable 'v' upon returning to the caller}}
}
ClassWithDestructor make2(AddressVector<ClassWithDestructor> &v) {
return make1(v);
// no-elide-warning at -1 {{Address of stack memory associated with temporary \
object of type 'ClassWithDestructor' is still referred \
-to by the stack variable 'v' upon returning to the caller}}
+to by the caller variable 'v' upon returning to the caller}}
}
ClassWithDestructor make3(AddressVector<ClassWithDestructor> &v) {
return make2(v);
// no-elide-warning at -1 {{Address of stack memory associated with temporary \
object of type 'ClassWithDestructor' is still referred \
-to by the stack variable 'v' upon returning to the caller}}
+to by the caller variable 'v' upon returning to the caller}}
}
void testMultipleReturnsWithDestructors() {
@@ -360,7 +360,7 @@ void testMultipleReturnsWithDestructors() {
void consume(ClassWithDestructor c) {
c.push();
// expected-warning at -1 {{Address of stack memory associated with local \
-variable 'c' is still referred to by the stack variable 'v' upon returning \
+variable 'c' is still referred to by the caller variable 'v' upon returning \
to the caller}}
}
@@ -407,7 +407,7 @@ struct Foo {
Foo make1(Foo **r) {
return Foo(r);
// no-elide-warning at -1 {{Address of stack memory associated with temporary \
-object of type 'Foo' is still referred to by the stack \
+object of type 'Foo' is still referred to by the caller \
variable 'z' upon returning to the caller}}
}
diff --git a/clang/test/Analysis/incorrect-checker-names.cpp b/clang/test/Analysis/incorrect-checker-names.cpp
index 9854a503fc4f62..8dee0b6f468581 100644
--- a/clang/test/Analysis/incorrect-checker-names.cpp
+++ b/clang/test/Analysis/incorrect-checker-names.cpp
@@ -16,5 +16,5 @@ char const *p;
void f0() {
char const str[] = "This will change";
p = str;
-} // expected-warning{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference [core.StackAddressEscape]}}
-// expected-note at -1{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference}}
+} // expected-warning at -1{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference [core.StackAddressEscape]}}
+// expected-note at -2{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference}}
diff --git a/clang/test/Analysis/loop-block-counts.c b/clang/test/Analysis/loop-block-counts.c
index 9af67b1b632f51..ae3705f8bd7ccc 100644
--- a/clang/test/Analysis/loop-block-counts.c
+++ b/clang/test/Analysis/loop-block-counts.c
@@ -6,7 +6,7 @@ void callee(void **p) {
int x;
*p = &x;
// expected-warning at -1 {{Address of stack memory associated with local \
-variable 'x' is still referred to by the stack variable 'arr' upon \
+variable 'x' is still referred to by the caller variable 'arr' upon \
returning to the caller}}
}
diff --git a/clang/test/Analysis/stack-addr-ps.c b/clang/test/Analysis/stack-addr-ps.c
index 2e14b7820be136..138b8c16b02bde 100644
--- a/clang/test/Analysis/stack-addr-ps.c
+++ b/clang/test/Analysis/stack-addr-ps.c
@@ -98,13 +98,13 @@ void callTestRegister(void) {
void top_level_leaking(int **out) {
int local = 42;
- *out = &local; // no-warning FIXME
+ *out = &local; // expected-warning{{Address of stack memory associated with local variable 'local' is still referred to by the caller variable 'out'}}
}
void callee_leaking_via_param(int **out) {
int local = 1;
*out = &local;
- // expected-warning at -1{{Address of stack memory associated with local variable 'local' is still referred to by the stack variable 'ptr'}}
+ // expected-warning at -1{{Address of stack memory associated with local variable 'local' is still referred to by the caller variable 'ptr'}}
}
void caller_for_leaking_callee() {
@@ -115,7 +115,7 @@ void caller_for_leaking_callee() {
void callee_nested_leaking(int **out) {
int local = 1;
*out = &local;
- // expected-warning at -1{{Address of stack memory associated with local variable 'local' is still referred to by the stack variable 'ptr'}}
+ // expected-warning at -1{{Address of stack memory associated with local variable 'local' is still referred to by the caller variable 'ptr'}}
}
void caller_mid_for_nested_leaking(int **mid) {
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index 95a6e3cbd25c7c..88bf6512165201 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -143,7 +143,7 @@ void write_stack_address_to(char **q) {
char local;
*q = &local;
// expected-warning at -1 {{Address of stack memory associated with local \
-variable 'local' is still referred to by the stack variable 'p' upon \
+variable 'local' is still referred to by the caller variable 'p' upon \
returning to the caller}}
}
@@ -188,14 +188,14 @@ void* global_ptr;
void global_direct_pointer() {
int local = 42;
- global_ptr = &local;
-} // expected-warning{{local variable 'local' is still referred to by the global variable 'global_ptr'}}
+ global_ptr = &local; // expected-warning{{local variable 'local' is still referred to by the global variable 'global_ptr'}}
+}
void static_direct_pointer_top() {
int local = 42;
static int* p = &local;
- (void)p;
-} // expected-warning{{local variable 'local' is still referred to by the static variable 'p'}}
+ (void)p; // expected-warning{{local variable 'local' is still referred to by the static variable 'p'}}
+}
void static_direct_pointer_callee() {
int local = 42;
@@ -219,7 +219,7 @@ void lambda_to_context_direct_pointer() {
int *p = nullptr;
auto lambda = [&] {
int local = 42;
- p = &local; // expected-warning{{local variable 'local' is still referred to by the stack variable 'p'}}
+ p = &local; // expected-warning{{local variable 'local' is still referred to by the caller variable 'p'}}
};
lambda();
(void)p;
@@ -245,7 +245,7 @@ void lambda_to_context_direct_pointer_lifetime_extended() {
int *p = nullptr;
auto lambda = [&] {
int&& local = 42;
- p = &local; // expected-warning{{'int' lifetime extended by local variable 'local' is still referred to by the stack variable 'p'}}
+ p = &local; // expected-warning{{'int' lifetime extended by local variable 'local' is still referred to by the caller variable 'p'}}
};
lambda();
(void)p;
@@ -254,7 +254,7 @@ void lambda_to_context_direct_pointer_lifetime_extended() {
template<typename Callback>
void lambda_param_capture_direct_pointer_callee(Callback& callee) {
int local = 42;
- callee(local); // expected-warning{{'local' is still referred to by the stack variable 'p'}}
+ callee(local); // expected-warning{{'local' is still referred to by the caller variable 'p'}}
}
void lambda_param_capture_direct_pointer_caller() {
@@ -279,19 +279,19 @@ void** global_pp;
void global_ptr_local_to_ptr() {
int local = 42;
int* p = &local;
- global_pp = (void**)&p;
-} // expected-warning{{local variable 'p' is still referred to by the global variable 'global_pp'}}
+ global_pp = (void**)&p; // expected-warning{{local variable 'p' is still referred to by the global variable 'global_pp'}}
+}
void global_ptr_to_ptr() {
int local = 42;
- *global_pp = &local; // no-warning FIXME
+ *global_pp = &local; // expected-warning{{local variable 'local' is still referred to by the global variable 'global_pp'}}
}
void *** global_ppp;
void global_ptr_to_ptr_to_ptr() {
int local = 42;
- **global_ppp = &local; // no-warning FIXME
+ **global_ppp = &local; // expected-warning{{local variable 'local' is still referred to by the global variable 'global_ppp'}}
}
void** get_some_pp();
@@ -304,12 +304,12 @@ void static_ptr_to_ptr() {
void param_ptr_to_ptr_top(void** pp) {
int local = 42;
- *pp = &local; // no-warning FIXME
+ *pp = &local; // expected-warning{{local variable 'local' is still referred to by the caller variable 'pp'}}
}
void param_ptr_to_ptr_callee(void** pp) {
int local = 42;
- *pp = &local; // expected-warning{{local variable 'local' is still referred to by the stack variable 'p'}}
+ *pp = &local; // expected-warning{{local variable 'local' is still referred to by the caller variable 'p'}}
}
void param_ptr_to_ptr_caller() {
@@ -319,12 +319,12 @@ void param_ptr_to_ptr_caller() {
void param_ptr_to_ptr_to_ptr_top(void*** ppp) {
int local = 42;
- **ppp = &local; // no-warning FIXME
+ **ppp = &local; // expected-warning {{local variable 'local' is still referred to by the caller variable 'ppp'}}
}
void param_ptr_to_ptr_to_ptr_callee(void*** ppp) {
int local = 42;
- **ppp = &local; // expected-warning{{local variable 'local' is still referred to by the stack variable 'pp'}}
+ **ppp = &local; // expected-warning{{local variable 'local' is still referred to by the caller variable 'pp'}}
}
void param_ptr_to_ptr_to_ptr_caller(void** pp) {
@@ -334,7 +334,7 @@ void param_ptr_to_ptr_to_ptr_caller(void** pp) {
void lambda_to_context_ptr_to_ptr(int **pp) {
auto lambda = [&] {
int local = 42;
- *pp = &local; // expected-warning{{local variable 'local' is still referred to by the stack variable 'pp'}}
+ *pp = &local; // expected-warning{{local variable 'local' is still referred to by the caller variable 'pp'}}
};
lambda();
(void)*pp;
@@ -342,7 +342,7 @@ void lambda_to_context_ptr_to_ptr(int **pp) {
void param_ptr_to_ptr_fptr(int **pp) {
int local = 42;
- *pp = &local; // expected-warning{{local variable 'local' is still referred to by the stack variable 'p'}}
+ *pp = &local; // expected-warning{{local variable 'local' is still referred to by the caller variable 'p'}}
}
void param_ptr_to_ptr_fptr_caller(void (*fptr)(int**)) {
@@ -363,7 +363,7 @@ void*& global_rtp = *make_ptr_to_ptr();
void global_ref_to_ptr() {
int local = 42;
int* p = &local;
- global_rtp = p; // no-warning FIXME
+ global_rtp = p; // expected-warning{{local variable 'local' is still referred to by the global variable 'global_rtp'}}
}
void static_ref_to_ptr() {
@@ -376,13 +376,13 @@ void static_ref_to_ptr() {
void param_ref_to_ptr_top(void*& rp) {
int local = 42;
int* p = &local;
- rp = p; // no-warning FIXME
+ rp = p; // expected-warning{{local variable 'local' is still referred to by the caller variable 'rp'}}
}
void param_ref_to_ptr_callee(void*& rp) {
int local = 42;
int* p = &local;
- rp = p; // expected-warning{{local variable 'local' is still referred to by the stack variable 'p'}}
+ rp = p; // expected-warning{{local variable 'local' is still referred to by the caller variable 'p'}}
}
void param_ref_to_ptr_caller() {
@@ -418,26 +418,26 @@ void* global_aop[2];
void global_arr_of_ptr() {
int local = 42;
int* p = &local;
- global_aop[1] = p;
-} // expected-warning{{local variable 'local' is still referred to by the global variable 'global_aop'}}
+ global_aop[1] = p; // expected-warning{{local variable 'local' is still referred to by the global variable 'global_aop'}}
+}
void static_arr_of_ptr() {
int local = 42;
static void* arr[2];
arr[1] = &local;
- (void)arr[1];
-} // expected-warning{{local variable 'local' is still referred to by the static variable 'arr'}}
+ (void)arr[1]; // expected-warning{{local variable 'local' is still referred to by the static variable 'arr'}}
+}
void param_arr_of_ptr_top(void* arr[2]) {
int local = 42;
int* p = &local;
- arr[1] = p; // no-warning FIXME
+ arr[1] = p; // expected-warning{{local variable 'local' is still referred to by the caller variable 'arr'}}
}
void param_arr_of_ptr_callee(void* arr[2]) {
int local = 42;
int* p = &local;
- arr[1] = p; // expected-warning{{local variable 'local' is still referred to by the stack variable 'arrStack'}}
+ arr[1] = p; // expected-warning{{local variable 'local' is still referred to by the caller variable 'arrStack'}}
}
void param_arr_of_ptr_caller() {
@@ -474,26 +474,26 @@ void* global_aop[2];
void global_arr_of_ptr(int idx) {
int local = 42;
int* p = &local;
- global_aop[idx] = p;
-} // expected-warning{{local variable 'local' is still referred to by the global variable 'global_aop'}}
+ global_aop[idx] = p; // expected-warning{{local variable 'local' is still referred to by the global variable 'global_aop'}}
+}
void static_arr_of_ptr(int idx) {
int local = 42;
static void* arr[2];
arr[idx] = &local;
- (void)arr[idx];
-} // expected-warning{{local variable 'local' is still referred to by the static variable 'arr'}}
+ (void)arr[idx]; // expected-warning{{local variable 'local' is still referred to by the static variable 'arr'}}
+}
void param_arr_of_ptr_top(void* arr[2], int idx) {
int local = 42;
int* p = &local;
- arr[idx] = p; // no-warning FIXME
+ arr[idx] = p; // expected-warning{{local variable 'local' is still referred to by the caller variable 'arr'}}
}
void param_arr_of_ptr_callee(void* arr[2], int idx) {
int local = 42;
int* p = &local;
- arr[idx] = p; // expected-warning{{local variable 'local' is still referred to by the stack variable 'arrStack'}}
+ arr[idx] = p; // expected-warning{{local variable 'local' is still referred to by the caller variable 'arrStack'}}
}
void param_arr_of_ptr_caller(int idx) {
@@ -519,7 +519,7 @@ 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 stack variable 's'}}
+ return s; // expected-warning{{'local' is still referred to by the caller variable 's'}}
}
void returned_struct_with_ptr_caller() {
@@ -531,15 +531,15 @@ S global_s;
void global_struct_with_ptr() {
int local = 42;
- global_s.p = &local;
-} // expected-warning{{'local' is still referred to by the global variable 'global_s'}}
+ global_s.p = &local; // expected-warning{{'local' is still referred to by the global variable 'global_s'}}
+}
void static_struct_with_ptr() {
int local = 42;
static S s;
s.p = &local;
- (void)s.p;
-} // expected-warning{{'local' is still referred to by the static variable 's'}}
+ (void)s.p; // expected-warning{{'local' is still referred to by the static variable 's'}}
+}
} // namespace leaking_via_struct_with_ptr
namespace leaking_via_ref_to_struct_with_ptr {
@@ -551,7 +551,7 @@ S &global_s = *(new S);
void global_ref_to_struct_with_ptr() {
int local = 42;
- global_s.p = &local; // no-warning FIXME
+ global_s.p = &local; // expected-warning{{'local' is still referred to by the global variable 'global_s'}}
}
void static_ref_to_struct_with_ptr() {
@@ -563,12 +563,12 @@ void static_ref_to_struct_with_ptr() {
void param_ref_to_struct_with_ptr_top(S &s) {
int local = 42;
- s.p = &local; // no-warning FIXME
+ s.p = &local; // expected-warning{{'local' is still referred to by the caller variable 's'}}
}
void param_ref_to_struct_with_ptr_callee(S &s) {
int local = 42;
- s.p = &local; // expected-warning{{'local' is still referred to by the stack variable 'sStack'}}
+ s.p = &local; // expected-warning{{'local' is still referred to by the caller variable 'sStack'}}
}
void param_ref_to_struct_with_ptr_caller() {
@@ -579,7 +579,7 @@ void param_ref_to_struct_with_ptr_caller() {
template<typename Callable>
void lambda_param_capture_callee(Callable& callee) {
int local = 42;
- callee(local); // expected-warning{{'local' is still referred to by the stack variable 'p'}}
+ callee(local); // expected-warning{{'local' is still referred to by the caller variable 'p'}}
}
void lambda_param_capture_caller() {
@@ -619,7 +619,7 @@ S* global_s;
void global_ptr_to_struct_with_ptr() {
int local = 42;
- global_s->p = &local; // no-warning FIXME
+ global_s->p = &local; // expected-warning{{'local' is still referred to by the global variable 'global_s'}}
}
void static_ptr_to_struct_with_ptr_new() {
@@ -639,12 +639,12 @@ void static_ptr_to_struct_with_ptr_generated() {
void param_ptr_to_struct_with_ptr_top(S* s) {
int local = 42;
- s->p = &local; // no-warning FIXME
+ s->p = &local; // expected-warning{{'local' is still referred to by the caller variable 's'}}
}
void param_ptr_to_struct_with_ptr_callee(S* s) {
int local = 42;
- s->p = &local; // expected-warning{{'local' is still referred to by the stack variable 's'}}
+ s->p = &local; // expected-warning{{'local' is still referred to by the caller variable 's'}}
}
void param_ptr_to_struct_with_ptr_caller() {
@@ -682,8 +682,8 @@ S global_s[2];
void global_ptr_to_struct_with_ptr() {
int local = 42;
- global_s[1].p = &local;
-} // expected-warning{{'local' is still referred to by the global variable 'global_s'}}
+ global_s[1].p = &local; // expected-warning{{'local' is still referred to by the global variable 'global_s'}}
+}
void static_ptr_to_struct_with_ptr_new() {
int local = 42;
@@ -702,12 +702,12 @@ void static_ptr_to_struct_with_ptr_generated() {
void param_ptr_to_struct_with_ptr_top(S s[2]) {
int local = 42;
- s[1].p = &local; // no-warning FIXME
+ s[1].p = &local; // expected-warning{{'local' is still referred to by the caller variable 's'}}
}
void param_ptr_to_struct_with_ptr_callee(S s[2]) {
int local = 42;
- s[1].p = &local; // expected-warning{{'local' is still referred to by the stack variable 's'}}
+ s[1].p = &local; // expected-warning{{'local' is still referred to by the caller variable 's'}}
}
void param_ptr_to_struct_with_ptr_caller() {
@@ -727,17 +727,17 @@ NestedAndTransitive global_nat;
void global_nested_and_transitive() {
int local = 42;
- *global_nat.next[2]->next[1]->p = &local; // no-warning FIXME
+ *global_nat.next[2]->next[1]->p = &local; // expected-warning{{'local' is still referred to by the global variable 'global_nat'}}
}
void param_nested_and_transitive_top(NestedAndTransitive* nat) {
int local = 42;
- *nat->next[2]->next[1]->p = &local; // no-warning FIXME
+ *nat->next[2]->next[1]->p = &local; // expected-warning{{'local' is still referred to by the caller variable 'nat'}}
}
void param_nested_and_transitive_callee(NestedAndTransitive* nat) {
int local = 42;
- *nat->next[2]->next[1]->p = &local; // expected-warning{{local variable 'local' is still referred to by the stack variable 'natCaller'}}
+ *nat->next[2]->next[1]->p = &local; // expected-warning{{'local' is still referred to by the caller variable 'natCaller'}}
}
void param_nested_and_transitive_caller(NestedAndTransitive natCaller) {
@@ -769,7 +769,7 @@ void leaker(int ***leakerArg) {
// is no longer relevant.
// The message must refer to 'original_arg' instead, but there is no easy way to
// connect the SymRegion stored in 'original_arg' and 'original_arg' as variable.
- **leakerArg = &local; // expected-warning{{ 'local' is still referred to by the stack variable 'arg'}}
+ **leakerArg = &local; // expected-warning{{ 'local' is still referred to by the caller variable 'arg'}}
}
int **tweak();
@@ -780,3 +780,14 @@ void foo(int **arg) {
leaker(&original_arg);
}
} // namespace origin_region_limitation
+
+namespace leaking_via_indirect_global_invalidated {
+void** global_pp;
+void opaque();
+void global_ptr_to_ptr() {
+ int local = 42;
+ *global_pp = &local;
+ opaque();
+ *global_pp = nullptr;
+}
+} // namespace leaking_via_indirect_global_invalidated
diff --git a/clang/test/Analysis/stack-capture-leak-no-arc.mm b/clang/test/Analysis/stack-capture-leak-no-arc.mm
index 84bbc5fdc5253a..1831e239a42d25 100644
--- a/clang/test/Analysis/stack-capture-leak-no-arc.mm
+++ b/clang/test/Analysis/stack-capture-leak-no-arc.mm
@@ -40,7 +40,7 @@ dispatch_block_t test_block_inside_block_async_leak() {
// called.
void output_block(dispatch_block_t * blk) {
int x = 0;
- *blk = ^{ f(x); }; // expected-warning {{Address of stack-allocated block declared on line 43 is still referred to by the stack variable 'blk' upon returning to the caller. This will be a dangling reference [core.StackAddressEscape]}}
+ *blk = ^{ f(x); }; // expected-warning {{Address of stack-allocated block declared on line 43 is still referred to by the caller variable 'blk' upon returning to the caller. This will be a dangling reference [core.StackAddressEscape]}}
}
// The block literal captures nothing thus is treated as a constant.
@@ -54,7 +54,7 @@ void test_block_leak() {
__block dispatch_block_t blk;
int x = 0;
dispatch_block_t p = ^{
- blk = ^{ // expected-warning {{Address of stack-allocated block declared on line 57 is still referred to by the stack variable 'blk' upon returning to the caller. This will be a dangling reference [core.StackAddressEscape]}}
+ blk = ^{ // expected-warning {{Address of stack-allocated block declared on line 57 is still referred to by the caller variable 'blk' upon returning to the caller. This will be a dangling reference [core.StackAddressEscape]}}
f(x);
};
};
diff --git a/clang/test/Analysis/stackaddrleak.c b/clang/test/Analysis/stackaddrleak.c
index 39c29f2a2635b5..5f508275ba9c8d 100644
--- a/clang/test/Analysis/stackaddrleak.c
+++ b/clang/test/Analysis/stackaddrleak.c
@@ -7,7 +7,7 @@ char const *p;
void f0(void) {
char const str[] = "This will change";
p = str;
-} // expected-warning{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference}}
+} // expected-warning at -1{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference}}
void f1(void) {
char const str[] = "This will change";
@@ -17,7 +17,7 @@ void f1(void) {
void f2(void) {
p = (const char *) __builtin_alloca(12);
-} // expected-warning{{Address of stack memory allocated by call to alloca() on line 19 is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference}}
+} // expected-warning at -1{{Address of stack memory allocated by call to alloca() on line 19 is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference}}
// PR 7383 - previously the stack address checker would crash on this example
// because it would attempt to do a direct load from 'pr7383_list'.
@@ -33,7 +33,7 @@ void test_multi_return(void) {
int x;
a = &x;
b = &x;
-} // expected-warning{{Address of stack memory associated with local variable 'x' is still referred to by the static variable 'a' upon returning}} expected-warning{{Address of stack memory associated with local variable 'x' is still referred to by the static variable 'b' upon returning}}
+} // expected-warning at -1{{Address of stack memory associated with local variable 'x' is still referred to by the static variable 'a' upon returning}} expected-warning at -1{{Address of stack memory associated with local variable 'x' is still referred to by the static variable 'b' upon returning}}
intptr_t returnAsNonLoc(void) {
int x;
@@ -49,7 +49,7 @@ void assignAsNonLoc(void) {
extern intptr_t ip;
int x;
ip = (intptr_t)&x;
-} // expected-warning{{Address of stack memory associated with local variable 'x' is still referred to by the global variable 'ip' upon returning}}
+} // expected-warning at -1{{Address of stack memory associated with local variable 'x' is still referred to by the global variable 'ip' upon returning}}
void assignAsBool(void) {
extern bool b;
>From 9075f2e1687df62e59a6e838f7ea1c9b5a6db39f Mon Sep 17 00:00:00 2001
From: Arseniy Zaostrovnykh <necto.ne at gmail.com>
Date: Tue, 27 Aug 2024 09:13:28 +0200
Subject: [PATCH 2/4] Rename InvalidatedRegions to Excluded Regions to avoid
confusion
---
.../Checkers/StackAddrEscapeChecker.cpp | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 82f03e5f7d09d3..c7909a654eaee5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -415,26 +415,34 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
return false;
}
- void updateInvalidatedRegions(const MemRegion *Region) {
+ // Keep track of the variables that were invalidated through an opaque
+ // function call. Even if the initial values of such variables were bound to
+ // an address of a local variable, we cannot claim anything now, at the
+ // function exit, so skip them to avoid false positives.
+ void recordInvalidatedRegions(const MemRegion *Region) {
if (const auto *SymReg = Region->getAs<SymbolicRegion>()) {
SymbolRef Symbol = SymReg->getSymbol();
if (const auto *DerS = dyn_cast<SymbolDerived>(Symbol);
DerS && isa_and_nonnull<SymbolConjured>(DerS->getParentSymbol())) {
- InvalidatedRegions.insert(Symbol->getOriginRegion()->getBaseRegion());
+ ExcludedRegions.insert(Symbol->getOriginRegion()->getBaseRegion());
}
}
}
public:
SmallVector<std::pair<const MemRegion *, const MemRegion *>, 10> V;
- llvm::SmallPtrSet<const MemRegion *, 4> InvalidatedRegions;
+ // ExcludedRegions are skipped from reporting.
+ // I.e., if a referrer in this set, skip the related bug report.
+ // It is useful to avoid false positive for the variables that were
+ // reset to a conjured value after an opaque function call.
+ llvm::SmallPtrSet<const MemRegion *, 4> ExcludedRegions;
CallBack(CheckerContext &CC, bool TopFrame)
: Ctx(CC), PoppedFrame(CC.getStackFrame()), TopFrame(TopFrame) {}
bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Region,
SVal Val) override {
- updateInvalidatedRegions(Region);
+ recordInvalidatedRegions(Region);
const MemRegion *VR = Val.getAsRegion();
if (!VR)
return true;
@@ -473,7 +481,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
for (const auto &P : Cb.V) {
const MemRegion *Referrer = P.first->getBaseRegion();
const MemRegion *Referred = P.second;
- if (Cb.InvalidatedRegions.contains(getOriginBaseRegion(Referrer))) {
+ if (Cb.ExcludedRegions.contains(getOriginBaseRegion(Referrer))) {
continue;
}
>From 9e048c8d43fdeb225bd64ddbcacf726868347f3e Mon Sep 17 00:00:00 2001
From: Arseniy Zaostrovnykh <necto.ne at gmail.com>
Date: Tue, 27 Aug 2024 15:12:28 +0200
Subject: [PATCH 3/4] Use the same canonicalization logic for insert as in
lookup
in ExcludedRegions
---
.../Checkers/StackAddrEscapeChecker.cpp | 25 ++++++++++++-------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index c7909a654eaee5..de16f7b672ae30 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -350,6 +350,18 @@ std::optional<std::string> printReferrer(const MemRegion *Referrer) {
return buf;
}
+/// Check whether \p Region refers to a freshly minted symbol after an opaque
+/// function call.
+bool isInvalidatedSymbolRegion(const MemRegion *Region) {
+ const auto *SymReg = Region->getAs<SymbolicRegion>();
+ if (!SymReg)
+ return false;
+ SymbolRef Symbol = SymReg->getSymbol();
+
+ const auto *DerS = dyn_cast<SymbolDerived>(Symbol);
+ return DerS && isa_and_nonnull<SymbolConjured>(DerS->getParentSymbol());
+}
+
void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
CheckerContext &Ctx) const {
if (!ChecksEnabled[CK_StackAddrEscapeChecker])
@@ -419,14 +431,9 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
// function call. Even if the initial values of such variables were bound to
// an address of a local variable, we cannot claim anything now, at the
// function exit, so skip them to avoid false positives.
- void recordInvalidatedRegions(const MemRegion *Region) {
- if (const auto *SymReg = Region->getAs<SymbolicRegion>()) {
- SymbolRef Symbol = SymReg->getSymbol();
- if (const auto *DerS = dyn_cast<SymbolDerived>(Symbol);
- DerS && isa_and_nonnull<SymbolConjured>(DerS->getParentSymbol())) {
- ExcludedRegions.insert(Symbol->getOriginRegion()->getBaseRegion());
- }
- }
+ void recordInInvalidatedRegions(const MemRegion *Region) {
+ if (isInvalidatedSymbolRegion(Region))
+ ExcludedRegions.insert(getOriginBaseRegion(Region));
}
public:
@@ -442,7 +449,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Region,
SVal Val) override {
- recordInvalidatedRegions(Region);
+ recordInInvalidatedRegions(Region);
const MemRegion *VR = Val.getAsRegion();
if (!VR)
return true;
>From bc41e99d6d34f418de225e0bf6a438d8b7f6a30c Mon Sep 17 00:00:00 2001
From: Arseniy Zaostrovnykh <necto.ne at gmail.com>
Date: Tue, 27 Aug 2024 15:14:31 +0200
Subject: [PATCH 4/4] Rename Referrer to Reg
---
.../StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index de16f7b672ae30..ec867ab60fac15 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -305,12 +305,12 @@ static const MemSpaceRegion *getStackOrGlobalSpaceRegion(const MemRegion *R) {
return nullptr;
}
-const MemRegion *getOriginBaseRegion(const MemRegion *Referrer) {
- Referrer = Referrer->getBaseRegion();
- while (const auto *SymReg = dyn_cast<SymbolicRegion>(Referrer)) {
- Referrer = SymReg->getSymbol()->getOriginRegion()->getBaseRegion();
+const MemRegion *getOriginBaseRegion(const MemRegion *Reg) {
+ Reg = Reg->getBaseRegion();
+ while (const auto *SymReg = dyn_cast<SymbolicRegion>(Reg)) {
+ Reg = SymReg->getSymbol()->getOriginRegion()->getBaseRegion();
}
- return Referrer;
+ return Reg;
}
std::optional<std::string> printReferrer(const MemRegion *Referrer) {
More information about the cfe-commits
mailing list