<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Mar 26, 2020 at 7:14 PM Fangrui Song <<a href="mailto:i@maskray.me">i@maskray.me</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The code should be unreachable if MC had no bugs :)<br></blockquote><div><br>Then it sounds like it should be an assert? Unless there are known bugs that are too mainstream and expensive to fix right now, in which case test cases for those bugs might be useful to demonstrate why this codepath is still here (& demonstrate the diagnostic message - with or without the name) & those tests can be repurposed to test the bug fixes when the bugs are fixed.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> I ran into this once while playing with my -fno-semantic-interposition patches.<br>
<br>
Why playing locally, I created .Lfoo$local with<br>
MCContext::createTempSymbol() but did not define it in some code<br>
paths. When the symbol was used, this error would trigger.<br>
I felt sad that the diagnostic did not tell me about the name, so I<br>
made the commit to improve the diagnostic a bit.<br>
<br>
Now, with <a href="https://reviews.llvm.org/D75097" rel="noreferrer" target="_blank">https://reviews.llvm.org/D75097</a> , all temp symbols when<br>
emitting object files are unnamed. This improved diagnostic will not<br>
help at all. If a developer knows bool UseNamesOnTempLabels =<br>
false; in MCContext.h, they can set that to true to enjoy the improve<br>
diagnostic.<br>
<br>
On Thu, Mar 26, 2020 at 6:43 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
><br>
> We can still test broken things - if it's reachable by input. (generally report_fatal_error is for things that are at least reachable by external state (input, etc) rather than only internal code errors (which would be assertions) - but so weird/rare it's not quite worth having proper error handling for it).<br>
><br>
> Could you describe what it takes to reproduce this? Is it reliably reproducible with (albeit bogus/broken) static inputs?<br>
><br>
> On Thu, Mar 26, 2020 at 6:37 PM Fangrui Song <<a href="mailto:i@maskray.me" target="_blank">i@maskray.me</a>> wrote:<br>
>><br>
>> There can't be a test case. If this error is ever triggered, it means<br>
>> something seriously broken happened.<br>
>><br>
>> I ran into this once while playing with my -fno-semantic-interposition patches.<br>
>><br>
>> On Thu, Mar 26, 2020 at 5:45 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
>> ><br>
>> > Test case?<br>
>> ><br>
>> > On Mon, Jan 20, 2020 at 11:17 PM Fangrui Song via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
>> >><br>
>> >><br>
>> >> Author: Fangrui Song<br>
>> >> Date: 2020-01-20T23:13:18-08:00<br>
>> >> New Revision: 02c1321139d61a9e56a5319a07bb8f27570e7f77<br>
>> >><br>
>> >> URL: <a href="https://github.com/llvm/llvm-project/commit/02c1321139d61a9e56a5319a07bb8f27570e7f77" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/02c1321139d61a9e56a5319a07bb8f27570e7f77</a><br>
>> >> DIFF: <a href="https://github.com/llvm/llvm-project/commit/02c1321139d61a9e56a5319a07bb8f27570e7f77.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/02c1321139d61a9e56a5319a07bb8f27570e7f77.diff</a><br>
>> >><br>
>> >> LOG: [MC] Improve a report_fatal_error<br>
>> >><br>
>> >> Added:<br>
>> >><br>
>> >><br>
>> >> Modified:<br>
>> >> llvm/lib/MC/ELFObjectWriter.cpp<br>
>> >><br>
>> >> Removed:<br>
>> >><br>
>> >><br>
>> >><br>
>> >> ################################################################################<br>
>> >> diff --git a/llvm/lib/MC/ELFObjectWriter.cpp b/llvm/lib/MC/ELFObjectWriter.cpp<br>
>> >> index 6b4b45eb8eff..804f999a326a 100644<br>
>> >> --- a/llvm/lib/MC/ELFObjectWriter.cpp<br>
>> >> +++ b/llvm/lib/MC/ELFObjectWriter.cpp<br>
>> >> @@ -640,7 +640,7 @@ void ELFWriter::computeSymbolTable(<br>
>> >> continue;<br>
>> >><br>
>> >> if (Symbol.isTemporary() && Symbol.isUndefined()) {<br>
>> >> - Ctx.reportError(SMLoc(), "Undefined temporary symbol");<br>
>> >> + Ctx.reportError(SMLoc(), "Undefined temporary symbol " + Symbol.getName());<br>
>> >> continue;<br>
>> >> }<br>
>> >><br>
>> >><br>
>> >><br>
>> >><br>
>> >> _______________________________________________<br>
>> >> llvm-commits mailing list<br>
>> >> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
>> >> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>