[Analyzer] Replace `CXXSelfAssignmentBRVisitor` with `NoteTags`
authorAdam Balogh <adam.balogh@ericsson.com>
Tue, 28 May 2019 13:07:09 +0000 (13:07 +0000)
committerAdam Balogh <adam.balogh@ericsson.com>
Tue, 28 May 2019 13:07:09 +0000 (13:07 +0000)
The `cplusplus.SelfAssignment` checker has a visitor that is added
to every `BugReport` to mark the to branch of the self assignment
operator with e.g. `rhs == *this` and `rhs != *this`. With the new
`NoteTag` feature this visitor is not needed anymore. Instead the
checker itself marks the two branches using the `NoteTag`s.

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

llvm-svn: 361818

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
clang/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

index e624d0f..1a09714 100644 (file)
@@ -294,20 +294,6 @@ public:
                                                  BugReport &BR) override;
 };
 
-class CXXSelfAssignmentBRVisitor final : public BugReporterVisitor {
-  bool Satisfied = false;
-
-public:
-  CXXSelfAssignmentBRVisitor() = default;
-
-  void Profile(llvm::FoldingSetNodeID &ID) const override {}
-
-  std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *Succ,
-                                                 BugReporterContext &BRC,
-                                                 BugReport &BR) override;
-};
-
-
 /// 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
index 1233849..01f5b9c 100644 (file)
@@ -50,10 +50,26 @@ void CXXSelfAssignmentChecker::checkBeginFunction(CheckerContext &C) const {
       State->getSVal(SVB.getCXXThis(MD, LCtx->getStackFrame()));
   auto Param = SVB.makeLoc(State->getRegion(MD->getParamDecl(0), LCtx));
   auto ParamVal = State->getSVal(Param);
+
   ProgramStateRef SelfAssignState = State->bindLoc(Param, ThisVal, LCtx);
-  C.addTransition(SelfAssignState);
+  const NoteTag *SelfAssignTag =
+    C.getNoteTag([MD](BugReport &BR) -> std::string {
+        SmallString<256> Msg;
+        llvm::raw_svector_ostream Out(Msg);
+        Out << "Assuming " << MD->getParamDecl(0)->getName() << " == *this";
+        return Out.str();
+      });
+  C.addTransition(SelfAssignState, SelfAssignTag);
+
   ProgramStateRef NonSelfAssignState = State->bindLoc(Param, ParamVal, LCtx);
-  C.addTransition(NonSelfAssignState);
+  const NoteTag *NonSelfAssignTag =
+    C.getNoteTag([MD](BugReport &BR) -> std::string {
+        SmallString<256> Msg;
+        llvm::raw_svector_ostream Out(Msg);
+        Out << "Assuming " << MD->getParamDecl(0)->getName() << " != *this";
+        return Out.str();
+      });
+  C.addTransition(NonSelfAssignState, NonSelfAssignTag);
 }
 
 void ento::registerCXXSelfAssignmentChecker(CheckerManager &Mgr) {
index cc93675..738ad9a 100644 (file)
@@ -2610,7 +2610,6 @@ std::pair<BugReport*, std::unique_ptr<VisitorsDiagnosticsTy>> findValidReport(
     // Register additional node visitors.
     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 21320b1..d11ecd9 100644 (file)
@@ -2371,59 +2371,6 @@ UndefOrNullArgVisitor::VisitNode(const ExplodedNode *N,
   return nullptr;
 }
 
-std::shared_ptr<PathDiagnosticPiece>
-CXXSelfAssignmentBRVisitor::VisitNode(const ExplodedNode *Succ,
-                                      BugReporterContext &BRC, BugReport &) {
-  if (Satisfied)
-    return nullptr;
-
-  const auto Edge = Succ->getLocation().getAs<BlockEdge>();
-  if (!Edge.hasValue())
-    return nullptr;
-
-  auto Tag = Edge->getTag();
-  if (!Tag)
-    return nullptr;
-
-  if (Tag->getTagDescription() != "cplusplus.SelfAssignment")
-    return nullptr;
-
-  Satisfied = true;
-
-  const auto *Met =
-      dyn_cast<CXXMethodDecl>(Succ->getCodeDecl().getAsFunction());
-  assert(Met && "Not a C++ method.");
-  assert((Met->isCopyAssignmentOperator() || Met->isMoveAssignmentOperator()) &&
-         "Not a copy/move assignment operator.");
-
-  const auto *LCtx = Edge->getLocationContext();
-
-  const auto &State = Succ->getState();
-  auto &SVB = State->getStateManager().getSValBuilder();
-
-  const auto Param =
-      State->getSVal(State->getRegion(Met->getParamDecl(0), LCtx));
-  const auto This =
-      State->getSVal(SVB.getCXXThis(Met, LCtx->getStackFrame()));
-
-  auto L = PathDiagnosticLocation::create(Met, BRC.getSourceManager());
-
-  if (!L.isValid() || !L.asLocation().isValid())
-    return nullptr;
-
-  SmallString<256> Buf;
-  llvm::raw_svector_ostream Out(Buf);
-
-  Out << "Assuming " << Met->getParamDecl(0)->getName() <<
-    ((Param == This) ? " == " : " != ") << "*this";
-
-  auto Piece = std::make_shared<PathDiagnosticEventPiece>(L, Out.str());
-  Piece->addRange(Met->getSourceRange());
-
-  return std::move(Piece);
-}
-
-
 FalsePositiveRefutationBRVisitor::FalsePositiveRefutationBRVisitor()
     : Constraints(ConstraintRangeTy::Factory().getEmptyMap()) {}
 
index b300847..5889a97 100644 (file)
@@ -732,6 +732,15 @@ PathDiagnosticLocation::create(const ProgramPoint& P,
 
     } else {
       S = BSrc->getTerminatorCondition();
+      if (!S) {
+        // If the BlockEdge has no terminator condition statement but its
+        // source is the entry of the CFG (e.g. a checker crated the branch at
+        // the beginning of a function), use the function's declaration instead.
+        assert(BSrc == &BSrc->getParent()->getEntry() && "CFGBlock has no "
+               "TerminatorCondition and is not the enrty block of the CFG");
+        return PathDiagnosticLocation::createBegin(
+            P.getLocationContext()->getDecl(), SMng);
+      }
     }
   } else if (Optional<StmtPoint> SP = P.getAs<StmtPoint>()) {
     S = SP->getStmt();