[clang] [analyzer] Detect leak of a stack address through output arguments 2/3 (PR #105653)
Arseniy Zaostrovnykh via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 23 00:53:30 PDT 2024
https://github.com/necto updated https://github.com/llvm/llvm-project/pull/105653
>From db68dcfba96bfbf9367ba4159b6bf179c8c56f4f Mon Sep 17 00:00:00 2001
From: Arseniy Zaostrovnykh <necto.ne at gmail.com>
Date: Tue, 20 Aug 2024 10:26:38 +0200
Subject: [PATCH 1/4] [analyzer] [NFC] Add tests for and refactor
StackAddrEscapeChecker
These tests and refactoring are preparatory for the upcoming changes:
detection of the indirect leak via global variables and output parameters.
CPP-4734
---
.../Checkers/StackAddrEscapeChecker.cpp | 71 ++-
clang/test/Analysis/stack-addr-ps.c | 31 +
clang/test/Analysis/stack-addr-ps.cpp | 596 ++++++++++++++++++
3 files changed, 665 insertions(+), 33 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index ea09c43cc5ce90..2bd4ca4528de8b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -288,12 +288,37 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
EmitStackError(C, R, RetE);
}
+std::optional<std::string> printReferrer(const MemRegion *Referrer) {
+ assert(Referrer);
+ const StringRef ReferrerMemorySpace = [](const MemSpaceRegion *Space) {
+ if (isa<StaticGlobalSpaceRegion>(Space))
+ return "static";
+ if (isa<GlobalsSpaceRegion>(Space))
+ return "global";
+ assert(isa<StackSpaceRegion>(Space));
+ return "stack";
+ }(Referrer->getMemorySpace());
+
+ // We should really only have VarRegions here.
+ // Anything else is really surprising, and we should get notified if such
+ // ever happens.
+ const auto *ReferrerVar = dyn_cast<VarRegion>(Referrer);
+ if (!ReferrerVar) {
+ assert(false && "We should have a VarRegion here");
+ return std::nullopt; // Defensively skip this one.
+ }
+ const std::string ReferrerVarName =
+ ReferrerVar->getDecl()->getDeclName().getAsString();
+
+ return (ReferrerMemorySpace + " variable '" + ReferrerVarName + "'").str();
+}
+
void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
CheckerContext &Ctx) const {
if (!ChecksEnabled[CK_StackAddrEscapeChecker])
return;
- ProgramStateRef State = Ctx.getState();
+ ExplodedNode *Node = Ctx.getPredecessor();
// Iterate over all bindings to global variables and see if it contains
// a memory region in the stack space.
@@ -315,15 +340,10 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
if (!ReferrerMemSpace || !ReferredMemSpace)
return false;
- const auto *ReferrerFrame = ReferrerMemSpace->getStackFrame();
- const auto *ReferredFrame = ReferredMemSpace->getStackFrame();
-
- if (ReferrerMemSpace && ReferredMemSpace) {
- if (ReferredFrame == PoppedFrame &&
- ReferrerFrame->isParentOf(PoppedFrame)) {
- V.emplace_back(Referrer, Referred);
- return true;
- }
+ if (ReferredMemSpace->getStackFrame() == PoppedFrame &&
+ ReferrerMemSpace->getStackFrame()->isParentOf(PoppedFrame)) {
+ V.emplace_back(Referrer, Referred);
+ return true;
}
return false;
}
@@ -352,6 +372,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
};
CallBack Cb(Ctx);
+ ProgramStateRef State = Node->getState();
State->getStateManager().getStoreManager().iterBindings(State->getStore(),
Cb);
@@ -359,7 +380,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
return;
// Generate an error node.
- ExplodedNode *N = Ctx.generateNonFatalErrorNode(State);
+ ExplodedNode *N = Ctx.generateNonFatalErrorNode(State, Node);
if (!N)
return;
@@ -374,13 +395,13 @@ 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());
if (isa<CXXTempObjectRegion, CXXLifetimeExtendedObjectRegion>(Referrer)) {
- Out << " is still referred to by a temporary object on the stack "
+ Out << " is still referred to by a temporary object on the stack"
<< CommonSuffix;
auto Report =
std::make_unique<PathSensitiveBugReport>(*BT_stackleak, Out.str(), N);
@@ -390,28 +411,12 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
return;
}
- const StringRef ReferrerMemorySpace = [](const MemSpaceRegion *Space) {
- if (isa<StaticGlobalSpaceRegion>(Space))
- return "static";
- if (isa<GlobalsSpaceRegion>(Space))
- return "global";
- assert(isa<StackSpaceRegion>(Space));
- return "stack";
- }(Referrer->getMemorySpace());
-
- // We should really only have VarRegions here.
- // Anything else is really surprising, and we should get notified if such
- // ever happens.
- const auto *ReferrerVar = dyn_cast<VarRegion>(Referrer);
- if (!ReferrerVar) {
- assert(false && "We should have a VarRegion here");
- continue; // Defensively skip this one.
+ auto ReferrerVariable = printReferrer(Referrer);
+ if (!ReferrerVariable) {
+ continue;
}
- const std::string ReferrerVarName =
- ReferrerVar->getDecl()->getDeclName().getAsString();
- Out << " is still referred to by the " << ReferrerMemorySpace
- << " variable '" << ReferrerVarName << "' " << CommonSuffix;
+ Out << " is still referred to by the " << *ReferrerVariable << CommonSuffix;
auto Report =
std::make_unique<PathSensitiveBugReport>(*BT_stackleak, Out.str(), N);
if (Range.isValid())
diff --git a/clang/test/Analysis/stack-addr-ps.c b/clang/test/Analysis/stack-addr-ps.c
index e69ab4189b524f..2e14b7820be136 100644
--- a/clang/test/Analysis/stack-addr-ps.c
+++ b/clang/test/Analysis/stack-addr-ps.c
@@ -95,3 +95,34 @@ void callTestRegister(void) {
char buf[20];
testRegister(buf); // no-warning
}
+
+void top_level_leaking(int **out) {
+ int local = 42;
+ *out = &local; // no-warning FIXME
+}
+
+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'}}
+}
+
+void caller_for_leaking_callee() {
+ int *ptr = 0;
+ callee_leaking_via_param(&ptr);
+}
+
+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'}}
+}
+
+void caller_mid_for_nested_leaking(int **mid) {
+ callee_nested_leaking(mid);
+}
+
+void caller_for_nested_leaking() {
+ int *ptr = 0;
+ caller_mid_for_nested_leaking(&ptr);
+}
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index bd856be2b8d690..68ccc322bf2ef2 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -161,3 +161,599 @@ C make1() {
void test_copy_elision() {
C c1 = make1();
}
+
+namespace leaking_via_direct_pointer {
+void* returned_direct_pointer_top() {
+ int local = 42;
+ int* p = &local;
+ return p; // expected-warning{{associated with local variable 'local' returned}}
+}
+
+int* returned_direct_pointer_callee() {
+ int local = 42;
+ int* p = &local;
+ return p; // expected-warning{{associated with local variable 'local' returned}}
+}
+
+void returned_direct_pointer_caller() {
+ int* loc_ptr = nullptr;
+ loc_ptr = returned_direct_pointer_callee();
+ (void)loc_ptr;
+}
+
+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'}}
+
+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 static_direct_pointer_callee() {
+ int local = 42;
+ static int* p = &local;
+ (void)p; // expected-warning{{local variable 'local' is still referred to by the static variable 'p'}}
+}
+
+void static_direct_pointer_caller() {
+ static_direct_pointer_callee();
+}
+
+void lambda_to_global_direct_pointer() {
+ auto lambda = [&] {
+ int local = 42;
+ global_ptr = &local; // expected-warning{{local variable 'local' is still referred to by the global variable 'global_ptr'}}
+ };
+ lambda();
+}
+
+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'}}
+ };
+ lambda();
+ (void)p;
+}
+
+template<typename Callable>
+class MyFunction {
+ Callable* fptr;
+ public:
+ MyFunction(Callable* callable) :fptr(callable) {}
+};
+
+void* lambda_to_context_direct_pointer_uncalled() {
+ int *p = nullptr;
+ auto lambda = [&] {
+ int local = 42;
+ p = &local; // no-warning: analyzed only as top-level, ignored explicitly by the checker
+ };
+ return new MyFunction(&lambda);
+}
+
+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'}}
+ };
+ lambda();
+ (void)p;
+}
+
+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'}}
+}
+
+void lambda_param_capture_direct_pointer_caller() {
+ int* p = nullptr;
+ auto capt = [&p](int& param) {
+ p = ¶m;
+ };
+ lambda_param_capture_direct_pointer_callee(capt);
+}
+} // namespace leaking_via_direct_pointer
+
+namespace leaking_via_ptr_to_ptr {
+void** returned_ptr_to_ptr_top() {
+ int local = 42;
+ int* p = &local;
+ void** pp = (void**)&p;
+ return pp; // expected-warning{{associated with local variable 'p' returned}}
+}
+
+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'}}
+
+void global_ptr_to_ptr() {
+ int local = 42;
+ *global_pp = &local; // no-warning FIXME
+}
+
+void *** global_ppp;
+
+void global_ptr_to_ptr_to_ptr() {
+ int local = 42;
+ **global_ppp = &local; // no-warning FIXME
+}
+
+void** get_some_pp();
+
+void static_ptr_to_ptr() {
+ int local = 42;
+ static void** pp = get_some_pp();
+ *pp = &local;
+} // no-warning False Negative, requires relating multiple bindings to cross the invented pointer.
+
+void param_ptr_to_ptr_top(void** pp) {
+ int local = 42;
+ *pp = &local; // no-warning FIXME
+}
+
+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'}}
+}
+
+void param_ptr_to_ptr_caller() {
+ void* p = nullptr;
+ param_ptr_to_ptr_callee((void**)&p);
+}
+
+void param_ptr_to_ptr_to_ptr_top(void*** ppp) {
+ int local = 42;
+ **ppp = &local; // no-warning FIXME
+}
+
+void param_ptr_to_ptr_to_ptr_callee(void*** ppp) {
+ int local = 42;
+ **ppp = &local; // no-warning FIXME
+}
+
+void param_ptr_to_ptr_to_ptr_caller(void** pp) {
+ param_ptr_to_ptr_to_ptr_callee(&pp);
+}
+
+void lambda_to_context_ptr_to_ptr(int **pp) {
+ auto lambda = [&] {
+ int local = 42;
+ *pp = &local; // no-warning FIXME
+ };
+ lambda();
+ (void)*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'}}
+}
+
+void param_ptr_to_ptr_fptr_caller(void (*fptr)(int**)) {
+ int* p = nullptr;
+ fptr(&p);
+}
+
+void param_ptr_to_ptr_caller_caller() {
+ void (*fptr)(int**) = param_ptr_to_ptr_fptr;
+ param_ptr_to_ptr_fptr_caller(fptr);
+}
+} // namespace leaking_via_ptr_to_ptr
+
+namespace leaking_via_ref_to_ptr {
+void** make_ptr_to_ptr();
+void*& global_rtp = *make_ptr_to_ptr();
+
+void global_ref_to_ptr() {
+ int local = 42;
+ int* p = &local;
+ global_rtp = p; // no-warning FIXME
+}
+
+void static_ref_to_ptr() {
+ int local = 42;
+ static void*& p = *make_ptr_to_ptr();
+ p = &local;
+ (void)p;
+} // no-warning False Negative, requires relating multiple bindings to cross the invented pointer.
+
+void param_ref_to_ptr_top(void*& rp) {
+ int local = 42;
+ int* p = &local;
+ rp = p; // no-warning FIXME
+}
+
+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'}}
+}
+
+void param_ref_to_ptr_caller() {
+ void* p = nullptr;
+ param_ref_to_ptr_callee(p);
+}
+} // namespace leaking_via_ref_to_ptr
+
+namespace leaking_via_arr_of_ptr_static_idx {
+void** returned_arr_of_ptr_top() {
+ int local = 42;
+ int* p = &local;
+ void** arr = new void*[2];
+ arr[1] = p;
+ return arr;
+} // no-warning False Negative
+
+void** returned_arr_of_ptr_callee() {
+ int local = 42;
+ int* p = &local;
+ void** arr = new void*[2];
+ arr[1] = p;
+ return arr;
+} // no-warning False Negative
+
+void returned_arr_of_ptr_caller() {
+ void** arr = returned_arr_of_ptr_callee();
+ (void)arr[1];
+}
+
+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'}}
+
+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 param_arr_of_ptr_top(void* arr[2]) {
+ int local = 42;
+ int* p = &local;
+ arr[1] = p; // no-warning FIXME
+}
+
+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'}}
+}
+
+void param_arr_of_ptr_caller() {
+ void* arrStack[2];
+ param_arr_of_ptr_callee(arrStack);
+ (void)arrStack[1];
+}
+} // namespace leaking_via_arr_of_ptr_static_idx
+
+namespace leaking_via_arr_of_ptr_dynamic_idx {
+void** returned_arr_of_ptr_top(int idx) {
+ int local = 42;
+ int* p = &local;
+ void** arr = new void*[2];
+ arr[idx] = p;
+ return arr;
+} // no-warning False Negative
+
+void** returned_arr_of_ptr_callee(int idx) {
+ int local = 42;
+ int* p = &local;
+ void** arr = new void*[2];
+ arr[idx] = p;
+ return arr;
+} // no-warning False Negative
+
+void returned_arr_of_ptr_caller(int idx) {
+ void** arr = returned_arr_of_ptr_callee(idx);
+ (void)arr[idx];
+}
+
+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'}}
+
+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 param_arr_of_ptr_top(void* arr[2], int idx) {
+ int local = 42;
+ int* p = &local;
+ arr[idx] = p; // no-warning FIXME
+}
+
+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'}}
+}
+
+void param_arr_of_ptr_caller(int idx) {
+ void* arrStack[2];
+ param_arr_of_ptr_callee(arrStack, idx);
+ (void)arrStack[idx];
+}
+} // namespace leaking_via_arr_of_ptr_dynamic_idx
+
+namespace leaking_via_struct_with_ptr {
+struct S {
+ int* p;
+};
+
+S returned_struct_with_ptr_top() {
+ int local = 42;
+ S s;
+ s.p = &local;
+ return s;
+} // no-warning False Negative, requires traversing returned LazyCompoundVals
+
+S returned_struct_with_ptr_callee() {
+ int local = 42;
+ S s;
+ s.p = &local;
+ return s; // expected-warning{{'local' is still referred to by the stack variable 's'}}
+}
+
+void returned_struct_with_ptr_caller() {
+ S s = returned_struct_with_ptr_callee();
+ (void)s.p;
+}
+
+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'}}
+
+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'}}
+} // namespace leaking_via_struct_with_ptr
+
+namespace leaking_via_ref_to_struct_with_ptr {
+struct S {
+ int* p;
+};
+
+S &global_s = *(new S);
+
+void global_ref_to_struct_with_ptr() {
+ int local = 42;
+ global_s.p = &local; // no-warning FIXME
+}
+
+void static_ref_to_struct_with_ptr() {
+ int local = 42;
+ static S &s = *(new S);
+ s.p = &local;
+ (void)s.p;
+} // no-warning False Negative, requires relating multiple bindings to cross a heap region.
+
+void param_ref_to_struct_with_ptr_top(S &s) {
+ int local = 42;
+ s.p = &local; // no-warning FIXME
+}
+
+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'}}
+}
+
+void param_ref_to_struct_with_ptr_caller() {
+ S sStack;
+ param_ref_to_struct_with_ptr_callee(sStack);
+}
+
+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'}}
+}
+
+void lambda_param_capture_caller() {
+ int* p = nullptr;
+ auto capt = [&p](int& param) {
+ p = ¶m;
+ };
+ lambda_param_capture_callee(capt);
+}
+} // namespace leaking_via_ref_to_struct_with_ptr
+
+namespace leaking_via_ptr_to_struct_with_ptr {
+struct S {
+ int* p;
+};
+
+S* returned_ptr_to_struct_with_ptr_top() {
+ int local = 42;
+ S* s = new S;
+ s->p = &local;
+ return s;
+} // no-warning False Negative
+
+S* returned_ptr_to_struct_with_ptr_callee() {
+ int local = 42;
+ S* s = new S;
+ s->p = &local;
+ return s;
+} // no-warning False Negative
+
+void returned_ptr_to_struct_with_ptr_caller() {
+ S* s = returned_ptr_to_struct_with_ptr_callee();
+ (void)s->p;
+}
+
+S* global_s;
+
+void global_ptr_to_struct_with_ptr() {
+ int local = 42;
+ global_s->p = &local; // no-warning FIXME
+}
+
+void static_ptr_to_struct_with_ptr_new() {
+ int local = 42;
+ static S* s = new S;
+ s->p = &local;
+ (void)s->p;
+} // no-warning False Negative, requires relating multiple bindings to cross a heap region.
+
+S* get_some_s();
+
+void static_ptr_to_struct_with_ptr_generated() {
+ int local = 42;
+ static S* s = get_some_s();
+ s->p = &local;
+} // no-warning False Negative, requires relating multiple bindings to cross the invented pointer.
+
+void param_ptr_to_struct_with_ptr_top(S* s) {
+ int local = 42;
+ s->p = &local; // no-warning FIXME
+}
+
+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'}}
+}
+
+void param_ptr_to_struct_with_ptr_caller() {
+ S s;
+ param_ptr_to_struct_with_ptr_callee(&s);
+ (void)s.p;
+}
+} // namespace leaking_via_ptr_to_struct_with_ptr
+
+namespace leaking_via_arr_of_struct_with_ptr {
+struct S {
+ int* p;
+};
+
+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
+
+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
+
+void returned_ptr_to_struct_with_ptr_caller() {
+ S* s = returned_ptr_to_struct_with_ptr_callee();
+ (void)s[1].p;
+}
+
+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'}}
+
+void static_ptr_to_struct_with_ptr_new() {
+ int local = 42;
+ static S* s = new S[2];
+ s[1].p = &local;
+ (void)s[1].p;
+}
+
+S* get_some_s();
+
+void static_ptr_to_struct_with_ptr_generated() {
+ int local = 42;
+ static S* s = get_some_s();
+ s[1].p = &local;
+} // no-warning False Negative, requires relating multiple bindings to cross the invented pointer.
+
+void param_ptr_to_struct_with_ptr_top(S s[2]) {
+ int local = 42;
+ s[1].p = &local; // no-warning FIXME
+}
+
+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'}}
+}
+
+void param_ptr_to_struct_with_ptr_caller() {
+ S s[2];
+ param_ptr_to_struct_with_ptr_callee(s);
+ (void)s[1].p;
+}
+} // namespace leaking_via_arr_of_struct_with_ptr
+
+namespace leaking_via_nested_and_indirect {
+struct NestedAndTransitive {
+ int** p;
+ NestedAndTransitive* next[3];
+};
+
+NestedAndTransitive global_nat;
+
+void global_nested_and_transitive() {
+ int local = 42;
+ *global_nat.next[2]->next[1]->p = &local; // no-warning FIXME
+}
+
+void param_nested_and_transitive_top(NestedAndTransitive* nat) {
+ int local = 42;
+ *nat->next[2]->next[1]->p = &local; // no-warning FIXME
+}
+
+void param_nested_and_transitive_callee(NestedAndTransitive* nat) {
+ int local = 42;
+ *nat->next[2]->next[1]->p = &local; // no-warning FIXME
+}
+
+void param_nested_and_transitive_caller(NestedAndTransitive natCaller) {
+ param_nested_and_transitive_callee(&natCaller);
+}
+
+} // namespace leaking_via_nested_and_indirect
+
+namespace leaking_as_member {
+class CRef {
+ int& ref; // expected-note{{reference member declared here}}
+ CRef(int x) : ref(x) {}
+ // expected-warning at -1 {{binding reference member 'ref' to stack allocated parameter 'x'}}
+};
+
+class CPtr {
+ int* ptr;
+ void memFun(int x) {
+ ptr = &x;
+ }
+};
+} // namespace leaking_as_member
>From d6f5423ff8fb9d344d0b73451690ce0f43a6a8e6 Mon Sep 17 00:00:00 2001
From: Arseniy Zaostrovnykh <necto.ne at gmail.com>
Date: Tue, 20 Aug 2024 10:53:25 +0200
Subject: [PATCH 2/4] [analyzer] Detect leak of a stack address through output
arguments
At this point only functions called from other functions (i.e., not
top-level) are covered. Top-level functions have a different exit
sequence, and will be handled by a subsequent change.
CPP-4734
---
.../Checkers/StackAddrEscapeChecker.cpp | 62 ++++++++++++++-----
clang/test/Analysis/stack-addr-ps.cpp | 6 +-
2 files changed, 49 insertions(+), 19 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 2bd4ca4528de8b..a704c4ff2eeb02 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -288,6 +288,23 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
EmitStackError(C, R, RetE);
}
+static const MemSpaceRegion *getStackOrGlobalSpaceRegion(const MemRegion *R) {
+ assert(R);
+ if (const auto *MemSpace = R->getMemorySpace()) {
+ if (const auto *SSR = MemSpace->getAs<StackSpaceRegion>())
+ return SSR;
+ if (const auto *GSR = MemSpace->getAs<GlobalsSpaceRegion>())
+ return GSR;
+ }
+ // If R describes a lambda capture, it will be a symbolic region
+ // referring to a field region of another symbolic region.
+ if (const auto *SymReg = R->getBaseRegion()->getAs<SymbolicRegion>()) {
+ if (const auto *OriginReg = SymReg->getSymbol()->getOriginRegion())
+ return getStackOrGlobalSpaceRegion(OriginReg);
+ }
+ return nullptr;
+}
+
std::optional<std::string> printReferrer(const MemRegion *Referrer) {
assert(Referrer);
const StringRef ReferrerMemorySpace = [](const MemSpaceRegion *Space) {
@@ -297,20 +314,29 @@ std::optional<std::string> printReferrer(const MemRegion *Referrer) {
return "global";
assert(isa<StackSpaceRegion>(Space));
return "stack";
- }(Referrer->getMemorySpace());
-
- // We should really only have VarRegions here.
- // Anything else is really surprising, and we should get notified if such
- // ever happens.
- const auto *ReferrerVar = dyn_cast<VarRegion>(Referrer);
- if (!ReferrerVar) {
- assert(false && "We should have a VarRegion here");
- return std::nullopt; // Defensively skip this one.
+ }(getStackOrGlobalSpaceRegion(Referrer));
+
+ while (!Referrer->canPrintPretty()) {
+ if (const auto *SymReg = dyn_cast<SymbolicRegion>(Referrer)) {
+ Referrer = SymReg->getSymbol()->getOriginRegion()->getBaseRegion();
+ } else if (isa<CXXThisRegion>(Referrer)) {
+ // Skip members of a class, it is handled by
+ // warn_bind_ref_member_to_parameter_addr
+ return std::nullopt;
+ } else {
+ Referrer->dump();
+ assert(false && "Unexpected referrer region type.");
+ return std::nullopt;
+ }
}
- const std::string ReferrerVarName =
- ReferrerVar->getDecl()->getDeclName().getAsString();
+ assert(Referrer);
+ assert(Referrer->canPrintPretty());
- return (ReferrerMemorySpace + " variable '" + ReferrerVarName + "'").str();
+ std::string buf;
+ llvm::raw_string_ostream os(buf);
+ os << ReferrerMemorySpace << " variable ";
+ Referrer->printPretty(os);
+ return buf;
}
void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
@@ -332,16 +358,20 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
/// referred by an other stack variable from different stack frame.
bool checkForDanglingStackVariable(const MemRegion *Referrer,
const MemRegion *Referred) {
- const auto *ReferrerMemSpace =
- Referrer->getMemorySpace()->getAs<StackSpaceRegion>();
+ const auto *ReferrerMemSpace = getStackOrGlobalSpaceRegion(Referrer);
const auto *ReferredMemSpace =
Referred->getMemorySpace()->getAs<StackSpaceRegion>();
if (!ReferrerMemSpace || !ReferredMemSpace)
return false;
+ const auto *ReferrerStackSpace =
+ ReferrerMemSpace->getAs<StackSpaceRegion>();
+ if (!ReferrerStackSpace)
+ return false;
+
if (ReferredMemSpace->getStackFrame() == PoppedFrame &&
- ReferrerMemSpace->getStackFrame()->isParentOf(PoppedFrame)) {
+ ReferrerStackSpace->getStackFrame()->isParentOf(PoppedFrame)) {
V.emplace_back(Referrer, Referred);
return true;
}
@@ -387,7 +417,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
if (!BT_stackleak)
BT_stackleak =
std::make_unique<BugType>(CheckNames[CK_StackAddrEscapeChecker],
- "Stack address stored into global variable");
+ "Stack address leaks outside of stack frame");
for (const auto &P : Cb.V) {
const MemRegion *Referrer = P.first->getBaseRegion();
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index 68ccc322bf2ef2..b8bd70ff5d98f5 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -321,7 +321,7 @@ void param_ptr_to_ptr_to_ptr_top(void*** ppp) {
void param_ptr_to_ptr_to_ptr_callee(void*** ppp) {
int local = 42;
- **ppp = &local; // no-warning FIXME
+ **ppp = &local; // expected-warning{{local variable 'local' is still referred to by the stack variable 'pp'}}
}
void param_ptr_to_ptr_to_ptr_caller(void** pp) {
@@ -331,7 +331,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; // no-warning FIXME
+ *pp = &local; // expected-warning{{local variable 'local' is still referred to by the stack variable 'pp'}}
};
lambda();
(void)*pp;
@@ -734,7 +734,7 @@ void param_nested_and_transitive_top(NestedAndTransitive* nat) {
void param_nested_and_transitive_callee(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 variable 'local' is still referred to by the stack variable 'natCaller'}}
}
void param_nested_and_transitive_caller(NestedAndTransitive natCaller) {
>From f223714bc7d42863b6f15d96daa72f4dedeac600 Mon Sep 17 00:00:00 2001
From: Arseniy Zaostrovnykh <necto.ne at gmail.com>
Date: Fri, 23 Aug 2024 09:43:56 +0200
Subject: [PATCH 3/4] Record limitation of the message
---
clang/test/Analysis/stack-addr-ps.cpp | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index b8bd70ff5d98f5..b25e433b6e2986 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -1,7 +1,10 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s -Wno-undefined-bool-conversion
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s -Wno-undefined-bool-conversion
typedef __INTPTR_TYPE__ intptr_t;
+template <typename T>
+void clang_analyzer_dump(T x);
+
const int& g() {
int s;
return s; // expected-warning{{Address of stack memory associated with local variable 's' returned}} expected-warning{{reference to stack memory associated with local variable 's' returned}}
@@ -757,3 +760,23 @@ class CPtr {
}
};
} // namespace leaking_as_member
+
+namespace origin_region_limitation {
+void leaker(int ***arg) {
+ int local;
+ clang_analyzer_dump(*arg); // expected-warning{{&SymRegion{reg_$0<int ** arg>}}}
+ // Incorrect message: 'arg', after it is reinitialized with value returned by 'tweak'
+ // 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.
+ **arg = &local; // expected-warning{{ 'local' is still referred to by the stack variable 'arg'}}
+}
+
+int **tweak();
+
+void foo(int **arg) {
+ int **original_arg = arg;
+ arg = tweak();
+ leaker(&original_arg);
+}
+} // namespace origin_region_limitation
>From 4afbb63cdf791e027a0890702671d4613e3dc4f9 Mon Sep 17 00:00:00 2001
From: Arseniy Zaostrovnykh <necto.ne at gmail.com>
Date: Fri, 23 Aug 2024 09:53:09 +0200
Subject: [PATCH 4/4] Defend agains null dereference on getOriginRegion
---
clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index a704c4ff2eeb02..f2dd02e1555595 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -317,7 +317,8 @@ std::optional<std::string> printReferrer(const MemRegion *Referrer) {
}(getStackOrGlobalSpaceRegion(Referrer));
while (!Referrer->canPrintPretty()) {
- if (const auto *SymReg = dyn_cast<SymbolicRegion>(Referrer)) {
+ if (const auto *SymReg = dyn_cast<SymbolicRegion>(Referrer);
+ SymReg && SymReg->getSymbol()->getOriginRegion()) {
Referrer = SymReg->getSymbol()->getOriginRegion()->getBaseRegion();
} else if (isa<CXXThisRegion>(Referrer)) {
// Skip members of a class, it is handled by
More information about the cfe-commits
mailing list