On Tue, Sep 11, 2012 at 6:47 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word"><div><div class="h5"><div><div>On Sep 11, 2012, at 6:29 PM, Chandler Carruth wrote:</div><blockquote type="cite"><div class="gmail_extra"><div class="gmail_quote">On Tue, Sep 11, 2012 at 6:09 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>On Tue, Sep 11, 2012 at 6:04 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br>
</div></div><div class="gmail_quote"><div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>On Sep 11, 2012, at 4:41 PM, Richard Smith wrote:<br>
> On Tue, Sep 11, 2012 at 11:33 AM, John McCall <<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>> wrote:<br>
> On Sep 11, 2012, at 11:07 AM, Chandler Carruth wrote:<br>
> > On Fri, Aug 31, 2012 at 1:01 PM, Eli Friedman <<a href="mailto:eli.friedman@gmail.com" target="_blank">eli.friedman@gmail.com</a>> wrote:<br>
</div><div>> >> Another nasty case I just thought of:<br>
> >><br>
> >> struct x { int i : 24; };<br>
> >> struct y { int i : 24; char j; };<br>
> >> union z {<br>
> >> struct x x;<br>
> >> struct y y;<br>
> >> };<br>
> >> union z a;<br>
> >> void f(int i) {<br>
> >> a.x.i = i;<br>
> >> }<br>
> >> void g(char j) {<br>
> >> a.y.j = j<br>
> >> }<br>
> >><br>
> >> The two writes are to separate memory locations. :)<br>
> ><br>
> > Wait, hold on... I'm deeply confused. Maybe because I don't know how C11 unions work?<br>
> ><br>
> > With C++11, only one side of the union is allowed to be active, and so I don't think they are separate memory locations?<br>
><br>
> I agree that this isn't a problem, but the analysis is a bit more complicated;<br>
> it hinges on the fact that it's okay to *read* from an inactive union member<br>
> under the common-prefix exception, but it's not okay to *write* to it. The<br>
> same analysis applies in both standards:<br>
><br>
> Is this still OK if the extra union member is volatile? Chandler and I have discussed this, and it's far from clear that it would be. (In particular, we can conceive of a seemingly-reasonable case where the union sits on an MMIO port, and only the fourth byte has volatile semantics.)<br>
<br>
</div>I see no reason why making the member volatile changes anything.<br>
Other members in the union can be assumed not to exist, because the<br>
active union member *must* be the one we're assigning to — indeed,<br>
in C such an assignment is how you change the active union member.<br></blockquote><div><br></div></div></div><div>I'm talking about the load-widening case, not the store-widening. Given:</div><div><br></div><div>union U {</div>
<div> struct X { int a : 24; volatile char b; } x;</div><div> struct Y { int c : 24; } y;</div><div>};</div><div><br></div><div>... if x is the active member, and we load y.c, can we really load from the storage location containing x.b?</div>
</div>
</blockquote></div><br></div><div class="gmail_extra">After going back and forth a few times amongst the committee members here, we propose this:</div><div class="gmail_extra"><br></div><div class="gmail_extra">This is a defect. We should disallow reading from the common prefix of a union using a non-active member if there are any volatile members following the common prefix. There doesn't appear to be any way to support these union types and common-prefix reads and the committee's express desire that load widening is a valid compiler optimization.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">Reasons why this truly seems like a defect:</div><div class="gmail_extra"><br></div><div class="gmail_extra">1) We can't even load widen through the padding bytes:</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">union U {<br> struct X { int a : 24; volatile char b; } x;</div><div class="gmail_extra"> struct Y { int a : 24; int c; } y;</div><div class="gmail_extra">};</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">2) We can't even load widen when the next field in all members after the common prefix is non-volatile:</div><div class="gmail_extra"><br></div><div class="gmail_extra">
union U {<br> struct X { int a : 33; char b; volatile char c; } x;</div><div class="gmail_extra"> struct Y { int a : 33; } y;</div><div class="gmail_extra">};</div><div class="gmail_extra"><br></div><div class="gmail_extra">
<br></div><div class="gmail_extra">Seem plausible?</div>
</blockquote></div><br></div></div><div>I could accept this as a defect. I don't think it's required.</div><div><br></div><div>For example, I claim that, if we need x and z, we are allowed to use a 32-bit</div><div>
load here (although, granted, it might not be a good idea):</div><div> struct [[align(4)]] A {</div><div> char x;</div><div> volatile char y;</div><div> char z;</div><div> };</div></div></blockquote><div><br></div>
<div><div>[The common initial sequence exemption only applies to structs as union members, so that case is fine. Assuming these are each wrapped in a struct...]</div><div><br></div><div>If 'y' is the active member of your union, then the intent of the standard certainly seems to be that an access to 'x' or 'z' actually accesses the object denoted by 'y'. Hence such an access is to a volatile object, but the access is through a non-volatile glvalue, so behavior is undefined by <a href="http://7.1.6.1/6">7.1.6.1/6</a>. So we can load widen here when accessing 'x' or 'z'.</div>
<div><br></div><div>The same analysis can be applied to my example (and Chandler's examples) too, with a bit of work. The standard doesn't explicitly say which bytes are accessed by an lvalue-to-rvalue conversion on a non-class type, but the implication is that a read of y.c (y.a in Chandler's examples) is permitted to the entirety of the memory location containing it. So if we define the memory location of y.c to be widenend to include padding (which is exactly what I believe Chandler's current patch does), then a load of y.c has undefined behavior if the active member of the union contains a volatile subobject overlapping y.c, and in any other case, the widened load is unobservable. We can use the same argument to widen loads of arbitrary struct fields into padding, but not into adjacent fields (because memory locations for fields cannot overlap).</div>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>We are allowed to do this by dint of declaring "our implementation supports</div>
<div>memory-mapped I/O ports but does not define semantics for aggregates</div><div>which partially overlap them". Voilà, the widened load can be assumed to</div><div>have no side effects and is therefore permitted.</div>
</div></blockquote><div><br></div><div><div>I'm not convinced by your argument: as-if doesn't apply to the observable behavior of the program, so we can't assume the widened load has no side-effects. However, since I've reached the same conclusion via a different route, I don't think it's worth debating.</div>
</div><div>-- </div><div><div>Richard</div></div></div>