[PATCH] D23036: [SelectionDAG] Use a union of bitfield structs for SDNode::SubclassData.

[Justin Lebar via llvm-commits]

https://reviews.llvm.org/D24223, probably wrong.  :)

On Sun, Sep 4, 2016 at 4:36 PM, Chandler Carruth <chandlerc at google.com> wrote:
> On Sun, Sep 4, 2016 at 4:23 PM Justin Lebar via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>>
>> Sorry for taking so long to come back to this; I was sick after I got
>> back from my trip.
>>
>> Now that I look at it, I don't think the new code is right.
>>
>>    unsigned getRawSubclassData() const {
>>      uint16_t Data;
>>      memcpy(&Data, &SDNodeBits, sizeof(SDNodeBits));
>>      static_assert(sizeof(SDNodeBits) <= sizeof(uint16_t),
>>        "SDNodeBits field too large?");
>>      return Data;
>>    }
>>
>> If sizeof(SDNodeBits) == 1, then we have a few problems:
>>
>> * Our code is endianness-unsafe.  Which doesn't matter because we're
>> just using this as input into a hashtable key, but still.
>> * Half of Data is uninitialized.
>> * We're not copying all of the bits from the anonymous union, which is
>> the intent of the code.
>>
>> The obvious thing would be to add a uint16_t member to the anon union,
>> but I am pretty sure Chandler explained to me this would be UB.
>
>
> It just doesn't solve the problem. You can't *use* that member to get
> defined bits back out. When we store a subset of the bits, we're allowed to
> store crazy undef things into the other bits in the uint16_t because you can
> no longer access them. =/