Author: jrose
Date: Wed Sep  5 12:11:15 2012
New Revision: 163218

URL: http://llvm.org/viewvc/llvm-project?rev=163218&view=rev
Log:
Revert "[analyzer] Treat all struct values as regions (even rvalues)."

This turned out to have many implications, but what eventually seemed to
make it unworkable was the fact that we can get struct values (as
LazyCompoundVals) from other places besides return-by-value function calls;
that is, we weren't actually able to "treat all struct values as regions"
consistently across the entire analyzer core.

Hopefully we'll be able to come up with an alternate solution soon.

This reverts r163066 / 02df4f0aef142f00d4637cd851e54da2a123ca8e.

Removed:
    cfe/trunk/test/Analysis/array-struct-region.cpp
Modified:
    
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
    cfe/trunk/test/Analysis/reference.cpp

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h?rev=163218&r1=163217&r2=163218&view=diff
==============================================================================
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h 
(original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h 
Wed Sep  5 12:11:15 2012
@@ -93,9 +93,6 @@
   /// 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());
   }

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h?rev=163218&r1=163217&r2=163218&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h Wed Sep  
5 12:11:15 2012
@@ -246,16 +246,8 @@
   }
 
   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->isRecordType();
+           T->isReferenceType();
   }
 };
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=163218&r1=163217&r2=163218&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Wed Sep  5 12:11:15 2012
@@ -1744,26 +1744,6 @@
   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);

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp?rev=163218&r1=163217&r2=163218&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp Wed Sep  5 12:11:15 2012
@@ -365,6 +365,9 @@
   if (T->isIntegerType())
     return T->isScalarType();
 
+  if (T->isRecordType() && !T->isUnionType())
+    return true;
+
   return false;
 }
 

Removed: cfe/trunk/test/Analysis/array-struct-region.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/array-struct-region.cpp?rev=163217&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/array-struct-region.cpp (original)
+++ cfe/trunk/test/Analysis/array-struct-region.cpp (removed)
@@ -1,87 +0,0 @@
-// 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

Modified: cfe/trunk/test/Analysis/reference.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/reference.cpp?rev=163218&r1=163217&r2=163218&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/reference.cpp (original)
+++ cfe/trunk/test/Analysis/reference.cpp Wed Sep  5 12:11:15 2012
@@ -116,11 +116,8 @@
 
   struct S { int &x; };
 
-  extern S getS();
-  clang_analyzer_eval(&getS().x != 0); // expected-warning{{TRUE}}
-
-  extern S *getSP();
-  clang_analyzer_eval(&getSP()->x != 0); // expected-warning{{TRUE}}
+  extern S *getS();
+  clang_analyzer_eval(&getS()->x != 0); // expected-warning{{TRUE}}
 }
 
 
@@ -153,3 +150,10 @@
     return *x; // should warn here!
   }
 }
+
+void testReferenceFieldAddress() {
+  struct S { int &x; };
+
+  extern S getS();
+  clang_analyzer_eval(&getS().x != 0); // expected-warning{{UNKNOWN}}
+}


_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to