Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -196,37 +196,6 @@ return {}; } -//===----------------------------------------------------------------------===// -// Implementation of BugReporterVisitor. -//===----------------------------------------------------------------------===// - -std::shared_ptr -BugReporterVisitor::getEndPath(BugReporterContext &, - const ExplodedNode *, BugReport &) { - return nullptr; -} - -void -BugReporterVisitor::finalizeVisitor(BugReporterContext &, - const ExplodedNode *, BugReport &) {} - -std::shared_ptr BugReporterVisitor::getDefaultEndPath( - BugReporterContext &BRC, const ExplodedNode *EndPathNode, BugReport &BR) { - PathDiagnosticLocation L = - PathDiagnosticLocation::createEndOfPath(EndPathNode,BRC.getSourceManager()); - - const auto &Ranges = BR.getRanges(); - - // Only add the statement itself as a range if we didn't specify any - // special ranges for this report. - auto P = std::make_shared( - L, BR.getDescription(), Ranges.begin() == Ranges.end()); - for (SourceRange Range : Ranges) - P->addRange(Range); - - return P; -} - /// \return name of the macro inside the location \p Loc. static StringRef getMacroName(SourceLocation Loc, BugReporterContext &BRC) { @@ -251,36 +220,70 @@ } /// \return Whether \c RegionOfInterest was modified at \p N, -/// where \p ReturnState is a state associated with the return -/// from the current frame. -static bool wasRegionOfInterestModifiedAt( - const SubRegion *RegionOfInterest, - const ExplodedNode *N, - SVal ValueAfter) { +/// where \p ValueAfter is \c RegionOfInterest's value at the end of the +/// stack frame. +static bool wasRegionOfInterestModifiedAt(const SubRegion *RegionOfInterest, + const ExplodedNode *N, + SVal ValueAfter) { ProgramStateRef State = N->getState(); ProgramStateManager &Mgr = N->getState()->getStateManager(); - if (!N->getLocationAs() - && !N->getLocationAs() - && !N->getLocationAs()) + if (!N->getLocationAs() && !N->getLocationAs() && + !N->getLocationAs()) return false; // Writing into region of interest. if (auto PS = N->getLocationAs()) if (auto *BO = PS->getStmtAs()) if (BO->isAssignmentOp() && RegionOfInterest->isSubRegionOf( - N->getSVal(BO->getLHS()).getAsRegion())) + N->getSVal(BO->getLHS()).getAsRegion())) return true; // SVal after the state is possibly different. SVal ValueAtN = N->getState()->getSVal(RegionOfInterest); - if (!Mgr.getSValBuilder().areEqual(State, ValueAtN, ValueAfter).isConstrainedTrue() && + if (!Mgr.getSValBuilder() + .areEqual(State, ValueAtN, ValueAfter) + .isConstrainedTrue() && (!ValueAtN.isUndef() || !ValueAfter.isUndef())) return true; return false; } +//===----------------------------------------------------------------------===// +// Implementation of BugReporterVisitor. +//===----------------------------------------------------------------------===// + +std::shared_ptr +BugReporterVisitor::getEndPath(BugReporterContext &, + const ExplodedNode *, BugReport &) { + return nullptr; +} + +void +BugReporterVisitor::finalizeVisitor(BugReporterContext &, + const ExplodedNode *, BugReport &) {} + +std::shared_ptr BugReporterVisitor::getDefaultEndPath( + BugReporterContext &BRC, const ExplodedNode *EndPathNode, BugReport &BR) { + PathDiagnosticLocation L = + PathDiagnosticLocation::createEndOfPath(EndPathNode,BRC.getSourceManager()); + + const auto &Ranges = BR.getRanges(); + + // Only add the statement itself as a range if we didn't specify any + // special ranges for this report. + auto P = std::make_shared( + L, BR.getDescription(), Ranges.begin() == Ranges.end()); + for (SourceRange Range : Ranges) + P->addRange(Range); + + return P; +} + +//===----------------------------------------------------------------------===// +// Implementation of NoStoreFuncVisitor. +//===----------------------------------------------------------------------===// namespace { @@ -311,6 +314,7 @@ llvm::SmallPtrSet FramesModifyingCalculated; using RegionVector = SmallVector; + public: NoStoreFuncVisitor(const SubRegion *R) : RegionOfInterest(R), MmrMgr(*R->getMemRegionManager()), @@ -330,371 +334,409 @@ std::shared_ptr VisitNode(const ExplodedNode *N, BugReporterContext &BR, - BugReport &R) override { + BugReport &R) override; + +private: + /// Attempts to find the region of interest in a given CXX decl, + /// by either following the base classes or fields. + /// Dereferences fields up to a given recursion limit. + /// Note that \p Vec is passed by value, leading to quadratic copying cost, + /// but it's OK in practice since its length is limited to DEREFERENCE_LIMIT. + /// \return A chain fields leading to the region of interest or None. + const Optional + findRegionOfInterestInRecord(const RecordDecl *RD, ProgramStateRef State, + const MemRegion *R, const RegionVector &Vec = {}, + int depth = 0); + /// Check and lazily calculate whether the region of interest is + /// modified in the stack frame to which \p N belongs. + /// The calculation is cached in FramesModifyingRegion. + bool isRegionOfInterestModifiedInFrame(const ExplodedNode *N) { const LocationContext *Ctx = N->getLocationContext(); const StackFrameContext *SCtx = Ctx->getStackFrame(); - ProgramStateRef State = N->getState(); - auto CallExitLoc = N->getLocationAs(); + if (!FramesModifyingCalculated.count(SCtx)) + findModifyingFrames(N); + return FramesModifyingRegion.count(SCtx); + } - // No diagnostic if region was modified inside the frame. - if (!CallExitLoc || isRegionOfInterestModifiedInFrame(N)) - return nullptr; + /// Write to \c FramesModifyingRegion all stack frames along + /// the path in the current stack frame which modify \c RegionOfInterest. + void findModifyingFrames(const ExplodedNode *N); - CallEventRef<> Call = - BR.getStateManager().getCallEventManager().getCaller(SCtx, State); - - // Region of interest corresponds to an IVar, exiting a method - // which could have written into that IVar, but did not. - if (const auto *MC = dyn_cast(Call)) { - if (const auto *IvarR = dyn_cast(RegionOfInterest)) { - const MemRegion *SelfRegion = MC->getReceiverSVal().getAsRegion(); - if (RegionOfInterest->isSubRegionOf(SelfRegion) && - potentiallyWritesIntoIvar(Call->getRuntimeDefinition().getDecl(), - IvarR->getDecl())) - return maybeEmitNote(R, *Call, N, {}, SelfRegion, "self", - /*FirstIsReferenceType=*/false, 1); - } - } + /// Consume the information on the no-store stack frame in order to + /// either emit a note or suppress the report enirely. + /// \return Diagnostics piece for region not modified in the current function, + /// if it decides to emit one. + std::shared_ptr + maybeEmitNote(BugReport &R, const CallEvent &Call, const ExplodedNode *N, + const RegionVector &FieldChain, const MemRegion *MatchedRegion, + StringRef FirstElement, bool FirstIsReferenceType, + unsigned IndirectionLevel); - if (const auto *CCall = dyn_cast(Call)) { - const MemRegion *ThisR = CCall->getCXXThisVal().getAsRegion(); - if (RegionOfInterest->isSubRegionOf(ThisR) - && !CCall->getDecl()->isImplicit()) - return maybeEmitNote(R, *Call, N, {}, ThisR, "this", - /*FirstIsReferenceType=*/false, 1); + /// Pretty-print region \p MatchedRegion to \p os. + /// \return Whether printing succeeded. + bool prettyPrintRegionName(StringRef FirstElement, bool FirstIsReferenceType, + const MemRegion *MatchedRegion, + const RegionVector &FieldChain, + int IndirectionLevel, + llvm::raw_svector_ostream &os); - // Do not generate diagnostics for not modified parameters in - // constructors. - return nullptr; - } + /// Print first item in the chain, return new separator. + static StringRef prettyPrintFirstElement(StringRef FirstElement, + bool MoreItemsExpected, + int IndirectionLevel, + llvm::raw_svector_ostream &os); +}; - ArrayRef parameters = getCallParameters(Call); - for (unsigned I = 0; I < Call->getNumArgs() && I < parameters.size(); ++I) { - const ParmVarDecl *PVD = parameters[I]; - SVal V = Call->getArgSVal(I); - bool ParamIsReferenceType = PVD->getType()->isReferenceType(); - std::string ParamName = PVD->getNameAsString(); - - int IndirectionLevel = 1; - QualType T = PVD->getType(); - while (const MemRegion *MR = V.getAsRegion()) { - if (RegionOfInterest->isSubRegionOf(MR) && !isPointerToConst(T)) - return maybeEmitNote(R, *Call, N, {}, MR, ParamName, - ParamIsReferenceType, IndirectionLevel); +} // end of anonymous namespace - QualType PT = T->getPointeeType(); - if (PT.isNull() || PT->isVoidType()) break; +/// \return Whether the method declaration \p Parent +/// syntactically has a binary operation writing into the ivar \p Ivar. +static bool potentiallyWritesIntoIvar(const Decl *Parent, + const ObjCIvarDecl *Ivar) { + using namespace ast_matchers; + const char *IvarBind = "Ivar"; + if (!Parent || !Parent->hasBody()) + return false; + StatementMatcher WriteIntoIvarM = binaryOperator( + hasOperatorName("="), + hasLHS(ignoringParenImpCasts( + objcIvarRefExpr(hasDeclaration(equalsNode(Ivar))).bind(IvarBind)))); + StatementMatcher ParentM = stmt(hasDescendant(WriteIntoIvarM)); + auto Matches = match(ParentM, *Parent->getBody(), Parent->getASTContext()); + for (BoundNodes &Match : Matches) { + auto IvarRef = Match.getNodeAs(IvarBind); + if (IvarRef->isFreeIvar()) + return true; - if (const RecordDecl *RD = PT->getAsRecordDecl()) - if (auto P = findRegionOfInterestInRecord(RD, State, MR)) - return maybeEmitNote(R, *Call, N, *P, RegionOfInterest, ParamName, - ParamIsReferenceType, IndirectionLevel); + const Expr *Base = IvarRef->getBase(); + if (const auto *ICE = dyn_cast(Base)) + Base = ICE->getSubExpr(); - V = State->getSVal(MR, PT); - T = PT; - IndirectionLevel++; - } - } + if (const auto *DRE = dyn_cast(Base)) + if (const auto *ID = dyn_cast(DRE->getDecl())) + if (ID->getParameterKind() == ImplicitParamDecl::ObjCSelf) + return true; - return nullptr; + return false; } + return false; +} -private: - /// Attempts to find the region of interest in a given CXX decl, - /// by either following the base classes or fields. - /// Dereferences fields up to a given recursion limit. - /// Note that \p Vec is passed by value, leading to quadratic copying cost, - /// but it's OK in practice since its length is limited to DEREFERENCE_LIMIT. - /// \return A chain fields leading to the region of interest or None. - const Optional - findRegionOfInterestInRecord(const RecordDecl *RD, ProgramStateRef State, - const MemRegion *R, - const RegionVector &Vec = {}, - int depth = 0) { +/// Get parameters associated with runtime definition in order +/// to get the correct parameter name. +static ArrayRef getCallParameters(CallEventRef<> Call) { + // Use runtime definition, if available. + RuntimeDefinition RD = Call->getRuntimeDefinition(); + if (const auto *FD = dyn_cast_or_null(RD.getDecl())) + return FD->parameters(); + if (const auto *MD = dyn_cast_or_null(RD.getDecl())) + return MD->parameters(); + + return Call->parameters(); +} + +/// \return whether \p Ty points to a const type, or is a const reference. +static bool isPointerToConst(QualType Ty) { + return !Ty->getPointeeType().isNull() && + Ty->getPointeeType().getCanonicalType().isConstQualified(); +} - if (depth == DEREFERENCE_LIMIT) // Limit the recursion depth. +/// Attempts to find the region of interest in a given CXX decl, +/// by either following the base classes or fields. +/// Dereferences fields up to a given recursion limit. +/// Note that \p Vec is passed by value, leading to quadratic copying cost, +/// but it's OK in practice since its length is limited to DEREFERENCE_LIMIT. +/// \return A chain fields leading to the region of interest or None. +const Optional +NoStoreFuncVisitor::findRegionOfInterestInRecord( + const RecordDecl *RD, ProgramStateRef State, const MemRegion *R, + const NoStoreFuncVisitor::RegionVector &Vec /* = {} */, + int depth /* = 0 */) { + + if (depth == DEREFERENCE_LIMIT) // Limit the recursion depth. + return None; + + if (const auto *RDX = dyn_cast(RD)) + if (!RDX->hasDefinition()) return None; - if (const auto *RDX = dyn_cast(RD)) - if (!RDX->hasDefinition()) - return None; - - // Recursively examine the base classes. - // Note that following base classes does not increase the recursion depth. - if (const auto *RDX = dyn_cast(RD)) - for (const auto II : RDX->bases()) - if (const RecordDecl *RRD = II.getType()->getAsRecordDecl()) - if (auto Out = findRegionOfInterestInRecord(RRD, State, R, Vec, depth)) - return Out; - - for (const FieldDecl *I : RD->fields()) { - QualType FT = I->getType(); - const FieldRegion *FR = MmrMgr.getFieldRegion(I, cast(R)); - const SVal V = State->getSVal(FR); - const MemRegion *VR = V.getAsRegion(); - - RegionVector VecF = Vec; - VecF.push_back(FR); - - if (RegionOfInterest == VR) - return VecF; - - if (const RecordDecl *RRD = FT->getAsRecordDecl()) - if (auto Out = - findRegionOfInterestInRecord(RRD, State, FR, VecF, depth + 1)) + // Recursively examine the base classes. + // Note that following base classes does not increase the recursion depth. + if (const auto *RDX = dyn_cast(RD)) + for (const auto II : RDX->bases()) + if (const RecordDecl *RRD = II.getType()->getAsRecordDecl()) + if (Optional Out = + findRegionOfInterestInRecord(RRD, State, R, Vec, depth)) return Out; - QualType PT = FT->getPointeeType(); - if (PT.isNull() || PT->isVoidType() || !VR) continue; + for (const FieldDecl *I : RD->fields()) { + QualType FT = I->getType(); + const FieldRegion *FR = MmrMgr.getFieldRegion(I, cast(R)); + const SVal V = State->getSVal(FR); + const MemRegion *VR = V.getAsRegion(); - if (const RecordDecl *RRD = PT->getAsRecordDecl()) - if (auto Out = - findRegionOfInterestInRecord(RRD, State, VR, VecF, depth + 1)) - return Out; + RegionVector VecF = Vec; + VecF.push_back(FR); - } + if (RegionOfInterest == VR) + return VecF; - return None; + if (const RecordDecl *RRD = FT->getAsRecordDecl()) + if (auto Out = + findRegionOfInterestInRecord(RRD, State, FR, VecF, depth + 1)) + return Out; + + QualType PT = FT->getPointeeType(); + if (PT.isNull() || PT->isVoidType() || !VR) + continue; + + if (const RecordDecl *RRD = PT->getAsRecordDecl()) + if (Optional Out = + findRegionOfInterestInRecord(RRD, State, VR, VecF, depth + 1)) + return Out; } - /// \return Whether the method declaration \p Parent - /// syntactically has a binary operation writing into the ivar \p Ivar. - bool potentiallyWritesIntoIvar(const Decl *Parent, - const ObjCIvarDecl *Ivar) { - using namespace ast_matchers; - const char * IvarBind = "Ivar"; - if (!Parent || !Parent->hasBody()) - return false; - StatementMatcher WriteIntoIvarM = binaryOperator( - hasOperatorName("="), - hasLHS(ignoringParenImpCasts( - objcIvarRefExpr(hasDeclaration(equalsNode(Ivar))).bind(IvarBind)))); - StatementMatcher ParentM = stmt(hasDescendant(WriteIntoIvarM)); - auto Matches = match(ParentM, *Parent->getBody(), Parent->getASTContext()); - for (BoundNodes &Match : Matches) { - auto IvarRef = Match.getNodeAs(IvarBind); - if (IvarRef->isFreeIvar()) - return true; + return None; +} - const Expr *Base = IvarRef->getBase(); - if (const auto *ICE = dyn_cast(Base)) - Base = ICE->getSubExpr(); +std::shared_ptr +NoStoreFuncVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BR, + BugReport &R) { - if (const auto *DRE = dyn_cast(Base)) - if (const auto *ID = dyn_cast(DRE->getDecl())) - if (ID->getParameterKind() == ImplicitParamDecl::ObjCSelf) - return true; + const LocationContext *Ctx = N->getLocationContext(); + const StackFrameContext *SCtx = Ctx->getStackFrame(); + ProgramStateRef State = N->getState(); + auto CallExitLoc = N->getLocationAs(); - return false; - } - return false; - } + // No diagnostic if region was modified inside the frame. + if (!CallExitLoc || isRegionOfInterestModifiedInFrame(N)) + return nullptr; - /// Check and lazily calculate whether the region of interest is - /// modified in the stack frame to which \p N belongs. - /// The calculation is cached in FramesModifyingRegion. - bool isRegionOfInterestModifiedInFrame(const ExplodedNode *N) { - const LocationContext *Ctx = N->getLocationContext(); - const StackFrameContext *SCtx = Ctx->getStackFrame(); - if (!FramesModifyingCalculated.count(SCtx)) - findModifyingFrames(N); - return FramesModifyingRegion.count(SCtx); + CallEventRef<> Call = + BR.getStateManager().getCallEventManager().getCaller(SCtx, State); + + // Region of interest corresponds to an IVar, exiting a method + // which could have written into that IVar, but did not. + if (const auto *MC = dyn_cast(Call)) { + if (const auto *IvarR = dyn_cast(RegionOfInterest)) { + const MemRegion *SelfRegion = MC->getReceiverSVal().getAsRegion(); + if (RegionOfInterest->isSubRegionOf(SelfRegion) && + potentiallyWritesIntoIvar(Call->getRuntimeDefinition().getDecl(), + IvarR->getDecl())) + return maybeEmitNote(R, *Call, N, {}, SelfRegion, "self", + /*FirstIsReferenceType=*/false, 1); + } } + if (const auto *CCall = dyn_cast(Call)) { + const MemRegion *ThisR = CCall->getCXXThisVal().getAsRegion(); + if (RegionOfInterest->isSubRegionOf(ThisR) && + !CCall->getDecl()->isImplicit()) + return maybeEmitNote(R, *Call, N, {}, ThisR, "this", + /*FirstIsReferenceType=*/false, 1); - /// Write to \c FramesModifyingRegion all stack frames along - /// the path in the current stack frame which modify \c RegionOfInterest. - void findModifyingFrames(const ExplodedNode *N) { - assert(N->getLocationAs()); - ProgramStateRef LastReturnState = N->getState(); - SVal ValueAtReturn = LastReturnState->getSVal(RegionOfInterest); - const LocationContext *Ctx = N->getLocationContext(); - const StackFrameContext *OriginalSCtx = Ctx->getStackFrame(); + // Do not generate diagnostics for not modified parameters in + // constructors. + return nullptr; + } - do { - ProgramStateRef State = N->getState(); - auto CallExitLoc = N->getLocationAs(); - if (CallExitLoc) { - LastReturnState = State; - ValueAtReturn = LastReturnState->getSVal(RegionOfInterest); - } + ArrayRef parameters = getCallParameters(Call); + for (unsigned I = 0; I < Call->getNumArgs() && I < parameters.size(); ++I) { + const ParmVarDecl *PVD = parameters[I]; + SVal V = Call->getArgSVal(I); + bool ParamIsReferenceType = PVD->getType()->isReferenceType(); + std::string ParamName = PVD->getNameAsString(); - FramesModifyingCalculated.insert( - N->getLocationContext()->getStackFrame()); + int IndirectionLevel = 1; + QualType T = PVD->getType(); + while (const MemRegion *MR = V.getAsRegion()) { + if (RegionOfInterest->isSubRegionOf(MR) && !isPointerToConst(T)) + return maybeEmitNote(R, *Call, N, {}, MR, ParamName, + ParamIsReferenceType, IndirectionLevel); - if (wasRegionOfInterestModifiedAt(RegionOfInterest, N, ValueAtReturn)) { - const StackFrameContext *SCtx = N->getStackFrame(); - while (!SCtx->inTopFrame()) { - auto p = FramesModifyingRegion.insert(SCtx); - if (!p.second) - break; // Frame and all its parents already inserted. - SCtx = SCtx->getParent()->getStackFrame(); - } - } + QualType PT = T->getPointeeType(); + if (PT.isNull() || PT->isVoidType()) + break; - // Stop calculation at the call to the current function. - if (auto CE = N->getLocationAs()) - if (CE->getCalleeContext() == OriginalSCtx) - break; + if (const RecordDecl *RD = PT->getAsRecordDecl()) + if (Optional P = + findRegionOfInterestInRecord(RD, State, MR)) + return maybeEmitNote(R, *Call, N, *P, RegionOfInterest, ParamName, + ParamIsReferenceType, IndirectionLevel); - N = N->getFirstPred(); - } while (N); + V = State->getSVal(MR, PT); + T = PT; + IndirectionLevel++; + } } - /// Get parameters associated with runtime definition in order - /// to get the correct parameter name. - ArrayRef getCallParameters(CallEventRef<> Call) { - // Use runtime definition, if available. - RuntimeDefinition RD = Call->getRuntimeDefinition(); - if (const auto *FD = dyn_cast_or_null(RD.getDecl())) - return FD->parameters(); - if (const auto *MD = dyn_cast_or_null(RD.getDecl())) - return MD->parameters(); - - return Call->parameters(); - } + return nullptr; +} - /// \return whether \p Ty points to a const type, or is a const reference. - bool isPointerToConst(QualType Ty) { - return !Ty->getPointeeType().isNull() && - Ty->getPointeeType().getCanonicalType().isConstQualified(); - } +void NoStoreFuncVisitor::findModifyingFrames(const ExplodedNode *N) { + assert(N->getLocationAs()); + ProgramStateRef LastReturnState = N->getState(); + SVal ValueAtReturn = LastReturnState->getSVal(RegionOfInterest); + const LocationContext *Ctx = N->getLocationContext(); + const StackFrameContext *OriginalSCtx = Ctx->getStackFrame(); - /// Consume the information on the no-store stack frame in order to - /// either emit a note or suppress the report enirely. - /// \return Diagnostics piece for region not modified in the current function, - /// if it decides to emit one. - std::shared_ptr - maybeEmitNote(BugReport &R, const CallEvent &Call, const ExplodedNode *N, - const RegionVector &FieldChain, const MemRegion *MatchedRegion, - StringRef FirstElement, bool FirstIsReferenceType, - unsigned IndirectionLevel) { - // Optimistically suppress uninitialized value bugs that result - // from system headers having a chance to initialize the value - // but failing to do so. It's too unlikely a system header's fault. - // It's much more likely a situation in which the function has a failure - // mode that the user decided not to check. If we want to hunt such - // omitted checks, we should provide an explicit function-specific note - // describing the precondition under which the function isn't supposed to - // initialize its out-parameter, and additionally check that such - // precondition can actually be fulfilled on the current path. - if (Call.isInSystemHeader()) { - // We make an exception for system header functions that have no branches. - // Such functions unconditionally fail to initialize the variable. - // If they call other functions that have more paths within them, - // this suppression would still apply when we visit these inner functions. - // One common example of a standard function that doesn't ever initialize - // its out parameter is operator placement new; it's up to the follow-up - // constructor (if any) to initialize the memory. - if (!N->getStackFrame()->getCFG()->isLinear()) - R.markInvalid(getTag(), nullptr); - return nullptr; + do { + ProgramStateRef State = N->getState(); + auto CallExitLoc = N->getLocationAs(); + if (CallExitLoc) { + LastReturnState = State; + ValueAtReturn = LastReturnState->getSVal(RegionOfInterest); } - PathDiagnosticLocation L = - PathDiagnosticLocation::create(N->getLocation(), SM); + FramesModifyingCalculated.insert(N->getLocationContext()->getStackFrame()); - // For now this shouldn't trigger, but once it does (as we add more - // functions to the body farm), we'll need to decide if these reports - // are worth suppressing as well. - if (!L.hasValidLocation()) - return nullptr; + if (wasRegionOfInterestModifiedAt(RegionOfInterest, N, ValueAtReturn)) { + const StackFrameContext *SCtx = N->getStackFrame(); + while (!SCtx->inTopFrame()) { + auto p = FramesModifyingRegion.insert(SCtx); + if (!p.second) + break; // Frame and all its parents already inserted. + SCtx = SCtx->getParent()->getStackFrame(); + } + } - SmallString<256> sbuf; - llvm::raw_svector_ostream os(sbuf); - os << "Returning without writing to '"; + // Stop calculation at the call to the current function. + if (auto CE = N->getLocationAs()) + if (CE->getCalleeContext() == OriginalSCtx) + break; - // Do not generate the note if failed to pretty-print. - if (!prettyPrintRegionName(FirstElement, FirstIsReferenceType, - MatchedRegion, FieldChain, IndirectionLevel, os)) - return nullptr; + N = N->getFirstPred(); + } while (N); +} - os << "'"; - return std::make_shared(L, os.str()); +std::shared_ptr NoStoreFuncVisitor::maybeEmitNote( + BugReport &R, const CallEvent &Call, const ExplodedNode *N, + const RegionVector &FieldChain, const MemRegion *MatchedRegion, + StringRef FirstElement, bool FirstIsReferenceType, + unsigned IndirectionLevel) { + // Optimistically suppress uninitialized value bugs that result + // from system headers having a chance to initialize the value + // but failing to do so. It's too unlikely a system header's fault. + // It's much more likely a situation in which the function has a failure + // mode that the user decided not to check. If we want to hunt such + // omitted checks, we should provide an explicit function-specific note + // describing the precondition under which the function isn't supposed to + // initialize its out-parameter, and additionally check that such + // precondition can actually be fulfilled on the current path. + if (Call.isInSystemHeader()) { + // We make an exception for system header functions that have no branches. + // Such functions unconditionally fail to initialize the variable. + // If they call other functions that have more paths within them, + // this suppression would still apply when we visit these inner functions. + // One common example of a standard function that doesn't ever initialize + // its out parameter is operator placement new; it's up to the follow-up + // constructor (if any) to initialize the memory. + if (!N->getStackFrame()->getCFG()->isLinear()) + R.markInvalid(getTag(), nullptr); + return nullptr; } - /// Pretty-print region \p MatchedRegion to \p os. - /// \return Whether printing succeeded. - bool prettyPrintRegionName(StringRef FirstElement, bool FirstIsReferenceType, - const MemRegion *MatchedRegion, - const RegionVector &FieldChain, - int IndirectionLevel, - llvm::raw_svector_ostream &os) { + PathDiagnosticLocation L = + PathDiagnosticLocation::create(N->getLocation(), SM); - if (FirstIsReferenceType) - IndirectionLevel--; + // For now this shouldn't trigger, but once it does (as we add more + // functions to the body farm), we'll need to decide if these reports + // are worth suppressing as well. + if (!L.hasValidLocation()) + return nullptr; - RegionVector RegionSequence; + SmallString<256> sbuf; + llvm::raw_svector_ostream os(sbuf); + os << "Returning without writing to '"; - // Add the regions in the reverse order, then reverse the resulting array. - assert(RegionOfInterest->isSubRegionOf(MatchedRegion)); - const MemRegion *R = RegionOfInterest; - while (R != MatchedRegion) { - RegionSequence.push_back(R); - R = cast(R)->getSuperRegion(); - } - std::reverse(RegionSequence.begin(), RegionSequence.end()); - RegionSequence.append(FieldChain.begin(), FieldChain.end()); + // Do not generate the note if failed to pretty-print. + if (!prettyPrintRegionName(FirstElement, FirstIsReferenceType, MatchedRegion, + FieldChain, IndirectionLevel, os)) + return nullptr; - StringRef Sep; - for (const MemRegion *R : RegionSequence) { + os << "'"; + return std::make_shared(L, os.str()); +} - // Just keep going up to the base region. - // Element regions may appear due to casts. - if (isa(R) || isa(R)) - continue; +bool NoStoreFuncVisitor::prettyPrintRegionName(StringRef FirstElement, + bool FirstIsReferenceType, + const MemRegion *MatchedRegion, + const RegionVector &FieldChain, + int IndirectionLevel, + llvm::raw_svector_ostream &os) { - if (Sep.empty()) - Sep = prettyPrintFirstElement(FirstElement, - /*MoreItemsExpected=*/true, - IndirectionLevel, os); + if (FirstIsReferenceType) + IndirectionLevel--; - os << Sep; + RegionVector RegionSequence; - // Can only reasonably pretty-print DeclRegions. - if (!isa(R)) - return false; + // Add the regions in the reverse order, then reverse the resulting array. + assert(RegionOfInterest->isSubRegionOf(MatchedRegion)); + const MemRegion *R = RegionOfInterest; + while (R != MatchedRegion) { + RegionSequence.push_back(R); + R = cast(R)->getSuperRegion(); + } + std::reverse(RegionSequence.begin(), RegionSequence.end()); + RegionSequence.append(FieldChain.begin(), FieldChain.end()); - const auto *DR = cast(R); - Sep = DR->getValueType()->isAnyPointerType() ? "->" : "."; - DR->getDecl()->getDeclName().print(os, PP); - } + StringRef Sep; + for (const MemRegion *R : RegionSequence) { + + // Just keep going up to the base region. + // Element regions may appear due to casts. + if (isa(R) || isa(R)) + continue; if (Sep.empty()) - prettyPrintFirstElement(FirstElement, - /*MoreItemsExpected=*/false, IndirectionLevel, - os); - return true; - } + Sep = prettyPrintFirstElement(FirstElement, + /*MoreItemsExpected=*/true, + IndirectionLevel, os); - /// Print first item in the chain, return new separator. - StringRef prettyPrintFirstElement(StringRef FirstElement, - bool MoreItemsExpected, - int IndirectionLevel, - llvm::raw_svector_ostream &os) { - StringRef Out = "."; - - if (IndirectionLevel > 0 && MoreItemsExpected) { - IndirectionLevel--; - Out = "->"; - } + os << Sep; - if (IndirectionLevel > 0 && MoreItemsExpected) - os << "("; + // Can only reasonably pretty-print DeclRegions. + if (!isa(R)) + return false; - for (int i=0; i(R); + Sep = DR->getValueType()->isAnyPointerType() ? "->" : "."; + DR->getDecl()->getDeclName().print(os, PP); + } - if (IndirectionLevel > 0 && MoreItemsExpected) - os << ")"; + if (Sep.empty()) + prettyPrintFirstElement(FirstElement, + /*MoreItemsExpected=*/false, IndirectionLevel, os); + return true; +} - return Out; +StringRef NoStoreFuncVisitor::prettyPrintFirstElement( + StringRef FirstElement, bool MoreItemsExpected, int IndirectionLevel, + llvm::raw_svector_ostream &os) { + StringRef Out = "."; + + if (IndirectionLevel > 0 && MoreItemsExpected) { + IndirectionLevel--; + Out = "->"; } -}; -} // end of anonymous namespace + if (IndirectionLevel > 0 && MoreItemsExpected) + os << "("; + + for (int i = 0; i < IndirectionLevel; i++) + os << "*"; + os << FirstElement; + + if (IndirectionLevel > 0 && MoreItemsExpected) + os << ")"; + + return Out; +} + +//===----------------------------------------------------------------------===// +// Implementation of MacroNullReturnSuppressionVisitor. +//===----------------------------------------------------------------------===// namespace {