<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 17, 2015 at 2:34 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:rgb(0,0,0)"><br><hr><blockquote style="border-left-width:2px;border-left-style:solid;border-left-color:rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><b>From: </b>"Daniel Berlin" <<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>><br><b>To: </b>"Jeroen Dobbelaere" <<a href="mailto:jeroen.dobbelaere@gmail.com" target="_blank">jeroen.dobbelaere@gmail.com</a>><br><b>Cc: </b><a href="mailto:reviews%2BD8056%2Bpublic%2B7b63cedb34d51bb1@reviews.llvm.org" target="_blank">reviews+D8056+public+7b63cedb34d51bb1@reviews.llvm.org</a>, "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>>, <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a>, "<a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a> Developers" <<a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a>><br><b>Sent: </b>Tuesday, March 17, 2015 4:24:01 PM<br><b>Subject: </b>Re: [PATCH] Fix for bug 21725: wrong results with union and strict-aliasing<span class=""><br><br><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 17, 2015 at 2:03 PM, Jeroen Dobbelaere <span dir="ltr"><<a href="mailto:jeroen.dobbelaere@gmail.com" target="_blank">jeroen.dobbelaere@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div><div><div class="gmail_quote">On Tue, Mar 17, 2015 at 7:16 PM, <a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a> <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><span>In <a href="http://reviews.llvm.org/D8056#141899" target="_blank">http://reviews.llvm.org/D8056#141899</a>, @jeroen.dobbelaere wrote:<br>
<br>
> Further investigation shows that llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp
is not ready to handle struct types where multiple members have the
same (0) offset. The clang fix only makes sense once that issue is
resolved.<br>
<br>
<br>
</span>Can you work on this? If not (or even if you can), can you file a bug report with an example?<br>
<div><div><br>
<br>
<a href="http://reviews.llvm.org/D8056" target="_blank">http://reviews.llvm.org/D8056</a><br>
<br>
EMAIL PREFERENCES<br>
<a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
<br>
<br></div></div></blockquote><div> </div></div></div></div>Yes, I am looking into it. I have a version now where 'most things' work, but it seems that the tbaa information that clang produces is not expressive enough when the struct/union has array members. For example:<br></div><div><br>union U03 {<br> short mS[4];<br> int mI;<br>};<br><br>results in:<br><br>!5 = !{!"omnipotent char", !6, i64 0}<br>!6 = !{!"Simple C/C++ TBAA"}<br>!7 = !{!8, !8, i64 0}<br>!8 = !{!"int", !5, i64 0}<br>!9 = !{!10, !10, i64 0}<br>!10 = !{!"short", !5, i64 0}<br>!40 = !{!41, !8, i64 0}<br>!41 = !{!"_ZTS3U03", !5, i64 0, !8, i64 0}<br><br></div><div>Note that In !41, we loose the information about the 'short' member.<br><br></div></div></blockquote><div>I would start here :)<br></div><div><br></div><div>You should make it produce info about all the members.</div></div></div></div></span></blockquote><br>I agree.<span class=""><br><br><blockquote style="border-left-width:2px;border-left-style:solid;border-left-color:rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div> <br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div></div><div>A function like:<br><br>void test_u03b(union U03* a, union U03* b)<br>{<br> a->mS[0]=1;<br> b->mI=2;<br><br> // CHECK: Function: _Z9test_u03b{{.*}}<br> //FIXME: CHECK: MayAlias: store i32 2, i32* %{{.*}}, align 4, !tbaa !{{.*}} <-> store i16 1, i16* %{{.*}}, align 2, !tbaa !{{.*}}<br>}<br><br></div><div>results in wrong information: an access to mS[0] results in a 'short' (!10) access. The access to mI results in '!40'.<br></div><div>Note that for the short '!10' access, we loose the information about the associated union.<br></div></div></blockquote><div> <br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div></div><div>Because !41 refers to char '!5' and int '8', but not to short '!10', we cannot detect that there might <br>be overlap between '!40' and '!10'.<br></div></div></blockquote><div><br></div><div>RIght, but this is clearly a bug in the info above.</div><div> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div></div><div>When we have two arrays of different type, the problem becomes even worse as we now have no<br>connection to the union any more from the two accesses...<br></div></div></blockquote><div><br></div><div>I went through this on the whiteboard with folks when they designed this, and pointed this out - it's pretty easy to get into situations where you have two random pieces of memory, and they were part of a union, and you don't know this.</div><div><br></div><div>You can make them as easy or as hard as you like.</div><div><br></div><div>The answer I got was "we're fine with people having to make this explicit".</div></div></div></div></blockquote><br></span>I don't understand what this means. How should they do that? </div></div></blockquote><div><br></div><div>So, his point was the arrays have no connections to the union. This is not the only case this occurs in.</div><div><br></div><div>Let's take the canonical example:</div><div><br></div><div><br></div><div>For example</div><div><br></div><div>union foo {</div><div>int a;</div><div>float b;</div><div>};</div><div><br></div><div>int ihateunions(int *a, float *b)</div><div>{</div><div><do a thing with a and b></div><div>}</div><div><br></div><div>int passtounion()</div><div>{</div><div>union foo bar;</div><div>ihateunions(&bar.a, &bar.b);</div><div><br></div><div>}</div><div><br></div><div>Inside ihateunions, you will have no idea that a and b are connected to unions.</div><div><br></div><div>Let's say this example is easy, i mean, they are both at offset 0, so they can alias, right?</div><div><br></div><div>Add two different structs in the union, and pass pointers, we'll still generate wrong struct-path tbaa for this, because it has no idea that it is part of a union.</div><div>:)<br></div><div><div>You can make this arbitrarily as complex or as simple as you want (put them in different translation units, etc).</div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:rgb(0,0,0)">The language has a defined set of semantics, and we need to follow them.</div></div></blockquote><div><br></div><div>What do you think should happen in the above? :)</div><div>(Last time we went around this on the GCC mailing list, this spawned a very long 30 message thread on the gcc mailing list, with the TL;DR being "nobody really knows, but we're fine with telling people if they want something sane here, they should make the union access in ihateunions visible and explicit". DR reports were filed, no good answer was given :P)<br></div><div><br></div><div> </div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:rgb(0,0,0)"> At the very least, if we don't want to fully represent the TBAA hierarchy for some types (I'm not sure why we'd not do this, but...) then we should omit the metadata all together.<br></div></div></blockquote><div> </div><div><br></div><div>This part isn't about not fully representing, but not knowing what part of the hierarchy you are really in.</div><div><br></div><div>Remember that TBAA struct path is not really TBAA info.</div><div>It's access path info.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div style="font-family:arial,helvetica,sans-serif;font-size:10pt;color:rgb(0,0,0)"> -Hal<span class=""><br><br><blockquote style="border-left-width:2px;border-left-style:solid;border-left-color:rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>One issue with getting array accesses right that I see, is that the 'offset' field can suddenly be variable.<br></div><div><br></div></div></blockquote><div><br></div><div>Not really. You can't say where it points, so the range would be "everything" anyway.</div><div>You should just say it accesses the entire array (but not anything else).</div><div><br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Maybe a good enough (and correct) fallback that we should consider is to make such _union_ accesses alias with everything... (omnipotent char) in stead of mapping it to the basic type.<br></div></div></blockquote><div><br></div><div>It is possible, with the sets we have, to get the aliasing right :)</div><div>I know this because GCC does it.</div><div> </div><div>This design is nearly identical to GCC's TBAA and subvariable design :)</div><div><br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>Any thoughts ?<span><font color="#888888"><br></font></span></div><span><font color="#888888"><div><br></div><div>Jeroen Dobbelaere<br><br></div></font></span></div>
</blockquote></div><br></div></div>
</blockquote><br><br><br></span><span class=""><font color="#888888">-- <br><div><span name="x"></span>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<span name="x"></span><br></div></font></span></div></div></blockquote></div><br></div></div>