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

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Sun Sep 4 16:36:44 PDT 2016


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. =/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160904/8cc02fbf/attachment-0001.html>


More information about the llvm-commits mailing list