]> granicus.if.org Git - clang/commitdiff
[analyzer] Add a prunable note for skipping vbase inits in subclasses.
authorArtem Dergachev <artem.dergachev@gmail.com>
Fri, 24 May 2019 23:37:11 +0000 (23:37 +0000)
committerArtem Dergachev <artem.dergachev@gmail.com>
Fri, 24 May 2019 23:37:11 +0000 (23:37 +0000)
When initialization of virtual base classes is skipped, we now tell the user
about it, because this aspect of C++ isn't very well-known.

The implementation is based on the new "note tags" feature (r358781).
In order to make use of it, allow note tags to produce prunable notes,
and move the note tag factory to CoreEngine.

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

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

include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
lib/StaticAnalyzer/Core/CoreEngine.cpp
lib/StaticAnalyzer/Core/PathDiagnostic.cpp
test/Analysis/diagnostics/initializer.cpp [new file with mode: 0644]

index 46b15d0c6f78cf2729ff0a32d1bb1e0199dfce26..4cccb38ce24fab5ec3d01dbf108862e88029060b 100644 (file)
@@ -604,8 +604,10 @@ private:
   static int Kind;
 
   const Callback Cb;
+  const bool IsPrunable;
 
-  NoteTag(Callback &&Cb) : ProgramPointTag(&Kind), Cb(std::move(Cb)) {}
+  NoteTag(Callback &&Cb, bool IsPrunable)
+      : ProgramPointTag(&Kind), Cb(std::move(Cb)), IsPrunable(IsPrunable) {}
 
 public:
   static bool classof(const ProgramPointTag *T) {
@@ -628,15 +630,17 @@ public:
     return "Note Tag";
   }
 
