This revision was automatically updated to reflect the committed changes.
Closed by commit rL289309: [analyzer] Improve VirtualCallChecker diagnostics
and move into optin package. (authored by dcoughlin).
Changed prior to commit:
https://reviews.llvm.org/D26768?vs=80979&id=80984#toc
Repository:
rL LLVM
https://reviews.llvm.org/D26768
Files:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
cfe/trunk/test/Analysis/virtualcall.cpp
cfe/trunk/test/Analysis/virtualcall.h
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
@@ -42,6 +42,7 @@
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 @@
} // 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"
//===----------------------------------------------------------------------===//
Index: cfe/trunk/test/Analysis/virtualcall.cpp
===================================================================
--- cfe/trunk/test/Analysis/virtualcall.cpp
+++ cfe/trunk/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 @@
};
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 {
Index: cfe/trunk/test/Analysis/virtualcall.h
===================================================================
--- cfe/trunk/test/Analysis/virtualcall.h
+++ cfe/trunk/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: cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ cfe/trunk/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<WorkListUnit, 20> 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<CXXConstructorDecl>(rootMethod) ||
+ isa<CXXDestructorDecl>(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<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,29 +251,41 @@
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();
}
}
// 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();
}
}
};
}
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);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
