Fix a couple bugs:

- NonLoc::MakeVal() would use sizeof(unsigned) (literally) instead of consulting
  ASTContext for the size (in bits) of 'int'. While it worked, it was a
  conflation of concepts and using ASTContext.IntTy is 100% correct.
- RegionStore::getSizeInElements() no longer assumes that a VarRegion has the
  type "ConstantArray", and handles the case when uses use ordinary variables
  as if they were arrays.
- Fixed ElementRegion::getRValueType() to just return the rvalue type of its
  "array region" in the case the array didn't have ArrayType.
- All of this fixes <rdar://problem/6541136>

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@63347 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Analysis/PathSensitive/BasicValueFactory.h b/include/clang/Analysis/PathSensitive/BasicValueFactory.h
index bf6bb8c80774e8bdf58734863df7f3f99cd817f9..1e08fb9a026639bcaf52ec35f24112220e067345 100644 (file)
--- a/include/clang/Analysis/PathSensitive/BasicValueFactory.h
+++ b/include/clang/Analysis/PathSensitive/BasicValueFactory.h
@@ -76,6 +76,11 @@ public:
   const llvm::APSInt& getValue(uint64_t X, unsigned BitWidth, bool isUnsigned);
   const llvm::APSInt& getValue(uint64_t X, QualType T);
 
+  const llvm::APSInt& getIntValue(uint64_t X, bool isUnsigned) {
+    QualType T = isUnsigned ? Ctx.UnsignedIntTy : Ctx.IntTy;
+    return getValue(X, T);
+  }
+
   inline const llvm::APSInt& getZeroWithPtrWidth(bool isUnsigned = true) {
     return getValue(0, Ctx.getTypeSize(Ctx.VoidPtrTy), isUnsigned);
   }

diff --git a/include/clang/Analysis/PathSensitive/MemRegion.h b/include/clang/Analysis/PathSensitive/MemRegion.h
index 80efac4dbf011f4a4a3d9d2c6eda6e37e42b10e8..8a62eff7a99646e7b9a4f265148552a5b25a996d 100644 (file)
--- a/include/clang/Analysis/PathSensitive/MemRegion.h
+++ b/include/clang/Analysis/PathSensitive/MemRegion.h
@@ -173,6 +173,14 @@ public:
     // FIXME: We can possibly optimize this later to cache this value.
     return C.getPointerType(getRValueType(C));
   }
