[llvm] r281051 - [SelectionDAG] Fix two issues with SDNode::getRawSubclassData().

[Justin Lebar via llvm-commits]

Author: jlebar
Date: Fri Sep  9 11:07:06 2016
New Revision: 281051

URL: http://llvm.org/viewvc/llvm-project?rev=281051&view=rev
Log:
[SelectionDAG] Fix two issues with SDNode::getRawSubclassData().

1) On some platforms, sizeof(SDNodeBits) == 1, so we were only copying
one byte out of the bitfield when we wanted to copy two, and we were
leaving half of the return value of getRawSubclassData() undefined.

2) Something something bitfields, not sure exactly what the issue or fix
is, yet.  (TODO)

Summary:
Previously we were assuming that SDNodeBits covered all of SDNode's
anonymous subclass data bitfield union.  But that's not right; it might
have size 1, in which it clearly doesn't.

This patch adds a field that does cover the whole union and adds
static_asserts to ensure it stays correct.

Reviewers: ahatanak, chandlerc

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D24223

Modified:
    llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h

Modified: llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h?rev=281051&r1=281050&r2=281051&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h (original)
+++ llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h Fri Sep  9 11:07:06 2016
@@ -463,6 +463,7 @@ protected:
   };
 
   union {
+    char RawSDNodeBits[sizeof(uint16_t)];
     SDNodeBitfields SDNodeBits;
     ConstantSDNodeBitfields ConstantSDNodeBits;
     MemSDNodeBitfields MemSDNodeBits;
@@ -471,6 +472,21 @@ protected:
     StoreSDNodeBitfields StoreSDNodeBits;
   };
 
+  // RawSDNodeBits must cover the entirety of the union.  This means that all
+  // of the union's members must have size <= RawSDNodeBits.
+  static_assert(sizeof(SDNodeBits) <= sizeof(RawSDNodeBits),
+                "SDNodeBits too wide");
+  static_assert(sizeof(ConstantSDNodeBits) <= sizeof(RawSDNodeBits),
+                "ConstantSDNodeBits too wide");
+  static_assert(sizeof(MemSDNodeBits) <= sizeof(RawSDNodeBits),
+                "MemSDNodeBits too wide");
+  static_assert(sizeof(LSBaseSDNodeBits) <= sizeof(RawSDNodeBits),
+                "LSBaseSDNodeBits too wide");
+  static_assert(sizeof(LoadSDNodeBits) <= sizeof(RawSDNodeBits),
+                "LoadSDNodeBits too wide");
+  static_assert(sizeof(StoreSDNodeBits) <= sizeof(RawSDNodeBits),
+                "StoreSDNodeBits too wide");
+
 private:
   /// Unique id per SDNode in the DAG.
   int NodeId;
@@ -876,7 +892,7 @@ protected:
       : NodeType(Opc), NodeId(-1), OperandList(nullptr), ValueList(VTs.VTs),
         UseList(nullptr), NumOperands(0), NumValues(VTs.NumVTs), IROrder(Order),
         debugLoc(std::move(dl)) {
-    memset(&SDNodeBits, 0, sizeof(SDNodeBits));
+    memset(&RawSDNodeBits, 0, sizeof(RawSDNodeBits));
     assert(debugLoc.hasTrivialDestructor() && "Expected trivial destructor");
     assert(NumValues == VTs.NumVTs &&
            "NumValues wasn't wide enough for its operands!");
@@ -1095,9 +1111,7 @@ public:
   /// function should only be used to compute a FoldingSetNodeID value.
   unsigned getRawSubclassData() const {
     uint16_t Data;
-    memcpy(&Data, &SDNodeBits, sizeof(SDNodeBits));
-    static_assert(sizeof(SDNodeBits) <= sizeof(uint16_t),
-                  "SDNodeBits field too large?");
+    memcpy(&Data, &RawSDNodeBits, sizeof(RawSDNodeBits));
     return Data;
   }