https://github.com/necto updated
https://github.com/llvm/llvm-project/pull/105653
>From 902e1d63b436db3ca9e21b022e821a0182bf992c Mon Sep 17 00:00:00 2001
From: Arseniy Zaostrovnykh <necto...@gmail.com>
Date: Tue, 20 Aug 2024 10:53:25 +0200
Subject: [PATCH] [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 | 64 ++++++++++++++-----
clang/test/Analysis/stack-addr-ps.cpp | 31 +++++++--
2 files changed, 75 insertions(+), 20 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 2bd4ca4528de8b..dcf6801a73de2d 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,31 @@ 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);
+ SymReg && SymReg->getSymbol()->getOriginRegion()) {
+ Referrer = SymReg->getSymbol()->getOriginRegion()->getBaseRegion();
+ } else if (isa<CXXThisRegion>(Referrer)) {
+ // Skip members of a class, it is handled in CheckExprLifetime.cpp as
+ // warn_bind_ref_member_to_parameter or
+ // warn_init_ptr_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 +360,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 +419,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..95a6e3cbd25c7c 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}}
@@ -321,7 +324,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 +334,7 @@ void param_ptr_to_ptr_to_ptr_caller(void** pp) {
void lambda_to_context_ptr_to_ptr(int **pp) {
auto lambda = [&] {
int local = 42;
- *pp = &local; // no-warning FIXME
+ *pp = &local; // expected-warning{{local variable 'local' is still
referred to by the stack variable 'pp'}}
};
lambda();
(void)*pp;
@@ -734,7 +737,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) {
@@ -757,3 +760,23 @@ class CPtr {
}
};
} // namespace leaking_as_member
+
+namespace origin_region_limitation {
+void leaker(int ***leakerArg) {
+ int local;
+ clang_analyzer_dump(*leakerArg); //
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.
+ **leakerArg = &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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
