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 @@ -1,4 +1,4 @@ -//===- BugReporterVisitors.h - Generate PathDiagnostics ---------*- C++ -*-===// +//===- BugReporterVisitors.h - Bug explaining and suppression ---*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -6,8 +6,9 @@ // //===----------------------------------------------------------------------===// // -// This file declares BugReporterVisitors, which are used to generate enhanced -// diagnostic traces. +/// \file +/// This file declares BugReporterVisitors, which are used to generate enhanced +/// diagnostic traces. They are also useful for suppressing false positives. // //===----------------------------------------------------------------------===// @@ -40,7 +41,8 @@ class PathDiagnosticPiece; using PathDiagnosticPieceRef = std::shared_ptr; -/// BugReporterVisitors are used to add custom diagnostics along a path. +/// BugReporterVisitors are used to add custom diagnostics along a \emoji bug +/// path. They also could serve false positive suppression. class BugReporterVisitor : public llvm::FoldingSetNode { public: BugReporterVisitor() = default; @@ -48,41 +50,53 @@ BugReporterVisitor(BugReporterVisitor &&) {} virtual ~BugReporterVisitor(); - /// Return a diagnostic piece which should be associated with the - /// given node. - /// Note that this function does *not* get run on the very last node - /// of the report, as the PathDiagnosticPiece associated with the - /// last node should be unique. - /// Use {@code getEndPath} to customize the note associated with the report - /// end instead. + /// \returns A diagnostic piece which should be associated with the + /// given node \p N. /// - /// The last parameter can be used to register a new visitor with the given + /// \note This function does *not* get run on the very last node of the + /// report, as the PathDiagnosticPiece associated with the last node should + /// be unique. Use getEndPath() to customize the note associated with the end + /// of the report instead. + /// + /// \note The \p BR can be used to register a new visitor with the given /// BugReport while processing a node. - virtual PathDiagnosticPieceRef VisitNode(const ExplodedNode *Succ, + virtual PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &BR) = 0; - /// Last function called on the visitor, no further calls to VisitNode - /// would follow. + /// Last function called on the visitor, no further calls to + /// BugReporterVisitor::VisitNode() would follow. virtual void finalizeVisitor(BugReporterContext &BRC, const ExplodedNode *EndPathNode, PathSensitiveBugReport &BR); - /// Provide custom definition for the final diagnostic piece on the + /// Provides a custom definition for the final diagnostic piece on the /// path - the piece, which is displayed before the path is expanded. /// - /// NOTE that this function can be implemented on at most one used visitor, - /// and otherwise it crahes at runtime. + /// \returns The last diagnostic piece. + /// + /// \note This function can be implemented on at most one used visitor, + /// and otherwise it crashes at runtime. virtual PathDiagnosticPieceRef getEndPath(BugReporterContext &BRC, const ExplodedNode *N, PathSensitiveBugReport &BR); - virtual void Profile(llvm::FoldingSetNodeID &ID) const = 0; - /// Generates the default final diagnostic piece. static PathDiagnosticPieceRef getDefaultEndPath(const BugReporterContext &BRC, const ExplodedNode *N, const PathSensitiveBugReport &BR); + + /// The tag helps to determine which visitor created a PathDiagnosticPiece. + /// + /// \returns The unique tag. + /// + /// \note Every BugReporterVisitor needs to implement this method. + static const char *getTag() { return "None"; } + + /// The usual Profile() method for FoldingSet. + /// + /// \sa https://llvm.org/doxygen/FoldingSet_8h_source.html + virtual void Profile(llvm::FoldingSetNodeID &ID) const = 0; }; namespace bugreporter { @@ -102,33 +116,52 @@ /// Attempts to add visitors to track expression value back to its point of /// origin. /// -/// \param N A node "downstream" from the evaluation of the statement. -/// \param E The expression value which we are tracking +/// \param N The error node (CheckerContext::generateErrorNode()). +/// \param E The expression value which we are tracking. /// \param R The bug report to which visitors should be attached. /// \param EnableNullFPSuppression Whether we should employ false positive -/// suppression (inlined defensive checks, returned null). +/// suppression (inlined defensive checks, returned null). +/// +/// \returns Whether the function was able to add visitors for the expression +/// \p E. +/// +/// \note Returning \c true does not actually imply that any visitors were +/// added. /// -/// \return Whether or not the function was able to add visitors for this -/// statement. Note that returning \c true does not actually imply -/// that any visitors were added. +/// \note At the moment this function is imprecise and may could not point +/// out where the value originates from. bool trackExpressionValue(const ExplodedNode *N, const Expr *E, PathSensitiveBugReport &R, TrackingKind TKind = TrackingKind::Thorough, bool EnableNullFPSuppression = true); +/// Given that statement \p S represents a pointer that would be dereferenced, +/// try to find a sub-expression from which the pointer came from. +/// This is used for tracking down origins of a null or undefined value: +/// "this is null because that is null because that is null" etc. +/// We wipe away field and element offsets because they merely add offsets. +/// We also wipe away all casts except lvalue-to-rvalue casts, because the +/// latter represent an actual pointer dereference; however, we remove +/// the final lvalue-to-rvalue cast before returning from this function +/// because it demonstrates more clearly from where the pointer rvalue was +/// loaded. Examples: +/// ``` +/// x->y.z ==> x // (lvalue) +/// foo()->y.z ==> foo() // (rvalue) +/// ``` const Expr *getDerefExpr(const Stmt *S); } // namespace bugreporter -/// Finds last store into the given region, -/// which is different from a given symbolic value. +/// Finds last store into the given region, which is different from the given +/// symbolic value. class FindLastStoreBRVisitor final : public BugReporterVisitor { const MemRegion *R; SVal V; bool Satisfied = false; - /// If the visitor is tracking the value directly responsible for the - /// bug, we are going to employ false positive suppression. + // If the visitor is tracking the value directly responsible for the + // bug, we are going to employ false positive suppression. bool EnableNullFPSuppression; using TrackingKind = bugreporter::TrackingKind; @@ -136,8 +169,9 @@ const StackFrameContext *OriginSFC; public: - /// \param V We're searching for the store where \c R received this value. - /// \param R The region we're tracking. + /// \param V We are searching for the store where \c R received this value. + /// \param InEnableNullFPSuppression Whether to suppress false positives. + /// \param R The region we are tracking. /// \param TKind May limit the amount of notes added to the bug report. /// \param OriginSFC Only adds notes when the last store happened in a /// different stackframe to this one. Disregarded if the tracking kind @@ -154,11 +188,19 @@ assert(R); } - void Profile(llvm::FoldingSetNodeID &ID) const override; - PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &BR) override; + + static const char *getTag() { return "FindLastStoreBRVisitor"; } + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.AddPointer(getTag()); + ID.AddPointer(R); + ID.Add(V); + ID.AddInteger(static_cast(TKind)); + ID.AddBoolean(EnableNullFPSuppression); + } }; class TrackConstraintBRVisitor final : public BugReporterVisitor { @@ -167,8 +209,8 @@ bool IsSatisfied = false; bool IsZeroCheck; - /// We should start tracking from the last node along the path in which the - /// value is constrained. + // We should start tracking from the last node along the path in which the + // value is constrained. bool IsTrackingTurnedOn = false; public: @@ -176,57 +218,52 @@ : Constraint(constraint), Assumption(assumption), IsZeroCheck(!Assumption && Constraint.getAs()) {} - void Profile(llvm::FoldingSetNodeID &ID) const override; - - /// Return the tag associated with this visitor. This tag will be used - /// to make all PathDiagnosticPieces created by this visitor. - static const char *getTag(); - PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &BR) override; + static const char *getTag() { return "TrackConstraintBRVisitor"; } + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.AddPointer(getTag()); + ID.Add(Constraint); + ID.AddBoolean(Assumption); + ID.AddBoolean(IsSatisfied); + ID.AddBoolean(IsZeroCheck); + ID.AddBoolean(IsTrackingTurnedOn); + } + private: - /// Checks if the constraint is valid in the current state. + // Checks if the constraint is valid in the current state. bool isUnderconstrained(const ExplodedNode *N) const; }; -/// \class NilReceiverBRVisitor /// Prints path notes when a message is sent to a nil receiver. class NilReceiverBRVisitor final : public BugReporterVisitor { public: void Profile(llvm::FoldingSetNodeID &ID) const override { - static int x = 0; - ID.AddPointer(&x); + ID.AddPointer(getTag()); } + static const char *getTag() { return "NilReceiverBRVisitor"; } + PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &BR) override; - /// If the statement is a message send expression with nil receiver, returns - /// the receiver expression. Returns NULL otherwise. + /// \returns The receiver expression if the statement is a message send + /// expression with nil receiver or nullptr otherwise. static const Expr *getNilReceiver(const Stmt *S, const ExplodedNode *N); }; /// Visitor that tries to report interesting diagnostics from conditions. class ConditionBRVisitor final : public BugReporterVisitor { - // FIXME: constexpr initialization isn't supported by MSVC2013. - constexpr static llvm::StringLiteral GenericTrueMessage = + static constexpr llvm::StringLiteral GenericTrueMessage = "Assuming the condition is true"; - constexpr static llvm::StringLiteral GenericFalseMessage = + static constexpr llvm::StringLiteral GenericFalseMessage = "Assuming the condition is false"; public: - void Profile(llvm::FoldingSetNodeID &ID) const override { - static int x = 0; - ID.AddPointer(&x); - } - - /// Return the tag associated with this visitor. This tag will be used - /// to make all PathDiagnosticPieces created by this visitor. - static const char *getTag(); - PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &BR) override; @@ -289,6 +326,12 @@ bool IsSameFieldName); static bool isPieceMessageGeneric(const PathDiagnosticPiece *Piece); + + static const char *getTag() { return "ConditionBRVisitor"; } + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.AddPointer(getTag()); + } }; /// Suppress reports that might lead to known false positives. @@ -297,9 +340,8 @@ class LikelyFalsePositiveSuppressionBRVisitor final : public BugReporterVisitor { public: - static void *getTag() { - static int Tag = 0; - return static_cast(&Tag); + static const char *getTag() { + return "LikelyFalsePositiveSuppressionBRVisitor"; } void Profile(llvm::FoldingSetNodeID &ID) const override { @@ -327,9 +369,10 @@ public: UndefOrNullArgVisitor(const MemRegion *InR) : R(InR) {} + static const char *getTag() { return "UndefOrNullArgVisitor"; } + void Profile(llvm::FoldingSetNodeID &ID) const override { - static int Tag = 0; - ID.AddPointer(&Tag); + ID.AddPointer(getTag()); ID.AddPointer(R); } @@ -339,64 +382,73 @@ }; class SuppressInlineDefensiveChecksVisitor final : public BugReporterVisitor { - /// The symbolic value for which we are tracking constraints. - /// This value is constrained to null in the end of path. + // The symbolic value for which we are tracking constraints. + // This value is constrained to null in the end of path. DefinedSVal V; - /// Track if we found the node where the constraint was first added. + // Track if we found the node where the constraint was first added. bool IsSatisfied = false; - /// Since the visitors can be registered on nodes previous to the last - /// node in the BugReport, but the path traversal always starts with the last - /// node, the visitor invariant (that we start with a node in which V is null) - /// might not hold when node visitation starts. We are going to start tracking - /// from the last node in which the value is null. + // Since the visitors can be registered on nodes previous to the last + // node in the BugReport, but the path traversal always starts with the last + // node, the visitor invariant (that we start with a node in which V is null) + // might not hold when node visitation starts. We are going to start tracking + // from the last node in which the value is null. bool IsTrackingTurnedOn = false; public: SuppressInlineDefensiveChecksVisitor(DefinedSVal Val, const ExplodedNode *N); - void Profile(llvm::FoldingSetNodeID &ID) const override; - - /// Return the tag associated with this visitor. This tag will be used - /// to make all PathDiagnosticPieces created by this visitor. - static const char *getTag(); - PathDiagnosticPieceRef VisitNode(const ExplodedNode *Succ, BugReporterContext &BRC, PathSensitiveBugReport &BR) override; + + static const char *getTag() { return "SuppressInlineDefensiveChecksVisitor"; } + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.AddPointer(getTag()); + ID.Add(V); + } }; /// The bug visitor will walk all the nodes in a path and collect all the /// constraints. When it reaches the root node, will create a refutation /// manager and check if the constraints are satisfiable class FalsePositiveRefutationBRVisitor final : public BugReporterVisitor { -private: /// Holds the constraints in a given path ConstraintRangeTy Constraints; public: FalsePositiveRefutationBRVisitor(); - void Profile(llvm::FoldingSetNodeID &ID) const override; - PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &BR) override; void finalizeVisitor(BugReporterContext &BRC, const ExplodedNode *EndPathNode, PathSensitiveBugReport &BR) override; + + static const char *getTag() { return "FalsePositiveRefutationBRVisitor"; } + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.AddPointer(getTag()); + ID.Add(Constraints); + } }; /// The visitor detects NoteTags and displays the event notes they contain. class TagVisitor : public BugReporterVisitor { public: - void Profile(llvm::FoldingSetNodeID &ID) const override; - PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &R) override; + + static const char *getTag() { return "TagVisitor"; } + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.AddPointer(getTag()); + } }; } // namespace ento Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -1,15 +1,10 @@ -//===- BugReporterVisitors.cpp - Helpers for reporting bugs ---------------===// +//===- BugReporterVisitors.cpp - Bug explaining and suppression -*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // //===----------------------------------------------------------------------===// -// -// This file defines a set of BugReporter "visitors" which can be used to -// enhance the diagnostics reported for a bug. -// -//===----------------------------------------------------------------------===// #include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h" #include "clang/AST/ASTContext.h" @@ -82,18 +77,6 @@ return nullptr; } -/// Given that expression S represents a pointer that would be dereferenced, -/// try to find a sub-expression from which the pointer came from. -/// This is used for tracking down origins of a null or undefined value: -/// "this is null because that is null because that is null" etc. -/// We wipe away field and element offsets because they merely add offsets. -/// We also wipe away all casts except lvalue-to-rvalue casts, because the -/// latter represent an actual pointer dereference; however, we remove -/// the final lvalue-to-rvalue cast before returning from this function -/// because it demonstrates more clearly from where the pointer rvalue was -/// loaded. Examples: -/// x->y.z ==> x (lvalue) -/// foo()->y.z ==> foo() (rvalue) const Expr *bugreporter::getDerefExpr(const Stmt *S) { const auto *E = dyn_cast(S); if (!E) @@ -362,21 +345,17 @@ SM(MmrMgr.getContext().getSourceManager()), PP(MmrMgr.getContext().getPrintingPolicy()), TKind(TKind) {} - void Profile(llvm::FoldingSetNodeID &ID) const override { - static int Tag = 0; - ID.AddPointer(&Tag); - ID.AddPointer(RegionOfInterest); - } - - void *getTag() const { - static int Tag = 0; - return static_cast(&Tag); - } - PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, BugReporterContext &BR, PathSensitiveBugReport &R) override; + static const char *getTag() { return "NoStoreFuncVisitor"; } + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.AddPointer(getTag()); + ID.AddPointer(RegionOfInterest); + } + private: /// Attempts to find the region of interest in a given record decl, /// by either following the base classes or fields. @@ -837,10 +816,7 @@ R->getAs(), V)); } - void* getTag() const { - static int Tag = 0; - return static_cast(&Tag); - } + static const char *getTag() { return "MacroNullReturnSuppressionVisitor"; } void Profile(llvm::FoldingSetNodeID &ID) const override { ID.AddPointer(getTag()); @@ -903,13 +879,10 @@ : CalleeSFC(Frame), EnableNullFPSuppression(Suppressed), Options(Options), TKind(TKind) {} - static void *getTag() { - static int Tag = 0; - return static_cast(&Tag); - } + static const char *getTag() { return "ReturnVisitor"; } void Profile(llvm::FoldingSetNodeID &ID) const override { - ID.AddPointer(ReturnVisitor::getTag()); + ID.AddPointer(getTag()); ID.AddPointer(CalleeSFC); ID.AddBoolean(EnableNullFPSuppression); } @@ -1192,7 +1165,7 @@ void finalizeVisitor(BugReporterContext &, const ExplodedNode *, PathSensitiveBugReport &BR) override { if (EnableNullFPSuppression && ShouldInvalidate) - BR.markInvalid(ReturnVisitor::getTag(), CalleeSFC); + BR.markInvalid(getTag(), CalleeSFC); } }; @@ -1202,15 +1175,6 @@ // Implementation of FindLastStoreBRVisitor. //===----------------------------------------------------------------------===// -void FindLastStoreBRVisitor::Profile(llvm::FoldingSetNodeID &ID) const { - static int tag = 0; - ID.AddPointer(&tag); - ID.AddPointer(R); - ID.Add(V); - ID.AddInteger(static_cast(TKind)); - ID.AddBoolean(EnableNullFPSuppression); -} - /// Returns true if \p N represents the DeclStmt declaring and initializing /// \p VR. static bool isInitializationOfVar(const ExplodedNode *N, const VarRegion *VR) { @@ -1240,7 +1204,8 @@ return FrameSpace->getStackFrame() == LCtx->getStackFrame(); } -/// Show diagnostics for initializing or declaring a region \p R with a bad value. +/// Show diagnostics for initializing or declaring a region \p R with a bad +/// value. static void showBRDiagnostics(const char *action, llvm::raw_svector_ostream &os, const MemRegion *R, SVal V, const DeclStmt *DS) { if (R->canPrintPretty()) { @@ -1524,19 +1489,6 @@ // Implementation of TrackConstraintBRVisitor. //===----------------------------------------------------------------------===// -void TrackConstraintBRVisitor::Profile(llvm::FoldingSetNodeID &ID) const { - static int tag = 0; - ID.AddPointer(&tag); - ID.AddBoolean(Assumption); - ID.Add(Constraint); -} - -/// Return the tag associated with this visitor. This tag will be used -/// to make all PathDiagnosticPieces created by this visitor. -const char *TrackConstraintBRVisitor::getTag() { - return "TrackConstraintBRVisitor"; -} - bool TrackConstraintBRVisitor::isUnderconstrained(const ExplodedNode *N) const { if (IsZeroCheck) return N->getState()->isNull(Constraint).isUnderconstrained(); @@ -1608,17 +1560,6 @@ IsSatisfied = true; } -void SuppressInlineDefensiveChecksVisitor::Profile( - llvm::FoldingSetNodeID &ID) const { - static int id = 0; - ID.AddPointer(&id); - ID.Add(V); -} - -const char *SuppressInlineDefensiveChecksVisitor::getTag() { - return "IDCVisitor"; -} - PathDiagnosticPieceRef SuppressInlineDefensiveChecksVisitor::VisitNode(const ExplodedNode *Succ, BugReporterContext &BRC, @@ -1715,14 +1656,15 @@ TrackControlDependencyCondBRVisitor(const ExplodedNode *O) : Origin(O), ControlDeps(&O->getCFG()) {} - void Profile(llvm::FoldingSetNodeID &ID) const override { - static int x = 0; - ID.AddPointer(&x); - } - PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &BR) override; + + static const char *getTag() { return "TrackControlDependencyCondBRVisitor"; } + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.AddPointer(getTag()); + } }; } // end of anonymous namespace @@ -2132,10 +2074,6 @@ // Visitor that tries to report interesting diagnostics from conditions. //===----------------------------------------------------------------------===// -/// Return the tag associated with this visitor. This tag will be used -/// to make all PathDiagnosticPieces created by this visitor. -const char *ConditionBRVisitor::getTag() { return "ConditionBRVisitor"; } - PathDiagnosticPieceRef ConditionBRVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &BR) { @@ -2871,23 +2809,12 @@ return nullptr; } -void FalsePositiveRefutationBRVisitor::Profile( - llvm::FoldingSetNodeID &ID) const { - static int Tag = 0; - ID.AddPointer(&Tag); -} - //===----------------------------------------------------------------------===// // Implementation of TagVisitor. //===----------------------------------------------------------------------===// int NoteTag::Kind = 0; -void TagVisitor::Profile(llvm::FoldingSetNodeID &ID) const { - static int Tag = 0; - ID.AddPointer(&Tag); -} - PathDiagnosticPieceRef TagVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &R) { Index: clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp +++ clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp @@ -14,6 +14,7 @@ #include "clang/Basic/Version.h" #include "clang/Lex/Preprocessor.h" #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h" #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringMap.h" @@ -238,8 +239,9 @@ // FIXME: What should be reported here? break; case PathDiagnosticPiece::Event: - return Piece.getTagStr() == "ConditionBRVisitor" ? Importance::Important - : Importance::Essential; + return Piece.getTag() == ConditionBRVisitor::getTag() + ? Importance::Important + : Importance::Essential; case PathDiagnosticPiece::ControlFlow: return Importance::Unimportant; }