NoQ updated this revision to Diff 208473.
NoQ added a comment.

Fix the crash on enabling both checkers, add a test.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64274/new/

https://reviews.llvm.org/D64274

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/virtualcall.cpp
  clang/test/Analysis/virtualcall.h

Index: clang/test/Analysis/virtualcall.h
===================================================================
--- clang/test/Analysis/virtualcall.h
+++ clang/test/Analysis/virtualcall.h
@@ -2,12 +2,7 @@
   class Z {
   public:
     Z() {
-      foo();
-#if !PUREONLY
-	// expected-warning-re@-2 {{{{^}}Call to virtual function during construction}}
-	// expected-note-re@-3 {{{{^}}This constructor of an object of type 'Z' has not returned when the virtual method was called}}
-	// expected-note-re@-4 {{{{^}}Call to virtual function during construction}}	
-#endif
+      foo(); // impure-warning {{Call to virtual function during construction}}
     }
     virtual int foo();
   };
Index: clang/test/Analysis/virtualcall.cpp
===================================================================
--- clang/test/Analysis/virtualcall.cpp
+++ clang/test/Analysis/virtualcall.cpp
@@ -1,6 +1,12 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -analyzer-output=text -verify -std=c++11 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN:                    -std=c++11 -verify=expected,impure %s
 
-// RUN: %clang_analyze_cc1 -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -analyzer-config optin.cplusplus.VirtualCall:PureOnly=true -DPUREONLY=1 -analyzer-output=text -verify -std=c++11 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
+// RUN:                    -std=c++11 -verify=expected -std=c++11 %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
+// RUN:                    -analyzer-checker=optin.cplusplus.VirtualCall \
+// RUN:                    -std=c++11 -verify=expected,impure -std=c++11 %s
 
 #include "virtualcall.h"
 
@@ -13,54 +19,31 @@
   virtual int foo() = 0;
   virtual void bar() = 0;
   void f() {
-    foo();
-	// expected-warning-re@-1 {{{{^}}Call to pure virtual function during construction}}
-	// expected-note-re@-2 {{{{^}}Call to pure virtual function during construction}}
+    foo(); // expected-warning{{Call to pure virtual function during construction}}
   }
 };
 
 class B : public A {
 public:
-  B() { // expected-note {{Calling default constructor for 'A'}}
-    foo(); 
-#if !PUREONLY
-  	// expected-warning-re@-2 {{{{^}}Call to virtual function during construction}}
-	// expected-note-re@-3 {{{{^}}This constructor of an object of type 'B' has not returned when the virtual method was called}}
-  	// expected-note-re@-4 {{{{^}}Call to virtual function during construction}}
-#endif
+  B() {
+    foo(); // impure-warning {{Call to virtual function during construction}}
   }
   ~B();
 
   virtual int foo();
   virtual void bar() {
-    foo(); 
-#if !PUREONLY
-  	// expected-warning-re@-2 {{{{^}}Call to virtual function during destruction}}
-  	// expected-note-re@-3 {{{{^}}Call to virtual function during destruction}}
-#endif
-  } 
+    foo(); // impure-warning{{Call to virtual function during destruction}}
+  }
 };
 
-A::A() { 
-  f(); 
-// expected-note-re@-1 {{{{^}}This constructor of an object of type 'A' has not returned when the virtual method was called}}
-// expected-note-re@-2 {{{{^}}Calling 'A::f'}}
+A::A() {
+  f();
 }
 
 B::~B() {
   this->B::foo(); // no-warning
   this->B::bar();
-#if !PUREONLY
- 	 // expected-note-re@-2 {{{{^}}This destructor of an object of type '~B' has not returned when the virtual method was called}}
- 	 // expected-note-re@-3 {{{{^}}Calling 'B::bar'}}
-#endif
-  this->foo(); 
-#if !PUREONLY
- 	 // expected-warning-re@-2 {{{{^}}Call to virtual function during destruction}}
- 	 // expected-note-re@-3 {{{{^}}This destructor of an object of type '~B' has not returned when the virtual method was called}}
- 	 // expected-note-re@-4 {{{{^}}Call to virtual function during destruction}}
-#endif
-	
+  this->foo(); // impure-warning {{Call to virtual function during destruction}}
 }
 
 class C : public B {
@@ -73,12 +56,7 @@
 };
 
 C::C() {
-  f(foo()); 
-#if !PUREONLY
-  	// expected-warning-re@-2 {{{{^}}Call to virtual function during construction}}
-	// expected-note-re@-3 {{{{^}}This constructor of an object of type 'C' has not returned when the virtual method was called}}
-  	// expected-note-re@-4 {{{{^}}Call to virtual function during construction}}
-#endif
+  f(foo()); // impure-warning {{Call to virtual function during construction}}
 }
 
 class D : public B {
@@ -97,9 +75,6 @@
     foo(); // no-warning
   }
   ~E() { bar(); }
-#if !PUREONLY
- 	 // expected-note-re@-2 2{{{{^}}Calling '~B'}}
-#endif
   int foo() override;
 };
 
