https://github.com/necto created https://github.com/llvm/llvm-project/pull/105653
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 ------- This is the second of three commits constituting https://github.com/llvm/llvm-project/pull/105648 it must not be merged before https://github.com/llvm/llvm-project/pull/105652 >From db68dcfba96bfbf9367ba4159b6bf179c8c56f4f Mon Sep 17 00:00:00 2001 From: Arseniy Zaostrovnykh <necto...@gmail.com> Date: Tue, 20 Aug 2024 10:26:38 +0200 Subject: [PATCH 1/2] [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@-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@-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@-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...@gmail.com> Date: Tue, 20 Aug 2024 10:53:25 +0200 Subject: [PATCH 2/2] [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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
