diff options
| -rw-r--r-- | clang/include/clang/StaticAnalyzer/Checkers/Checkers.td | 5 | ||||
| -rw-r--r-- | clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp | 121 | ||||
| -rw-r--r-- | clang/test/Analysis/virtualcall.cpp | 64 | ||||
| -rw-r--r-- | clang/test/Analysis/virtualcall.h | 10 |
4 files changed, 152 insertions, 48 deletions
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 6ea500fa749..1b2fd405c88 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -42,6 +42,7 @@ def Nullability : Package<"nullability">; def Cplusplus : Package<"cplusplus">; def CplusplusAlpha : Package<"cplusplus">, InPackage<Alpha>, Hidden; +def CplusplusOptIn : Package<"cplusplus">, InPackage<OptIn>; def Valist : Package<"valist">; def ValistAlpha : Package<"valist">, InPackage<Alpha>, Hidden; @@ -262,13 +263,13 @@ def CXXSelfAssignmentChecker : Checker<"SelfAssignment">, } // end: "cplusplus" -let ParentPackage = CplusplusAlpha in { +let ParentPackage = CplusplusOptIn in { def VirtualCallChecker : Checker<"VirtualCall">, HelpText<"Check virtual function calls during construction or destruction">, DescFile<"VirtualCallChecker.cpp">; -} // end: "alpha.cplusplus" +} // end: "optin.cplusplus" //===----------------------------------------------------------------------===// diff --git a/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp index 55025030261..15e8ea31c4c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp @@ -32,6 +32,18 @@ class WalkAST : public StmtVisitor<WalkAST> { 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<WorkListUnit, 20> DFSWorkList; @@ -59,9 +71,16 @@ class WalkAST : public StmtVisitor<WalkAST> { 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<CXXConstructorDecl>(rootMethod) || + isa<CXXDestructorDecl>(rootMethod)); + } bool hasWork() const { return !WList.empty(); } @@ -132,7 +151,8 @@ void WalkAST::VisitChildren(Stmt *S) { void WalkAST::VisitCallExpr(CallExpr *CE) { VisitChildren(CE); - Enqueue(CE); + if (IsInterprocedural) + Enqueue(CE); } void WalkAST::VisitCXXMemberCallExpr(CallExpr *CE) { @@ -164,51 +184,64 @@ void WalkAST::VisitCXXMemberCallExpr(CallExpr *CE) { !MD->getParent()->hasAttr<FinalAttr>()) 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<const CallExpr *>::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<const CallExpr *>::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<CXXConstructorDecl>(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 @@ void WalkAST::ReportVirtualCall(const CallExpr *CE, bool isPure) { namespace { class VirtualCallChecker : public Checker<check::ASTDecl<CXXRecordDecl> > { 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 @@ public: // 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 @@ public: } void ento::registerVirtualCallChecker(CheckerManager &mgr) { - mgr.registerChecker<VirtualCallChecker>(); + VirtualCallChecker *checker = mgr.registerChecker<VirtualCallChecker>(); + checker->isInterprocedural = + mgr.getAnalyzerOptions().getBooleanOption("Interprocedural", false, + checker); + + checker->isPureOnly = + mgr.getAnalyzerOptions().getBooleanOption("PureOnly", false, + checker); } diff --git a/clang/test/Analysis/virtualcall.cpp b/clang/test/Analysis/virtualcall.cpp index 8ce1d4103b9..e42b898a073 100644 --- a/clang/test/Analysis/virtualcall.cpp +++ b/clang/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=optin.cplusplus.VirtualCall -analyzer-store region -verify -std=c++11 %s +// RUN: %clang_cc1 -analyze -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -analyzer-config optin.cplusplus.VirtualCall:Interprocedural=true -DINTERPROCEDURAL=1 -verify -std=c++11 %s +// RUN: %clang_cc1 -analyze -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -analyzer-config optin.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 @@ public: }; 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 { diff --git a/clang/test/Analysis/virtualcall.h b/clang/test/Analysis/virtualcall.h index 9f7094dc63e..c2ad8a6444c 100644 --- a/clang/test/Analysis/virtualcall.h +++ b/clang/test/Analysis/virtualcall.h @@ -18,7 +18,15 @@ namespace header { 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(); |
