[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