spaits created this revision. spaits added reviewers: NoQ, steakhal, Szelethus. Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: All. spaits requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
As discussed in my previous patch (D142354 <https://reviews.llvm.org/D142354>) the analyzer can understand std::variant and std::any through regular inlining. However warnings coming from these classes were suppressed by NoStateChangeFuncVisitor, more specificly by an instance of NoStoreFuncVisitor. The reason behind that is that we suppress bug reports when a system header function was supposed to initialize its parameter but failed to (as you can read about it in a comment block visible in this patch). Now the problem is that these bug report were suppressed, but they had nothing to do with uninitialized values. In a follow up patch I intend to fix that and see how this suppression logic works as it was meant to be, but my initial testing shows that they no longer falsely suppress reports like this: std::variant<int, std::string> v = 0; int i = std::get<int>(v); 10/i; // FIXME: This should warn for division by zero! In this patch I only separated these two functionalities because I don't think this suppression should be implemented in a NoStateChangeVisitor but in it's own. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D145069 Files: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -452,6 +452,58 @@ CallEventRef<> Call = BR.getStateManager().getCallEventManager().getCaller(SCtx, State); + if (Call->isInSystemHeader()) + return nullptr; + + if (const auto *MC = dyn_cast<ObjCMethodCall>(Call)) { + // If we failed to construct a piece for self, we still want to check + // whether the entity of interest is in a parameter. + if (PathDiagnosticPieceRef Piece = maybeEmitNoteForObjCSelf(R, *MC, N)) + return Piece; + } + + if (const auto *CCall = dyn_cast<CXXConstructorCall>(Call)) { + // Do not generate diagnostics for not modified parameters in + // constructors. + return maybeEmitNoteForCXXThis(R, *CCall, N); + } + + return maybeEmitNoteForParameters(R, *Call, N); +} + +//===----------------------------------------------------------------------===// +// Implementation of SuppressSystemHeaderWarningVistor. +//===----------------------------------------------------------------------===// + +namespace { +class SuppressSystemHeaderWarningVisitor : public BugReporterVisitor { +public: + virtual PathDiagnosticPieceRef VisitNode(const ExplodedNode *, + BugReporterContext &, + PathSensitiveBugReport &) override; + void Profile(llvm::FoldingSetNodeID &ID) const override { + static int Tag = 0; + ID.AddPointer(&Tag); + } +}; +} // namespace + +PathDiagnosticPieceRef +SuppressSystemHeaderWarningVisitor::VisitNode(const ExplodedNode *Succ, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) { + const LocationContext *Ctx = Succ->getLocationContext(); + const StackFrameContext *SCtx = Ctx->getStackFrame(); + ProgramStateRef State = Succ->getState(); + auto CallExitLoc = Succ->getLocationAs<CallExitBegin>(); + + // No diagnostic if region was modified inside the frame. + if (!CallExitLoc) + return nullptr; + + CallEventRef<> Call = + BRC.getStateManager().getCallEventManager().getCaller(SCtx, State); + // 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. @@ -469,27 +521,13 @@ // 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()) { + + if (!Succ->getStackFrame()->getCFG()->isLinear()) { static int i = 0; - R.markInvalid(&i, nullptr); + BR.markInvalid(&i, nullptr); } - return nullptr; - } - - if (const auto *MC = dyn_cast<ObjCMethodCall>(Call)) { - // If we failed to construct a piece for self, we still want to check - // whether the entity of interest is in a parameter. - if (PathDiagnosticPieceRef Piece = maybeEmitNoteForObjCSelf(R, *MC, N)) - return Piece; - } - - if (const auto *CCall = dyn_cast<CXXConstructorCall>(Call)) { - // Do not generate diagnostics for not modified parameters in - // constructors. - return maybeEmitNoteForCXXThis(R, *CCall, N); } - - return maybeEmitNoteForParameters(R, *Call, N); + return nullptr; } //===----------------------------------------------------------------------===// @@ -2350,7 +2388,7 @@ // Mark both the variable region and its contents as interesting. SVal V = LVState->getRawSVal(loc::MemRegionVal(R)); Report.addVisitor<NoStoreFuncVisitor>(cast<SubRegion>(R), Opts.Kind); - + Report.addVisitor<SuppressSystemHeaderWarningVisitor>(); // When we got here, we do have something to track, and we will // interrupt. Result.FoundSomethingToTrack = true;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits