[analyzer] Detect leaks of stack addresses via output params, indirect globals 3/3 #105648
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Arseniy Zaostrovnykh (necto) ChangesNote, I prepared this PR to be rebased and merged with three commits that are self-sufficient and build on each other. Fix some false negatives of StackAddrEscapeChecker:
Note that now StackAddrEscapeChecker produces a diagnostic if a function with an output parameter is analyzed as top-level or as a callee. I took special care to make sure the reports point to the same primary location and, in many cases, feature the same primary message. That is the motivation to modify Core/BugReporter.cpp and Core/ExplodedGraph.cpp To avoid false positive reports when a global indirect pointer is assigned a local address, invalidated, and then reset, I rely on the fact that the invalidation symbol will be a DerivedSymbol of a ConjuredSymbol that refers to the same memory region. The checker still has a false negative for non-trivial escaping via a returned value. It requires an active https://sonarsource.atlassian.net/browse/CPP-4734 Patch is 40.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/105648.diff 10 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index ea09c43cc5ce90..ef477a808fac17 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -288,12 +288,89 @@ 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;
+}
+
+const MemRegion *getOriginBaseRegion(const MemRegion *Referrer) {
+ Referrer = Referrer->getBaseRegion();
+ while (const auto *SymReg = dyn_cast<SymbolicRegion>(Referrer)) {
+ Referrer = SymReg->getSymbol()->getOriginRegion()->getBaseRegion();
+ }
+ return Referrer;
+}
+
+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";
+ // UnknownSpaceRegion usually refers to pointers-to-pointers,
+ // might be in top frame or not with pointers-to-pointers-to-pointers
+ assert((isa<UnknownSpaceRegion, StackSpaceRegion>(Space)));
+ // These two cases are deliberately combined to keep the message identical
+ // between the top-level and inlined analysis of the same function
+ return "caller";
+ }(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();
+
+ bool ExitingTopFrame =
+ Ctx.getPredecessor()->getLocationContext()->inTopFrame();
+
+ if (ExitingTopFrame && Node->getLocation().getTag() &&
+ Node->getLocation().getTag()->getTagDescription() ==
+ "ExprEngine : Clean Node" &&
+ Node->getFirstPred()) {
+ // When finishing analysis of a top-level function, engine proactively
+ // removes dead symbols thus preventing this checker from looking through
+ // the output parameters. Take 1 step back, to the node where these symbols
+ // and their bindings are still present
+ Node = Node->getFirstPred();
+ }
// Iterate over all bindings to global variables and see if it contains
// a memory region in the stack space.
@@ -301,40 +378,64 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
private:
CheckerContext &Ctx;
const StackFrameContext *PoppedFrame;
+ const bool TopFrame;
/// Look for stack variables referring to popped stack variables.
/// Returns true only if it found some dangling stack variables
/// 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 (ReferrerMemSpace && ReferredMemSpace) {
- if (ReferredFrame == PoppedFrame &&
- ReferrerFrame->isParentOf(PoppedFrame)) {
- V.emplace_back(Referrer, Referred);
- return true;
- }
+ if (!ReferrerStackSpace)
+ return false;
+
+ if (const auto *ReferredFrame = ReferredMemSpace->getStackFrame();
+ ReferredFrame != PoppedFrame) {
+ return false;
+ }
+
+ if (ReferrerStackSpace->getStackFrame()->isParentOf(PoppedFrame)) {
+ V.emplace_back(Referrer, Referred);
+ return true;
+ }
+ if (isa<StackArgumentsSpaceRegion>(ReferrerMemSpace) &&
+ ReferrerStackSpace->getStackFrame() == PoppedFrame && TopFrame) {
+ // Output parameter of a top-level function
+ V.emplace_back(Referrer, Referred);
+ return true;
}
return false;
}
+ void updateInvalidatedRegions(const MemRegion *Region) {
+ if (const auto *SymReg = Region->getAs<SymbolicRegion>()) {
+ SymbolRef Symbol = SymReg->getSymbol();
+ if (const auto *DerS = dyn_cast<SymbolDerived>(Symbol);
+ DerS && isa_and_nonnull<SymbolConjured>(DerS->getParentSymbol())) {
+ InvalidatedRegions.insert(Symbol->getOriginRegion()->getBaseRegion());
+ }
+ }
+ }
+
public:
SmallVector<std::pair<const MemRegion *, const MemRegion *>, 10> V;
+ llvm::SmallPtrSet<const MemRegion *, 4> InvalidatedRegions;
- CallBack(CheckerContext &CC) : Ctx(CC), PoppedFrame(CC.getStackFrame()) {}
+ CallBack(CheckerContext &CC, bool TopFrame)
+ : Ctx(CC), PoppedFrame(CC.getStackFrame()), TopFrame(TopFrame) {}
bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Region,
SVal Val) override {
+ updateInvalidatedRegions(Region);
const MemRegion *VR = Val.getAsRegion();
if (!VR)
return true;
@@ -343,7 +444,8 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
return true;
// Check the globals for the same.
- if (!isa<GlobalsSpaceRegion>(Region->getMemorySpace()))
+ if (!isa_and_nonnull<GlobalsSpaceRegion>(
+ getStackOrGlobalSpaceRegion(Region)))
return true;
if (VR && VR->hasStackStorage() && !isNotInCurrentFrame(VR, Ctx))
V.emplace_back(Region, VR);
@@ -351,7 +453,8 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
}
};
- CallBack Cb(Ctx);
+ CallBack Cb(Ctx, ExitingTopFrame);
+ ProgramStateRef State = Node->getState();
State->getStateManager().getStoreManager().iterBindings(State->getStore(),
Cb);
@@ -359,28 +462,31 @@ 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();
const MemRegion *Referred = P.second;
+ if (Cb.InvalidatedRegions.contains(getOriginBaseRegion(Referrer))) {
+ continue;
+ }
// 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 +496,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/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index d73dc40cf03fbb..ea916c3585cadc 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2436,8 +2436,19 @@ PathSensitiveBugReport::getLocation() const {
if (auto FE = P.getAs<FunctionExitPoint>()) {
if (const ReturnStmt *RS = FE->getStmt())
return PathDiagnosticLocation::createBegin(RS, SM, LC);
+
+ // If we are exiting a destructor call, it is more useful to point to the
+ // next stmt which is usually the temporary declaration.
+ // For non-destructor and non-top-level calls, the next stmt will still
+ // refer to the last executed stmt of the body.
+ S = ErrorNode->getNextStmtForDiagnostics();
+ // If next stmt is not found, it is likely the end of a top-level function
+ // analysis. find the last execution statement then.
+ if (!S)
+ S = ErrorNode->getPreviousStmtForDiagnostics();
}
- S = ErrorNode->getNextStmtForDiagnostics();
+ if (!S)
+ S = ErrorNode->getNextStmtForDiagnostics();
}
if (S) {
diff --git a/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp b/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
index f84da769d182f8..11cef00ada330b 100644
--- a/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
@@ -376,7 +376,7 @@ const Stmt *ExplodedNode::getNextStmtForDiagnostics() const {
const Stmt *ExplodedNode::getPreviousStmtForDiagnostics() const {
for (const ExplodedNode *N = getFirstPred(); N; N = N->getFirstPred())
- if (const Stmt *S = N->getStmtForDiagnostics())
+ if (const Stmt *S = N->getStmtForDiagnostics(); S && !isa<CompoundStmt>(S))
return S;
return nullptr;
diff --git a/clang/test/Analysis/copy-elision.cpp b/clang/test/Analysis/copy-elision.cpp
index 991f325c05853d..423c4519f5bfc6 100644
--- a/clang/test/Analysis/copy-elision.cpp
+++ b/clang/test/Analysis/copy-elision.cpp
@@ -158,19 +158,19 @@ ClassWithoutDestructor make1(AddressVector<ClassWithoutDestructor> &v) {
return ClassWithoutDestructor(v);
// no-elide-warning@-1 {{Address of stack memory associated with temporary \
object of type 'ClassWithoutDestructor' is still \
-referred to by the stack variable 'v' upon returning to the caller}}
+referred to by the caller variable 'v' upon returning to the caller}}
}
ClassWithoutDestructor make2(AddressVector<ClassWithoutDestructor> &v) {
return make1(v);
// no-elide-warning@-1 {{Address of stack memory associated with temporary \
object of type 'ClassWithoutDestructor' is still \
-referred to by the stack variable 'v' upon returning to the caller}}
+referred to by the caller variable 'v' upon returning to the caller}}
}
ClassWithoutDestructor make3(AddressVector<ClassWithoutDestructor> &v) {
return make2(v);
// no-elide-warning@-1 {{Address of stack memory associated with temporary \
object of type 'ClassWithoutDestructor' is still \
-referred to by the stack variable 'v' upon returning to the caller}}
+referred to by the caller variable 'v' upon returning to the caller}}
}
void testMultipleReturns() {
@@ -193,7 +193,7 @@ void testMultipleReturns() {
void consume(ClassWithoutDestructor c) {
c.push();
// expected-warning@-1 {{Address of stack memory associated with local \
-variable 'c' is still referred to by the stack variable 'v' upon returning \
+variable 'c' is still referred to by the caller variable 'v' upon returning \
to the caller}}
}
@@ -267,7 +267,7 @@ struct TestCtorInitializer {
: c(ClassWithDestructor(v)) {}
// no-elide-warning@-1 {{Address of stack memory associated with temporary \
object of type 'ClassWithDestructor' is still referred \
-to by the stack variable 'v' upon returning to the caller}}
+to by the caller variable 'v' upon returning to the caller}}
};
void testCtorInitializer() {
@@ -303,19 +303,19 @@ ClassWithDestructor make1(AddressVector<ClassWithDestructor> &v) {
return ClassWithDestructor(v);
// no-elide-warning@-1 {{Address of stack memory associated with temporary \
object of type 'ClassWithDestructor' is still referred \
-to by the stack variable 'v' upon returning to the caller}}
+to by the caller variable 'v' upon returning to the caller}}
}
ClassWithDestructor make2(AddressVector<ClassWithDestructor> &v) {
return make1(v);
// no-elide-warning@-1 {{Address of stack memory associated with temporary \
object of type 'ClassWithDestructor' is still referred \
-to by the stack variable 'v' upon returning to the caller}}
+to by the caller variable 'v' upon returning to the caller}}
}
ClassWithDestructor make3(AddressVector<ClassWithDestructor> &v) {
return make2(v);
// no-elide-warning@-1 {{Address of stack memory associated with temporary \
object of type 'ClassWithDestructor' is still referred \
-to by the stack variable 'v' upon returning to the caller}}
+to by the caller variable 'v' upon returning to the caller}}
}
void testMultipleReturnsWithDestructors() {
@@ -360,7 +360,7 @@ void testMultipleReturnsWithDestructors() {
void consume(ClassWithDestructor c) {
c.push();
// expected-warning@-1 {{Address of stack memory associated with local \
-variable 'c' is still referred to by the stack variable 'v' upon returning \
+variable 'c' is still referred to by the caller variable 'v' upon returning \
to the caller}}
}
@@ -407,7 +407,7 @@ struct Foo {
Foo make1(Foo **r) {
return Foo(r);
// no-elide-warning@-1 {{Address of stack memory associated with temporary \
-object of type 'Foo' is still referred to by the stack \
+object of type 'Foo' is still referred to by the caller \
variable 'z' upon returning to the caller}}
}
diff --git a/clang/test/Analysis/incorrect-checker-names.cpp b/clang/test/Analysis/incorrect-checker-names.cpp
index 9854a503fc4f62..8dee0b6f468581 100644
--- a/clang/test/Analysis/incorrect-checker-names.cpp
+++ b/clang/test/Analysis/incorrect-checker-names.cpp
@@ -16,5 +16,5 @@ char const *p;
void f0() {
char const str[] = "This will change";
p = str;
-} // expected-warning{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference [core.StackAddressEscape]}}
-// expected-note@-1{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference}}
+} // expected-warning@-1{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference [core.StackAddressEscape]}}
+// expected-note@-2{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference}}
diff --git a/clang/test/Analysis/loop-block-counts.c b/clang/test/Analysis/loop-block-counts.c
index 9af67b1b632f51..ae3705f8bd7ccc 100644
--- a/clang/test/Analysis/loop-block-counts.c
+++ b/clang/test/Analysis/loop-block-counts.c
@@ -6,7 +6,7 @@ void callee(void **p) {
int x;
*p = &x;
// expected-warning@-1 {{Address of stack memory associated with local \
-variable 'x' is still referred to by the stack variable 'arr' upon \
+variable 'x' is still referred to by the caller variable 'arr' upon \
returning to the caller}}
}
diff --git a/clang/test/Analysis/stack-addr-ps.c b/clang/test/Analysis/stack-addr-ps.c
index e69ab4189b524f..138b8c16b02bde 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; // expected-warning{{Address of stack memory associated with local variable 'local' is still referred to by the caller variable 'out'}}
+}
+
+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 caller 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 caller 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..b98fc3beade761 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -140,7 +140,7 @@ void write_stack_address_to(char **q) {
char local;
*q = &local;
// expected-warning@-1 {{Address of stack memory associated with local \
-variable 'local' is still referred to by the stack variable 'p' upon \
+variable 'local' is still referred to by the caller variable 'p' upon \
returning to the caller}}
}
@@ -161,3 +161,610 @@ 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}}
+}
+...
[truncated]
|
|
@llvm/pr-subscribers-clang Author: Arseniy Zaostrovnykh (necto) ChangesNote, I prepared this PR to be rebased and merged with three commits that are self-sufficient and build on each other. Fix some false negatives of StackAddrEscapeChecker:
Note that now StackAddrEscapeChecker produces a diagnostic if a function with an output parameter is analyzed as top-level or as a callee. I took special care to make sure the reports point to the same primary location and, in many cases, feature the same primary message. That is the motivation to modify Core/BugReporter.cpp and Core/ExplodedGraph.cpp To avoid false positive reports when a global indirect pointer is assigned a local address, invalidated, and then reset, I rely on the fact that the invalidation symbol will be a DerivedSymbol of a ConjuredSymbol that refers to the same memory region. The checker still has a false negative for non-trivial escaping via a returned value. It requires an active https://sonarsource.atlassian.net/browse/CPP-4734 Patch is 40.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/105648.diff 10 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index ea09c43cc5ce90..ef477a808fac17 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -288,12 +288,89 @@ 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;
+}
+
+const MemRegion *getOriginBaseRegion(const MemRegion *Referrer) {
+ Referrer = Referrer->getBaseRegion();
+ while (const auto *SymReg = dyn_cast<SymbolicRegion>(Referrer)) {
+ Referrer = SymReg->getSymbol()->getOriginRegion()->getBaseRegion();
+ }
+ return Referrer;
+}
+
+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";
+ // UnknownSpaceRegion usually refers to pointers-to-pointers,
+ // might be in top frame or not with pointers-to-pointers-to-pointers
+ assert((isa<UnknownSpaceRegion, StackSpaceRegion>(Space)));
+ // These two cases are deliberately combined to keep the message identical
+ // between the top-level and inlined analysis of the same function
+ return "caller";
+ }(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();
+
+ bool ExitingTopFrame =
+ Ctx.getPredecessor()->getLocationContext()->inTopFrame();
+
+ if (ExitingTopFrame && Node->getLocation().getTag() &&
+ Node->getLocation().getTag()->getTagDescription() ==
+ "ExprEngine : Clean Node" &&
+ Node->getFirstPred()) {
+ // When finishing analysis of a top-level function, engine proactively
+ // removes dead symbols thus preventing this checker from looking through
+ // the output parameters. Take 1 step back, to the node where these symbols
+ // and their bindings are still present
+ Node = Node->getFirstPred();
+ }
// Iterate over all bindings to global variables and see if it contains
// a memory region in the stack space.
@@ -301,40 +378,64 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
private:
CheckerContext &Ctx;
const StackFrameContext *PoppedFrame;
+ const bool TopFrame;
/// Look for stack variables referring to popped stack variables.
/// Returns true only if it found some dangling stack variables
/// 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 (ReferrerMemSpace && ReferredMemSpace) {
- if (ReferredFrame == PoppedFrame &&
- ReferrerFrame->isParentOf(PoppedFrame)) {
- V.emplace_back(Referrer, Referred);
- return true;
- }
+ if (!ReferrerStackSpace)
+ return false;
+
+ if (const auto *ReferredFrame = ReferredMemSpace->getStackFrame();
+ ReferredFrame != PoppedFrame) {
+ return false;
+ }
+
+ if (ReferrerStackSpace->getStackFrame()->isParentOf(PoppedFrame)) {
+ V.emplace_back(Referrer, Referred);
+ return true;
+ }
+ if (isa<StackArgumentsSpaceRegion>(ReferrerMemSpace) &&
+ ReferrerStackSpace->getStackFrame() == PoppedFrame && TopFrame) {
+ // Output parameter of a top-level function
+ V.emplace_back(Referrer, Referred);
+ return true;
}
return false;
}
+ void updateInvalidatedRegions(const MemRegion *Region) {
+ if (const auto *SymReg = Region->getAs<SymbolicRegion>()) {
+ SymbolRef Symbol = SymReg->getSymbol();
+ if (const auto *DerS = dyn_cast<SymbolDerived>(Symbol);
+ DerS && isa_and_nonnull<SymbolConjured>(DerS->getParentSymbol())) {
+ InvalidatedRegions.insert(Symbol->getOriginRegion()->getBaseRegion());
+ }
+ }
+ }
+
public:
SmallVector<std::pair<const MemRegion *, const MemRegion *>, 10> V;
+ llvm::SmallPtrSet<const MemRegion *, 4> InvalidatedRegions;
- CallBack(CheckerContext &CC) : Ctx(CC), PoppedFrame(CC.getStackFrame()) {}
+ CallBack(CheckerContext &CC, bool TopFrame)
+ : Ctx(CC), PoppedFrame(CC.getStackFrame()), TopFrame(TopFrame) {}
bool HandleBinding(StoreManager &SMgr, Store S, const MemRegion *Region,
SVal Val) override {
+ updateInvalidatedRegions(Region);
const MemRegion *VR = Val.getAsRegion();
if (!VR)
return true;
@@ -343,7 +444,8 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
return true;
// Check the globals for the same.
- if (!isa<GlobalsSpaceRegion>(Region->getMemorySpace()))
+ if (!isa_and_nonnull<GlobalsSpaceRegion>(
+ getStackOrGlobalSpaceRegion(Region)))
return true;
if (VR && VR->hasStackStorage() && !isNotInCurrentFrame(VR, Ctx))
V.emplace_back(Region, VR);
@@ -351,7 +453,8 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
}
};
- CallBack Cb(Ctx);
+ CallBack Cb(Ctx, ExitingTopFrame);
+ ProgramStateRef State = Node->getState();
State->getStateManager().getStoreManager().iterBindings(State->getStore(),
Cb);
@@ -359,28 +462,31 @@ 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();
const MemRegion *Referred = P.second;
+ if (Cb.InvalidatedRegions.contains(getOriginBaseRegion(Referrer))) {
+ continue;
+ }
// 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 +496,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/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index d73dc40cf03fbb..ea916c3585cadc 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2436,8 +2436,19 @@ PathSensitiveBugReport::getLocation() const {
if (auto FE = P.getAs<FunctionExitPoint>()) {
if (const ReturnStmt *RS = FE->getStmt())
return PathDiagnosticLocation::createBegin(RS, SM, LC);
+
+ // If we are exiting a destructor call, it is more useful to point to the
+ // next stmt which is usually the temporary declaration.
+ // For non-destructor and non-top-level calls, the next stmt will still
+ // refer to the last executed stmt of the body.
+ S = ErrorNode->getNextStmtForDiagnostics();
+ // If next stmt is not found, it is likely the end of a top-level function
+ // analysis. find the last execution statement then.
+ if (!S)
+ S = ErrorNode->getPreviousStmtForDiagnostics();
}
- S = ErrorNode->getNextStmtForDiagnostics();
+ if (!S)
+ S = ErrorNode->getNextStmtForDiagnostics();
}
if (S) {
diff --git a/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp b/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
index f84da769d182f8..11cef00ada330b 100644
--- a/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
@@ -376,7 +376,7 @@ const Stmt *ExplodedNode::getNextStmtForDiagnostics() const {
const Stmt *ExplodedNode::getPreviousStmtForDiagnostics() const {
for (const ExplodedNode *N = getFirstPred(); N; N = N->getFirstPred())
- if (const Stmt *S = N->getStmtForDiagnostics())
+ if (const Stmt *S = N->getStmtForDiagnostics(); S && !isa<CompoundStmt>(S))
return S;
return nullptr;
diff --git a/clang/test/Analysis/copy-elision.cpp b/clang/test/Analysis/copy-elision.cpp
index 991f325c05853d..423c4519f5bfc6 100644
--- a/clang/test/Analysis/copy-elision.cpp
+++ b/clang/test/Analysis/copy-elision.cpp
@@ -158,19 +158,19 @@ ClassWithoutDestructor make1(AddressVector<ClassWithoutDestructor> &v) {
return ClassWithoutDestructor(v);
// no-elide-warning@-1 {{Address of stack memory associated with temporary \
object of type 'ClassWithoutDestructor' is still \
-referred to by the stack variable 'v' upon returning to the caller}}
+referred to by the caller variable 'v' upon returning to the caller}}
}
ClassWithoutDestructor make2(AddressVector<ClassWithoutDestructor> &v) {
return make1(v);
// no-elide-warning@-1 {{Address of stack memory associated with temporary \
object of type 'ClassWithoutDestructor' is still \
-referred to by the stack variable 'v' upon returning to the caller}}
+referred to by the caller variable 'v' upon returning to the caller}}
}
ClassWithoutDestructor make3(AddressVector<ClassWithoutDestructor> &v) {
return make2(v);
// no-elide-warning@-1 {{Address of stack memory associated with temporary \
object of type 'ClassWithoutDestructor' is still \
-referred to by the stack variable 'v' upon returning to the caller}}
+referred to by the caller variable 'v' upon returning to the caller}}
}
void testMultipleReturns() {
@@ -193,7 +193,7 @@ void testMultipleReturns() {
void consume(ClassWithoutDestructor c) {
c.push();
// expected-warning@-1 {{Address of stack memory associated with local \
-variable 'c' is still referred to by the stack variable 'v' upon returning \
+variable 'c' is still referred to by the caller variable 'v' upon returning \
to the caller}}
}
@@ -267,7 +267,7 @@ struct TestCtorInitializer {
: c(ClassWithDestructor(v)) {}
// no-elide-warning@-1 {{Address of stack memory associated with temporary \
object of type 'ClassWithDestructor' is still referred \
-to by the stack variable 'v' upon returning to the caller}}
+to by the caller variable 'v' upon returning to the caller}}
};
void testCtorInitializer() {
@@ -303,19 +303,19 @@ ClassWithDestructor make1(AddressVector<ClassWithDestructor> &v) {
return ClassWithDestructor(v);
// no-elide-warning@-1 {{Address of stack memory associated with temporary \
object of type 'ClassWithDestructor' is still referred \
-to by the stack variable 'v' upon returning to the caller}}
+to by the caller variable 'v' upon returning to the caller}}
}
ClassWithDestructor make2(AddressVector<ClassWithDestructor> &v) {
return make1(v);
// no-elide-warning@-1 {{Address of stack memory associated with temporary \
object of type 'ClassWithDestructor' is still referred \
-to by the stack variable 'v' upon returning to the caller}}
+to by the caller variable 'v' upon returning to the caller}}
}
ClassWithDestructor make3(AddressVector<ClassWithDestructor> &v) {
return make2(v);
// no-elide-warning@-1 {{Address of stack memory associated with temporary \
object of type 'ClassWithDestructor' is still referred \
-to by the stack variable 'v' upon returning to the caller}}
+to by the caller variable 'v' upon returning to the caller}}
}
void testMultipleReturnsWithDestructors() {
@@ -360,7 +360,7 @@ void testMultipleReturnsWithDestructors() {
void consume(ClassWithDestructor c) {
c.push();
// expected-warning@-1 {{Address of stack memory associated with local \
-variable 'c' is still referred to by the stack variable 'v' upon returning \
+variable 'c' is still referred to by the caller variable 'v' upon returning \
to the caller}}
}
@@ -407,7 +407,7 @@ struct Foo {
Foo make1(Foo **r) {
return Foo(r);
// no-elide-warning@-1 {{Address of stack memory associated with temporary \
-object of type 'Foo' is still referred to by the stack \
+object of type 'Foo' is still referred to by the caller \
variable 'z' upon returning to the caller}}
}
diff --git a/clang/test/Analysis/incorrect-checker-names.cpp b/clang/test/Analysis/incorrect-checker-names.cpp
index 9854a503fc4f62..8dee0b6f468581 100644
--- a/clang/test/Analysis/incorrect-checker-names.cpp
+++ b/clang/test/Analysis/incorrect-checker-names.cpp
@@ -16,5 +16,5 @@ char const *p;
void f0() {
char const str[] = "This will change";
p = str;
-} // expected-warning{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference [core.StackAddressEscape]}}
-// expected-note@-1{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference}}
+} // expected-warning@-1{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference [core.StackAddressEscape]}}
+// expected-note@-2{{Address of stack memory associated with local variable 'str' is still referred to by the global variable 'p' upon returning to the caller. This will be a dangling reference}}
diff --git a/clang/test/Analysis/loop-block-counts.c b/clang/test/Analysis/loop-block-counts.c
index 9af67b1b632f51..ae3705f8bd7ccc 100644
--- a/clang/test/Analysis/loop-block-counts.c
+++ b/clang/test/Analysis/loop-block-counts.c
@@ -6,7 +6,7 @@ void callee(void **p) {
int x;
*p = &x;
// expected-warning@-1 {{Address of stack memory associated with local \
-variable 'x' is still referred to by the stack variable 'arr' upon \
+variable 'x' is still referred to by the caller variable 'arr' upon \
returning to the caller}}
}
diff --git a/clang/test/Analysis/stack-addr-ps.c b/clang/test/Analysis/stack-addr-ps.c
index e69ab4189b524f..138b8c16b02bde 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; // expected-warning{{Address of stack memory associated with local variable 'local' is still referred to by the caller variable 'out'}}
+}
+
+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 caller 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 caller 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..b98fc3beade761 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -140,7 +140,7 @@ void write_stack_address_to(char **q) {
char local;
*q = &local;
// expected-warning@-1 {{Address of stack memory associated with local \
-variable 'local' is still referred to by the stack variable 'p' upon \
+variable 'local' is still referred to by the caller variable 'p' upon \
returning to the caller}}
}
@@ -161,3 +161,610 @@ 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}}
+}
+...
[truncated]
|
|
Review commit by commit. |
|
It turns out that you cannot rebase&merge in llvm-project repo, so I'll create two more PRs stacked PRs - one per commit |
necto
force-pushed
the
az/CPP-4734-stack-leak-out-param
branch
from
6635877 to
daec8e8
Compare
This was referenced Aug 22, 2024
steakhal
changed the title
|
I already reviewed these three. LGTM |
|
Please ping me when this commit is in a clean state that can be reviewed (e.g. updates on earlier commits are incorporated). Thanks! |
necto
force-pushed
the
az/CPP-4734-stack-leak-out-param
branch
from
4081a03 to
153ad98
Compare
steakhal
pushed a commit
that referenced
this pull request
Aug 26, 2024
#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 #105648 it must not be merged before #105652
Fix some false negatives of StackAddrEscapeChecker:
- Output parameters
```
void top(int **out) {
int local = 42;
*out = &local; // Noncompliant
}
```
- Indirect global pointers
```
int **global;
void top() {
int local = 42;
*global = &local; // Noncompliant
}
```
Note that now StackAddrEscapeChecker produces a diagnostic if a function
with an output parameter is analyzed as top-level or as a callee. I took
special care to make sure the reports point to the same primary location
and, in many cases, feature the same primary message. That is the
motivation to modify Core/BugReporter.cpp and Core/ExplodedGraph.cpp
To avoid false positive reports when a global indirect pointer is
assigned a local address, invalidated, and then reset, I rely on the
fact that the invalidation symbol will be a DerivedSymbol of a
ConjuredSymbol that refers to the same memory region.
The checker still has a false negative for non-trivial escaping via a
returned value. It requires a more sophisticated traversal akin to
scanReachableSymbols, which out of the scope of this change.
CPP-4734
necto
force-pushed
the
az/CPP-4734-stack-leak-out-param
branch
from
153ad98 to
991f176
Compare
@NagyDonat , the earlier commits are now merged and I rebased this PR. Feel free to have a look |
NagyDonat
reviewed
Aug 26, 2024
There was a problem hiding this comment.
I read the commit once and I have a serious question about using getOriginRegion in the invalidation logic (see inline comment for details). Apart from that, the code LGTM (I might return later with a few additional comments from a more through review, but I don't except anything important).
NagyDonat
approved these changes
Aug 27, 2024
Xazax-hun
reviewed
Aug 27, 2024
in ExcludedRegions
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
NagyDonat
approved these changes
Aug 27, 2024
|
Hello, The following starts crashing with this patch: Result: |
necto
added a commit
to necto/llvm-project
that referenced
this pull request
Aug 29, 2024
As reported in llvm#105648 (comment) commit 08ad8dc7154bf3ab79f750e6d5fb7df597c7601a introduced a nullptr dereference in the case when store contains a binding to a symbol that has no origin region associated with it, such as the symbol generated when a pointer is passed to an opaque function.
Thank you for the report! |
steakhal
pushed a commit
that referenced
this pull request
Aug 29, 2024
…ion (#106568) As reported in #105648 (comment) commit 08ad8dc7154bf3ab79f750e6d5fb7df597c7601a introduced a nullptr dereference in the case when store contains a binding to a symbol that has no origin region associated with it, such as the symbol generated when a pointer is passed to an opaque function.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix some false negatives of StackAddrEscapeChecker:
Note that now StackAddrEscapeChecker produces a diagnostic if a function
with an output parameter is analyzed as top-level or as a callee. I took
special care to make sure the reports point to the same primary location
and, in many cases, feature the same primary message. That is the
motivation to modify Core/BugReporter.cpp and Core/ExplodedGraph.cpp
To avoid false positive reports when a global indirect pointer is
assigned a local address, invalidated, and then reset, I rely on the
fact that the invalidation symbol will be a DerivedSymbol of a
ConjuredSymbol that refers to the same memory region.
The checker still has a false negative for non-trivial escaping via a
returned value. It requires a more sophisticated traversal akin to
scanReachableSymbols, which out of the scope of this change.
CPP-4734
This is the last of the 3 stacked PRs, it must not be merged before #105652 and #105653