+  bool isPrunable() const { return IsPrunable; }
+
   // Manage memory for NoteTag objects.
   class Factory {
     std::vector<std::unique_ptr<NoteTag>> Tags;
 
   public:
-    const NoteTag *makeNoteTag(Callback &&Cb) {
+    const NoteTag *makeNoteTag(Callback &&Cb, bool IsPrunable = false) {
       // 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)));
+      std::unique_ptr<NoteTag> T(new NoteTag(std::move(Cb), IsPrunable));
       Tags.push_back(std::move(T));
       return Tags.back().get();
     }
index 019acc0b7d9f86887b41bc7d8f7c47eae3c4690f..278193ef99edea68ca4750d856b19b44fc4f45fc 100644 (file)
@@ -19,6 +19,7 @@
 #include "clang/Analysis/CFG.h"
 #include "clang/Analysis/ProgramPoint.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/BlockCounter.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
@@ -95,6 +96,10 @@ private:
   /// (This data is owned by AnalysisConsumer.)
   FunctionSummariesTy *FunctionSummaries;
 
+  /// Add path note tags along the path when we see that something interesting
+  /// is happening. This field is the allocator for such tags.
+  NoteTag::Factory NoteTags;
+
   void generateNode(const ProgramPoint &Loc,
                     ProgramStateRef State,
                     ExplodedNode *Pred);
@@ -194,6 +199,8 @@ public:
 
   /// Enqueue a single node created as a result of statement processing.
   void enqueueStmtNode(ExplodedNode *N, const CFGBlock *Block, unsigned Idx);
+
+  NoteTag::Factory &getNoteTags() { return NoteTags; }
 };
 
 // TODO: Turn into a class.
index f0b01f182bf8c9611f3e097d34afcc2e27cad34e..8bc599a96a596ca5e70222b89fe4a87c7579409f 100644 (file)
@@ -156,8 +156,6 @@ 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,
@@ -399,7 +397,7 @@ public:
   SymbolManager &getSymbolManager() { return SymMgr; }
   MemRegionManager &getRegionManager() { return MRMgr; }
 
-  NoteTag::Factory &getNoteTags() { return NoteTags; }
+  NoteTag::Factory &getNoteTags() { return Engine.getNoteTags(); }
 
 
   // Functions for external checking of whether we have unfinished work
index bc34472020c40ffef36a29e327dfa86ce5c8821a..21320b1cdd8847b6d13fc257e88d003ebcaf7d08 100644 (file)
@@ -2501,7 +2501,9 @@ TagVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC,
   if (Optional<std::string> Msg = T->generateMessage(BRC, R)) {
     PathDiagnosticLocation Loc =
         PathDiagnosticLocation::create(PP, BRC.getSourceManager());
-    return std::make_shared<PathDiagnosticEventPiece>(Loc, *Msg);
+    auto Piece = std::make_shared<PathDiagnosticEventPiece>(Loc, *Msg);
+    Piece->setPrunable(T->isPrunable());
+    return Piece;
   }
 
   return nullptr;
index 500995b053ef9b0f766ce053b9e726db8a193a0a..431d07dab1e1896a10dc24eaadc7744c8d3c37e8 100644 (file)
@@ -216,6 +216,25 @@ void CoreEngine::HandleBlockEdge(const BlockEdge &L, ExplodedNode *Pred) {
                                            LC->getDecl(),
                                            LC->getCFG()->getNumBlockIDs());
 
+  // Display a prunable path note to the user if it's a virtual bases branch
+  // and we're taking the path that skips virtual base constructors.
+  if (L.getSrc()->getTerminator().isVirtualBaseBranch() &&
+      L.getDst() == *L.getSrc()->succ_begin()) {
+    ProgramPoint P = L.withTag(getNoteTags().makeNoteTag(
+        [](BugReporterContext &, BugReport &) -> std::string {
+          // TODO: Just call out the name of the most derived class
+          // when we know it.
+          return "Virtual base initialization skipped because "
+                 "it has already been handled by the most derived class";
+        }, /*IsPrunable=*/true));
+    // Perform the transition.
+    ExplodedNodeSet Dst;
+    NodeBuilder Bldr(Pred, Dst, BuilderCtx);
+    Pred = Bldr.generateNode(P, Pred->getState(), Pred);
+    if (!Pred)
+      return;
+  }
+
   // Check if we are entering the EXIT block.
   if (Blk == &(L.getLocationContext()->getCFG()->getExit())) {
     assert(L.getLocationContext()->getCFG()->getExit().empty() &&
index 903206889210031afc59b2ebb677b04d22b3989a..b3008479fe35834df11d42ec564e6d36bdefcbe4 100644 (file)
@@ -724,7 +724,15 @@ PathDiagnosticLocation::create(const ProgramPoint& P,
   const Stmt* S = nullptr;
   if (Optional<BlockEdge> BE = P.getAs<BlockEdge>()) {
     const CFGBlock *BSrc = BE->getSrc();
-    S = BSrc->getTerminatorCondition();
+    if (BSrc->getTerminator().isVirtualBaseBranch()) {
+      // TODO: VirtualBaseBranches should also appear for destructors.
+      // In this case we should put the diagnostic at the end of decl.
+      return PathDiagnosticLocation::createBegin(
+          P.getLocationContext()->getDecl(), SMng);
+
+    } else {
+      S = BSrc->getTerminatorCondition();
+    }
   } else if (Optional<StmtPoint> SP = P.getAs<StmtPoint>()) {
     S = SP->getStmt();
     if (P.getAs<PostStmtPurgeDeadSymbols>())
diff --git a/test/Analysis/diagnostics/initializer.cpp b/test/Analysis/diagnostics/initializer.cpp
new file mode 100644 (file)
index 0000000..db744ef
--- /dev/null
@@ -0,0 +1,44 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core -analyzer-output=text \
+// RUN:   -verify %s
+
+namespace note_on_skipped_vbases {
+struct A {
+  int x;
+  A() : x(0) {} // expected-note{{The value 0 is assigned to 'c.x'}}
+  A(int x) : x(x) {}
+};
+
+struct B : virtual A {
+  int y;
+  // This note appears only once, when this constructor is called from C.
+  // When this constructor is called from D, this note is still correct but
+  // it doesn't appear because it's pruned out because it's irrelevant to the
+  // bug report.
+  B(): // expected-note{{Virtual base initialization skipped because it has already been handled by the most derived class}}
+    A(1),
+    y(1 / x) // expected-warning{{Division by zero}}
+             // expected-note@-1{{Division by zero}}
+  {}
+};
+
+struct C : B {
+  C(): // expected-note{{Calling default constructor for 'A'}}
+       // expected-note@-1{{Returning from default constructor for 'A'}}
+    B() // expected-note{{Calling default constructor for 'B'}}
+  {}
+};
+
+void test_note() {
+  C c; // expected-note{{Calling default constructor for 'C'}}
+}
+
+struct D: B {
+  D() : A(1), B() {}
+};
+
+void test_prunability() {
+  D d;
+  1 / 0; // expected-warning{{Division by zero}}
+         // expected-note@-1{{Division by zero}}
+}
+} // namespace note_on_skipped_vbases