]> granicus.if.org Git - clang/commitdiff
Reapply "[analyzer] Introduce a simplified API for adding custom path notes."
authorArtem Dergachev <artem.dergachev@gmail.com>
Fri, 19 Apr 2019 20:23:29 +0000 (20:23 +0000)
committerArtem Dergachev <artem.dergachev@gmail.com>
Fri, 19 Apr 2019 20:23:29 +0000 (20:23 +0000)
This reapplies commit r357323, fixing memory leak found by LSan.

Differential Revision: https://reviews.llvm.org/D58367

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@358781 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Analysis/ProgramPoint.h
include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Checkers/MIGChecker.cpp
lib/StaticAnalyzer/Core/BugReporter.cpp
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
test/Analysis/mig.mm

index 623f5dc7072b41c7a43d4cf0275842ac8c9e5979..5b554c150947df851a997dba10cd131174b2fb30 100644 (file)
@@ -42,12 +42,11 @@ public:
   virtual ~ProgramPointTag();
   virtual StringRef getTagDescription() const = 0;
 
-protected:
   /// Used to implement 'isKind' in subclasses.
-  const void *getTagKind() { return TagKind; }
+  const void *getTagKind() const { return TagKind; }
 
 private:
-  const void *TagKind;
+  const void *const TagKind;
 };
 
 class SimpleProgramPointTag : public ProgramPointTag {
index a53efbbb99372e9786410d3dacdba9354ac5ec20..46b15d0c6f78cf2729ff0a32d1bb1e0199dfce26 100644 (file)
@@ -592,6 +592,59 @@ public:
   NodeMapClosure& getNodeResolver() { return NMC; }
 };
 
+
+/// The tag upon which the TagVisitor reacts. Add these in order to display
+/// additional PathDiagnosticEventPieces along the path.
+class NoteTag : public ProgramPointTag {
+public:
+  using Callback =
+      std::function<std::string(BugReporterContext &, BugReport &)>;
+
+private:
+  static int Kind;
+
+  const Callback Cb;
+
+  NoteTag(Callback &&Cb) : ProgramPointTag(&Kind), Cb(std::move(Cb)) {}
+
+public:
+  static bool classof(const ProgramPointTag *T) {
+    return T->getTagKind() == &Kind;
+  }
+
+  Optional<std::string> generateMessage(BugReporterContext &BRC,
+                                        BugReport &R) const {
+    std::string Msg = Cb(BRC, R);
+    if (Msg.empty())
+      return None;
+
+    return std::move(Msg);
+  }
+
+  StringRef getTagDescription() const override {
+    // TODO: Remember a few examples of generated messages
+    // and display them in the ExplodedGraph dump by
+    // returning them from this function.
+    return "Note Tag";
+  }
+
+  // Manage memory for NoteTag objects.
+  class Factory {
+    std::vector<std::unique_ptr<NoteTag>> Tags;
+
+  public:
+    const NoteTag *makeNoteTag(Callback &&Cb) {
+      // We cannot use make_unique because we cannot access the private
+      // constructor from inside it.
+      std::unique_ptr<NoteTag> T(new NoteTag(std::move(Cb)));
+      Tags.push_back(std::move(T));
+      return Tags.back().get();
+    }
+  };
+
+  friend class TagVisitor;
+};
+
 } // namespace ento
 
 } // namespace clang
index 03b6560e987b010a2a3e2f69427eaf391d7589e7..e624d0fc026c4ce0d27767a6cf7ef28dd7c80591 100644 (file)
@@ -14,6 +14,7 @@
 #ifndef LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_BUGREPORTERVISITORS_H
 #define LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_BUGREPORTERVISITORS_H
 
+#include "clang/Analysis/ProgramPoint.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
@@ -328,6 +329,17 @@ public:
                        BugReport &BR) override;
 };
 
+
+/// The visitor detects NoteTags and displays the event notes they contain.
+class TagVisitor : public BugReporterVisitor {
+public:
+  void Profile(llvm::FoldingSetNodeID &ID) const override;
+
+  std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
+                                                 BugReporterContext &BRC,
+                                                 BugReport &R) override;
+};
+
 namespace bugreporter {
 
 /// Attempts to add visitors to track expression value back to its point of
index 5710ee7971455d2b897f4e484e4cde15ab0c6078..6a49aebc14ce45c5893767c4e0561c74d4c0374e 100644 (file)
@@ -219,6 +219,24 @@ public:
     Eng.getBugReporter().emitReport(std::move(R));
   }
 
