Index: include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- include/clang/StaticAnalyzer/Checkers/Checkers.td +++ include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -260,16 +260,11 @@ HelpText<"Checks C++ copy and move assignment operators for self assignment">, DescFile<"CXXSelfAssignmentChecker.cpp">; -} // end: "cplusplus" - -let ParentPackage = CplusplusAlpha in { - def VirtualCallChecker : Checker<"VirtualCall">, - HelpText<"Check virtual function calls during construction or destruction">, + HelpText<"Check for virtual function calls during construction or destruction">, DescFile<"VirtualCallChecker.cpp">; -} // end: "alpha.cplusplus" - +} // end: "cplusplus" //===----------------------------------------------------------------------===// // Valist checkers. Index: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp +++ lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp @@ -32,6 +32,10 @@ BugReporter &BR; AnalysisDeclContext *AC; + /// Whether the checker should walk into bodies of called functions. + /// Controlled by the "Interprocedural" analyzer-config option. + bool IsInterprocedural = false; + typedef const CallExpr * WorkListUnit; typedef SmallVector DFSWorkList; @@ -60,8 +64,9 @@ public: WalkAST(const CheckerBase *checker, BugReporter &br, - AnalysisDeclContext *ac) - : Checker(checker), BR(br), AC(ac), visitingCallExpr(nullptr) {} + AnalysisDeclContext *ac, bool isInterprocedural) + : Checker(checker), BR(br), AC(ac), visitingCallExpr(nullptr), + IsInterprocedural(isInterprocedural) { } bool hasWork() const { return !WList.empty(); } @@ -132,7 +137,8 @@ void WalkAST::VisitCallExpr(CallExpr *CE) { VisitChildren(CE); - Enqueue(CE); + if (IsInterprocedural) + Enqueue(CE); } void WalkAST::VisitCXXMemberCallExpr(CallExpr *CE) { @@ -164,51 +170,56 @@ !MD->getParent()->hasAttr()) ReportVirtualCall(CE, MD->isPure()); - Enqueue(CE); + if (IsInterprocedural) + Enqueue(CE); } void WalkAST::ReportVirtualCall(const CallExpr *CE, bool isPure) { SmallString<100> buf; llvm::raw_svector_ostream os(buf); - os << "Call Path : "; - // Name of current visiting CallExpr. - os << *CE->getDirectCallee(); - - // Name of the CallExpr whose body is current walking. - if (visitingCallExpr) - os << " <-- " << *visitingCallExpr->getDirectCallee(); - // Names of FunctionDecls in worklist with state PostVisited. - for (SmallVectorImpl::iterator I = WList.end(), + // FIXME: The interprocedural diagnostic experience here is not good. + // Ultimately this checker should be re-written to be path sensitive. + // For now, only diagnose intraprocedurally, by default. + if (IsInterprocedural) { + os << "Call Path : "; + // Name of current visiting CallExpr. + os << *CE->getDirectCallee(); + + // Name of the CallExpr whose body is current being walked. + if (visitingCallExpr) + os << " <-- " << *visitingCallExpr->getDirectCallee(); + // Names of FunctionDecls in worklist with state PostVisited. + for (SmallVectorImpl::iterator I = WList.end(), E = WList.begin(); I != E; --I) { - const FunctionDecl *FD = (*(I-1))->getDirectCallee(); - assert(FD); - if (VisitedFunctions[FD] == PostVisited) - os << " <-- " << *FD; + const FunctionDecl *FD = (*(I-1))->getDirectCallee(); + assert(FD); + if (VisitedFunctions[FD] == PostVisited) + os << " <-- " << *FD; + } + + os << "\n"; } PathDiagnosticLocation CELoc = PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC); SourceRange R = CE->getCallee()->getSourceRange(); - if (isPure) { - os << "\n" << "Call pure virtual functions during construction or " - << "destruction may leads undefined behaviour"; - BR.EmitBasicReport(AC->getDecl(), Checker, - "Call pure virtual function during construction or " - "Destruction", - "Cplusplus", os.str(), CELoc, R); - return; - } - else { - os << "\n" << "Call virtual functions during construction or " - << "destruction will never go to a more derived class"; - BR.EmitBasicReport(AC->getDecl(), Checker, - "Call virtual function during construction or " - "Destruction", - "Cplusplus", os.str(), CELoc, R); - return; - } + os << "Call to "; + if (isPure) + os << "pure "; + + os << "virtual function during construction or destruction "; + + if (isPure) + os << "has undefined behavior"; + else + os << "will not dispatch to derived class"; + + BR.EmitBasicReport(AC->getDecl(), Checker, + "Call to virtual function during construction or " + "destruction", + "C++ Object Lifecycle", os.str(), CELoc, R); } //===----------------------------------------------------------------------===// @@ -218,9 +229,11 @@ namespace { class VirtualCallChecker : public Checker > { public: + DefaultBool isInterprocedural; + void checkASTDecl(const CXXRecordDecl *RD, AnalysisManager& mgr, BugReporter &BR) const { - WalkAST walker(this, BR, mgr.getAnalysisDeclContext(RD)); + WalkAST walker(this, BR, mgr.getAnalysisDeclContext(RD), isInterprocedural); // Check the constructors. for (const auto *I : RD->ctors()) { @@ -242,5 +255,8 @@ } void ento::registerVirtualCallChecker(CheckerManager &mgr) { - mgr.registerChecker(); + VirtualCallChecker *checker = mgr.registerChecker(); + checker->isInterprocedural = + mgr.getAnalyzerOptions().getBooleanOption("Interprocedural", false, + checker); } Index: test/Analysis/dtor.cpp =================================================================== --- test/Analysis/dtor.cpp +++ test/Analysis/dtor.cpp @@ -184,7 +184,7 @@ virtual int get() { return 1; } ~A() { - *out1 = get(); + *out1 = get(); // expected-warning {{Call to virtual function during construction or destruction will not dispatch to derived class}} } }; @@ -193,7 +193,7 @@ virtual int get() { return 2; } ~B() { - *out2 = get(); + *out2 = get(); // expected-warning {{Call to virtual function during construction or destruction will not dispatch to derived class}} } }; @@ -202,7 +202,7 @@ virtual int get() { return 3; } ~C() { - *out3 = get(); + *out3 = get(); // expected-warning {{Call to virtual function during construction or destruction will not dispatch to derived class}} } }; Index: test/Analysis/virtualcall.h =================================================================== --- test/Analysis/virtualcall.h +++ test/Analysis/virtualcall.h @@ -18,7 +18,12 @@ class A { public: A() { - foo(); // expected-warning{{Call virtual functions during construction or destruction will never go to a more derived class}} + foo(); +#if INTERPROCEDURAL + // expected-warning-re@-2 {{{{^}}Call Path : fooCall to virtual function during construction or destruction will not dispatch to derived class}} +#else + // expected-warning-re@-4 {{{{^}}Call to virtual function during construction or destruction will not dispatch to derived class}} +#endif } virtual int foo(); Index: test/Analysis/virtualcall.cpp =================================================================== --- test/Analysis/virtualcall.cpp +++ test/Analysis/virtualcall.cpp @@ -1,4 +1,5 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.cplusplus.VirtualCall -analyzer-store region -verify -std=c++11 %s +// RUN: %clang_cc1 -analyze -analyzer-checker=cplusplus.VirtualCall -analyzer-store region -verify -std=c++11 %s +// RUN: %clang_cc1 -analyze -analyzer-checker=cplusplus.VirtualCall -analyzer-store region -analyzer-config cplusplus.VirtualCall:Interprocedural=true -DINTERPROCEDURAL=1 -verify -std=c++11 %s class A { public: @@ -8,19 +9,30 @@ virtual int foo() = 0; virtual void bar() = 0; void f() { - foo(); // expected-warning{{Call pure virtual functions during construction or destruction may leads undefined behaviour}} + foo(); +#if INTERPROCEDURAL + // expected-warning-re@-2 {{{{^}}Call Path : foo <-- fCall to pure virtual function during construction or destruction has undefined behavior}} +#endif } }; class B : public A { public: B() { - foo(); // expected-warning{{Call virtual functions during construction or destruction will never go to a more derived class}} + foo(); +#if INTERPROCEDURAL + // expected-warning-re@-2 {{{{^}}Call Path : fooCall to virtual function during construction or destruction will not dispatch to derived class}} +#else + // expected-warning-re@-4 {{{{^}}Call to virtual function during construction or destruction will not dispatch to derived class}} +#endif } ~B(); virtual int foo(); - virtual void bar() { foo(); } // expected-warning{{Call virtual functions during construction or destruction will never go to a more derived class}} + virtual void bar() { foo(); } +#if INTERPROCEDURAL + // expected-warning-re@-2 {{{{^}}Call Path : foo <-- barCall to virtual function during construction or destruction will not dispatch to derived class}} +#endif }; A::A() { @@ -30,7 +42,13 @@ B::~B() { this->B::foo(); // no-warning this->B::bar(); - this->foo(); // expected-warning{{Call virtual functions during construction or destruction will never go to a more derived class}} + this->foo(); +#if INTERPROCEDURAL + // expected-warning-re@-2 {{{{^}}Call Path : fooCall to virtual function during construction or destruction will not dispatch to derived class}} +#else + // expected-warning-re@-4 {{{{^}}Call to virtual function during construction or destruction will not dispatch to derived class}} +#endif + } class C : public B { @@ -43,7 +61,12 @@ }; C::C() { - f(foo()); // expected-warning{{Call virtual functions during construction or destruction will never go to a more derived class}} + f(foo()); +#if INTERPROCEDURAL + // expected-warning-re@-2 {{{{^}}Call Path : fooCall to virtual function during construction or destruction will not dispatch to derived class}} +#else + // expected-warning-re@-4 {{{{^}}Call to virtual function during construction or destruction will not dispatch to derived class}} +#endif } class D : public B {