[PATCH] D93045: [ELF] AArch64: Handle DT_AARCH64_VARIANT_PCS

[Adhemerval Zanella via Phabricator via llvm-commits]

zatrazz added a comment.

In D93045#2446599 <https://reviews.llvm.org/D93045#2446599>, @MaskRay wrote:

>> st_other marked with STO_AARCH64_VARIANT_PCS indicates a may follow a variant
>
> a -> it

Ack.

>> if there are R_<CLS>_JUMP_SLOT relocations that reference that symbols.
>
> Question: The ABI does use the `<CLS>` notation. Does it mean it can be AARCH32 in the future?

This is better described on section "5.7.3.1Relocation names and class" from the documentation and the another possible <CLS> would be AARCH64_P32 (basically the AArch64 ILP32 ABI that has been developed out of tree and is on beta status regarding ABI).

>> It fixes https://bugs.llvm.org/show_bug.cgi?id=48368.
>
> I think "fixes" is used for bug fixes. This is a new feature.

'It implements' would be better then?

> binutils `elfNN_aarch64_merge_symbol_attribute` has an untested diagnostic  `_bfd_error_handler (_("unknown attribute for symbol `%s': 0x%02x"), h->root.root.string, isym_sto);` (@nsz) 
> Is it useful to LLD?

I think it might be useful to warn whether old lld is being used along objects built with newer ABI extensions. I can work on add a warning on a subsequent patch.



================
Comment at: lld/test/ELF/aarch64-variant_pcs.s:12
+# PCSSYM: Symbol table '.dynsym'
+# PCSSYM: {{0+}} 0 NOTYPE GLOBAL DEFAULT               UND func_global_undef
+# PCSSYM: {{0+}} 0 NOTYPE GLOBAL DEFAULT [VARIANT_PCS] UND pcs_func_global_undef
----------------
MaskRay wrote:
> `{{0+}}` is not useful.
> 
> (Even if you want to check this is not a canonical PLT entry (which is probably out of scope of this test), `{{0+}}` can match trailing zeros in a non-zero address `12340`)
Indeed,  this is not really required. I will removed and check only if 'func_global_undef' is being exported.


================
Comment at: lld/test/ELF/aarch64-variant_pcs.s:13
+# PCSSYM: {{0+}} 0 NOTYPE GLOBAL DEFAULT               UND func_global_undef
+# PCSSYM: {{0+}} 0 NOTYPE GLOBAL DEFAULT [VARIANT_PCS] UND pcs_func_global_undef
+# PCSSYM:        0 NOTYPE GLOBAL DEFAULT                 7 func_global_def
----------------
MaskRay wrote:
> Use `-NEXT`. The FileCheck diagnostic will be more friendly if a newline is added or a line is removed.
Ack.


================
Comment at: lld/test/ELF/aarch64-variant_pcs.s:14
+# PCSSYM: {{0+}} 0 NOTYPE GLOBAL DEFAULT [VARIANT_PCS] UND pcs_func_global_undef
+# PCSSYM:        0 NOTYPE GLOBAL DEFAULT                 7 func_global_def
+# PCSSYM:        0 IFUNC  GLOBAL DEFAULT                 7 ifunc_global_def
----------------
MaskRay wrote:
> Replace `7` with `[[#]]` to prevent updatess when section indexes change.
Ack.


================
Comment at: lld/test/ELF/aarch64-variant_pcs.s:34
+
+.global pcs_func_global_def
+.global pcs_func_global_undef
----------------
MaskRay wrote:
> Optional: `.global a, b, c` can mark several symbols
> 
Ack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93045