[analyzer] Correctly devirtualize virtual method calls in destructors.

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

clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h

@@ -65,10 +65,14 @@ class DynamicTypeInfo {
 
 public:
   DynamicTypeInfo() : T(QualType()) {}
-  DynamicTypeInfo(QualType WithType, bool CanBeSub = true):
+  DynamicTypeInfo(QualType WithType, bool CanBeSub = true)
-    T(WithType), CanBeASubClass(CanBeSub) {}
+    : T(WithType), CanBeASubClass(CanBeSub) {}
-  QualType getType() { return T; }
+
-  bool canBeASubClass() { return CanBeASubClass; }
+  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);

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<ImplicitCastExpr> > {
   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<CXXDestructorCall>(&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<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);
+  }
+}
+
 void DynamicTypePropagation::checkPostCall(const CallEvent &Call,
                                            CheckerContext &C) const {
   // We can obtain perfect type info for return values from some calls.

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<TypedValueRegion>(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<CXXMethodDecl>(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<CXXMethodDecl>(D);
   if (!MD->isVirtual())
     return AnyFunctionCall::getRuntimeDefinition();
 
-  // If the method is virtual, see if we can find the actual implementation
+  // Do we know the implicit 'this' object being called?
-  // based on context-sensitivity.
+  const MemRegion *R = getCXXThisVal().getAsRegion();
-  // FIXME: Virtual method calls behave differently when an object is being
+  if (!R)
-  // constructed or destructed. It's not as simple as "no devirtualization"
+    return RuntimeDefinition();
-  // 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);
 
-  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(

clang/lib/StaticAnalyzer/Core/ProgramState.cpp

@@ -745,14 +745,16 @@ template<> struct ProgramStateTrait<DynamicTypeMap>
 }}
 
 DynamicTypeInfo ProgramState::getDynamicTypeInfo(const MemRegion *Reg) const {
+  Reg = Reg->StripCasts();
+
   // Look up the dynamic type in the GDM.
   const DynamicTypeInfo *GDMType = get<DynamicTypeMap>(Reg);
   if (GDMType)
     return *GDMType;
 
   // Otherwise, fall back to what we know about the region.
-  if (const TypedValueRegion *TR = dyn_cast<TypedValueRegion>(Reg))
+  if (const TypedRegion *TR = dyn_cast<TypedRegion>(Reg))
-    return DynamicTypeInfo(TR->getValueType());
+    return DynamicTypeInfo(TR->getLocationType(), /*CanBeSubclass=*/false);
 
   if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(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<DynamicTypeMap>(Reg, NewTy);
   assert(NewState);
   return NewState;

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}}
+  }
+}