[PATCH] D27973: [ELF] - Do not call fatal() in Target.cpp, call error() instead.

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

In https://reviews.llvm.org/D27973#627795, @davide wrote:

> In https://reviews.llvm.org/D27973#627791, @grimar wrote:
>
> > In https://reviews.llvm.org/D27973#627788, @davide wrote:
> >
> > > Instead of checking in a binary, I'll see if you can craft a broken object using `yaml2obj`.
> > >  I'm not a huge fan of the tool itself, but I find this a very legitimate usecase for it (creating broken binaries).
> >
> >
> > No it looks not possible. It showed me a error when I tried to create a relocation using numeric value and not a pre-defined enum. So I believe it is a unreasonable limitaion of yaml2obj now.
>
>
> Can you please take a look at how hard is to fix `yaml2obj` for this testcase? I think we should avoid checking in a binary unless really necessary, as it makes reconstructing the testcase harder in the future.
>
> In https://reviews.llvm.org/D27973#627794, @grimar wrote:
>
> > In https://reviews.llvm.org/D27973#627793, @davide wrote:
> >
> > > In https://reviews.llvm.org/D27973#627791, @grimar wrote:
> > >
> > > > In https://reviews.llvm.org/D27973#627788, @davide wrote:
> > > >
> > > > > Instead of checking in a binary, I'll see if you can craft a broken object using `yaml2obj`.
> > > > >  I'm not a huge fan of the tool itself, but I find this a very legitimate usecase for it (creating broken binaries).
> > > >
> > > >
> > > > No it looks not possible. It showed me a error when I tried to create a relocation using numeric value and not a pre-defined enum. So I believe it is a unreasonable limitaion of yaml2obj now.
> > >
> > >
> > > Can you please take a look at how hard is to fix `yaml2obj` for this testcase? I think we should avoid checking in a binary unless really necessary, as it makes reconstructing the testcase harder in the future.
> >
> >
> > Sure. I'll take a look on that tomorrow than. I hope that should not affect on that patch though. But that would be a good change I think anyways. I assume yaml2obj is best using when need to create something unusual, so I would expect to have as less limitations as possible here.
>
>
> *sigh* It sounds fixing this might be non-trivial as the YAML parser expects an enumerated scalar (which probably will get from `ELF.h` or something). So, I'd leave this as is but please add specific instructions on how to reproduce (source test file, commands, and tools version, so that it's not a pain to recreate this in the future).


Hmm, ok, I'll do.


https://reviews.llvm.org/D27973