diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp index b67e6cd86c3d60..2904eab0097dc8 100644 --- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -2416,6 +2416,28 @@ PathSensitiveBugReport::getRanges() const { return Ranges; } +static bool exitingDestructor(const ExplodedNode *N) { + // Need to loop here, as some times the Error node is already outside of the + // destructor context, and the previous node is an edge that is also outside. + while (N && !N->getLocation().getAs()) { + N = N->getFirstPred(); + } + return N && isa(N->getLocationContext()->getDecl()); +} + +static const Stmt * +findReasonableStmtCloseToFunctionExit(const ExplodedNode *N) { + if (exitingDestructor(N)) { + // If we are exiting a destructor call, it is more useful to point to + // the next stmt which is usually the temporary declaration. + if (const Stmt *S = N->getNextStmtForDiagnostics()) + return S; + // If next stmt is not found, it is likely the end of a top-level + // function analysis. find the last execution statement then. + } + return N->getPreviousStmtForDiagnostics(); +} + PathDiagnosticLocation PathSensitiveBugReport::getLocation() const { assert(ErrorNode && "Cannot create a location with a null node."); @@ -2433,15 +2455,7 @@ PathSensitiveBugReport::getLocation() const { 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 = findReasonableStmtCloseToFunctionExit(ErrorNode); } if (!S) S = ErrorNode->getNextStmtForDiagnostics(); diff --git a/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp b/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp index 11cef00ada330b..1e0cc2eea9ed85 100644 --- a/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp @@ -349,6 +349,8 @@ const Stmt *ExplodedNode::getStmtForDiagnostics() const { const Stmt *ExplodedNode::getNextStmtForDiagnostics() const { for (const ExplodedNode *N = getFirstSucc(); N; N = N->getFirstSucc()) { + if (N->getLocation().isPurgeKind()) + continue; if (const Stmt *S = N->getStmtForDiagnostics()) { // Check if the statement is '?' or '&&'/'||'. These are "merges", // not actual statement points. diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 2ca24d0c5ab22b..02facf786830d2 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -353,14 +353,19 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { ExplodedNodeSet CleanedNodes; if (LastSt && Blk && AMgr.options.AnalysisPurgeOpt != PurgeNone) { static SimpleProgramPointTag retValBind("ExprEngine", "Bind Return Value"); - PostStmt Loc(LastSt, calleeCtx, &retValBind); + auto Loc = isa(LastSt) + ? ProgramPoint{PostStmt(LastSt, calleeCtx, &retValBind)} + : ProgramPoint{EpsilonPoint(calleeCtx, /*Data1=*/nullptr, + /*Data2=*/nullptr, &retValBind)}; + const CFGBlock *PrePurgeBlock = + isa(LastSt) ? Blk : &CEBNode->getCFG().getExit(); bool isNew; ExplodedNode *BindedRetNode = G.getNode(Loc, state, false, &isNew); BindedRetNode->addPredecessor(CEBNode, G); if (!isNew) return; - NodeBuilderContext Ctx(getCoreEngine(), Blk, BindedRetNode); + NodeBuilderContext Ctx(getCoreEngine(), PrePurgeBlock, BindedRetNode); currBldrCtx = &Ctx; // Here, we call the Symbol Reaper with 0 statement and callee location // context, telling it to clean up everything in the callee's context diff --git a/clang/test/Analysis/copy-elision.cpp b/clang/test/Analysis/copy-elision.cpp index 423c4519f5bfc6..cd941854fc7f19 100644 --- a/clang/test/Analysis/copy-elision.cpp +++ b/clang/test/Analysis/copy-elision.cpp @@ -263,17 +263,17 @@ void testVariable() { struct TestCtorInitializer { ClassWithDestructor c; - TestCtorInitializer(AddressVector &v) - : c(ClassWithDestructor(v)) {} - // 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}} + TestCtorInitializer(AddressVector &refParam) + : c(ClassWithDestructor(refParam)) {} }; void testCtorInitializer() { AddressVector v; { TestCtorInitializer t(v); + // 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}} // Check if the last destructor is an automatic destructor. // A temporary destructor would have fired by now. #if ELIDE diff --git a/clang/test/Analysis/cxx-uninitialized-object-unguarded-access.cpp b/clang/test/Analysis/cxx-uninitialized-object-unguarded-access.cpp index 53e72e7c5f4f7c..611e1d8255976c 100644 --- a/clang/test/Analysis/cxx-uninitialized-object-unguarded-access.cpp +++ b/clang/test/Analysis/cxx-uninitialized-object-unguarded-access.cpp @@ -163,8 +163,8 @@ class UnguardedFieldThroughMethodTest { Volume = 0; break; case A: - Area = 0; // expected-warning {{1 uninitialized field}} - break; + Area = 0; + break; // expected-warning {{1 uninitialized field}} } } @@ -201,8 +201,8 @@ class UnguardedPublicFieldsTest { Volume = 0; break; case A: - Area = 0; // expected-warning {{1 uninitialized field}} - break; + Area = 0; + break; // expected-warning {{1 uninitialized field}} } } diff --git a/clang/test/Analysis/void-call-exit-modelling.c b/clang/test/Analysis/void-call-exit-modelling.c new file mode 100644 index 00000000000000..6fc8cb54f00bcd --- /dev/null +++ b/clang/test/Analysis/void-call-exit-modelling.c @@ -0,0 +1,26 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc -analyzer-output text -verify %s + +typedef __typeof(sizeof(int)) size_t; +void *malloc(size_t size); + +void inf_loop_break_callee() { + void* data = malloc(10); // expected-note{{Memory is allocated}} + while (1) { // expected-note{{Loop condition is true}} + (void)data; + break; // No note that we jump to the line above from this break + } // expected-note@-1{{Execution jumps to the end of the function}} +} // expected-warning{{Potential leak of memory pointed to by 'data'}} +// expected-note@-1 {{Potential leak of memory pointed to by 'data'}} + +void inf_loop_break_caller() { + inf_loop_break_callee(); // expected-note{{Calling 'inf_loop_break_callee'}} +} + +void inf_loop_break_top() { + void* data = malloc(10); // expected-note{{Memory is allocated}} + while (1) { // expected-note{{Loop condition is true}} + (void)data; + break; // No note that we jump to the line above from this break + } // expected-note@-1{{Execution jumps to the end of the function}} +} // expected-warning{{Potential leak of memory pointed to by 'data'}} +// expected-note@-1 {{Potential leak of memory pointed to by 'data'}}