llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Arseniy Zaostrovnykh (necto) <details> <summary>Changes</summary> 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 --- Patch is 23.52 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/105653.diff 3 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp (+70-35) - (modified) clang/test/Analysis/stack-addr-ps.c (+31) - (modified) clang/test/Analysis/stack-addr-ps.cpp (+596) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp index ea09c43cc5ce90..a704c4ff2eeb02 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp @@ -288,12 +288,63 @@ 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) { + if (isa<StaticGlobalSpaceRegion>(Space)) + return "static"; + if (isa<GlobalsSpaceRegion>(Space)) + return "global"; + assert(isa<StackSpaceRegion>(Space)); + return "stack"; + }(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; + } + } + assert(Referrer); + assert(Referrer->canPrintPretty()); + + std::string buf; + llvm::raw_string_ostream os(buf); + os << ReferrerMemorySpace << " variable "; + Referrer->printPretty(os); + return buf; +} + 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. @@ -307,23 +358,22 @@ 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 *ReferrerFrame = ReferrerMemSpace->getStackFrame(); - const auto *ReferredFrame = ReferredMemSpace->getStackFrame(); + const auto *ReferrerStackSpace = + ReferrerMemSpace->getAs<StackSpaceRegion>(); + if (!ReferrerStackSpace) + return false; - if (ReferrerMemSpace && ReferredMemSpace) { - if (ReferredFrame == PoppedFrame && - ReferrerFrame->isParentOf(PoppedFrame)) { - V.emplace_back(Referrer, Referred); - return true; - } + if (ReferredMemSpace->getStackFrame() == PoppedFrame && + ReferrerStackSpace->getStackFrame()->isParentOf(PoppedFrame)) { + V.emplace_back(Referrer, Referred); + return true; } return false; } @@ -352,6 +402,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS, }; CallBack Cb(Ctx); + ProgramStateRef State = Node->getState(); State->getStateManager().getStoreManager().iterBindings(State->getStore(), Cb); @@ -359,14 +410,14 @@ 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; 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(); @@ -374,13 +425,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 +441,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..b8bd70ff5d98f5 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; // expected-warning{{local variable 'local' is still referred to by the stack variable 'pp'}} +} + +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; // expected-warning{{local variable 'local' is still referred to by the stack variable 'pp'}} + }; + 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_struc... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/105653 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
