[PATCH] D41613: [ELF] - Do not use HeaderSize for conditions in PltSection.

[Rafael Avila de Espindola via llvm-commits]

Rui Ueyama <ruiu at google.com> writes:

> On Tue, Jan 9, 2018 at 4:52 PM, Rafael Avila de Espindola <
> rafael.espindola at gmail.com> wrote:
>
>> Rui Ueyama via Phabricator <reviews at reviews.llvm.org> writes:
>>
>> > ruiu added a comment.
>> >
>> > Isn't it an ABI violation not to write the PLT header for PPC64? If so,
>> we fix that ABI violation rather than "correctly" emitting ABI-violating
>> executables.
>>
>> The PPC64 implementation is incomplete. Finishing it would be nice, but
>> independent of this change.
>>
>> Without this change a target must set Target->PltHeaderSize to avoid
>> having lld confuse the plt and iplt sections, which is not a good design
>> IMHO.
>>
>
> It is okay to be incomplete, but I'm not very fond of making a change that
> is not needed for the proper ABI. How about setting a non-zero value to the
> PPC64 PLT header? We don't have to fill the actual instruction sequence.
> Setting a non-zero value should fix the issue, right?

I think the way to look at this patch is a cleanup, not a fix.

I find

  if (IsIplt) {

A lot easier to read than

  if (HeaderSize == 0) {

I does happen to fix a problem with the ppc64 target, but that is almost
incidental. And yes, it could be made a pure cleanup by setting the
ppc64 PltHeaderSize to a nonzero value first.

Cheers,
Rafael