From 0f6d63be06a32a7b6e53719f6f68462f1d27af87 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Wed, 15 Aug 2012 00:51:56 +0000 Subject: [PATCH] [analyzer] Correctly devirtualize virtual method calls in destructors. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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, and the object to which the call applies is the object under 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. llvm-svn: 161915 --- .../Core/PathSensitive/ProgramState.h | 12 ++-- .../Checkers/DynamicTypePropagation.cpp | 37 ++++++++++- clang/lib/StaticAnalyzer/Core/CallEvent.cpp | 62 ++++++++++--------- .../lib/StaticAnalyzer/Core/ProgramState.cpp | 7 ++- clang/test/Analysis/dtor.cpp | 54 ++++++++++++++++ 5 files changed, 135 insertions(+), 37 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h index d36aa1bd1bcb..b0c51dd5b928 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h @@ -65,10 +65,14 @@ class DynamicTypeInfo { public: DynamicTypeInfo() : T(QualType()) {} - DynamicTypeInfo(QualType WithType, bool CanBeSub = true): - T(WithType), CanBeASubClass(CanBeSub) {} - QualType getType() { return T; } - bool canBeASubClass() { return CanBeASubClass; } + DynamicTypeInfo(QualType WithType, bool CanBeSub = true) + : T(WithType), CanBeASubClass(CanBeSub) {} + + bool isValid() const { return !T.isNull(); } + + QualType getType() const { return T; } + bool canBeASubClass() const { return CanBeASubClass; } + void Profile(llvm::FoldingSetNodeID &ID) const { T.Profile(ID); ID.AddInteger((unsigned)CanBeASubClass); diff --git a/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp b/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp index fea57337bbda..036686e964c2 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp @@ -25,7 +25,8 @@ using namespace ento; namespace { class DynamicTypePropagation: - public Checker< check::PostCall, + public Checker< check::PreCall, + check::PostCall, check::PostStmt > { const ObjCObjectType *getObjectTypeForAllocAndNew(const ObjCMessageExpr *MsgE, CheckerContext &C) const; @@ -34,11 +35,45 @@ class DynamicTypePropagation: const ObjCObjectPointerType *getBetterObjCType(const Expr *CastE, CheckerContext &C) const; public: + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPostCall(const CallEvent &Call, CheckerContext &C) const; void checkPostStmt(const ImplicitCastExpr *CastE, CheckerContext &C) const; }; } +void DynamicTypePropagation::checkPreCall(const CallEvent &Call, + CheckerContext &C) const { + if (const CXXDestructorCall *Dtor = dyn_cast(&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, + // and the object to which the call applies is the object under + // 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. + + const MemRegion *Target = Dtor->getCXXThisVal().getAsRegion(); + if (!Target) + return; + + // FIXME: getRuntimeDefinition() can be expensive. It would be better to do + // this when we are entering the stack frame for the destructor. + const Decl *D = Dtor->getRuntimeDefinition().getDecl(); + if (!D) + return; + + const CXXRecordDecl *Class = cast(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); + } +} + void DynamicTypePropagation::checkPostCall(const CallEvent &Call, CheckerContext &C) const { // We can obtain perfect type info for return values from some calls. diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp index 7a0cb4abe42b..db48fba60e80 100644 --- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -376,47 +376,49 @@ void CXXInstanceCall::getExtraInvalidatedRegions(RegionList &Regions) const { Regions.push_back(R); } -static const CXXMethodDecl *devirtualize(const CXXMethodDecl *MD, SVal ThisVal){ - const MemRegion *R = ThisVal.getAsRegion(); - if (!R) - return 0; - - const TypedValueRegion *TR = dyn_cast(R->StripCasts()); - if (!TR) - return 0; - - const CXXRecordDecl *RD = TR->getValueType()->getAsCXXRecordDecl(); - if (!RD) - return 0; - - const CXXMethodDecl *Result = MD->getCorrespondingMethodInClass(RD); - const FunctionDecl *Definition; - if (!Result->hasBody(Definition)) - return 0; - - return cast(Definition); -} - RuntimeDefinition CXXInstanceCall::getRuntimeDefinition() const { + // Do we have a decl at all? const Decl *D = getDecl(); if (!D) return RuntimeDefinition(); + // If the method is non-virtual, we know we can inline it. const CXXMethodDecl *MD = cast(D); if (!MD->isVirtual()) return AnyFunctionCall::getRuntimeDefinition(); - // If the method is virtual, see if we can find the actual implementation - // based on context-sensitivity. - // FIXME: Virtual method calls behave differently when an object is being - // constructed or destructed. It's not as simple as "no devirtualization" - // because a /partially/ constructed object can be referred to through a - // base pointer. We'll eventually want to use DynamicTypeInfo here. - if (const CXXMethodDecl *Devirtualized = devirtualize(MD, getCXXThisVal())) - return RuntimeDefinition(Devirtualized); + // Do we know the implicit 'this' object being called? + const MemRegion *R = getCXXThisVal().getAsRegion(); + if (!R) + return RuntimeDefinition(); - return RuntimeDefinition(); + // Do we know anything about the type of 'this'? + DynamicTypeInfo DynType = getState()->getDynamicTypeInfo(R); + if (!DynType.isValid()) + return RuntimeDefinition(); + + // Is the type a C++ class? (This is mostly a defensive check.) + QualType RegionType = DynType.getType()->getPointeeType(); + const CXXRecordDecl *RD = RegionType->getAsCXXRecordDecl(); + if (!RD) + return RuntimeDefinition(); + + // Find the decl for this method in that class. + const CXXMethodDecl *Result = MD->getCorrespondingMethodInClass(RD); + assert(Result && "At the very least the static decl should show up."); + + // Does the decl that we found have an implementation? + const FunctionDecl *Definition; + if (!Result->hasBody(Definition)) + return RuntimeDefinition(); + + // We found a definition. If we're not sure that this devirtualization is + // actually what will happen at runtime, make sure to provide the region so + // that ExprEngine can decide what to do with it. + if (DynType.canBeASubClass()) + return RuntimeDefinition(Definition, R->StripCasts()); + return RuntimeDefinition(Definition, /*DispatchRegion=*/0); } void CXXInstanceCall::getInitialStackFrameContents( diff --git a/clang/lib/StaticAnalyzer/Core/ProgramState.cpp b/clang/lib/StaticAnalyzer/Core/ProgramState.cpp index dc988cc5f1fc..2000338ee0a6 100644 --- a/clang/lib/StaticAnalyzer/Core/ProgramState.cpp +++ b/clang/lib/StaticAnalyzer/Core/ProgramState.cpp @@ -745,14 +745,16 @@ template<> struct ProgramStateTrait }} DynamicTypeInfo ProgramState::getDynamicTypeInfo(const MemRegion *Reg) const { + Reg = Reg->StripCasts(); + // Look up the dynamic type in the GDM. const DynamicTypeInfo *GDMType = get(Reg); if (GDMType) return *GDMType; // Otherwise, fall back to what we know about the region. - if (const TypedValueRegion *TR = dyn_cast(Reg)) - return DynamicTypeInfo(TR->getValueType()); + if (const TypedRegion *TR = dyn_cast(Reg)) + return DynamicTypeInfo(TR->getLocationType(), /*CanBeSubclass=*/false); if (const SymbolicRegion *SR = dyn_cast(Reg)) { SymbolRef Sym = SR->getSymbol(); @@ -764,6 +766,7 @@ DynamicTypeInfo ProgramState::getDynamicTypeInfo(const MemRegion *Reg) const { ProgramStateRef ProgramState::setDynamicTypeInfo(const MemRegion *Reg, DynamicTypeInfo NewTy) const { + Reg = Reg->StripCasts(); ProgramStateRef NewState = set(Reg, NewTy); assert(NewState); return NewState; diff --git a/clang/test/Analysis/dtor.cpp b/clang/test/Analysis/dtor.cpp index 620994858c7c..1f45925561c9 100644 --- a/clang/test/Analysis/dtor.cpp +++ b/clang/test/Analysis/dtor.cpp @@ -173,3 +173,57 @@ void testDefaultArg() { // Force a bug to be emitted. *(char *)0 = 1; // expected-warning{{Dereference of null pointer}} } + + +namespace DestructorVirtualCalls { + class A { + public: + int *out1, *out2, *out3; + + virtual int get() { return 1; } + + ~A() { + *out1 = get(); + } + }; + + class B : public A { + public: + virtual int get() { return 2; } + + ~B() { + *out2 = get(); + } + }; + + class C : public B { + public: + virtual int get() { return 3; } + + ~C() { + *out3 = get(); + } + }; + + void test() { + int a, b, c; + + // New scope for the C object. + { + C obj; + 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}} + + obj.out1 = &a; + obj.out2 = &b; + obj.out3 = &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}} + } +}