3 files changed, 109 insertions, 6 deletions

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 535de9fd98..d4abbdce5e 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -17,6 +17,7 @@
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/AST/CXXInheritance.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/ParentMap.h"
 #include "llvm/ADT/SmallSet.h"
@@ -118,6 +119,44 @@ static std::pair<const Stmt*,
   return std::pair<const Stmt*, const CFGBlock*>(S, Blk);
 }
 
+/// Adjusts a return value when the called function's return type does not
+/// match the caller's expression type. This can happen when a dynamic call
+/// is devirtualized, and the overridding method has a covariant (more specific)
+/// return type than the parent's method. For C++ objects, this means we need
+/// to add base casts.
+static SVal adjustReturnValue(SVal V, QualType ExpectedTy, QualType ActualTy,
+                              StoreManager &StoreMgr) {
+  // For now, the only adjustments we handle apply only to locations.
+  if (!isa<Loc>(V))
+    return V;
+
+  // If the types already match, don't do any unnecessary work.
+  if (ExpectedTy == ActualTy)
+    return V;
+
+  // No adjustment is needed between Objective-C pointer types.
+  if (ExpectedTy->isObjCObjectPointerType() &&
+      ActualTy->isObjCObjectPointerType())
+    return V;
+
+  // C++ object pointers may need "derived-to-base" casts.
+  const CXXRecordDecl *ExpectedClass = ExpectedTy->getPointeeCXXRecordDecl();
+  const CXXRecordDecl *ActualClass = ActualTy->getPointeeCXXRecordDecl();
+  if (ExpectedClass && ActualClass) {
+    CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/true,
+                       /*DetectVirtual=*/false);
+    if (ActualClass->isDerivedFrom(ExpectedClass, Paths) &&
+        !Paths.isAmbiguous(ActualTy->getCanonicalTypeUnqualified())) {
+      return StoreMgr.evalDerivedToBase(V, Paths.front());
+    }
+  }
+
+  // Unfortunately, Objective-C does not enforce that overridden methods have
+  // covariant return types, so we can't assert that that never happens.
+  // Be safe and return UnknownVal().
+  return UnknownVal();
+}
+
 /// The call exit is simulated with a sequence of nodes, which occur between 
 /// CallExitBegin and CallExitEnd. The following operations occur between the 
 /// two program points:
@@ -144,6 +183,11 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) {
   const CFGBlock *Blk = 0;
   llvm::tie(LastSt, Blk) = getLastStmt(CEBNode);
 
+  // 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 2: generate node with bound return value: CEBNode -> BindedRetNode.
 
   // If the callee returns an expression, bind its value to CallExpr.
@@ -151,6 +195,18 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) {
     if (const ReturnStmt *RS = dyn_cast_or_null<ReturnStmt>(LastSt)) {
       const LocationContext *LCtx = CEBNode->getLocationContext();
       SVal V = state->getSVal(RS, LCtx);
+
+      const Decl *Callee = calleeCtx->getDecl();
+      if (Callee != Call->getDecl()) {
+        QualType ReturnedTy = CallEvent::getDeclaredResultType(Callee);
+        if (!ReturnedTy.isNull()) {
+          if (const Expr *Ex = dyn_cast<Expr>(CE)) {
+            V = adjustReturnValue(V, Ex->getType(), ReturnedTy,
+                                  getStoreManager());
+          }
+        }
+      }
+
       state = state->BindExpr(CE, callerCtx, V);
     }
 
@@ -172,11 +228,6 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) {
     }
   }
 
-  // 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
diff --git a/test/Analysis/inline.cpp b/test/Analysis/inline.cpp
index b39b87101a..ddcf5d01c3 100644
--- a/test/Analysis/inline.cpp
+++ b/test/Analysis/inline.cpp
@@ -340,3 +340,30 @@ namespace QualifiedCalls {
     clang_analyzer_eval(object->A::getZero() == 0); // expected-warning{{TRUE}}
 }
 }
+
+
+namespace rdar12409977  {
+  struct Base {
+    int x;
+  };
+
+  struct Parent : public Base {
+    virtual Parent *vGetThis();
+    Parent *getThis() { return vGetThis(); }
+  };
+
+  struct Child : public Parent {
+    virtual Child *vGetThis() { return this; }
+  };
+
+  void test() {
+    Child obj;
+    obj.x = 42;
+
+    // Originally, calling a devirtualized method with a covariant return type
+    // caused a crash because the return value had the wrong type. When we then
+    // go to layer a CXXBaseObjectRegion on it, the base isn't a direct base of
+    // the object region and we get an assertion failure.
+    clang_analyzer_eval(obj.getThis()->x == 42); // expected-warning{{TRUE}}
+  }
+}
diff --git a/test/Analysis/inlining/InlineObjCInstanceMethod.m b/test/Analysis/inlining/InlineObjCInstanceMethod.m
index b09775bd38..d1b1d945a1 100644
--- a/test/Analysis/inlining/InlineObjCInstanceMethod.m
+++ b/test/Analysis/inlining/InlineObjCInstanceMethod.m
@@ -83,4 +83,29 @@
 // Don't crash if we don't know the receiver's region.
 void randomlyMessageAnObject(MyClass *arr[], int i) {
   (void)[arr[i] getInt];
-}
\ No newline at end of file
+}
+
+
+@interface EvilChild : MyParent
+- (id)getInt;
+@end
+
+@implementation EvilChild
+- (id)getInt { // expected-warning {{types are incompatible}}
+  return self;
+}
+@end
+
+int testNonCovariantReturnType() {
+  MyParent *obj = [[EvilChild alloc] init];
+
+  // Devirtualization allows us to directly call -[EvilChild getInt], but
+  // that returns an id, not an int. There is an off-by-default warning for
+  // this, -Woverriding-method-mismatch, and an on-by-default analyzer warning,
+  // osx.cocoa.IncompatibleMethodTypes. This code would probably crash at
+  // runtime, but at least the analyzer shouldn't crash.
+  int x = 1 + [obj getInt];
+
+  [obj release];
+  return 5/(x-1); // no-warning
+}