From 8b1460250e896b186527b6a9a7c27148387de837 Mon Sep 17 00:00:00 2001 From: Justin Lebar Date: Fri, 9 Sep 2016 16:07:06 +0000 Subject: [PATCH] [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 llvm-svn: 281051 --- llvm/include/llvm/CodeGen/SelectionDAGNodes.h | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h index fad77ae..a1dca49 100644 --- a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h +++ b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h @@ -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; } -- 2.7.4