Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h +++ clang/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: clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp +++ clang/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. - bool IsPureOnly = true; + // 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,15 +51,13 @@ private: void registerCtorDtorCallInState(bool IsBeginFunction, CheckerContext &C) const; - void reportBug(StringRef Msg, bool PureError, const MemRegion *Reg, - CheckerContext &C) const; }; } // end namespace // GDM (generic data map) to the memregion of this for the ctor and dtor. REGISTER_MAP_WITH_PROGRAMSTATE(CtorDtorMap, const MemRegion *, ObjectState) -// 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; @@ -106,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; @@ -118,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) { + assert(BT == BT_Impure && "Pure-only check must be enabled!"); + return; } + + auto Report = llvm::make_unique(*BT, OS.str(), N); + C.emitReport(std::move(Report)); } void VirtualCallChecker::registerCtorDtorCallInState(bool IsBeginFunction, @@ -182,34 +187,21 @@ } } -void VirtualCallChecker::reportBug(StringRef Msg, bool IsSink, - const MemRegion *Reg, - CheckerContext &C) const { - ExplodedNode *N; - if (IsSink) - N = C.generateErrorNode(); - else - N = C.generateNonFatalErrorNode(); - - if (!N) - return; - if (!BT) - BT.reset(new BugType( - this, "Call to virtual function during construction or destruction", - "C++ Object Lifecycle")); - - auto Reporter = llvm::make_unique(*BT, Msg, N); - C.emitReport(std::move(Reporter)); -} - void ento::registerPureVirtualCallChecker(CheckerManager &Mgr) { - Mgr.registerChecker(); + auto *Chk = Mgr.registerChecker(); + Chk->BT_Pure = llvm::make_unique( + Mgr.getCurrentCheckName(), "Pure virtual method call", + categories::CXXObjectLifecycle); } void ento::registerVirtualCallChecker(CheckerManager &Mgr) { auto *Chk = Mgr.getChecker(); - Chk->IsPureOnly = Mgr.getAnalyzerOptions().getCheckerBooleanOption( - Mgr.getCurrentCheckName(), "PureOnly"); + if (!Mgr.getAnalyzerOptions().getCheckerBooleanOption( + Mgr.getCurrentCheckName(), "PureOnly")) { + Chk->BT_Impure = llvm::make_unique( + Mgr.getCurrentCheckName(), "Unexpected loss of virtual dispatch", + categories::CXXObjectLifecycle); + } } bool ento::shouldRegisterPureVirtualCallChecker(const LangOptions &LO) { Index: clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp +++ clang/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: clang/test/Analysis/virtualcall-plist.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/virtualcall-plist.cpp @@ -0,0 +1,20 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus \ +// RUN: -analyzer-output=plist -o %t.plist -w -verify %s +// RUN: cat %t.plist | FileCheck %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus,optin.cplusplus \ +// RUN: -analyzer-output=plist -o %t.plist -w -verify %s +// RUN: cat %t.plist | FileCheck %s + +struct S { + virtual void foo(); + virtual void bar() = 0; + + S() { + // CHECK: Call to virtual method 'S::foo' during construction bypasses virtual dispatch + // CHECK: optin.cplusplus.VirtualCall + foo(); // expected-warning{{Call to virtual method 'S::foo' during construction bypasses virtual dispatch}} + // CHECK: Call to pure virtual method 'S::bar' during construction has undefined behavior + // CHECK: cplusplus.PureVirtualCall + bar(); // expected-warning{{Call to pure virtual method 'S::bar' during construction has undefined behavior}} + } +}; Index: clang/test/Analysis/virtualcall.h =================================================================== --- clang/test/Analysis/virtualcall.h +++ clang/test/Analysis/virtualcall.h @@ -2,7 +2,7 @@ class Z { public: Z() { - foo(); // impure-warning {{Call to virtual function during construction}} + foo(); // impure-warning {{Call to virtual method 'Z::foo' during construction bypasses virtual dispatch}} } virtual int foo(); }; Index: clang/test/Analysis/virtualcall.cpp =================================================================== --- clang/test/Analysis/virtualcall.cpp +++ clang/test/Analysis/virtualcall.cpp @@ -1,26 +1,33 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \ +// RUN: -analyzer-checker=debug.ExprInspection \ // RUN: -std=c++11 -verify=expected,impure %s // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \ +// RUN: -analyzer-checker=debug.ExprInspection \ // RUN: -std=c++11 -verify=expected -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=expected %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=expected,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=expected %s #include "virtualcall.h" +void clang_analyzer_warnIfReached(); + class A { public: A(); @@ -30,31 +37,32 @@ virtual int foo() = 0; virtual void bar() = 0; void f() { - foo(); // expected-warning{{Call to pure virtual function during construction}} + foo(); // expected-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() { - foo(); // impure-warning {{Call to virtual function during construction}} + foo(); // impure-warning {{Call to virtual method 'B::foo' during construction bypasses virtual dispatch}} } ~B(); virtual int foo(); virtual void bar() { - foo(); // impure-warning{{Call to virtual function during destruction}} + foo(); // impure-warning {{Call to virtual method 'B::foo' during destruction bypasses virtual dispatch}} } }; -A::A() { - f(); -} - B::~B() { this->B::foo(); // no-warning this->B::bar(); - this->foo(); // impure-warning {{Call to virtual function during destruction}} + this->foo(); // impure-warning {{Call to virtual method 'B::foo' during destruction bypasses virtual dispatch}} } class C : public B { @@ -67,7 +75,7 @@ }; C::C() { - f(foo()); // impure-warning {{Call to virtual function during construction}} + f(foo()); // impure-warning {{Call to virtual method 'C::foo' during construction bypasses virtual dispatch}} } class D : public B { @@ -121,23 +129,23 @@ G g; g.foo(); g.bar(); // no warning - f(); // impure-warning {{Call to virtual function during construction}} + f(); // impure-warning {{Call to virtual method 'H::f' during construction bypasses virtual dispatch}} H &h = *this; - h.f(); // impure-warning {{Call to virtual function during construction}} + h.f(); // impure-warning {{Call to virtual method 'H::f' during construction bypasses virtual dispatch}} } }; class X { public: X() { - g(); // impure-warning {{Call to virtual function during construction}} + g(); // impure-warning {{Call to virtual method 'X::g' during construction bypasses virtual dispatch}} } X(int i) { if (i > 0) { X x(i - 1); x.g(); // no warning } - g(); // impure-warning {{Call to virtual function during construction}} + g(); // impure-warning {{Call to virtual method 'X::g' during construction bypasses virtual dispatch}} } virtual void g(); }; @@ -158,7 +166,7 @@ virtual void foo(); }; void N::callFooOfM(M *m) { - m->foo(); // impure-warning {{Call to virtual function during construction}} + m->foo(); // impure-warning {{Call to virtual method 'M::foo' during construction bypasses virtual dispatch}} } class Y { @@ -166,7 +174,7 @@ virtual void foobar(); void fooY() { F f1; - foobar(); // impure-warning {{Call to virtual function during construction}} + foobar(); // impure-warning {{Call to virtual method 'Y::foobar' during construction bypasses virtual dispatch}} } Y() { fooY(); } };