[clang] [analyzer] Reapply recent stack addr escape checker changes + buildbot fix (PR #126620)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 10 15:03:44 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Michael Flanders (Flandini)
<details>
<summary>Changes</summary>
Reapplying changes from https://github.com/llvm/llvm-project/pull/125638 after buildbot failures.
Buildbot failures fixed in 029e7e98dc9956086adc6c1dfb0c655a273fbee6, latest commit on this PR. It was a problem with a declared class member with same name as its type. Sorry!
---
Patch is 27.61 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/126620.diff
5 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp (+129-53)
- (modified) clang/test/Analysis/copy-elision.cpp (+18-7)
- (modified) clang/test/Analysis/stack-addr-ps.cpp (+225-20)
- (modified) clang/test/Analysis/stackaddrleak.c (+12)
- (modified) clang/test/Analysis/stackaddrleak.cpp (+1-1)
``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index f4de3b500499c48..c9df15ceb3b4092 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -54,8 +54,8 @@ class StackAddrEscapeChecker
CheckerContext &C) const;
void checkAsyncExecutedBlockCaptures(const BlockDataRegion &B,
CheckerContext &C) const;
- void EmitStackError(CheckerContext &C, const MemRegion *R,
- const Expr *RetE) const;
+ void EmitReturnLeakError(CheckerContext &C, const MemRegion *LeakedRegion,
+ const Expr *RetE) const;
bool isSemaphoreCaptured(const BlockDecl &B) const;
static SourceRange genName(raw_ostream &os, const MemRegion *R,
ASTContext &Ctx);
@@ -147,9 +147,22 @@ StackAddrEscapeChecker::getCapturedStackRegions(const BlockDataRegion &B,
return Regions;
}
-void StackAddrEscapeChecker::EmitStackError(CheckerContext &C,
- const MemRegion *R,
- const Expr *RetE) const {
+static void EmitReturnedAsPartOfError(llvm::raw_ostream &OS, SVal ReturnedVal,
+ const MemRegion *LeakedRegion) {
+ if (const MemRegion *ReturnedRegion = ReturnedVal.getAsRegion()) {
+ if (isa<BlockDataRegion>(ReturnedRegion)) {
+ OS << " is captured by a returned block";
+ return;
+ }
+ }
+
+ // Generic message
+ OS << " returned to caller";
+}
+
+void StackAddrEscapeChecker::EmitReturnLeakError(CheckerContext &C,
+ const MemRegion *R,
+ const Expr *RetE) const {
ExplodedNode *N = C.generateNonFatalErrorNode();
if (!N)
return;
@@ -157,11 +170,15 @@ void StackAddrEscapeChecker::EmitStackError(CheckerContext &C,
BT_returnstack = std::make_unique<BugType>(
CheckNames[CK_StackAddrEscapeChecker],
"Return of address to stack-allocated memory");
+
// Generate a report for this bug.
SmallString<128> buf;
llvm::raw_svector_ostream os(buf);
+
+ // Error message formatting
SourceRange range = genName(os, R, C.getASTContext());
- os << " returned to caller";
+ EmitReturnedAsPartOfError(os, C.getSVal(RetE), R);
+
auto report =
std::make_unique<PathSensitiveBugReport>(*BT_returnstack, os.str(), N);
report->addRange(RetE->getSourceRange());
@@ -209,30 +226,6 @@ void StackAddrEscapeChecker::checkAsyncExecutedBlockCaptures(
}
}
-void StackAddrEscapeChecker::checkReturnedBlockCaptures(
- const BlockDataRegion &B, CheckerContext &C) const {
- for (const MemRegion *Region : getCapturedStackRegions(B, C)) {
- if (isNotInCurrentFrame(Region, C))
- continue;
- ExplodedNode *N = C.generateNonFatalErrorNode();
- if (!N)
- continue;
- if (!BT_capturedstackret)
- BT_capturedstackret = std::make_unique<BugType>(
- CheckNames[CK_StackAddrEscapeChecker],
- "Address of stack-allocated memory is captured");
- SmallString<128> Buf;
- llvm::raw_svector_ostream Out(Buf);
- SourceRange Range = genName(Out, Region, C.getASTContext());
- Out << " is captured by a returned block";
- auto Report = std::make_unique<PathSensitiveBugReport>(*BT_capturedstackret,
- Out.str(), N);
- if (Range.isValid())
- Report->addRange(Range);
- C.emitReport(std::move(Report));
- }
-}
-
void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
if (!ChecksEnabled[CK_StackAddrAsyncEscapeChecker])
@@ -247,45 +240,128 @@ void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call,
}
}
-void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
- CheckerContext &C) const {
- if (!ChecksEnabled[CK_StackAddrEscapeChecker])
- return;
+/// A visitor made for use with a ScanReachableSymbols scanner, used
+/// for finding stack regions within an SVal that live on the current
+/// stack frame of the given checker context. This visitor excludes
+/// NonParamVarRegion that data is bound to in a BlockDataRegion's
+/// bindings, since these are likely uninteresting, e.g., in case a
+/// temporary is constructed on the stack, but it captures values
+/// that would leak.
+class FindStackRegionsSymbolVisitor final : public SymbolVisitor {
+ CheckerContext &Ctxt;
+ const StackFrameContext *PoppedStackFrame;
+ SmallVectorImpl<const MemRegion *> &EscapingStackRegions;
- const Expr *RetE = RS->getRetValue();
- if (!RetE)
- return;
- RetE = RetE->IgnoreParens();
+public:
+ explicit FindStackRegionsSymbolVisitor(
+ CheckerContext &Ctxt,
+ SmallVectorImpl<const MemRegion *> &StorageForStackRegions)
+ : Ctxt(Ctxt), PoppedStackFrame(Ctxt.getStackFrame()),
+ EscapingStackRegions(StorageForStackRegions) {}
- SVal V = C.getSVal(RetE);
- const MemRegion *R = V.getAsRegion();
- if (!R)
- return;
+ bool VisitSymbol(SymbolRef sym) override { return true; }
- if (const BlockDataRegion *B = dyn_cast<BlockDataRegion>(R))
- checkReturnedBlockCaptures(*B, C);
+ bool VisitMemRegion(const MemRegion *MR) override {
+ SaveIfEscapes(MR);
- if (!isa<StackSpaceRegion>(R->getMemorySpace()) || isNotInCurrentFrame(R, C))
- return;
+ if (const BlockDataRegion *BDR = MR->getAs<BlockDataRegion>())
+ return VisitBlockDataRegionCaptures(BDR);
+
+ return true;
+ }
+
+private:
+ void SaveIfEscapes(const MemRegion *MR) {
+ const StackSpaceRegion *SSR =
+ MR->getMemorySpace()->getAs<StackSpaceRegion>();
+ if (SSR && SSR->getStackFrame() == PoppedStackFrame)
+ EscapingStackRegions.push_back(MR);
+ }
+
+ bool VisitBlockDataRegionCaptures(const BlockDataRegion *BDR) {
+ for (auto Var : BDR->referenced_vars()) {
+ SVal Val = Ctxt.getState()->getSVal(Var.getCapturedRegion());
+ const MemRegion *Region = Val.getAsRegion();
+ if (Region) {
+ SaveIfEscapes(Region);
+ VisitMemRegion(Region);
+ }
+ }
+
+ return false;
+ }
+};
+
+/// Given some memory regions that are flagged by FindStackRegionsSymbolVisitor,
+/// this function filters out memory regions that are being returned that are
+/// likely not true leaks:
+/// 1. If returning a block data region that has stack memory space
+/// 2. If returning a constructed object that has stack memory space
+static SmallVector<const MemRegion *> FilterReturnExpressionLeaks(
+ const SmallVectorImpl<const MemRegion *> &MaybeEscaped, CheckerContext &C,
+ const Expr *RetE, SVal &RetVal) {
+
+ SmallVector<const MemRegion *> WillEscape;
+
+ const MemRegion *RetRegion = RetVal.getAsRegion();
// Returning a record by value is fine. (In this case, the returned
// expression will be a copy-constructor, possibly wrapped in an
// ExprWithCleanups node.)
if (const ExprWithCleanups *Cleanup = dyn_cast<ExprWithCleanups>(RetE))
RetE = Cleanup->getSubExpr();
- if (isa<CXXConstructExpr>(RetE) && RetE->getType()->isRecordType())
- return;
+ bool IsConstructExpr =
+ isa<CXXConstructExpr>(RetE) && RetE->getType()->isRecordType();
// The CK_CopyAndAutoreleaseBlockObject cast causes the block to be copied
// so the stack address is not escaping here.
+ bool IsCopyAndAutoreleaseBlockObj = false;
if (const auto *ICE = dyn_cast<ImplicitCastExpr>(RetE)) {
- if (isa<BlockDataRegion>(R) &&
- ICE->getCastKind() == CK_CopyAndAutoreleaseBlockObject) {
- return;
- }
+ IsCopyAndAutoreleaseBlockObj =
+ isa_and_nonnull<BlockDataRegion>(RetRegion) &&
+ ICE->getCastKind() == CK_CopyAndAutoreleaseBlockObject;
+ }
+
+ for (const MemRegion *MR : MaybeEscaped) {
+ if (RetRegion == MR && (IsCopyAndAutoreleaseBlockObj || IsConstructExpr))
+ continue;
+
+ WillEscape.push_back(MR);
}
- EmitStackError(C, R, RetE);
+ return WillEscape;
+}
+
+/// For use in finding regions that live on the checker context's current
+/// stack frame, deep in the SVal representing the return value.
+static SmallVector<const MemRegion *>
+FindEscapingStackRegions(CheckerContext &C, const Expr *RetE, SVal RetVal) {
+ SmallVector<const MemRegion *> FoundStackRegions;
+
+ FindStackRegionsSymbolVisitor Finder(C, FoundStackRegions);
+ ScanReachableSymbols Scanner(C.getState(), Finder);
+ Scanner.scan(RetVal);
+
+ return FilterReturnExpressionLeaks(FoundStackRegions, C, RetE, RetVal);
+}
+
+void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
+ CheckerContext &C) const {
+ if (!ChecksEnabled[CK_StackAddrEscapeChecker])
+ return;
+
+ const Expr *RetE = RS->getRetValue();
+ if (!RetE)
+ return;
+ RetE = RetE->IgnoreParens();
+
+ SVal V = C.getSVal(RetE);
+
+ SmallVector<const MemRegion *> EscapedStackRegions =
+ FindEscapingStackRegions(C, RetE, V);
+
+ for (const MemRegion *ER : EscapedStackRegions)
+ EmitReturnLeakError(C, ER, RetE);
}
static const MemSpaceRegion *getStackOrGlobalSpaceRegion(const MemRegion *R) {
diff --git a/clang/test/Analysis/copy-elision.cpp b/clang/test/Analysis/copy-elision.cpp
index cd941854fc7f196..e69f52f351f1bc0 100644
--- a/clang/test/Analysis/copy-elision.cpp
+++ b/clang/test/Analysis/copy-elision.cpp
@@ -154,20 +154,26 @@ class ClassWithoutDestructor {
void push() { v.push(this); }
};
+// Two warnings on no-elide: arg v holds the address of the temporary, and we
+// are returning an object which holds v which holds the address of the temporary
ClassWithoutDestructor make1(AddressVector<ClassWithoutDestructor> &v) {
- return ClassWithoutDestructor(v);
+ return ClassWithoutDestructor(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithoutDestructor' returned to caller}}
// no-elide-warning at -1 {{Address of stack memory associated with temporary \
object of type 'ClassWithoutDestructor' is still \
referred to by the caller variable 'v' upon returning to the caller}}
}
+// Two warnings on no-elide: arg v holds the address of the temporary, and we
+// are returning an object which holds v which holds the address of the temporary
ClassWithoutDestructor make2(AddressVector<ClassWithoutDestructor> &v) {
- return make1(v);
+ return make1(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithoutDestructor' returned to caller}}
// no-elide-warning at -1 {{Address of stack memory associated with temporary \
object of type 'ClassWithoutDestructor' is still \
referred to by the caller variable 'v' upon returning to the caller}}
}
+// Two warnings on no-elide: arg v holds the address of the temporary, and we
+// are returning an object which holds v which holds the address of the temporary
ClassWithoutDestructor make3(AddressVector<ClassWithoutDestructor> &v) {
- return make2(v);
+ return make2(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithoutDestructor' returned to caller}}
// no-elide-warning at -1 {{Address of stack memory associated with temporary \
object of type 'ClassWithoutDestructor' is still \
referred to by the caller variable 'v' upon returning to the caller}}
@@ -298,21 +304,26 @@ to by the caller variable 'v' upon returning to the caller}}
#endif
}
-
+// Two warnings on no-elide: arg v holds the address of the temporary, and we
+// are returning an object which holds v which holds the address of the temporary
ClassWithDestructor make1(AddressVector<ClassWithDestructor> &v) {
- return ClassWithDestructor(v);
+ return ClassWithDestructor(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithDestructor' returned to caller}}
// no-elide-warning at -1 {{Address of stack memory associated with temporary \
object of type 'ClassWithDestructor' is still referred \
to by the caller variable 'v' upon returning to the caller}}
}
+// Two warnings on no-elide: arg v holds the address of the temporary, and we
+// are returning an object which holds v which holds the address of the temporary
ClassWithDestructor make2(AddressVector<ClassWithDestructor> &v) {
- return make1(v);
+ return make1(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithDestructor' returned to caller}}
// no-elide-warning at -1 {{Address of stack memory associated with temporary \
object of type 'ClassWithDestructor' is still referred \
to by the caller variable 'v' upon returning to the caller}}
}
+// Two warnings on no-elide: arg v holds the address of the temporary, and we
+// are returning an object which holds v which holds the address of the temporary
ClassWithDestructor make3(AddressVector<ClassWithDestructor> &v) {
- return make2(v);
+ return make2(v); // no-elide-warning{{Address of stack memory associated with temporary object of type 'ClassWithDestructor' returned to caller}}
// no-elide-warning at -1 {{Address of stack memory associated with temporary \
object of type 'ClassWithDestructor' is still referred \
to by the caller variable 'v' upon returning to the caller}}
diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index 73e9dbeca460f60..bf988d0a1695947 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -90,6 +90,18 @@ int *mf() {
return &x; // expected-warning{{Address of stack memory associated with local variable 's1' returned}} expected-warning {{address of stack memory associated with local variable 's1' returned}}
}
+int *return_assign_expr_leak() {
+ int x = 1;
+ int *y;
+ return y = &x; // expected-warning{{Address of stack memory associated with local variable 'x' returned}}
+}
+
+// Additional diagnostic from -Wreturn-stack-address in the simple case?
+int *return_comma_separated_expressions_leak() {
+ int x = 1;
+ return (x=14), &x; // expected-warning{{Address of stack memory associated with local variable 'x' returned to caller}} expected-warning{{address of stack memory associated with local variable 'x' returned}}
+}
+
void *lf() {
label:
void *const &x = &&label; // expected-note {{binding reference variable 'x' here}}
@@ -152,6 +164,18 @@ namespace rdar13296133 {
}
} // namespace rdar13296133
+void* ret_cpp_static_cast(short x) {
+ return static_cast<void*>(&x); // expected-warning {{Address of stack memory associated with local variable 'x' returned to caller}} expected-warning {{address of stack memory}}
+}
+
+int* ret_cpp_reinterpret_cast(double x) {
+ return reinterpret_cast<int*>(&x); // expected-warning {{Address of stack memory associated with local variable 'x' returned to caller}} expected-warning {{address of stack me}}
+}
+
+int* ret_cpp_const_cast(const int x) {
+ return const_cast<int*>(&x); // expected-warning {{Address of stack memory associated with local variable 'x' returned to caller}} expected-warning {{address of stack memory}}
+}
+
void write_stack_address_to(char **q) {
char local;
*q = &local;
@@ -178,6 +202,11 @@ void test_copy_elision() {
C c1 = make1();
}
+C&& return_bind_rvalue_reference_to_temporary() {
+ return C(); // expected-warning{{Address of stack memory associated with temporary object of type 'C' returned to caller}}
+ // expected-warning at -1{{returning reference to local temporary object}} -Wreturn-stack-address
+}
+
namespace leaking_via_direct_pointer {
void* returned_direct_pointer_top() {
int local = 42;
@@ -251,7 +280,7 @@ void* lambda_to_context_direct_pointer_uncalled() {
int local = 42;
p = &local; // no-warning: analyzed only as top-level, ignored explicitly by the checker
};
- return new MyFunction(&lambda);
+ return new MyFunction(&lambda); // expected-warning{{Address of stack memory associated with local variable 'lambda' returned to caller}}
}
void lambda_to_context_direct_pointer_lifetime_extended() {
@@ -340,6 +369,13 @@ void param_ptr_to_ptr_to_ptr_callee(void*** ppp) {
**ppp = &local; // expected-warning{{local variable 'local' is still referred to by the caller variable 'pp'}}
}
+void ***param_ptr_to_ptr_to_ptr_return(void ***ppp) {
+ int local = 0;
+ **ppp = &local;
+ return ppp;
+ // expected-warning at -1 {{Address of stack memory associated with local variable 'local' is still referred to by the caller variable 'ppp' upon returning to the caller. This will be a dangling reference}}
+}
+
void param_ptr_to_ptr_to_ptr_caller(void** pp) {
param_ptr_to_ptr_to_ptr_callee(&pp);
}
@@ -410,16 +446,16 @@ void** returned_arr_of_ptr_top() {
int* p = &local;
void** arr = new void*[2];
arr[1] = p;
- return arr;
-} // no-warning False Negative
+ return arr; // expected-warning{{Address of stack memory associated with local variable 'local' returned to caller}}
+}
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
+ return arr; // expected-warning{{Address of stack memory associated with local variable 'local' returned to caller}}
+}
void returned_arr_of_ptr_caller() {
void** arr = returned_arr_of_ptr_callee();
@@ -466,16 +502,16 @@ void** returned_arr_of_ptr_top(int idx) {
int* p = &local;
void** arr = new void*[2];
arr[idx] = p;
- return arr;
-} // no-warning False Negative
+ return arr; // expected-warning{{Address of stack memory associated with local variable 'local' returned to caller}}
+}
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
+ return arr; // expected-warning{{Address of stack memory associated with local variable 'local' returned to caller}}
+}
void returned_arr_of_ptr_caller(int idx) {
void** arr = returned_arr_of_ptr_callee(idx);
@@ -525,14 +561,25 @@ S returned_struct_with_ptr_top() {
int local = 42;
S s;
s.p = &local;
- return s;
-} // no-warning False Negative, requires traversing returned LazyCompoundVals
+ return s; // expected-warning{{Address of stack memory associated with local variable 'local' returned to caller}}
+}
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 caller variable 's'}}
+ return s; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}} expected-warning{{Address of stack memory associated with local variable 'local' is still referred to by the caller variable 's' upon returning to the caller. This will be a dangling reference}}
+}
+
+S leak_through_field_of_returned_object() {
+ int local = 14;
+ S s{&local};
+ return s; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}}
+}
+
+S leak_through_compound_literal() {
+ int local = 0;
+ return (S) { &local }; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}}
}
void returned_struct_with_ptr_caller() {
@@ -555,6 +602,30 @@ void static_struct_with_ptr() {
}
} // namespace leaking_via_struct_with_ptr
+namespace leaking_via_nested_structs_with_ptr {
+struct Inner {
+ int *ptr;
+};
+
+struct Outer {
+ Inner I;
+};
+
+struct Deriving : public Outer {};
+
+Outer leaks_through_nested_objects() {
+ int local = 0;
+ Outer O{&local};
+ return O; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}}
+}
+
+Deriving leaks_through_base_objects() {
+ int local = 0;
+ Deriving D{&local};
+ return D; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}}
+}
+} // namespace leaking_via_nested_structs_with_ptr
+
namespace leaking_via_ref_to_struct_with_ptr {
struct S {
int* p;
@@ -613,15 +684,15 @@ S* returned_ptr_to_struct_with_ptr_top() {
int local = 42;
S* s = new S;
s->p = &local;
- return s;...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/126620
More information about the cfe-commits
mailing list