[llvm] r265815 - [Object] Report an error if .alt_entry is used with ELF or COFF.

[David Blaikie via llvm-commits]

On Mon, Apr 11, 2016 at 12:44 PM, Lang Hames via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Hi Rafael,
>
> Yep - this was just a temporary hack. It's been fixed as of r265975.
>
> Regarding test cases - what's the bar for something like this. I'd have
> included a test that the directive was handled correctly on the platforms
> it's intended for, but testing every (unsupported) directive on every
> platform seems like overkill.
>

FWIW, my bar would be "if it's separate code, it probably deserves a
separate test". (100% block coverage is a pretty low bar for testing (& I
expect a bit more than that - it's easy to have coverage without
verification of the right behavior, for example), and, admittedly, not one
(m)any projects really have, especially with C++ & many implicit blocks
(maybe that's not so bad with exceptions disabled), but generally something
to strive for)

- Dave


>
> - Lang.
>
> On Sun, Apr 10, 2016 at 8:35 AM, Rafael Espíndola <
> rafael.espindola at gmail.com> wrote:
>
>> Thanks, but:
>>
>> * This should have a testcase.
>> * It is odd to parse and then issue an error. I think the correct fix
>> is to move the parsing to lib/MC/MCParser/DarwinAsmParser.cpp, that
>> way we get the natural error on ELF and COFF.
>>
>> Cheers,
>> Rafael
>>
>>
>>
>> On 8 April 2016 at 13:38, Lang Hames via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>> > Author: lhames
>> > Date: Fri Apr  8 12:38:51 2016
>> > New Revision: 265815
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=265815&view=rev
>> > Log:
>> > [Object] Report an error if .alt_entry is used with ELF or COFF.
>> >
>> > I'm looking into a better way to do this long-term, but for now at
>> least don't
>> > crash.
>> >
>> > Modified:
>> >     llvm/trunk/lib/MC/MCELFStreamer.cpp
>> >     llvm/trunk/lib/MC/WinCOFFStreamer.cpp
>> >
>> > Modified: llvm/trunk/lib/MC/MCELFStreamer.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCELFStreamer.cpp?rev=265815&r1=265814&r2=265815&view=diff
>> >
>> ==============================================================================
>> > --- llvm/trunk/lib/MC/MCELFStreamer.cpp (original)
>> > +++ llvm/trunk/lib/MC/MCELFStreamer.cpp Fri Apr  8 12:38:51 2016
>> > @@ -285,7 +285,7 @@ bool MCELFStreamer::EmitSymbolAttribute(
>> >      break;
>> >
>> >    case MCSA_AltEntry:
>> > -    llvm_unreachable("ELF doesn't support this attribute");
>> > +    report_fatal_error("ELF doesn't support the .alt_entry attribute");
>> >    }
>> >
>> >    return true;
>> >
>> > Modified: llvm/trunk/lib/MC/WinCOFFStreamer.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/WinCOFFStreamer.cpp?rev=265815&r1=265814&r2=265815&view=diff
>> >
>> ==============================================================================
>> > --- llvm/trunk/lib/MC/WinCOFFStreamer.cpp (original)
>> > +++ llvm/trunk/lib/MC/WinCOFFStreamer.cpp Fri Apr  8 12:38:51 2016
>> > @@ -107,6 +107,8 @@ bool MCWinCOFFStreamer::EmitSymbolAttrib
>> >    case MCSA_Global:
>> >      Symbol->setExternal(true);
>> >      break;
>> > +  case MCSA_AltEntry:
>> > +    report_fatal_error("COFF doesn't support the .alt_entry
>> attribute");
>> >    }
>> >
>> >    return true;
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://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/20160411/49da0b4d/attachment.html>