[analyzer] Invalidate union regions properly. Don't hesitate to load the default binding later.

[NoQ]

Summary

(1) We weren't invalidating our unions correctly. The previous behavior in invalidateRegionsWorker::VisitCluster() was to direct-bind an UnknownVal to the union (at offset 0). The proposed behavior is to default-bind a conjured symbol (of irrelevant type) to the union that's being invalidated, similarly to what we do for structures and classes.

(2) In order to keep ourselves afloat, we were never actually loading default bindings from our unions, because there never was any default binding to load, and the value that is presumed when there's no default binding to load is usually completely incorrect (eg. UndefinedVal for stack unions).

Fix bug (1) and then remove hack (2).

[NoQ]

test/Analysis/unions.cpp

82

This test got fixed because we're now loading the default-bound lazy compound value for vv from uu correctly.

[a.sidorin]

LGTM, thanks!

lib/StaticAnalyzer/Core/RegionStore.cpp

 
 typedef const RegionBindingsRef& RegionBindingsConstRef;
 
 Optional<SVal> RegionBindingsRef::getDirectBinding(const MemRegion *R) const {
   return Optional<SVal>::create(lookup(R, BindingKey::Direct));
 }
 
 Optional<SVal> RegionBindingsRef::getDefaultBinding(const MemRegion *R) const {
-  if (R->isBoundable())
-    if (const TypedValueRegion *TR = dyn_cast<TypedValueRegion>(R))
-      if (TR->getValueType()->isUnionType())
-        return UnknownVal();
-
   return Optional<SVal>::create(lookup(R, BindingKey::Default));
 }
 
 RegionBindingsRef RegionBindingsRef::addBinding(BindingKey K, SVal V) const {
   const MemRegion *Base = K.getBaseRegion();
 
   const ClusterBindings *ExistingCluster = lookup(Base);
   ClusterBindings Cluster =
void invalidateRegionsWorker::VisitCluster(const MemRegion *baseR,
 
   if (isInitiallyIncludedGlobalRegion(baseR)) {
     // If the region is a global and we are invalidating all globals,
     // erasing the entry is good enough.  This causes all globals to be lazily
     // symbolicated from the same base symbol.
     return;
   }
 
-  if (T->isStructureOrClassType()) {
+  if (T->isRecordType()) {
     // Invalidate the region by setting its default value to
     // conjured symbol. The type of the symbol is irrelevant.
     DefinedOrUnknownSVal V = svalBuilder.conjureSymbolVal(baseR, Ex, LCtx,
                                                           Ctx.IntTy, Count);
     B = B.addBinding(baseR, BindingKey::Default, V);
     return;
   }
 

test/Analysis/unions.cpp

namespace PR17596 {
   void testCopy() {
     IntOrString uu;
     uu.i = 4;
     clang_analyzer_eval(uu.i == 4); // expected-warning{{TRUE}}
 
     IntOrString vv;
     vv.i = 5;
     uu = vv;
-    // FIXME: Should be true.
-    clang_analyzer_eval(uu.i == 5); // expected-warning{{UNKNOWN}}
+    clang_analyzer_eval(uu.i == 5); // expected-warning{{TRUE}}
   }
 
   void testInvalidation() {
     IntOrString uu;
     uu.s = strdup("");
 
     IntOrString vv;
     char str[] = "abc";
void testIndirectInvalidation() {
     uu.s = str;
 
     clang_analyzer_eval(uu.s[0] == 'a'); // expected-warning{{TRUE}}
 
     process(uu);
     clang_analyzer_eval(uu.s[0] == 'a'); // expected-warning{{UNKNOWN}}
   }
 }
+
+namespace assume_union_contents {
+union U {
+  int x;
+};
+
+U get();
+
+void test() {
+  U u = get();
+  int y = 0;
+  if (u.x)
+    y = 1;
+  if (u.x)
+    y = 1 / y; // no-warning
+}
+} // end namespace assume_union_contents