https://github.com/necto updated 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] [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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits