r361682 - [analyzer] Add a prunable note for skipping vbase inits in subclasses.

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Fri May 24 16:37:11 PDT 2019


Author: dergachev
Date: Fri May 24 16:37:11 2019
New Revision: 361682

URL: http://llvm.org/viewvc/llvm-project?rev=361682&view=rev
Log:
[analyzer] Add a prunable note for skipping vbase inits in subclasses.

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

Added:
    cfe/trunk/test/Analysis/diagnostics/initializer.cpp
Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h?rev=361682&r1=361681&r2=361682&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Fri May 24 16:37:11 2019
@@ -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();
     }

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h?rev=361682&r1=361681&r2=361682&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h Fri May 24 16:37:11 2019
@@ -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.

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=361682&r1=361681&r2=361682&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Fri May 24 16:37:11 2019
@@ -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

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=361682&r1=361681&r2=361682&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Fri May 24 16:37:11 2019
@@ -2501,7 +2501,9 @@ TagVisitor::VisitNode(const ExplodedNode
   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;

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp?rev=361682&r1=361681&r2=361682&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp Fri May 24 16:37:11 2019
@@ -216,6 +216,25 @@ void CoreEngine::HandleBlockEdge(const B
                                            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() &&

Modified: cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp?rev=361682&r1=361681&r2=361682&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp Fri May 24 16:37:11 2019
@@ -724,7 +724,15 @@ PathDiagnosticLocation::create(const Pro
   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>())

Added: cfe/trunk/test/Analysis/diagnostics/initializer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/initializer.cpp?rev=361682&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/diagnostics/initializer.cpp (added)
+++ cfe/trunk/test/Analysis/diagnostics/initializer.cpp Fri May 24 16:37:11 2019
@@ -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 at -1{{Division by zero}}
+  {}
+};
+
+struct C : B {
+  C(): // expected-note{{Calling default constructor for 'A'}}
+       // expected-note at -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 at -1{{Division by zero}}
+}
+} // namespace note_on_skipped_vbases




More information about the cfe-commits mailing list