+  
+  QualType getDesugaredRValueType(ASTContext& C) const {
+    return getRValueType(C)->getDesugaredType();
+  }
+  
+  QualType getDesugaredLValueType(ASTContext& C) const {
+    return getLValueType(C)->getDesugaredType();
+  }
 
   static bool classof(const MemRegion* R) {
     unsigned k = R->getKind();

diff --git a/include/clang/Analysis/PathSensitive/SVals.h b/include/clang/Analysis/PathSensitive/SVals.h
index dbeba45c833aee42cde40eaa397ac055356395ce..a0bc3ed186b5a83d91a7402d2184a97c2a7274ca 100644 (file)
--- a/include/clang/Analysis/PathSensitive/SVals.h
+++ b/include/clang/Analysis/PathSensitive/SVals.h
@@ -171,8 +171,8 @@ public:
 
   static NonLoc MakeVal(SymbolRef sym);
 
-  static NonLoc MakeVal(BasicValueFactory& BasicVals, unsigned X, 
-                        bool isUnsigned);
+  static NonLoc MakeIntVal(BasicValueFactory& BasicVals, uint64_t X, 
+                           bool isUnsigned);
 
   static NonLoc MakeVal(BasicValueFactory& BasicVals, uint64_t X, 
                         unsigned BitWidth, bool isUnsigned);

diff --git a/lib/Analysis/MemRegion.cpp b/lib/Analysis/MemRegion.cpp
index 55c6935b8e85e9d82e2fbe0e6c8252d530854b1b..82f442354192db33000a817906c3acaa2bd7b6e3 100644 (file)
--- a/lib/Analysis/MemRegion.cpp
+++ b/lib/Analysis/MemRegion.cpp
@@ -114,8 +114,9 @@ QualType ElementRegion::getRValueType(ASTContext& C) const {
   if (ArrayType* AT = dyn_cast<ArrayType>(T.getTypePtr()))
     return AT->getElementType();
 
-  PointerType* PtrT = cast<PointerType>(T.getTypePtr());
-  return C.getCanonicalType(PtrT->getPointeeType());
+  // If the RValueType of the array region isn't an ArrayType, then essentially
+  // the element's  
+  return T;
 }
 
 //===----------------------------------------------------------------------===//

diff --git a/lib/Analysis/RegionStore.cpp b/lib/Analysis/RegionStore.cpp
index e50b0abb61e62075e6cc163f29fe0276b46cfcb6..8d36d10a8a515304d0407f8c5b5929b5018ccb88 100644 (file)
--- a/lib/Analysis/RegionStore.cpp
+++ b/lib/Analysis/RegionStore.cpp
@@ -403,20 +403,27 @@ SVal RegionStoreManager::getSizeInElements(const GRState* St,
                                            const MemRegion* R) {
   if (const VarRegion* VR = dyn_cast<VarRegion>(R)) {
     // Get the type of the variable.
-    QualType T = VR->getRValueType(getContext());
+    QualType T = VR->getDesugaredRValueType(getContext());
 
-    // It must be of array type. 
-    const ConstantArrayType* CAT = cast<ConstantArrayType>(T.getTypePtr());
-
-    // return the size as signed integer.
-    return NonLoc::MakeVal(getBasicVals(), CAT->getSize(), false);
+    // FIXME: Handle variable-length arrays.
+    if (isa<VariableArrayType>(T))
+      return UnknownVal();
+    
+    if (const ConstantArrayType* CAT = dyn_cast<ConstantArrayType>(T)) {
+      // return the size as signed integer.
+      return NonLoc::MakeVal(getBasicVals(), CAT->getSize(), false);
+    }
+    
+    // Clients can use ordinary variables as if they were arrays.  These
+    // essentially are arrays of size 1.
+    return NonLoc::MakeIntVal(getBasicVals(), 1, false);
   }
 
   if (const StringRegion* SR = dyn_cast<StringRegion>(R)) {
     const StringLiteral* Str = SR->getStringLiteral();
     // We intentionally made the size value signed because it participates in 
     // operations with signed indices.
-    return NonLoc::MakeVal(getBasicVals(), Str->getByteLength() + 1, false);
+    return NonLoc::MakeIntVal(getBasicVals(), Str->getByteLength()+1, false);
   }
 
   if (const AnonTypedRegion* ATR = dyn_cast<AnonTypedRegion>(R)) {
@@ -880,8 +887,8 @@ const GRState* RegionStoreManager::BindArray(const GRState* St,
 
   // When we are binding the whole array, it always has default value 0.
   GRStateRef state(St, StateMgr);
-  St = state.set<RegionDefaultValue>(R, NonLoc::MakeVal(getBasicVals(), 0, 
-                                                        false));
+  St = state.set<RegionDefaultValue>(R, NonLoc::MakeIntVal(getBasicVals(), 0, 
+                                                           false));
 
   Store store = St->getStore();
 
@@ -961,8 +968,8 @@ RegionStoreManager::BindStruct(const GRState* St, const TypedRegion* R, SVal V){
     // struct decl. In this case, mark the region as having default value.
     if (VI == VE) {
       GRStateRef state(St, StateMgr);
-      St = state.set<RegionDefaultValue>(R, NonLoc::MakeVal(getBasicVals(), 0, 
-                                                            false));
+      const NonLoc& Idx = NonLoc::MakeIntVal(getBasicVals(), 0, false);
+      St = state.set<RegionDefaultValue>(R, Idx);
       break;
     }
 

diff --git a/lib/Analysis/SVals.cpp b/lib/Analysis/SVals.cpp
index 6847764871d1e3614e7d4e5abb87fcf96e12cfa2..824d7229a33abfe7242026dd7f37d10abb985a29 100644 (file)
--- a/lib/Analysis/SVals.cpp
+++ b/lib/Analysis/SVals.cpp
@@ -247,10 +247,9 @@ NonLoc NonLoc::MakeVal(SymbolRef sym) {
   return nonloc::SymbolVal(sym);
 }
 
-NonLoc NonLoc::MakeVal(BasicValueFactory& BasicVals, unsigned X, 
-                       bool isUnsigned) {
-  return nonloc::ConcreteInt(BasicVals.getValue(X, sizeof(unsigned)*8, 
-                                                isUnsigned));
+NonLoc NonLoc::MakeIntVal(BasicValueFactory& BasicVals, uint64_t X, 
+                          bool isUnsigned) {
+  return nonloc::ConcreteInt(BasicVals.getIntValue(X, isUnsigned));
 }
 
 NonLoc NonLoc::MakeVal(BasicValueFactory& BasicVals, uint64_t X, 

diff --git a/test/Analysis/rdar-6541136-region.c b/test/Analysis/rdar-6541136-region.c
new file mode 100644 (file)
index 0000000..4fd2214
--- /dev/null
+++ b/test/Analysis/rdar-6541136-region.c
@@ -0,0 +1,19 @@
+// RUN: clang -verify -analyze -checker-cfref -analyzer-store-region %s
+
+struct tea_cheese { unsigned magic; };
+typedef struct tea_cheese kernel_tea_cheese_t;
+extern kernel_tea_cheese_t _wonky_gesticulate_cheese;
+
+// This test case exercises the ElementRegion::getRValueType() logic.
+
+
+void foo( void )
+{
+  kernel_tea_cheese_t *wonky = &_wonky_gesticulate_cheese;
+  struct load_wine *cmd = (void*) &wonky[1];
+  cmd = cmd;
+  char *p = (void*) &wonky[1];
+  *p = 1;
+  kernel_tea_cheese_t *q = &wonky[1];
+  kernel_tea_cheese_t r = *q; // expected-warning{{out-of-bound memory position}}
+}

diff --git a/test/Analysis/rdar-6541136.c b/test/Analysis/rdar-6541136.c
new file mode 100644 (file)
index 0000000..919a21d
--- /dev/null
+++ b/test/Analysis/rdar-6541136.c
@@ -0,0 +1,20 @@
+// clang -verify -analyze -checker-cfref -analyzer-store-basic %s
+
+struct tea_cheese { unsigned magic; };
+typedef struct tea_cheese kernel_tea_cheese_t;
+extern kernel_tea_cheese_t _wonky_gesticulate_cheese;
+
+// This test case exercises the ElementRegion::getRValueType() logic.
+// All it tests is that it does not crash or do anything weird.
+// The out-of-bounds-access on line 19 is caught using the region store variant.
+
+void foo( void )
+{
+  kernel_tea_cheese_t *wonky = &_wonky_gesticulate_cheese;
+  struct load_wine *cmd = (void*) &wonky[1];
+  cmd = cmd;
+  char *p = (void*) &wonky[1];
+  *p = 1;
+  kernel_tea_cheese_t *q = &wonky[1];
+  kernel_tea_cheese_t r = *q; // no-warning
+}