https://github.com/Flandini created https://github.com/llvm/llvm-project/pull/126620
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! >From a9fa79ba9c34eff2879816de449a7e923378a349 Mon Sep 17 00:00:00 2001 From: Michael Flanders <flanders.micha...@gmail.com> Date: Mon, 10 Feb 2025 17:00:21 -0600 Subject: [PATCH 1/2] =?UTF-8?q?Reapply=20"[analyzer]=20Remove=20some=20fal?= =?UTF-8?q?se=20negatives=20in=20StackAddrEscapeChec=E2=80=A6=20(#126614)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 3c6d1dd362009e0aebd91c5197d40f8ce59fcff4. Reapply recent stack addr escape checker changes after fixing buildbot errors. --- .../Checkers/StackAddrEscapeChecker.cpp | 182 +++++++++---- clang/test/Analysis/copy-elision.cpp | 25 +- clang/test/Analysis/stack-addr-ps.cpp | 245 ++++++++++++++++-- clang/test/Analysis/stackaddrleak.c | 12 + clang/test/Analysis/stackaddrleak.cpp | 2 +- 5 files changed, 385 insertions(+), 81 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp index f4de3b500499c48..826dc42e4f59948 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 *StackFrameContext; + 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), StackFrameContext(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() == StackFrameContext) + 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@-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@-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@-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@-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@-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@-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@-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@-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; -} // no-warning False Negative + return s; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}} +} S* returned_ptr_to_struct_with_ptr_callee() { int local = 42; S* s = new S; s->p = &local; - return s; -} // no-warning False Negative + return s; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}} +} void returned_ptr_to_struct_with_ptr_caller() { S* s = returned_ptr_to_struct_with_ptr_callee(); @@ -676,15 +747,15 @@ 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 + return s; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}} +} 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 + return s; // expected-warning {{Address of stack memory associated with local variable 'local' returned to caller}} +} void returned_ptr_to_struct_with_ptr_caller() { S* s = returned_ptr_to_struct_with_ptr_callee(); @@ -877,3 +948,137 @@ void top_malloc_no_crash_fn() { free(pptr); } } // namespace alloca_region_pointer + +// These all warn with -Wreturn-stack-address also +namespace return_address_of_true_positives { +int* ret_local_addrOf() { + int x = 1; + return &*&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}} +} + +int* ret_local_addrOf_paren() { + int x = 1; + return (&(*(&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}} +} + +int* ret_local_addrOf_ptr_arith() { + int x = 1; + return &*(&x+1); // 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}} +} + +int* ret_local_addrOf_ptr_arith2() { + int x = 1; + return &*(&x+1); // 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}} +} + +int* ret_local_field() { + struct { int x; } a; + return &a.x; // expected-warning {{Address of stack memory associated with local variable 'a' returned to caller}} expected-warning {{address of stack memory associated with local variable 'a' returned}} +} + +int& ret_local_field_ref() { + struct { int x; } a; + return a.x; // expected-warning {{Address of stack memory associated with local variable 'a' returned to caller}} expected-warning {{reference to stack memory associated with local variable 'a' returned}} +} +} //namespace return_address_of_true_positives + +namespace true_negatives_return_expressions { +struct Container { int *x; }; + +int test2() { + int x = 14; + return x; // no-warning +} + +void return_void() { + return void(); // no-warning +} + +int return_assign_expr_safe() { + int y, x = 14; + return y = x; // no-warning +} + +int return_comma_separated_expressions_safe() { + int x = 1; + int *y; + return (y=&x), (x=15); // no-warning +} + +int return_comma_separated_expressions_container_safe() { + int x = 1; + Container Other; + return Other = Container{&x}, x = 14; // no-warning +} + +int make_x(); +int return_symbol_safe() { + int x = make_x(); + clang_analyzer_dump(x); // expected-warning-re {{conj_$2{int, {{.+}}}}} + return x; // no-warning +} + +int *return_symbolic_region_safe(int **ptr) { + return *ptr; // no-warning +} + +int *return_arg_ptr(int *arg) { + return arg; // no-warning +} + +// This has a false positive with -Wreturn-stack-address, but CSA should not +// warn on this +int *return_conditional_never_false(int *arg) { + int x = 14; + return true ? arg : &x; // expected-warning {{address of stack memory associated with local variable 'x' returned}} +} + +// This has a false positive with -Wreturn-stack-address, but CSA should not +// warn on this +int *return_conditional_never_true(int *arg) { + int x = 14; + return false ? &x : arg; // expected-warning {{address of stack memory associated with local variable 'x' returned}} +} + +int *return_conditional_path_split(int *arg, bool b) { + int x = 14; + return b ? arg : &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}} -Wreturn-stack-address +} + +// compare of two symbolic regions +// maybe in some implementation, make_ptr returns interior pointers that are comparable +int *make_ptr(); +bool return_symbolic_exxpression() { + int *a = make_ptr(); + int *b = make_ptr(); + return a < b; // no-warning +} + +int *return_static() { + static int x = 0; + return &x; // no-warning +} + +int* return_cpp_reinterpret_cast_no_warning(long x) { + return reinterpret_cast<int*>(x); // no-warning +} +} + +// TODO: The tail call expression tree must not hold a reference to an arg or stack local: +// "The lifetimes of all local variables and function parameters end immediately before the +// [tail] call to the function. This means that it is undefined behaviour to pass a pointer or +// reference to a local variable to the called function, which is not the case without the +// attribute." +// These only warn from -Wreturn-stack-address for now +namespace with_attr_musttail { +void TakesIntAndPtr(int, int *); +void PassAddressOfLocal(int a, int *b) { + int c; + [[clang::musttail]] return TakesIntAndPtr(0, &c); // expected-warning {{address of stack memory associated with local variable 'c' pass\ +ed to musttail function}} False-negative on CSA +} +void PassAddressOfParam(int a, int *b) { + [[clang::musttail]] return TakesIntAndPtr(0, &a); // expected-warning {{address of stack memory associated with parameter 'a' passed to\ + musttail function}} False-negative on CSA +} +} // namespace with_attr_musttail diff --git a/clang/test/Analysis/stackaddrleak.c b/clang/test/Analysis/stackaddrleak.c index 5f508275ba9c8d4..f8101525401b0ff 100644 --- a/clang/test/Analysis/stackaddrleak.c +++ b/clang/test/Analysis/stackaddrleak.c @@ -56,3 +56,15 @@ void assignAsBool(void) { int x; b = &x; } // no-warning + +int *f(int* p __attribute__((lifetimebound))); +int *g() { + int i; + return f(&i); // expected-warning {{address of stack memory associated with local variable 'i' returned}} +} + +int *f_no_lifetime_bound(int *p); +int *g_no_lifetime_bound() { + int i = 0; + return f_no_lifetime_bound(&i); // no-warning +} diff --git a/clang/test/Analysis/stackaddrleak.cpp b/clang/test/Analysis/stackaddrleak.cpp index 3daffb35a6cd9a6..df202a2fee77628 100644 --- a/clang/test/Analysis/stackaddrleak.cpp +++ b/clang/test/Analysis/stackaddrleak.cpp @@ -18,7 +18,7 @@ struct myfunction { myfunction create_func() { int n; auto c = [&n] {}; - return c; // expected-warning {{Address of stack memory associated with local variable 'n' is still referred to by a temporary object on the stack upon returning to the caller. This will be a dangling reference}} + return c; // expected-warning {{Address of stack memory associated with local variable 'n' returned to caller}} expected-warning{{Address of stack memory associated with local variable 'n' is still referred to by a temporary object on the stack upon returning to the caller. This will be a dangling reference}} } void gh_66221() { create_func()(); >From 029e7e98dc9956086adc6c1dfb0c655a273fbee6 Mon Sep 17 00:00:00 2001 From: Michael Flanders <flanders.micha...@gmail.com> Date: Mon, 10 Feb 2025 16:37:03 -0600 Subject: [PATCH 2/2] [analyzer] Fix StackFrameContext define in stack addr escape checker --- .../lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp index 826dc42e4f59948..c9df15ceb3b4092 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp @@ -249,14 +249,14 @@ void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call, /// that would leak. class FindStackRegionsSymbolVisitor final : public SymbolVisitor { CheckerContext &Ctxt; - const StackFrameContext *StackFrameContext; + const StackFrameContext *PoppedStackFrame; SmallVectorImpl<const MemRegion *> &EscapingStackRegions; public: explicit FindStackRegionsSymbolVisitor( CheckerContext &Ctxt, SmallVectorImpl<const MemRegion *> &StorageForStackRegions) - : Ctxt(Ctxt), StackFrameContext(Ctxt.getStackFrame()), + : Ctxt(Ctxt), PoppedStackFrame(Ctxt.getStackFrame()), EscapingStackRegions(StorageForStackRegions) {} bool VisitSymbol(SymbolRef sym) override { return true; } @@ -274,7 +274,7 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor { void SaveIfEscapes(const MemRegion *MR) { const StackSpaceRegion *SSR = MR->getMemorySpace()->getAs<StackSpaceRegion>(); - if (SSR && SSR->getStackFrame() == StackFrameContext) + if (SSR && SSR->getStackFrame() == PoppedStackFrame) EscapingStackRegions.push_back(MR); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
