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,18 @@ BugReporter &BR; AnalysisDeclContext *AC; + /// The root constructor or destructor whose callees are being analyzed. + const CXXMethodDecl *RootMethod = nullptr; + + /// Whether the checker should walk into bodies of called functions. + /// Controlled by the "Interprocedural" analyzer-config option. + bool IsInterprocedural = false; + + /// Whether the checker should only warn for calls to pure virtual functions + /// (which is undefined behavior) or for all virtual functions (which may + /// may result in unexpected behavior). + bool ReportPureOnly = false; + typedef const CallExpr * WorkListUnit; typedef SmallVector DFSWorkList; @@ -59,9 +71,16 @@ const CallExpr *visitingCallExpr; public: - WalkAST(const CheckerBase *checker, BugReporter &br, - AnalysisDeclContext *ac) - : Checker(checker), BR(br), AC(ac), visitingCallExpr(nullptr) {} + WalkAST(const CheckerBase *checker, BugReporter &br, AnalysisDeclContext *ac, + const CXXMethodDecl *rootMethod, bool isInterprocedural, + bool reportPureOnly) + : Checker(checker), BR(br), AC(ac), RootMethod(rootMethod), + IsInterprocedural(isInterprocedural), ReportPureOnly(reportPureOnly), + visitingCallExpr(nullptr) { + // Walking should always start from either a constructor or a destructor. + assert(isa(rootMethod) || + isa(rootMethod)); + } bool hasWork() const { return !WList.empty(); } @@ -132,7 +151,8 @@ void WalkAST::VisitCallExpr(CallExpr *CE) { VisitChildren(CE); - Enqueue(CE); + if (IsInterprocedural) + Enqueue(CE); } void WalkAST::VisitCXXMemberCallExpr(CallExpr *CE) { @@ -164,51 +184,64 @@ !MD->getParent()->hasAttr()) ReportVirtualCall(CE, MD->isPure()); - Enqueue(CE); + if (IsInterprocedural) + Enqueue(CE); } void WalkAST::ReportVirtualCall(const CallExpr *CE, bool isPure) { + if (ReportPureOnly && !isPure) + return; + 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 "; + + if (isa(RootMethod)) + os << "construction "; + else + os << "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,14 +251,18 @@ namespace { class VirtualCallChecker : public Checker > { public: + DefaultBool isInterprocedural; + DefaultBool isPureOnly; + void checkASTDecl(const CXXRecordDecl *RD, AnalysisManager& mgr, BugReporter &BR) const { - WalkAST walker(this, BR, mgr.getAnalysisDeclContext(RD)); + AnalysisDeclContext *ADC = mgr.getAnalysisDeclContext(RD); // Check the constructors. for (const auto *I : RD->ctors()) { if (!I->isCopyOrMoveConstructor()) if (Stmt *Body = I->getBody()) { + WalkAST walker(this, BR, ADC, I, isInterprocedural, isPureOnly); walker.Visit(Body); walker.Execute(); } @@ -234,6 +271,7 @@ // Check the destructor. if (CXXDestructorDecl *DD = RD->getDestructor()) if (Stmt *Body = DD->getBody()) { + WalkAST walker(this, BR, ADC, DD, isInterprocedural, isPureOnly); walker.Visit(Body); walker.Execute(); } @@ -242,5 +280,12 @@ } void ento::registerVirtualCallChecker(CheckerManager &mgr) { - mgr.registerChecker(); + VirtualCallChecker *checker = mgr.registerChecker(); + checker->isInterprocedural = + mgr.getAnalyzerOptions().getBooleanOption("Interprocedural", false, + checker); + + checker->isPureOnly = + mgr.getAnalyzerOptions().getBooleanOption("PureOnly", 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 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 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 destruction will not dispatch to derived class}} } }; Index: test/Analysis/virtualcall.h =================================================================== --- test/Analysis/virtualcall.h +++ test/Analysis/virtualcall.h @@ -18,7 +18,15 @@ class A { public: A() { - foo(); // expected-warning{{Call virtual functions during construction or destruction will never go to a more derived class}} + foo(); +#if !PUREONLY +#if INTERPROCEDURAL + // expected-warning-re@-3 {{{{^}}Call Path : fooCall to virtual function during construction will not dispatch to derived class}} +#else + // expected-warning-re@-5 {{{{^}}Call to virtual function during construction will not dispatch to derived class}} +#endif +#endif + } virtual int foo(); Index: test/Analysis/virtualcall.cpp =================================================================== --- test/Analysis/virtualcall.cpp +++ test/Analysis/virtualcall.cpp @@ -1,36 +1,79 @@ -// 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 +// RUN: %clang_cc1 -analyze -analyzer-checker=cplusplus.VirtualCall -analyzer-store region -analyzer-config cplusplus.VirtualCall:PureOnly=true -DPUREONLY=1 -verify -std=c++11 %s + +/* When INTERPROCEDURAL is set, we expect diagnostics in all functions reachable + from a constructor or destructor. If it is not set, we expect diagnostics + only in the constructor or destructor. + + When PUREONLY is set, we expect diagnostics only for calls to pure virtual + functions not to non-pure virtual functions. +*/ class A { public: A(); + A(int i); + ~A() {}; - virtual int foo() = 0; + virtual int foo() = 0; // from Sema: expected-note {{'foo' declared here}} 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 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 !PUREONLY +#if INTERPROCEDURAL + // expected-warning-re@-3 {{{{^}}Call Path : fooCall to virtual function during construction will not dispatch to derived class}} +#else + // expected-warning-re@-5 {{{{^}}Call to virtual function during construction will not dispatch to derived class}} +#endif +#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 destruction will not dispatch to derived class}} +#endif }; A::A() { f(); } +A::A(int i) { + foo(); // From Sema: expected-warning {{call to pure virtual member function 'foo' has undefined behavior}} +#if INTERPROCEDURAL + // expected-warning-re@-2 {{{{^}}Call Path : fooCall to pure virtual function during construction has undefined behavior}} +#else + // expected-warning-re@-4 {{{{^}}Call to pure virtual function during construction has undefined behavior}} +#endif +} + 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 !PUREONLY +#if INTERPROCEDURAL + // expected-warning-re@-3 {{{{^}}Call Path : fooCall to virtual function during destruction will not dispatch to derived class}} +#else + // expected-warning-re@-5 {{{{^}}Call to virtual function during destruction will not dispatch to derived class}} +#endif +#endif + } class C : public B { @@ -43,7 +86,14 @@ }; C::C() { - f(foo()); // expected-warning{{Call virtual functions during construction or destruction will never go to a more derived class}} + f(foo()); +#if !PUREONLY +#if INTERPROCEDURAL + // expected-warning-re@-3 {{{{^}}Call Path : fooCall to virtual function during construction will not dispatch to derived class}} +#else + // expected-warning-re@-5 {{{{^}}Call to virtual function during construction will not dispatch to derived class}} +#endif +#endif } class D : public B {