@@ -135,52 +110,23 @@
     G g;
     g.foo();
     g.bar(); // no warning
-    f();     
-#if !PUREONLY
-  	// expected-warning-re@-2 {{{{^}}Call to virtual function during construction}}
-	// expected-note-re@-3 {{{{^}}This constructor of an object of type 'H' has not returned when the virtual method was called}}
-  	// expected-note-re@-4 {{{{^}}Call to virtual function during construction}}
-#endif
+    f(); // impure-warning {{Call to virtual function during construction}}
     H &h = *this;
-    h.f(); 
-#if !PUREONLY
-  	// expected-warning-re@-2 {{{{^}}Call to virtual function during construction}}
-  	// expected-note-re@-3 {{{{^}}This constructor of an object of type 'H' has not returned when the virtual method was called}}
-  	// expected-note-re@-4 {{{{^}}Call to virtual function during construction}}
-#endif
+    h.f(); // impure-warning {{Call to virtual function during construction}}
   }
 };
 
 class X {
 public:
   X() {
-    g(); 
-#if !PUREONLY
-  	// expected-warning-re@-2 {{{{^}}Call to virtual function during construction}}
-	// expected-note-re@-3 {{{{^}}This constructor of an object of type 'X' has not returned when the virtual method was called}}
-  	// expected-note-re@-4 {{{{^}}Call to virtual function during construction}}
-#endif
+    g(); // impure-warning {{Call to virtual function during construction}}
   }
   X(int i) {
     if (i > 0) {
-#if !PUREONLY
-	// expected-note-re@-2 {{{{^}}'i' is > 0}}
-	// expected-note-re@-3 {{{{^}}Taking true branch}}
-	// expected-note-re@-4 {{{{^}}'i' is <= 0}}
-	// expected-note-re@-5 {{{{^}}Taking false branch}}
-#endif
       X x(i - 1);
-#if !PUREONLY
-	// expected-note-re@-2 {{{{^}}Calling constructor for 'X'}}
-#endif
       x.g(); // no warning
     }
-    g(); 
-#if !PUREONLY
-  	// expected-warning-re@-2 {{{{^}}Call to virtual function during construction}}
-	// expected-note-re@-3 {{{{^}}This constructor of an object of type 'X' has not returned when the virtual method was called}}
-  	// expected-note-re@-4 {{{{^}}Call to virtual function during construction}}
-#endif
+    g(); // impure-warning {{Call to virtual function during construction}}
   }
   virtual void g();
 };
@@ -197,19 +143,11 @@
     N n;
     n.virtualMethod(); // no warning
     n.callFooOfM(this);