+
+  /// Produce a program point tag that displays an additional path note
+  /// to the user. This is a lightweight alternative to the
+  /// BugReporterVisitor mechanism: instead of visiting the bug report
+  /// node-by-node to restore the sequence of events that led to discovering
+  /// a bug, you can add notes as you add your transitions.
+  const NoteTag *getNoteTag(NoteTag::Callback &&Cb) {
+    return Eng.getNoteTags().makeNoteTag(std::move(Cb));
+  }
+
+  /// A shorthand version of getNoteTag that doesn't require you to accept
+  /// the BugReporterContext arguments when you don't need it.
+  const NoteTag *getNoteTag(std::function<std::string(BugReport &)> &&Cb) {
+    return getNoteTag(
+        [Cb](BugReporterContext &, BugReport &BR) { return Cb(BR); });
+  }
+
+
   /// Returns the word that should be used to refer to the declaration
   /// in the report.
   StringRef getDeclDescription(const Decl *D);
index 605e1c78bb0ec4b55947508ede49209872b7c9ad..185774b586cebd9474f14abef402d743241e1439 100644 (file)
@@ -22,6 +22,7 @@
 #include "clang/Analysis/ProgramPoint.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/FunctionSummary.h"
@@ -155,6 +156,8 @@ private:
   /// The flag, which specifies the mode of inlining for the engine.
   InliningModes HowToInline;
 
+  NoteTag::Factory NoteTags;
+
 public:
   ExprEngine(cross_tu::CrossTranslationUnitContext &CTU, AnalysisManager &mgr,
              SetOfConstDecls *VisitedCalleesIn,
@@ -396,6 +399,8 @@ public:
   SymbolManager &getSymbolManager() { return SymMgr; }
   MemRegionManager &getRegionManager() { return MRMgr; }
 
+  NoteTag::Factory &getNoteTags() { return NoteTags; }
+
 
   // Functions for external checking of whether we have unfinished work
   bool wasBlocksExhausted() const { return Engine.wasBlocksExhausted(); }
index bf73e8ac4270234c41f8fb6d849f63103ec9449a..33dd28e52a3614a9220936bb3ea0ac315ff7605e 100644 (file)
@@ -102,43 +102,10 @@ public:
     checkReturnAux(RS, C);
   }
 
-  class Visitor : public BugReporterVisitor {
-  public:
-    void Profile(llvm::FoldingSetNodeID &ID) const {
-      static int X = 0;
-      ID.AddPointer(&X);
-    }
-
-    std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
-        BugReporterContext &BRC, BugReport &R);
-  };
 };
 } // end anonymous namespace
 
