[cfe-commits] r161981 - in /cfe/trunk: include/clang/AST/DeclCXX.h lib/AST/DeclCXX.cpp lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp lib/StaticAnalyzer/Core/CallEvent.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp test/Analysis/ctor-inlining.mm
Jordan Rose
jordan_rose at apple.com
Wed Aug 15 13:07:18 PDT 2012
Author: jrose
Date: Wed Aug 15 15:07:17 2012
New Revision: 161981
URL: http://llvm.org/viewvc/llvm-project?rev=161981&view=rev
Log:
[analyzer] Correctly devirtualize virtual method calls in constructors.
This is the other half of C++11 [class.cdtor]p4 (the destructor side
was added in r161915). This also fixes an issue with post-call checks
where the 'this' value was already being cleaned out of the state, thus
being omitted from a reconstructed CXXConstructorCall.
Modified:
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
cfe/trunk/test/Analysis/ctor-inlining.mm
Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=161981&r1=161980&r2=161981&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Wed Aug 15 15:07:17 2012
@@ -1646,14 +1646,17 @@
/// \brief Find the method in RD that corresponds to this one.
///
/// Find if RD or one of the classes it inherits from override this method.
- /// If so, return it. RD is assumed to be a base class of the class defining
- /// this method (or be the class itself).
+ /// If so, return it. RD is assumed to be a subclass of the class defining
+ /// this method (or be the class itself), unless MayBeBase is set to true.
CXXMethodDecl *
- getCorrespondingMethodInClass(const CXXRecordDecl *RD);
+ getCorrespondingMethodInClass(const CXXRecordDecl *RD,
+ bool MayBeBase = false);
const CXXMethodDecl *
- getCorrespondingMethodInClass(const CXXRecordDecl *RD) const {
- return const_cast<CXXMethodDecl*>(this)->getCorrespondingMethodInClass(RD);
+ getCorrespondingMethodInClass(const CXXRecordDecl *RD,
+ bool MayBeBase = false) const {
+ return const_cast<CXXMethodDecl *>(this)
+ ->getCorrespondingMethodInClass(RD, MayBeBase);
}
// Implement isa/cast/dyncast/etc.
Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=161981&r1=161980&r2=161981&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Wed Aug 15 15:07:17 2012
@@ -1294,15 +1294,20 @@
}
CXXMethodDecl *
-CXXMethodDecl::getCorrespondingMethodInClass(const CXXRecordDecl *RD) {
+CXXMethodDecl::getCorrespondingMethodInClass(const CXXRecordDecl *RD,
+ bool MayBeBase) {
if (this->getParent()->getCanonicalDecl() == RD->getCanonicalDecl())
return this;
// Lookup doesn't work for destructors, so handle them separately.
if (isa<CXXDestructorDecl>(this)) {
CXXMethodDecl *MD = RD->getDestructor();
- if (MD && recursivelyOverrides(MD, this))
- return MD;
+ if (MD) {
+ if (recursivelyOverrides(MD, this))
+ return MD;
+ if (MayBeBase && recursivelyOverrides(this, MD))
+ return MD;
+ }
return NULL;
}
@@ -1313,6 +1318,8 @@
continue;
if (recursivelyOverrides(MD, this))
return MD;
+ if (MayBeBase && recursivelyOverrides(this, MD))
+ return MD;
}
for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(),
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp?rev=161981&r1=161980&r2=161981&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp Wed Aug 15 15:07:17 2012
@@ -41,9 +41,23 @@
};
}
+static void recordFixedType(const MemRegion *Region, const CXXMethodDecl *MD,
+ CheckerContext &C) {
+ assert(Region);
+ assert(MD);
+
+ ASTContext &Ctx = C.getASTContext();
+ QualType Ty = Ctx.getPointerType(Ctx.getRecordType(MD->getParent()));
+
+ ProgramStateRef State = C.getState();
+ State = State->setDynamicTypeInfo(Region, Ty, /*CanBeSubclass=*/false);
+ C.addTransition(State);
+ return;
+}
+
void DynamicTypePropagation::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
- if (const CXXDestructorCall *Dtor = dyn_cast<CXXDestructorCall>(&Call)) {
+ if (const CXXConstructorCall *Ctor = dyn_cast<CXXConstructorCall>(&Call)) {
// C++11 [class.cdtor]p4: When a virtual function is called directly or
// indirectly from a constructor or from a destructor, including during
// the construction or destruction of the classâs non-static data members,
@@ -51,7 +65,24 @@
// construction or destruction, the function called is the final overrider
// in the constructor's or destructor's class and not one overriding it in
// a more-derived class.
- // FIXME: We don't support this behavior yet for constructors.
+
+ switch (Ctor->getOriginExpr()->getConstructionKind()) {
+ case CXXConstructExpr::CK_Complete:
+ case CXXConstructExpr::CK_Delegating:
+ // No additional type info necessary.
+ return;
+ case CXXConstructExpr::CK_NonVirtualBase:
+ case CXXConstructExpr::CK_VirtualBase:
+ if (const MemRegion *Target = Ctor->getCXXThisVal().getAsRegion())
+ recordFixedType(Target, Ctor->getDecl(), C);
+ return;
+ }
+
+ return;
+ }
+
+ if (const CXXDestructorCall *Dtor = dyn_cast<CXXDestructorCall>(&Call)) {
+ // C++11 [class.cdtor]p4 (see above)
const MemRegion *Target = Dtor->getCXXThisVal().getAsRegion();
if (!Target)
@@ -63,14 +94,8 @@
if (!D)
return;
- const CXXRecordDecl *Class = cast<CXXDestructorDecl>(D)->getParent();
-
- ASTContext &Ctx = C.getASTContext();
- QualType Ty = Ctx.getPointerType(Ctx.getRecordType(Class));
-
- ProgramStateRef State = C.getState();
- State = State->setDynamicTypeInfo(Target, Ty, /*CanBeSubclass=*/false);
- C.addTransition(State);
+ recordFixedType(Target, cast<CXXDestructorDecl>(D), C);
+ return;
}
}
@@ -117,6 +142,31 @@
break;
}
}
+
+ return;
+ }
+
+ if (const CXXConstructorCall *Ctor = dyn_cast<CXXConstructorCall>(&Call)) {
+ // We may need to undo the effects of our pre-call check.
+ switch (Ctor->getOriginExpr()->getConstructionKind()) {
+ case CXXConstructExpr::CK_Complete:
+ case CXXConstructExpr::CK_Delegating:
+ // No additional work necessary.
+ // Note: This will leave behind the actual type of the object for
+ // complete constructors, but arguably that's a good thing, since it
+ // means the dynamic type info will be correct even for objects
+ // constructed with operator new.
+ return;
+ case CXXConstructExpr::CK_NonVirtualBase:
+ case CXXConstructExpr::CK_VirtualBase:
+ if (const MemRegion *Target = Ctor->getCXXThisVal().getAsRegion()) {
+ // We just finished a base constructor. Now we can use the subclass's
+ // type when resolving virtual calls.
+ const Decl *D = C.getLocationContext()->getDecl();
+ recordFixedType(Target, cast<CXXConstructorDecl>(D), C);
+ }
+ return;
+ }
}
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=161981&r1=161980&r2=161981&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Wed Aug 15 15:07:17 2012
@@ -405,7 +405,7 @@
return RuntimeDefinition();
// Find the decl for this method in that class.
- const CXXMethodDecl *Result = MD->getCorrespondingMethodInClass(RD);
+ const CXXMethodDecl *Result = MD->getCorrespondingMethodInClass(RD, true);
assert(Result && "At the very least the static decl should show up.");
// Does the decl that we found have an implementation?
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=161981&r1=161980&r2=161981&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Wed Aug 15 15:07:17 2012
@@ -154,6 +154,11 @@
}
}
+ // Generate a CallEvent /before/ cleaning the state, so that we can get the
+ // correct value for 'this' (if necessary).
+ CallEventManager &CEMgr = getStateManager().getCallEventManager();
+ CallEventRef<> Call = CEMgr.getCaller(calleeCtx, state);
+
// Step 3: BindedRetNode -> CleanedNodes
// If we can find a statement and a block in the inlined function, run remove
// dead bindings before returning from the call. This is important to ensure
@@ -203,21 +208,21 @@
&Ctx);
SaveAndRestore<unsigned> CBISave(currentStmtIdx, calleeCtx->getIndex());
- CallEventManager &CEMgr = getStateManager().getCallEventManager();
- CallEventRef<> Call = CEMgr.getCaller(calleeCtx, CEEState);
+ CallEventRef<> UpdatedCall = Call.cloneWithState(CEEState);
ExplodedNodeSet DstPostCall;
- getCheckerManager().runCheckersForPostCall(DstPostCall, CEENode, *Call,
- *this, true);
+ getCheckerManager().runCheckersForPostCall(DstPostCall, CEENode,
+ *UpdatedCall, *this,
+ /*WasInlined=*/true);
ExplodedNodeSet Dst;
- if (isa<ObjCMethodCall>(Call)) {
- getCheckerManager().runCheckersForPostObjCMessage(Dst, DstPostCall,
- cast<ObjCMethodCall>(*Call),
- *this, true);
+ if (const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(Call)) {
+ getCheckerManager().runCheckersForPostObjCMessage(Dst, DstPostCall, *Msg,
+ *this,
+ /*WasInlined=*/true);
} else if (CE) {
getCheckerManager().runCheckersForPostStmt(Dst, DstPostCall, CE,
- *this, true);
+ *this, /*WasInlined=*/true);
} else {
Dst.insert(DstPostCall);
}
Modified: cfe/trunk/test/Analysis/ctor-inlining.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/ctor-inlining.mm?rev=161981&r1=161980&r2=161981&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/ctor-inlining.mm (original)
+++ cfe/trunk/test/Analysis/ctor-inlining.mm Wed Aug 15 15:07:17 2012
@@ -39,3 +39,51 @@
clang_analyzer_eval(b.x == 42); // expected-warning{{TRUE}}
}
+
+namespace ConstructorVirtualCalls {
+ class A {
+ public:
+ int *out1, *out2, *out3;
+
+ virtual int get() { return 1; }
+
+ A(int *out1) {
+ *out1 = get();
+ }
+ };
+
+ class B : public A {
+ public:
+ virtual int get() { return 2; }
+
+ B(int *out1, int *out2) : A(out1) {
+ *out2 = get();
+ }
+ };
+
+ class C : public B {
+ public:
+ virtual int get() { return 3; }
+
+ C(int *out1, int *out2, int *out3) : B(out1, out2) {
+ *out3 = get();
+ }
+ };
+
+ void test() {
+ int a, b, c;
+
+ C obj(&a, &b, &c);
+ clang_analyzer_eval(a == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(b == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(c == 3); // expected-warning{{TRUE}}
+
+ clang_analyzer_eval(obj.get() == 3); // expected-warning{{TRUE}}
+
+ // Sanity check for devirtualization.
+ A *base = &obj;
+ clang_analyzer_eval(base->get() == 3); // expected-warning{{TRUE}}
+ }
+}
+
+
More information about the cfe-commits
mailing list