-#if !PUREONLY
-  	// expected-note-re@-2 {{{{^}}This constructor of an object of type 'M' has not returned when the virtual method was called}}
-	// expected-note-re@-3 {{{{^}}Calling 'N::callFooOfM'}}
-#endif
   }
   virtual void foo();
 };
 void N::callFooOfM(M *m) {
-  m->foo(); 
-#if !PUREONLY
-  	// expected-warning-re@-2 {{{{^}}Call to virtual function during construction}}
-  	// expected-note-re@-3 {{{{^}}Call to virtual function during construction}}
-#endif
+  m->foo(); // impure-warning {{Call to virtual function during construction}}
 }
 
 class Y {
@@ -217,65 +155,27 @@
   virtual void foobar();
   void fooY() {
     F f1;
-    foobar(); 
-#if !PUREONLY
-  	// expected-warning-re@-2 {{{{^}}Call to virtual function during construction}}
-  	// expected-note-re@-3 {{{{^}}Call to virtual function during construction}}
-#endif
+    foobar(); // impure-warning {{Call to virtual function during construction}}
   }
   Y() { fooY(); }
-#if !PUREONLY
-  	// expected-note-re@-2 {{{{^}}This constructor of an object of type 'Y' has not returned when the virtual method was called}}
-  	// expected-note-re@-3 {{{{^}}Calling 'Y::fooY'}}
-#endif
 };
 
 int main() {
   B b;
-#if PUREONLY
-	//expected-note-re@-2 {{{{^}}Calling default constructor for 'B'}}
-#else 
-	//expected-note-re@-4 2{{{{^}}Calling default constructor for 'B'}}
-#endif
   C c;
-#if !PUREONLY
-	//expected-note-re@-2 {{{{^}}Calling default constructor for 'C'}}
-#endif
   D d;
   E e;
   F f;
   G g;
   H h;
   H h1(1);
-#if !PUREONLY
-	//expected-note-re@-2 {{{{^}}Calling constructor for 'H'}}
-	//expected-note-re@-3 {{{{^}}Calling constructor for 'H'}}
-#endif
   X x; 
-#if !PUREONLY
-	//expected-note-re@-2 {{{{^}}Calling default constructor for 'X'}}
-#endif
   X x1(1);
-#if !PUREONLY
-	//expected-note-re@-2 {{{{^}}Calling constructor for 'X'}}
-#endif
   M m;
-#if !PUREONLY
-	//expected-note-re@-2 {{{{^}}Calling default constructor for 'M'}}
-#endif
   Y *y = new Y;
-#if !PUREONLY
-  //expected-note-re@-2 {{{{^}}Calling default constructor for 'Y'}}
-#endif
   delete y;
   header::Z z;
-#if !PUREONLY
-	// expected-note-re@-2 {{{{^}}Calling default constructor for 'Z'}}
-#endif
 }
-#if !PUREONLY
-	//expected-note-re@-2 2{{{{^}}Calling '~E'}}
-#endif
 
 namespace PR34451 {
 struct a {
Index: clang/test/Analysis/analyzer-config.c
===================================================================
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -70,7 +70,6 @@
 // CHECK-NEXT: optin.cplusplus.UninitializedObject:IgnoreRecordsWithField = ""
 // CHECK-NEXT: optin.cplusplus.UninitializedObject:NotesAsWarnings = false
 // CHECK-NEXT: optin.cplusplus.UninitializedObject:Pedantic = false
-// CHECK-NEXT: optin.cplusplus.VirtualCall:PureOnly = false
 // CHECK-NEXT: optin.osx.cocoa.localizability.NonLocalizedStringChecker:AggressiveReport = false
 // CHECK-NEXT: optin.performance.Padding:AllowedPad = 24
 // CHECK-NEXT: osx.NumberObjectConversion:Pedantic = false
@@ -90,4 +89,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 87
+// CHECK-NEXT: num-entries = 86
Index: clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -44,7 +44,7 @@
 
 public:
   // The flag to determine if pure virtual functions should be issued only.
-  DefaultBool IsPureOnly;
+  bool IsPureOnly = true;
 
   void checkBeginFunction(CheckerContext &C) const;
   void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const;
@@ -55,84 +55,12 @@
                                    CheckerContext &C) const;
   void reportBug(StringRef Msg, bool PureError, const MemRegion *Reg,
                  CheckerContext &C) const;
-
-  class VirtualBugVisitor : public BugReporterVisitor {
-  private:
-    const MemRegion *ObjectRegion;
-    bool Found;
-
-  public:
-    VirtualBugVisitor(const MemRegion *R) : ObjectRegion(R), Found(false) {}
-
-    void Profile(llvm::FoldingSetNodeID &ID) const override {
-      static int X = 0;
-      ID.AddPointer(&X);
-      ID.AddPointer(ObjectRegion);
-    }
-
-    std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
-                                                   BugReporterContext &BRC,
-                                                   BugReport &BR) override;
-  };
 };
 } // end namespace
 
 // GDM (generic data map) to the memregion of this for the ctor and dtor.
 REGISTER_MAP_WITH_PROGRAMSTATE(CtorDtorMap, const MemRegion *, ObjectState)
 
