Reapply "[analyzer] Treat fields of unions as having symbolic offsets."

This time, actually uncomment the code that's supposed to fix the problem.

This reverts r165671 / 8ceb837585ed973dc36fba8dfc57ef60fc8f2735.

llvm-svn: 165676

2 files changed, 55 insertions, 1 deletions

diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index 7c66739558a..fab10cfd3d0 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1168,8 +1168,12 @@ RegionOffset MemRegion::getAsOffset() const {
       R = FR->getSuperRegion();
 
       const RecordDecl *RD = FR->getDecl()->getParent();
-      if (!RD->isCompleteDefinition()) {
+      if (RD->isUnion() || !RD->isCompleteDefinition()) {
         // We cannot compute offset for incomplete type.
+        // For unions, we could treat everything as offset 0, but we'd rather
+        // treat each field as a symbolic offset so they aren't stored on top
+        // of each other, since we depend on things in typed regions actually
+        // matching their types.
         SymbolicOffsetBase = R;
       }
 
diff --git a/clang/test/Analysis/unions.cpp b/clang/test/Analysis/unions.cpp
new file mode 100644
index 00000000000..e7671a90b90
--- /dev/null
+++ b/clang/test/Analysis/unions.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core %s -verify
+
+namespace PR14054_reduced {
+  struct Definition;
+  struct ParseNode {
+    union {
+      Definition *lexdef;
+      ParseNode *data;
+    } pn_u;
+  };
+  struct Definition : public ParseNode { };
+
+  void CloneParseTree(ParseNode *opn, ParseNode *pn,  ParseNode *x) {
+    // This used to cause an assertion failure because:
+    // 1. The implicit operator= for unions assigns all members of the union,
+    //    not just the active one (b/c there's no way to know which is active).
+    // 2. RegionStore dutifully stored all the variants at the same offset;
+    //    the last one won.
+    // 3. We asked for the value of the first variant but got back a conjured
+    //    symbol for the second variant.
+    // 4. We ended up trying to add a base cast to a region of the wrong type.
+    //
+    // Now (at the time this test was added), we instead treat all variants of
+    // a union as different offsets, but only allow one to be active at a time.
+    *pn = *opn;
+    x = pn->pn_u.lexdef->pn_u.lexdef;
+  }
+}
+
+namespace PR14054_original {
+  struct Definition;
+  struct ParseNode {
+    union {
+      struct {
+        union {};
+        Definition *lexdef;
+      } name;
+      class {
+        int *target;
+        ParseNode *data;
+      } xmlpi;
+    } pn_u;
+  };
+  struct Definition : public ParseNode { };
+
+  void CloneParseTree(ParseNode *opn, ParseNode *pn,  ParseNode *x) {
+    pn->pn_u = opn->pn_u;
+    x = pn->pn_u.name.lexdef->pn_u.name.lexdef;
+  }
+}