Index: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -504,6 +504,15 @@ ]>, Documentation; +def VirtualCallModeling : Checker<"VirtualCallModeling">, + HelpText<"Auxiliary modeling for the virtual method call checkers">, + Documentation, + Hidden; + +def PureVirtualCallChecker : Checker<"PureVirtualCall">, + HelpText<"Check pure virtual function calls during construction/destruction">, + Dependencies<[VirtualCallModeling]>, + Documentation; } // end: "cplusplus" let ParentPackage = CplusplusOptIn in { @@ -552,14 +561,17 @@ Documentation; def VirtualCallChecker : Checker<"VirtualCall">, - HelpText<"Check virtual function calls during construction or destruction">, + HelpText<"Check virtual function calls during construction/destruction">, CheckerOptions<[ CmdLineOption + InAlpha> ]>, + Dependencies<[VirtualCallModeling]>, Documentation; } // end: "optin.cplusplus" Index: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h =================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h +++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h @@ -18,6 +18,7 @@ extern const char * const MemoryRefCount; extern const char * const MemoryError; extern const char * const UnixAPI; + extern const char * const CXXObjectLifecycle; } } } Index: cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h =================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h +++ cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h @@ -105,6 +105,7 @@ CheckName() = default; StringRef getName() const { return Name; } + operator StringRef() const { return Name; } }; enum class ObjCMessageVisitKind { Index: cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp @@ -6,7 +6,7 @@ // //===----------------------------------------------------------------------===// // -// This file defines a checker that checks virtual function calls during +// This file defines a checker that checks virtual method calls during // construction or destruction of C++ objects. // //===----------------------------------------------------------------------===// @@ -40,11 +40,9 @@ namespace { class VirtualCallChecker : public Checker { - mutable std::unique_ptr BT; - public: - // The flag to determine if pure virtual functions should be issued only. - DefaultBool IsPureOnly; + // These are going to be null if the respective check is disabled. + mutable std::unique_ptr BT_Pure, BT_Impure; void checkBeginFunction(CheckerContext &C) const; void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const; @@ -53,85 +51,13 @@ private: void registerCtorDtorCallInState(bool IsBeginFunction, 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); - } - - PathDiagnosticPieceRef 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) -PathDiagnosticPieceRef 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(LCtx->getDecl()); - const CXXDestructorDecl *DD = - dyn_cast_or_null(LCtx->getDecl()); - - if (!CD && !DD) - return nullptr; - - ProgramStateManager &PSM = State->getStateManager(); - auto &SVB = PSM.getSValBuilder(); - const auto *MD = dyn_cast(LCtx->getDecl()); - if (!MD) - return nullptr; - auto ThiSVal = - State->getSVal(SVB.getCXXThis(MD, LCtx->getStackFrame())); - const MemRegion *Reg = ThiSVal.castAs().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(Pos, InfoText, true); -} - -// The function to check if a callexpr is a virtual function. +// The function to check if a callexpr is a virtual method call. static bool isVirtualCall(const CallExpr *CE) { bool CallIsNonVirtual = false; @@ -176,11 +102,9 @@ const CXXMethodDecl *MD = dyn_cast_or_null(Call.getDecl()); if (!MD) return; + ProgramStateRef State = C.getState(); const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr()); - - if (IsPureOnly && !MD->isPure()) - return; if (!isVirtualCall(CE)) return; @@ -188,29 +112,40 @@ const ObjectState *ObState = State->get(Reg); if (!ObState) return; - // Check if a virtual method is called. - // The GDM of constructor and destructor should be true. - if (*ObState == ObjectState::CtorCalled) { - if (IsPureOnly && MD->isPure()) - reportBug("Call to pure virtual function during construction", true, Reg, - C); - else if (!MD->isPure()) - reportBug("Call to virtual function during construction", false, Reg, C); - else - reportBug("Call to pure virtual function during construction", false, Reg, - C); - } - if (*ObState == ObjectState::DtorCalled) { - if (IsPureOnly && MD->isPure()) - reportBug("Call to pure virtual function during destruction", true, Reg, - C); - else if (!MD->isPure()) - reportBug("Call to virtual function during destruction", false, Reg, C); - else - reportBug("Call to pure virtual function during construction", false, Reg, - C); + bool IsPure = MD->isPure(); + + // At this point we're sure that we're calling a virtual method + // during construction or destruction, so we'll emit a report. + SmallString<128> Msg; + llvm::raw_svector_ostream OS(Msg); + OS << "Call to "; + if (IsPure) + OS << "pure "; + OS << "virtual method '" << MD->getParent()->getNameAsString() + << "::" << MD->getNameAsString() << "' during "; + if (*ObState == ObjectState::CtorCalled) + OS << "construction "; + else + OS << "destruction "; + if (IsPure) + OS << "has undefined behavior"; + else + OS << "bypasses virtual dispatch"; + + ExplodedNode *N = + IsPure ? C.generateErrorNode() : C.generateNonFatalErrorNode(); + if (!N) + return; + + const std::unique_ptr &BT = IsPure ? BT_Pure : BT_Impure; + if (!BT) { + // The respective check is disabled. + return; } + + auto Report = std::make_unique(*BT, OS.str(), N); + C.emitReport(std::move(Report)); } void VirtualCallChecker::registerCtorDtorCallInState(bool IsBeginFunction, @@ -252,34 +187,35 @@ } } -void VirtualCallChecker::reportBug(StringRef Msg, bool IsSink, - const MemRegion *Reg, - CheckerContext &C) const { - ExplodedNode *N; - if (IsSink) - N = C.generateErrorNode(); - else - N = C.generateNonFatalErrorNode(); +void ento::registerVirtualCallModeling(CheckerManager &Mgr) { + Mgr.registerChecker(); +} - if (!N) - return; - if (!BT) - BT.reset(new BugType( - this, "Call to virtual function during construction or destruction", - "C++ Object Lifecycle")); +void ento::registerPureVirtualCallChecker(CheckerManager &Mgr) { + auto *Chk = Mgr.getChecker(); + Chk->BT_Pure = std::make_unique( + Mgr.getCurrentCheckName(), "Pure virtual method call", + categories::CXXObjectLifecycle); +} - auto Reporter = std::make_unique(*BT, Msg, N); - Reporter->addVisitor(std::make_unique(Reg)); - C.emitReport(std::move(Reporter)); +void ento::registerVirtualCallChecker(CheckerManager &Mgr) { + auto *Chk = Mgr.getChecker(); + if (!Mgr.getAnalyzerOptions().getCheckerBooleanOption( + Mgr.getCurrentCheckName(), "PureOnly")) { + Chk->BT_Impure = std::make_unique( + Mgr.getCurrentCheckName(), "Unexpected loss of virtual dispatch", + categories::CXXObjectLifecycle); + } } -void ento::registerVirtualCallChecker(CheckerManager &mgr) { - VirtualCallChecker *checker = mgr.registerChecker(); +bool ento::shouldRegisterVirtualCallModeling(const LangOptions &LO) { + return LO.CPlusPlus; +} - checker->IsPureOnly = - mgr.getAnalyzerOptions().getCheckerBooleanOption(checker, "PureOnly"); +bool ento::shouldRegisterPureVirtualCallChecker(const LangOptions &LO) { + return LO.CPlusPlus; } bool ento::shouldRegisterVirtualCallChecker(const LangOptions &LO) { - return true; + return LO.CPlusPlus; } Index: cfe/trunk/lib/StaticAnalyzer/Core/CommonBugCategories.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/CommonBugCategories.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/CommonBugCategories.cpp @@ -17,4 +17,5 @@ "Memory (Core Foundation/Objective-C/OSObject)"; const char * const MemoryError = "Memory error"; const char * const UnixAPI = "Unix API"; +const char * const CXXObjectLifecycle = "C++ object lifecycle"; }}} Index: cfe/trunk/test/Analysis/virtualcall-plist.cpp =================================================================== --- cfe/trunk/test/Analysis/virtualcall-plist.cpp +++ cfe/trunk/test/Analysis/virtualcall-plist.cpp @@ -0,0 +1,23 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus \ +// RUN: -analyzer-output=plist -o %t.plist -w -verify=pure %s +// RUN: cat %t.plist | FileCheck --check-prefixes=PURE %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus \ +// RUN: -analyzer-output=plist -o %t.plist -w -verify=impure %s +// RUN: cat %t.plist | FileCheck --check-prefixes=IMPURE %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus,optin.cplusplus \ +// RUN: -analyzer-output=plist -o %t.plist -w -verify=pure,impure %s +// RUN: cat %t.plist | FileCheck --check-prefixes=PURE,IMPURE %s + +struct S { + virtual void foo(); + virtual void bar() = 0; + + S() { + // IMPURE: Call to virtual method 'S::foo' during construction bypasses virtual dispatch + // IMPURE: optin.cplusplus.VirtualCall + foo(); // impure-warning{{Call to virtual method 'S::foo' during construction bypasses virtual dispatch}} + // PURE: Call to pure virtual method 'S::bar' during construction has undefined behavior + // PURE: cplusplus.PureVirtualCall + bar(); // pure-warning{{Call to pure virtual method 'S::bar' during construction has undefined behavior}} + } +}; Index: cfe/trunk/test/Analysis/virtualcall.h =================================================================== --- cfe/trunk/test/Analysis/virtualcall.h +++ cfe/trunk/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 method 'Z::foo' during construction bypasses virtual dispatch}} } virtual int foo(); }; Index: cfe/trunk/test/Analysis/virtualcall.cpp =================================================================== --- cfe/trunk/test/Analysis/virtualcall.cpp +++ cfe/trunk/test/Analysis/virtualcall.cpp @@ -1,9 +1,38 @@ -// 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: -analyzer-checker=debug.ExprInspection \ +// RUN: -std=c++11 -verify=impure %s + +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -std=c++11 -verify=pure -std=c++11 %s + +// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \ +// RUN: -analyzer-config \ +// RUN: optin.cplusplus.VirtualCall:PureOnly=true \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -std=c++11 -verify=none %s + +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \ +// RUN: -analyzer-checker=optin.cplusplus.VirtualCall \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -std=c++11 -verify=pure,impure -std=c++11 %s + +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \ +// RUN: -analyzer-checker=optin.cplusplus.VirtualCall \ +// RUN: -analyzer-config \ +// RUN: optin.cplusplus.VirtualCall:PureOnly=true \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -std=c++11 -verify=pure %s + + +// We expect no diagnostics when all checks are disabled. +// none-no-diagnostics -// 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 #include "virtualcall.h" +void clang_analyzer_warnIfReached(); + class A { public: A(); @@ -13,54 +42,32 @@ 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(); // pure-warning{{Call to pure virtual method 'A::foo' during construction has undefined behavior}} + clang_analyzer_warnIfReached(); // no-warning } }; -class B : public A { +A::A() { + f(); +} + +class B { 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 method 'B::foo' during construction bypasses virtual dispatch}} } ~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 method 'B::foo' during destruction bypasses virtual dispatch}} + } }; -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'}} -} - 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 method 'B::foo' during destruction bypasses virtual dispatch}} } class C : public B { @@ -73,12 +80,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 method 'C::foo' during construction bypasses virtual dispatch}} } class D : public B { @@ -97,9 +99,6 @@ foo(); // no-warning } ~E() { bar(); } -#if !PUREONLY - // expected-note-re@-2 2{{{{^}}Calling '~B'}} -#endif int foo() override; }; @@ -135,52 +134,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 method 'H::f' during construction bypasses virtual dispatch}} 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 method 'H::f' during construction bypasses virtual dispatch}} } }; 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 method 'X::g' during construction bypasses virtual dispatch}} } 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 method 'X::g' during construction bypasses virtual dispatch}} } virtual void g(); }; @@ -197,19 +167,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 method 'M::foo' during construction bypasses virtual dispatch}} } class Y { @@ -217,65 +179,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 method 'Y::foobar' during construction bypasses virtual dispatch}} } 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 {