-std::shared_ptr<PathDiagnosticPiece>
-VirtualCallChecker::VirtualBugVisitor::VisitNode(const ExplodedNode *N,
-                                                 BugReporterContext &BRC,
-                                                 BugReport &) {
-  // We need the last ctor/dtor which call the virtual function.
-  // The visitor walks the ExplodedGraph backwards.
-  if (Found)
-    return nullptr;
-
-  ProgramStateRef State = N->getState();
-  const LocationContext *LCtx = N->getLocationContext();
-  const CXXConstructorDecl *CD =
-      dyn_cast_or_null<CXXConstructorDecl>(LCtx->getDecl());
-  const CXXDestructorDecl *DD =
-      dyn_cast_or_null<CXXDestructorDecl>(LCtx->getDecl());
-
-  if (!CD && !DD)
-    return nullptr;
-
-  ProgramStateManager &PSM = State->getStateManager();
-  auto &SVB = PSM.getSValBuilder();
-  const auto *MD = dyn_cast<CXXMethodDecl>(LCtx->getDecl());
-  if (!MD)
-    return nullptr;
-  auto ThiSVal =
-      State->getSVal(SVB.getCXXThis(MD, LCtx->getStackFrame()));
-  const MemRegion *Reg = ThiSVal.castAs<loc::MemRegionVal>().getRegion();
-  if (!Reg)
-    return nullptr;
-  if (Reg != ObjectRegion)
-    return nullptr;
-
-  const Stmt *S = PathDiagnosticLocation::getStmt(N);
-  if (!S)
-    return nullptr;
-  Found = true;
-
-  std::string InfoText;
-  if (CD)
-    InfoText = "This constructor of an object of type '" +
-               CD->getNameAsString() +
-               "' has not returned when the virtual method was called";
-  else
-    InfoText = "This destructor of an object of type '" +
-               DD->getNameAsString() +
-               "' has not returned when the virtual method was called";
-
-  // Generate the extra diagnostic.
-  PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
-                             N->getLocationContext());
-  return std::make_shared<PathDiagnosticEventPiece>(Pos, InfoText, true);
-}
-
 // The function to check if a callexpr is a virtual function.
 static bool isVirtualCall(const CallExpr *CE) {
   bool CallIsNonVirtual = false;
@@ -271,15 +199,20 @@
         "C++ Object Lifecycle"));
 
   auto Reporter = llvm::make_unique<BugReport>(*BT, Msg, N);
-  Reporter->addVisitor(llvm::make_unique<VirtualBugVisitor>(Reg));
   C.emitReport(std::move(Reporter));
 }
 
-void ento::registerVirtualCallChecker(CheckerManager &mgr) {
-  VirtualCallChecker *checker = mgr.registerChecker<VirtualCallChecker>();
+void ento::registerPureVirtualCallChecker(CheckerManager &Mgr) {
+  Mgr.registerChecker<VirtualCallChecker>();
+}
+
+void ento::registerVirtualCallChecker(CheckerManager &Mgr) {
+  auto *Chk = Mgr.getChecker<VirtualCallChecker>();
+  Chk->IsPureOnly = false;
+}
 
-  checker->IsPureOnly =
-      mgr.getAnalyzerOptions().getCheckerBooleanOption(checker, "PureOnly");
+bool ento::shouldRegisterPureVirtualCallChecker(const LangOptions &LO) {
+  return true;
 }
 
 bool ento::shouldRegisterVirtualCallChecker(const LangOptions &LO) {
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -507,6 +507,9 @@
   ]>,
   Documentation<HasDocumentation>;
 
+def PureVirtualCallChecker : Checker<"PureVirtualCall">,
+  HelpText<"Check pure virtual function calls during construction/destruction">,
+  Documentation<HasDocumentation>;
 } // end: "cplusplus"
 
 let ParentPackage = CplusplusOptIn in {
@@ -555,14 +558,8 @@
   Documentation<HasAlphaDocumentation>;
 
 def VirtualCallChecker : Checker<"VirtualCall">,
-  HelpText<"Check virtual function calls during construction or destruction">,
-  CheckerOptions<[
-    CmdLineOption<Boolean,
-                  "PureOnly",
-                  "Whether to only report calls to pure virtual methods.",
-                  "false",
-                  Released>
-  ]>,
+  HelpText<"Check virtual function calls during construction/destruction">,
+  Dependencies<[PureVirtualCallChecker]>,
   Documentation<HasDocumentation>;
 
 } // end: "optin.cplusplus"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to