-// FIXME: It's a 'const ParmVarDecl *' but there's no ready-made GDM traits
-// specialization for this sort of types.
-REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, const void *)
-
-std::shared_ptr<PathDiagnosticPiece>
-MIGChecker::Visitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC,
-                               BugReport &R) {
-  const auto *NewPVD = static_cast<const ParmVarDecl *>(
-      N->getState()->get<ReleasedParameter>());
-  const auto *OldPVD = static_cast<const ParmVarDecl *>(
-      N->getFirstPred()->getState()->get<ReleasedParameter>());
-  if (OldPVD == NewPVD)
-    return nullptr;
-
-  assert(NewPVD && "What is deallocated cannot be un-deallocated!");
-  SmallString<64> Str;
-  llvm::raw_svector_ostream OS(Str);
-  OS << "Value passed through parameter '" << NewPVD->getName()
-     << "' is deallocated";
-
-  PathDiagnosticLocation Loc =
-      PathDiagnosticLocation::create(N->getLocation(), BRC.getSourceManager());
-  return std::make_shared<PathDiagnosticEventPiece>(Loc, OS.str());
-}
+REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, bool)
 
 static const ParmVarDecl *getOriginParam(SVal V, CheckerContext &C) {
   SymbolRef Sym = V.getAsSymbol();
@@ -217,7 +184,16 @@ void MIGChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const {
   if (!PVD)
     return;
 
-  C.addTransition(C.getState()->set<ReleasedParameter>(PVD));
+  const NoteTag *T = C.getNoteTag([this, PVD](BugReport &BR) -> std::string {
+    if (&BR.getBugType() != &BT)
+      return "";
+    SmallString<64> Str;
+    llvm::raw_svector_ostream OS(Str);
+    OS << "Value passed through parameter '" << PVD->getName()
+       << "\' is deallocated";
+    return OS.str();
+  });
+  C.addTransition(C.getState()->set<ReleasedParameter>(true), T);
 }
 
 // Returns true if V can potentially represent a "successful" kern_return_t.
@@ -282,7 +258,6 @@ void MIGChecker::checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const {
 
   R->addRange(RS->getSourceRange());
   bugreporter::trackExpressionValue(N, RS->getRetValue(), *R, false);
-  R->addVisitor(llvm::make_unique<Visitor>());
   C.emitReport(std::move(R));
 }
 
index 1cea44f1d850446cc2a889980c65aea71f36c059..168050955f2ef1eaea0dcf4997c1f4821a4b5124 100644 (file)
@@ -2611,6 +2611,7 @@ std::pair<BugReport*, std::unique_ptr<VisitorsDiagnosticsTy>> findValidReport(
     R->addVisitor(llvm::make_unique<NilReceiverBRVisitor>());
     R->addVisitor(llvm::make_unique<ConditionBRVisitor>());
     R->addVisitor(llvm::make_unique<CXXSelfAssignmentBRVisitor>());
+    R->addVisitor(llvm::make_unique<TagVisitor>());
 
     BugReporterContext BRC(Reporter, ErrorGraph.BackMap);
 
index cf29560b37ef6f72987990254b461d837f3acb24..09c0b9d171acdad0ebc59f30c8bbeb2d62777dcc 100644 (file)
@@ -2499,6 +2499,30 @@ FalsePositiveRefutationBRVisitor::VisitNode(const ExplodedNode *N,
   return nullptr;
 }
 
+int NoteTag::Kind = 0;
+
+void TagVisitor::Profile(llvm::FoldingSetNodeID &ID) const {
+  static int Tag = 0;
+  ID.AddPointer(&Tag);
+}
+
+std::shared_ptr<PathDiagnosticPiece>
+TagVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC,
+                      BugReport &R) {
+  ProgramPoint PP = N->getLocation();
+  const NoteTag *T = dyn_cast_or_null<NoteTag>(PP.getTag());
+  if (!T)
+    return nullptr;
+
+  if (Optional<std::string> Msg = T->generateMessage(BRC, R)) {
+    PathDiagnosticLocation Loc =
+        PathDiagnosticLocation::create(PP, BRC.getSourceManager());
+    return std::make_shared<PathDiagnosticEventPiece>(Loc, *Msg);
+  }
+
+  return nullptr;
+}
+
 void FalsePositiveRefutationBRVisitor::Profile(
     llvm::FoldingSetNodeID &ID) const {
   static int Tag = 0;
index 000edef1ca8e1b7259ca854c31218e935867aabb..ee9c0a42c1c926691589c625aa298f9a90094480 100644 (file)
@@ -201,7 +201,9 @@ ExprEngine::ExprEngine(cross_tu::CrossTranslationUnitContext &CTU,
       svalBuilder(StateMgr.getSValBuilder()),
       ObjCNoRet(mgr.getASTContext()),
       BR(mgr, *this),
-      VisitedCallees(VisitedCalleesIn), HowToInline(HowToInlineIn) {
+      VisitedCallees(VisitedCalleesIn),
+      HowToInline(HowToInlineIn)
+  {
   unsigned TrimInterval = mgr.options.GraphTrimInterval;
   if (TrimInterval != 0) {
     // Enable eager node reclamation when constructing the ExplodedGraph.
index bc432928ed8e4fae7297bf04948ab0a1c141edf1..ca6635a3289f9949cbc0c06c625ca3c50e978c95 100644 (file)
@@ -100,6 +100,14 @@ kern_return_t release_twice(mach_port_name_t port, vm_address_t addr1, vm_addres
                      // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
 }
 
+MIG_SERVER_ROUTINE
+kern_return_t no_unrelated_notes(mach_port_name_t port, vm_address_t address, vm_size_t size) {
+  vm_deallocate(port, address, size); // no-note
+  1 / 0; // expected-warning{{Division by zero}}
+         // expected-note@-1{{Division by zero}}
+  return KERN_SUCCESS;
+}
+
 // Make sure we find the bug when the object is destroyed within an
 // automatic destructor.
 MIG_SERVER_ROUTINE