[clang] 95b37a7 - [analyzer] Detect leak of a stack address through output arguments 2/3 (#105653)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 26 05:38:58 PDT 2024
Author: Arseniy Zaostrovnykh
Date: 2024-08-26T14:38:54+02:00
New Revision: 95b37a76493a1cd4b607f53f4318b5da5b5392f0
URL: https://github.com/llvm/llvm-project/commit/95b37a76493a1cd4b607f53f4318b5da5b5392f0
DIFF: https://github.com/llvm/llvm-project/commit/95b37a76493a1cd4b607f53f4318b5da5b5392f0.diff
LOG: [analyzer] Detect leak of a stack address through output arguments 2/3 (#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
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
clang/test/Analysis/stack-addr-ps.cpp
Removed:
################################################################################
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
diff erent 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
More information about the cfe-commits
mailing list