r176388 - [analyzer] Special-case bitfields when finding sub-region bindings.

[Jordan Rose]

Author: jrose
Date: Fri Mar  1 17:03:17 2013
New Revision: 176388

URL: http://llvm.org/viewvc/llvm-project?rev=176388&view=rev
Log:
[analyzer] Special-case bitfields when finding sub-region bindings.

Previously we were assuming that we'd never ask for the sub-region bindings
of a bitfield, since a bitfield cannot have subregions. However,
unification of code paths has made that assumption invalid. While we could
take advantage of this by just checking for the single possible binding,
it's probably better to do the right thing, so that if/when we someday
support unions we'll do the right thing there, too.

This fixes a handful of false positives in analyzing LLVM.

<rdar://problem/13325522>

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
    cfe/trunk/test/Analysis/fields.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=176388&r1=176387&r2=176388&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Fri Mar  1 17:03:17 2013
@@ -195,6 +195,10 @@ DefinedOrUnknownSVal TypedValueRegion::g
 }
 
 DefinedOrUnknownSVal FieldRegion::getExtent(SValBuilder &svalBuilder) const {
+  // Force callers to deal with bitfields explicitly.
+  if (getDecl()->isBitField())
+    return UnknownVal();
+
   DefinedOrUnknownSVal Extent = DeclRegion::getExtent(svalBuilder);
 
   // A zero-length array at the end of a struct often stands for dynamically-

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=176388&r1=176387&r2=176388&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Fri Mar  1 17:03:17 2013
@@ -757,9 +757,7 @@ collectSubRegionBindings(SmallVectorImpl
     TopKey = BindingKey::Make(Top, BindingKey::Default);
   }
 
-  // This assumes the region being invalidated is char-aligned. This isn't
-  // true for bitfields, but since bitfields have no subregions they shouldn't
-  // be using this function anyway.
+  // Find the length (in bits) of the region being invalidated.
   uint64_t Length = UINT64_MAX;
   SVal Extent = Top->getExtent(SVB);
   if (Optional<nonloc::ConcreteInt> ExtentCI =
@@ -768,6 +766,9 @@ collectSubRegionBindings(SmallVectorImpl
     assert(ExtentInt.isNonNegative() || ExtentInt.isUnsigned());
     // Extents are in bytes but region offsets are in bits. Be careful!
     Length = ExtentInt.getLimitedValue() * SVB.getContext().getCharWidth();
+  } else if (const FieldRegion *FR = dyn_cast<FieldRegion>(Top)) {
+    if (FR->getDecl()->isBitField())
+      Length = FR->getDecl()->getBitWidthValue(SVB.getContext());
   }
 
   for (ClusterBindings::iterator I = Cluster.begin(), E = Cluster.end();

Modified: cfe/trunk/test/Analysis/fields.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/fields.c?rev=176388&r1=176387&r2=176388&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/fields.c (original)
+++ cfe/trunk/test/Analysis/fields.c Fri Mar  1 17:03:17 2013
@@ -29,6 +29,10 @@ void test() {
   (void)(p = getit()).x;
 }
 
+#define true ((bool)1)
+#define false ((bool)0)
+typedef _Bool bool;
+
 
 void testLazyCompoundVal() {
   Point p = {42, 0};
@@ -36,3 +40,86 @@ void testLazyCompoundVal() {
   clang_analyzer_eval((q = p).x == 42); // expected-warning{{TRUE}}
   clang_analyzer_eval(q.x == 42); // expected-warning{{TRUE}}
 }
+
+
+struct Bits {
+  unsigned a : 1;
+  unsigned b : 2;
+  unsigned c : 1;
+
+  bool x;
+
+  struct InnerBits {
+    bool y;
+
+    unsigned d : 16;
+    unsigned e : 6;
+    unsigned f : 2;
+  } inner;
+};
+
+void testBitfields() {
+  struct Bits bits;
+
+  if (foo() && bits.b) // expected-warning {{garbage}}
+    return;
+  if (foo() && bits.inner.e) // expected-warning {{garbage}}
+    return;
+
+  bits.c = 1;
+  clang_analyzer_eval(bits.c == 1); // expected-warning {{TRUE}}
+
+  if (foo() && bits.b) // expected-warning {{garbage}}
+    return;
+  if (foo() && bits.x) // expected-warning {{garbage}}
+    return;
+
+  bits.x = true;
+  clang_analyzer_eval(bits.x == true); // expected-warning{{TRUE}}
+  bits.b = 2;
+  clang_analyzer_eval(bits.x == true); // expected-warning{{TRUE}}
+  if (foo() && bits.c) // no-warning
+    return;
+
+  bits.inner.e = 50;
+  if (foo() && bits.inner.e) // no-warning
+    return;
+  if (foo() && bits.inner.y) // expected-warning {{garbage}}
+    return;
+  if (foo() && bits.inner.f) // expected-warning {{garbage}}
+    return;
+
+  extern struct InnerBits getInner();
+  bits.inner = getInner();
+  
+  if (foo() && bits.inner.e) // no-warning
+    return;
+  if (foo() && bits.inner.y) // no-warning
+    return;
+  if (foo() && bits.inner.f) // no-warning
+    return;
+
+  bits.inner.f = 1;
+  
+  if (foo() && bits.inner.e) // no-warning
+    return;
+  if (foo() && bits.inner.y) // no-warning
+    return;
+  if (foo() && bits.inner.f) // no-warning
+    return;
+
+  if (foo() && bits.a) // expected-warning {{garbage}}
+    return;
+}
+
+
+//-----------------------------------------------------------------------------
+// Incorrect behavior
+//-----------------------------------------------------------------------------
+
+void testTruncation() {
+  struct Bits bits;
+  bits.c = 0x11; // expected-warning{{implicit truncation}}
+  // FIXME: We don't model truncation of bitfields.
+  clang_analyzer_eval(bits.c == 1); // expected-warning {{FALSE}}
+}