[PATCH] D149817: [llvm-objdump][X86] Add @plt symbols for .plt.got

Tue May 16 07:28:33 PDT 2023

bd1976llvm added a comment.

In D149817#4344774 <https://reviews.llvm.org/D149817#4344774>, @MaskRay wrote:

> In D149817#4342333 <https://reviews.llvm.org/D149817#4342333>, @bd1976llvm wrote:
>
>> Thanks for working on this and sorry for the lack of response.
>>
>> Generally looks good, LGTM from me.
>>
>> I'm happy with the missing corner case for x86-32, given that you mentioned that it is extremely unlikely to occur. Do the GNU tools support that corner case? If they have gone to the trouble to support it I'm wondering if they have a use-case that hasn't occurred to us?
>> In the description, I think we should have a link to the optimisation in the PS ABI documents in addition to your blog entry.
>> I wonder if the LLDB folks are aware of this? Might be worth informing them?
>> I have mentioned a few improvements that seemed worthwhile to me. Feel free to ignore these if they are not helpful.
>
> Thanks for the review! Sorry for the shameless plug. The official psABI mentions combining JUMP_SLOT and GLOB_DAT, but doesn't say anything about `.plt.got`, or the fact that `.got.plt` can be omitted...

No problem. I have been known to read your blog entries myself :)

> With `int main() {}` `gcc x.c -o x -z now -fuse-ld=bfd -m32`, we don't get `.got.plt`. If we add a libc function call, a regular PLT will be needed. To omit `.got.plt` again, we need to take the address of the libc function call. For every PLT we need an address-taken operation in the code. 
> This is why I said that the corner case nearly never happens in the wild.
> It seems that objdump does symbolize `.plt.got` in this case. I believe it takes quite some code to retrieve the value `_GLOBAL_OFFSET_TABLE_`.
>
> Thanks for mentioning lldb. This gives me a motivation to study how it performs symbolization...

Thanks for the extra information, makes sense to me.

LGTM.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149817/new/

https://reviews.llvm.org/D149817