<div dir="ltr"><div>LGTM</div><div><br></div><div>+ if (OldA->getLabel() != NewA->getLabel()) {</div><div>+<span class="" style="white-space:pre"> </span>// This redeclaration changes __asm__ label.</div><div>+ Diag(New->getLocation(), diag::err_different_asm_label);</div><div><br></div><div>Comment is underindented by one space.</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 11, 2015 at 1:17 PM, Nick Lewycky <span dir="ltr"><<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@google.com</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 dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="">On 11 December 2015 at 12:57, 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Fri, Dec 11, 2015 at 12:43 PM, Gao, Yunzhong via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</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 lang="EN-US" link="blue" vlink="purple">
<div>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">gcc 4.8.2 accepts the following case silently without error or warning:<u></u><u></u></span></p><span>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"> void f();<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"> void g() __asm__(“real_g”);<u></u><u></u></span></p>
</span><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"> void f() { g(); } // gcc emits a call to real_g() here<u></u><u></u></span></p><span>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"> void real_g() __asm__(“gold”);<u></u><u></u></span></p>
</span><p class="MsoNormal" style="text-indent:4.5pt"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">void real_g() { } // gcc generates a body for gold() here<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">gcc 4.8.2 generates a warning for the following test case:<u></u><u></u></span></p>
<p class="MsoNormal" style="text-indent:4.5pt"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">void g() __asm__(“func1”);<u></u><u></u></span></p>
<p class="MsoNormal" style="text-indent:4.5pt"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">void g() __asm__(“func2”); // gcc generates a warning and ignores this asm label<u></u><u></u></span></p>
<p class="MsoNormal" style="text-indent:4.5pt"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">void g() { }<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Comparing to gcc behavior, I think it is better to emit error in both cases.</span></p></div></div></blockquote><div><br></div></span><div>Why? I don't see anything wrong with the first example. We have a function whose source-level name is "g()" and whose asm name is "real_g", and we have a function whose source-level name is "real_g()" and whose asm name is "gold". What's wrong with that?</div></div></div></div></blockquote><div><br></div></span><div>Hah, I completely misread the example and fixed something else. What I added an error for is:</div><div><br></div><div> void f() __asm__("something");</div><div> void f() __asm__("somethingelse");</div><div><br></div><div>With my patch, we don't issue an error on the case Yunzhong actually supplied. Joerg has pointed out over IRC that this is a really useful construct and that NetBSD's stack smashing protection uses it. We shouldn't break that. I've added a testcase to make sure it continues working. Please review!</div><span class=""><font color="#888888"><div><br></div><div>Nick</div></font></span><div><div class="h5"><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 dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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"><span><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Looks good to me. Thanks!<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">- Gao<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<div style="border-style:none none none solid;border-left-color:blue;border-left-width:1.5pt;padding:0in 0in 0in 4pt">
<div>
<div style="border-style:solid none none;border-top-color:rgb(181,196,223);border-top-width:1pt;padding:3pt 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10pt;font-family:Tahoma,sans-serif">From:</span></b><span style="font-size:10pt;font-family:Tahoma,sans-serif"> Nick Lewycky [mailto:<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@google.com</a>]
<br>
<b>Sent:</b> Friday, December 11, 2015 12:44 AM<br>
<b>To:</b> Gao, Yunzhong<br>
<b>Cc:</b> Clang Commits<br>
<b>Subject:</b> Re: PATCH: error on code that redeclares with an __asm__ label after the first ODR use<u></u><u></u></span></p>
</div>
</div><div><div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<div>
<p class="MsoNormal">On 10 December 2015 at 17:42, Gao, Yunzhong <<a href="mailto:yunzhong_gao@playstation.sony.com" target="_blank">yunzhong_gao@playstation.sony.com</a>> wrote:<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Out of curiosity, is the following test case possible too?</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">void f();</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">void g() __asm__(“real_g”); // rename g into real_g.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">void f() {</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"> g(); // this would actually be calling real_g()</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">}</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">void real_g() __asm__("gold"); // re-declaring real_g() into gold <-- should this be an error too?</span><u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">I can't see any reason why not. Both clang and gcc will emit "real_g" here instead of "gold", but changing the asm label in a program is highly dubious. I added an error for this too.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Thanks!<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"> </span><u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-color:rgb(204,204,204);border-left-width:1pt;padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<div style="border-style:none none none solid;border-left-color:blue;border-left-width:1.5pt;padding:0in 0in 0in 4pt">
<div>
<div style="border-style:solid none none;border-top-color:rgb(181,196,223);border-top-width:1pt;padding:3pt 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10pt;font-family:Tahoma,sans-serif">From:</span></b><span style="font-size:10pt;font-family:Tahoma,sans-serif"> cfe-commits [mailto:<a href="mailto:cfe-commits-bounces@lists.llvm.org" target="_blank">cfe-commits-bounces@lists.llvm.org</a>]
<b>On Behalf Of </b>Nick Lewycky via cfe-commits<br>
<b>Sent:</b> Thursday, December 10, 2015 3:15 PM<br>
<b>To:</b> Clang Commits<br>
<b>Subject:</b> PATCH: error on code that redeclares with an __asm__ label after the first ODR use</span><u></u><u></u></p>
</div>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">PR22830 shows a case where some code was adding an __asm__ label after the first use of a function. GCC and Clang diverged on this code, GCC emitting the name in the last __asm__
label for all uses while clang would switch in the middle of the TU as the redeclaration was parsed. The attached patch detects this case and issues an error on such a redeclaration. If this breaks real code, we can turn it down to a warning.<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Please review!<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal">Nick<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div></div></div>
</div>
</div>
<br></span>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>