[cfe-commits] r165676 - in /cfe/trunk: lib/StaticAnalyzer/Core/MemRegion.cpp test/Analysis/unions.cpp

[Jordan Rose]

Author: jrose
Date: Wed Oct 10 18:23:21 2012
New Revision: 165676

URL: http://llvm.org/viewvc/llvm-project?rev=165676&view=rev
Log:
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.

Added:
    cfe/trunk/test/Analysis/unions.cpp
Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=165676&r1=165675&r2=165676&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Wed Oct 10 18:23:21 2012
@@ -1168,8 +1168,12 @@
       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;
       }
 

Added: cfe/trunk/test/Analysis/unions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/unions.cpp?rev=165676&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/unions.cpp (added)
+++ cfe/trunk/test/Analysis/unions.cpp Wed Oct 10 18:23:21 2012
@@ -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;
+  }
+}