6 files changed, 124 insertions, 13 deletions

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h b/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
index b4a9de76f4..56a932b709 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -90,6 +90,9 @@ public:
   /// Returns the type of the APSInt used to store values of the given QualType.
   APSIntType getAPSIntType(QualType T) const {
     assert(T->isIntegerType() || Loc::isLocType(T));
+    // Make sure all locations have the same representation in the analyzer.
+    if (Loc::isLocType(T))
+      T = Ctx.VoidPtrTy;
     return APSIntType(Ctx.getTypeSize(T),
                       !T->isSignedIntegerOrEnumerationType());
   }
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
index e0b5f64b90..52e52e0faa 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -246,8 +246,16 @@ public:
   }
 
   static inline bool isLocType(QualType T) {
+    // Why are record types included here? Because we want to make sure a
+    // record, even a record rvalue, is always represented with a region.
+    // This is especially necessary in C++, where you can call methods on
+    // struct prvalues, which then need to have a valid 'this' pointer.
+    //
+    // This necessitates a bit of extra hackery in the Store to deal with
+    // the case of binding a "struct value" into a struct region; in
+    // practice it just means "dereferencing" the value before binding.
     return T->isAnyPointerType() || T->isBlockPointerType() || 
-           T->isReferenceType();
+           T->isReferenceType() || T->isRecordType();
   }
 };
 
diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp
index b3cf208000..96342260a0 100644
--- a/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1744,6 +1744,26 @@ StoreRef RegionStoreManager::BindStruct(Store store, const TypedValueRegion* R,
   if (!RD->isCompleteDefinition())
     return StoreRef(store, *this);
 
+  // Handle Loc values by automatically dereferencing the location.
+  // This is necessary because we treat all struct values as regions even if
+  // they are rvalues; we may then be asked to bind one of these
+  // "rvalue regions" to an actual struct region.
+  // (This is necessary for many of the test cases in array-struct-region.cpp.)
+  //
+  // This also handles the case of a struct argument passed by value to an
+  // inlined function. In this case, the C++ standard says that the value
+  // is copy-constructed into the parameter variable. However, the copy-
+  // constructor is processed before we actually know if we're going to inline
+  // the function, and thus we don't actually have the parameter's region
+  // available. Instead, we use a temporary-object region, then copy the
+  // bindings over by value.
+  //
+  // FIXME: This will be a problem when we handle the destructors of
+  // temporaries; the inlined function will modify the parameter region,
+  // but the destructor will act on the temporary region.
+  if (const loc::MemRegionVal *MRV = dyn_cast<loc::MemRegionVal>(&V))
+    V = getBinding(store, *MRV);
+
   // Handle lazy compound values and symbolic values.
   if (isa<nonloc::LazyCompoundVal>(V) || isa<nonloc::SymbolVal>(V))
     return BindAggregate(store, R, V);
diff --git a/lib/StaticAnalyzer/Core/SymbolManager.cpp b/lib/StaticAnalyzer/Core/SymbolManager.cpp
index c21df4c318..65356a9458 100644
--- a/lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ b/lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -365,9 +365,6 @@ bool SymbolManager::canSymbolicate(QualType T) {
   if (T->isIntegerType())
     return T->isScalarType();
 
-  if (T->isRecordType() && !T->isUnionType())
-    return true;
-
   return false;
 }
 
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..ce0ee8ed57 100644
--- a/test/Analysis/reference.cpp
+++ b/test/Analysis/reference.cpp
@@ -116,8 +116,11 @@ void testReferenceAddress(int &x) {
 
   struct S { int &x; };
 
-  extern S *getS();
-  clang_analyzer_eval(&getS()->x != 0); // expected-warning{{TRUE}}
+  extern S getS();
+  clang_analyzer_eval(&getS().x != 0); // expected-warning{{TRUE}}
+
+  extern S *getSP();
+  clang_analyzer_eval(&getSP()->x != 0); // expected-warning{{TRUE}}
 }
 
 
@@ -150,10 +153,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}}
-}