[llvm] 02c1321 - [MC] Improve a report_fatal_error

[David Blaikie via llvm-commits]

On Thu, Mar 26, 2020 at 7:14 PM Fangrui Song <i at maskray.me> wrote:

> The code should be unreachable if MC had no bugs :)
>

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.


>
> > I ran into this once while playing with my -fno-semantic-interposition
> patches.
>
> Why playing locally, I created .Lfoo$local with
> MCContext::createTempSymbol() but did not define it in some code
> paths. When the symbol was used, this error would trigger.
> I felt sad that the diagnostic did not tell me about the name, so I
> made the commit to improve the diagnostic a bit.
>
> Now, with https://reviews.llvm.org/D75097 , all temp symbols when
> emitting object files are unnamed. This improved diagnostic will not
> help at all. If a developer knows     bool UseNamesOnTempLabels =
> false; in MCContext.h, they can set that to true to enjoy the improve
> diagnostic.
>
> On Thu, Mar 26, 2020 at 6:43 PM David Blaikie <dblaikie at gmail.com> wrote:
> >
> > 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).
> >
> > Could you describe what it takes to reproduce this? Is it reliably
> reproducible with (albeit bogus/broken) static inputs?
> >
> > On Thu, Mar 26, 2020 at 6:37 PM Fangrui Song <i at maskray.me> wrote:
> >>
> >> There can't be a test case. If this error is ever triggered, it means
> >> something seriously broken happened.
> >>
> >> I ran into this once while playing with my -fno-semantic-interposition
> patches.
> >>
> >> On Thu, Mar 26, 2020 at 5:45 PM David Blaikie <dblaikie at gmail.com>
> wrote:
> >> >
> >> > Test case?
> >> >
> >> > On Mon, Jan 20, 2020 at 11:17 PM Fangrui Song via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> >> >>
> >> >>
> >> >> Author: Fangrui Song
> >> >> Date: 2020-01-20T23:13:18-08:00
> >> >> New Revision: 02c1321139d61a9e56a5319a07bb8f27570e7f77
> >> >>
> >> >> URL:
> https://github.com/llvm/llvm-project/commit/02c1321139d61a9e56a5319a07bb8f27570e7f77
> >> >> DIFF:
> https://github.com/llvm/llvm-project/commit/02c1321139d61a9e56a5319a07bb8f27570e7f77.diff
> >> >>
> >> >> LOG: [MC] Improve a report_fatal_error
> >> >>
> >> >> Added:
> >> >>
> >> >>
> >> >> Modified:
> >> >>     llvm/lib/MC/ELFObjectWriter.cpp
> >> >>
> >> >> Removed:
> >> >>
> >> >>
> >> >>
> >> >>
> ################################################################################
> >> >> diff  --git a/llvm/lib/MC/ELFObjectWriter.cpp
> b/llvm/lib/MC/ELFObjectWriter.cpp
> >> >> index 6b4b45eb8eff..804f999a326a 100644
> >> >> --- a/llvm/lib/MC/ELFObjectWriter.cpp
> >> >> +++ b/llvm/lib/MC/ELFObjectWriter.cpp
> >> >> @@ -640,7 +640,7 @@ void ELFWriter::computeSymbolTable(
> >> >>        continue;
> >> >>
> >> >>      if (Symbol.isTemporary() && Symbol.isUndefined()) {
> >> >> -      Ctx.reportError(SMLoc(), "Undefined temporary symbol");
> >> >> +      Ctx.reportError(SMLoc(), "Undefined temporary symbol " +
> Symbol.getName());
> >> >>        continue;
> >> >>      }
> >> >>
> >> >>
> >> >>
> >> >>
> >> >> _______________________________________________
> >> >> llvm-commits mailing list
> >> >> llvm-commits at lists.llvm.org
> >> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200326/601e6278/attachment.html>