<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Sun, Sep 4, 2016 at 4:23 PM Justin Lebar via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Sorry for taking so long to come back to this; I was sick after I got<br>
back from my trip.<br>
<br>
Now that I look at it, I don't think the new code is right.<br>
<br>
   unsigned getRawSubclassData() const {<br>
     uint16_t Data;<br>
     memcpy(&Data, &SDNodeBits, sizeof(SDNodeBits));<br>
     static_assert(sizeof(SDNodeBits) <= sizeof(uint16_t),<br>
       "SDNodeBits field too large?");<br>
     return Data;<br>
   }<br>
<br>
If sizeof(SDNodeBits) == 1, then we have a few problems:<br>
<br>
* Our code is endianness-unsafe.  Which doesn't matter because we're<br>
just using this as input into a hashtable key, but still.<br>
* Half of Data is uninitialized.<br>
* We're not copying all of the bits from the anonymous union, which is<br>
the intent of the code.<br>
<br>
The obvious thing would be to add a uint16_t member to the anon union,<br>
but I am pretty sure Chandler explained to me this would be UB.</blockquote><div><br></div><div>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. =/</div></div></div>