5 files changed, 118 insertions, 19 deletions

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index d0d1585545..d9712208f0 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -504,13 +504,7 @@ public:
   virtual const Expr *getCXXThisExpr() const { return 0; }
 
   /// \brief Returns the value of the implicit 'this' object.
-  virtual SVal getCXXThisVal() const {
-    const Expr *Base = getCXXThisExpr();
-    // FIXME: This doesn't handle an overloaded ->* operator.
-    if (!Base)
-      return UnknownVal();
-    return getSVal(Base);
-  }
+  virtual SVal getCXXThisVal() const;
 
   virtual const FunctionDecl *getDecl() const;
 
diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp
index 9a2ec0f9c6..1cfa394a07 100644
--- a/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -384,6 +384,25 @@ void CXXInstanceCall::getExtraInvalidatedRegions(RegionList &Regions) const {
     Regions.push_back(R);
 }
 
+SVal CXXInstanceCall::getCXXThisVal() const {
+  const Expr *Base = getCXXThisExpr();
+  // FIXME: This doesn't handle an overloaded ->* operator.
+  if (!Base)
+    return UnknownVal();
+
+  SVal ThisVal = getSVal(Base);
+
+  // FIXME: This is only necessary because we can call member functions on
+  // struct rvalues, which do not have regions we can use for a 'this' pointer.
+  // Ideally this should eventually be changed to an assert, i.e. all
+  // non-Unknown, non-null 'this' values should be loc::MemRegionVals.
+  if (isa<DefinedSVal>(ThisVal))
+    if (!ThisVal.getAsRegion() && !ThisVal.isConstant())
+      return UnknownVal();
+
+  return ThisVal;
+}
+
 
 RuntimeDefinition CXXInstanceCall::getRuntimeDefinition() const {
   // Do we have a decl at all?
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 84001d3a0c..d3d2837fe5 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -34,9 +34,10 @@ void ExprEngine::CreateCXXTemporaryObject(const MaterializeTemporaryExpr *ME,
   // the expression to the location of the object.
   SVal V = state->getSVal(tempExpr, LCtx);
 
-  // If the object is a record, the constructor will have already created
-  // a temporary object region. If it is not, we need to copy the value over.
-  if (!ME->getType()->isRecordType()) {
+  // If the value is already a CXXTempObjectRegion, it is fine as it is.
+  // Otherwise, create a new CXXTempObjectRegion, and copy the value into it.
+  const MemRegion *MR = V.getAsRegion();
+  if (!MR || !isa<CXXTempObjectRegion>(MR)) {
     const MemRegion *R =
       svalBuilder.getRegionManager().getCXXTempObjectRegion(ME, LCtx);
 
diff --git a/test/Analysis/array-struct-region.cpp b/test/Analysis/array-struct-region.cpp
new file mode 100644
index 0000000000..3581566bdc
--- /dev/null
+++ b/test/Analysis/array-struct-region.cpp
@@ -0,0 +1,87 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c++ -analyzer-config c++-inlining=constructors %s
+
+void clang_analyzer_eval(int);
+
+struct S {
+  int field;
+
+#if __cplusplus
+  const struct S *getThis() const { return this; }
+#endif
+};
+
+struct S getS();
+
+
+void testAssignment() {
+  struct S s = getS();
+
+  if (s.field != 42) return;
+  clang_analyzer_eval(s.field == 42); // expected-warning{{TRUE}}
+
+  s.field = 0;
+  clang_analyzer_eval(s.field == 0); // expected-warning{{TRUE}}
+
+#if __cplusplus
+  clang_analyzer_eval(s.getThis() == &s); // expected-warning{{TRUE}}
+#endif
+}
+
+
+void testImmediateUse() {
+  int x = getS().field;
+
+  if (x != 42) return;
+  clang_analyzer_eval(x == 42); // expected-warning{{TRUE}}
+
+#if __cplusplus
+  clang_analyzer_eval((void *)getS().getThis() == (void *)&x); // expected-warning{{FALSE}}
+#endif
+}
+
+int getConstrainedField(struct S s) {
+  if (s.field != 42) return 42;
+  return s.field;
+}
+
+int getAssignedField(struct S s) {
+  s.field = 42;
+  return s.field;
+}
+
+void testArgument() {
+  clang_analyzer_eval(getConstrainedField(getS()) == 42); // expected-warning{{TRUE}}
+  clang_analyzer_eval(getAssignedField(getS()) == 42); // expected-warning{{TRUE}}
+}
+
+
+//--------------------
+// C++-only tests
+//--------------------
+
+#if __cplusplus
+void testReferenceAssignment() {
+  const S &s = getS();
+
+  if (s.field != 42) return;
+  clang_analyzer_eval(s.field == 42); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(s.getThis() == &s); // expected-warning{{TRUE}}
+}
+
+
+int getConstrainedFieldRef(const S &s) {
+  if (s.field != 42) return 42;
+  return s.field;
+}
+
+bool checkThis(const S &s) {
+  return s.getThis() == &s;
+}
+
+void testReferenceArgument() {
+  clang_analyzer_eval(getConstrainedFieldRef(getS()) == 42); // expected-warning{{TRUE}}
+  clang_analyzer_eval(checkThis(getS())); // expected-warning{{TRUE}}
+}
+#endif
diff --git a/test/Analysis/reference.cpp b/test/Analysis/reference.cpp
index 4a2cbb8e25..374f3f7261 100644
--- a/test/Analysis/reference.cpp
+++ b/test/Analysis/reference.cpp
@@ -116,8 +116,13 @@ void testReferenceAddress(int &x) {
 
   struct S { int &x; };
 
-  extern S *getS();
-  clang_analyzer_eval(&getS()->x != 0); // expected-warning{{TRUE}}
+  // FIXME: Should be TRUE. Fields of return-by-value structs are not yet
+  // symbolicated. Tracked by <rdar://problem/12137950>.
+  extern S getS();
+  clang_analyzer_eval(&getS().x != 0); // expected-warning{{UNKNOWN}}
+
+  extern S *getSP();
+  clang_analyzer_eval(&getSP()->x != 0); // expected-warning{{TRUE}}
 }
 
 
@@ -150,10 +155,3 @@ namespace rdar11212286 {
     return *x; // should warn here!
   }
 }
-
-void testReferenceFieldAddress() {
-  struct S { int &x; };
-
-  extern S getS();
-  clang_analyzer_eval(&getS().x != 0); // expected-warning{{UNKNOWN}}
-}