Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h +++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h @@ -651,6 +651,8 @@ /// The calculation is cached in FramesModifying. bool isModifiedInFrame(const ExplodedNode *CallExitBeginN); + void markFrameAsModifying(const StackFrameContext *SCtx); + /// Write to \c FramesModifying all stack frames along the path in the current /// stack frame which modifies the state. void findModifyingFrames(const ExplodedNode *const CallExitBeginN); @@ -658,11 +660,23 @@ protected: bugreporter::TrackingKind TKind; - /// \return Whether the state was modified from the current node, \CurrN, to - /// the end of the stack fram, at \p CallExitBeginN. + /// \return Whether the state was modified from the current node, \p CurrN, to + /// the end of the stack fram, at \p CallExitBeginN. \p CurrN and + /// \p CallExitBeginN are always in the same stack frame. virtual bool wasModifiedBeforeCallExit(const ExplodedNode *CurrN, - const ExplodedNode *CallExitBeginN) = 0; + const ExplodedNode *CallExitBeginN) { + return false; + } + + /// \return Whether the state was modified in the inlined function call in + /// between \p CallEnterN and \p CallExitBeginN. Mind that the stack frame + /// retrieved from a CallEnter is the *caller's* stack frame! The inlined + /// function's stack should be retrieved from \p CallExitBeginN. + virtual bool wasModifiedInFunction(const ExplodedNode *CallEnterN, + const ExplodedNode *CallExitEndN) { + return false; + } /// Consume the information on the non-modifying stack frame in order to /// either emit a note or not. May suppress the report entirely. Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -52,6 +52,7 @@ #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/ParentMap.h" +#include "clang/Analysis/ProgramPoint.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TargetInfo.h" @@ -76,8 +77,10 @@ #include "llvm/ADT/SetOperations.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/Support/Casting.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" +#include "llvm/Support/raw_ostream.h" #include #include #include @@ -755,6 +758,17 @@ Owners.insert(Region); return true; } + + LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); } + LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream &out) const { + out << "Owners: {\n"; + for (const MemRegion *Owner : Owners) { + out << " "; + Owner->dumpToStream(out); + out << ",\n"; + } + out << "}\n"; + } }; protected: @@ -768,31 +782,24 @@ return Ret; } - static const ExplodedNode *getCallExitEnd(const ExplodedNode *N) { - while (N && !N->getLocationAs()) - N = N->getFirstSucc(); - return N; + LLVM_DUMP_METHOD static std::string + getFunctionName(const ExplodedNode *CallEnterN) { + if (const CallExpr *CE = llvm::dyn_cast_or_null( + CallEnterN->getLocationAs()->getCallExpr())) + if (const FunctionDecl *FD = CE->getDirectCallee()) + return FD->getQualifiedNameAsString(); + return ""; } virtual bool - wasModifiedBeforeCallExit(const ExplodedNode *CurrN, - const ExplodedNode *CallExitN) override { - if (CurrN->getLocationAs()) - return true; - - // Its the state right *after* the call that is interesting. Any pointers - // inside the call that pointed to the allocated memory are of little - // consequence if their lifetime ends within the function. - CallExitN = getCallExitEnd(CallExitN); - if (!CallExitN) - return true; - + wasModifiedInFunction(const ExplodedNode *CurrN, + const ExplodedNode *CallExitEndN) override { if (CurrN->getState()->get(Sym) != - CallExitN->getState()->get(Sym)) + CallExitEndN->getState()->get(Sym)) return true; OwnerSet CurrOwners = getOwnersAtNode(CurrN); - OwnerSet ExitOwners = getOwnersAtNode(CallExitN); + OwnerSet ExitOwners = getOwnersAtNode(CallExitEndN); // Owners in the current set may be purged from the analyzer later on. // If a variable is dead (is not referenced directly or indirectly after Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -355,43 +355,70 @@ return FramesModifying.count(SCtx); } +void NoStateChangeFuncVisitor::markFrameAsModifying( + const StackFrameContext *SCtx) { + while (!SCtx->inTopFrame()) { + auto p = FramesModifying.insert(SCtx); + if (!p.second) + break; // Frame and all its parents already inserted. + SCtx = SCtx->getParent()->getStackFrame(); + } +} + +static const ExplodedNode *getCallExitEnd(const ExplodedNode *N) { + while (N && !N->getLocationAs()) + N = N->getFirstSucc(); + return N; +} + void NoStateChangeFuncVisitor::findModifyingFrames( const ExplodedNode *const CallExitBeginN) { assert(CallExitBeginN->getLocationAs()); - const ExplodedNode *LastReturnN = CallExitBeginN; + const StackFrameContext *const OriginalSCtx = CallExitBeginN->getLocationContext()->getStackFrame(); const ExplodedNode *CurrN = CallExitBeginN; - - do { - ProgramStateRef State = CurrN->getState(); - auto CallExitLoc = CurrN->getLocationAs(); - if (CallExitLoc) { - LastReturnN = CurrN; + const ExplodedNode *CurrCallExitBeginN = CallExitBeginN; + const StackFrameContext *CurrentSCtx = OriginalSCtx; + + while (CurrN) { + // Found a new inlined call. + if (CurrN->getLocationAs()) { + CurrCallExitBeginN = CurrN; + CurrentSCtx = CurrN->getStackFrame(); + FramesModifyingCalculated.insert(CurrentSCtx); + // We won't see a change in between two identical exploded nodes: skip. + CurrN = CurrN->getFirstPred(); + continue; } - FramesModifyingCalculated.insert( - CurrN->getLocationContext()->getStackFrame()); - - if (wasModifiedBeforeCallExit(CurrN, LastReturnN)) { - const StackFrameContext *SCtx = CurrN->getStackFrame(); - while (!SCtx->inTopFrame()) { - auto p = FramesModifying.insert(SCtx); - if (!p.second) - break; // Frame and all its parents already inserted. - SCtx = SCtx->getParent()->getStackFrame(); + if (auto CE = CurrN->getLocationAs()) { + if (const ExplodedNode *CallExitEndN = getCallExitEnd(CurrCallExitBeginN)) + if (wasModifiedInFunction(CurrN, CallExitEndN)) + markFrameAsModifying(CurrentSCtx); + + // We exited this inlined call, lets actualize the stack frame. + CurrentSCtx = CurrN->getStackFrame(); + + // Stop calculating at the current function, but always regard it as + // modifying, so we can avoid notes like this: + // void f(Foo &F) { + // F.field = 0; // note: 0 assigned to 'F.field' + // // note: returning without writing to 'F.field' + // } + if (CE->getCalleeContext() == OriginalSCtx) { + markFrameAsModifying(CurrentSCtx); + break; } } - // Stop calculation at the call to the current function. - if (auto CE = CurrN->getLocationAs()) - if (CE->getCalleeContext() == OriginalSCtx) - break; + if (wasModifiedBeforeCallExit(CurrN, CurrCallExitBeginN)) + markFrameAsModifying(CurrentSCtx); CurrN = CurrN->getFirstPred(); - } while (CurrN); + } } PathDiagnosticPieceRef NoStateChangeFuncVisitor::VisitNode(