Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ELFRelocs/AArch64: update canonical reference URL (NFC) #86955

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

artagnon
Copy link
Contributor

@artagnon artagnon commented Mar 28, 2024

Update the URL of the reference to be used for AArch64.def, and add some comments. The canonical aaelf64 document can be found at:

https://github.com/ARM-software/abi-aa/blob/main/aaelf64/aaelf64.rst

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 28, 2024

@llvm/pr-subscribers-llvm-binary-utilities

Author: Ramkumar Ramachandra (artagnon)

Changes

As per the released specification of aaelf64, there is no longer any R_AARCH64_AUTH_RELATIVE. Stripping it from the def file has no consequences, as it is unused in the code, so this change is non-functional.


Full diff: https://github.com/llvm/llvm-project/pull/86955.diff

1 Files Affected:

  • (modified) llvm/include/llvm/BinaryFormat/ELFRelocs/AArch64.def (+12-7)
diff --git a/llvm/include/llvm/BinaryFormat/ELFRelocs/AArch64.def b/llvm/include/llvm/BinaryFormat/ELFRelocs/AArch64.def
index 5fb3fa4aeb7beb..f01470d49bca35 100644
--- a/llvm/include/llvm/BinaryFormat/ELFRelocs/AArch64.def
+++ b/llvm/include/llvm/BinaryFormat/ELFRelocs/AArch64.def
@@ -1,18 +1,19 @@
-
 #ifndef ELF_RELOC
 #error "ELF_RELOC must be defined"
 #endif
 
-// Based on ABI release 1.1-beta, dated 6 November 2013. NB: The cover page of
-// this document, IHI0056C_beta_aaelf64.pdf, on infocenter.arm.com, still
-// labels this as release 1.0.
+// Based on released ABI: https://github.com/ARM-software/abi-aa, aaelf64.
+// ELF64
+// Null relocation: also 0x100 for ELF64
 ELF_RELOC(R_AARCH64_NONE,                                0)
+// Data relocations
 ELF_RELOC(R_AARCH64_ABS64,                           0x101)
 ELF_RELOC(R_AARCH64_ABS32,                           0x102)
 ELF_RELOC(R_AARCH64_ABS16,                           0x103)
 ELF_RELOC(R_AARCH64_PREL64,                          0x104)
 ELF_RELOC(R_AARCH64_PREL32,                          0x105)
 ELF_RELOC(R_AARCH64_PREL16,                          0x106)
+// Static relocations
 ELF_RELOC(R_AARCH64_MOVW_UABS_G0,                    0x107)
 ELF_RELOC(R_AARCH64_MOVW_UABS_G0_NC,                 0x108)
 ELF_RELOC(R_AARCH64_MOVW_UABS_G1,                    0x109)
@@ -60,11 +61,13 @@ ELF_RELOC(R_AARCH64_LD64_GOT_LO12_NC,                0x138)
 ELF_RELOC(R_AARCH64_LD64_GOTPAGE_LO15,               0x139)
 ELF_RELOC(R_AARCH64_PLT32,                           0x13a)
 ELF_RELOC(R_AARCH64_GOTPCREL32,                      0x13b)
+// General dynamic TLS relocations
 ELF_RELOC(R_AARCH64_TLSGD_ADR_PREL21,                0x200)
 ELF_RELOC(R_AARCH64_TLSGD_ADR_PAGE21,                0x201)
 ELF_RELOC(R_AARCH64_TLSGD_ADD_LO12_NC,               0x202)
 ELF_RELOC(R_AARCH64_TLSGD_MOVW_G1,                   0x203)
 ELF_RELOC(R_AARCH64_TLSGD_MOVW_G0_NC,                0x204)
+// Local dynamic TLS relocations
 ELF_RELOC(R_AARCH64_TLSLD_ADR_PREL21,                0x205)
 ELF_RELOC(R_AARCH64_TLSLD_ADR_PAGE21,                0x206)
 ELF_RELOC(R_AARCH64_TLSLD_ADD_LO12_NC,               0x207)
@@ -92,6 +95,7 @@ ELF_RELOC(R_AARCH64_TLSIE_MOVW_GOTTPREL_G0_NC,       0x21c)
 ELF_RELOC(R_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21,       0x21d)
 ELF_RELOC(R_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC,     0x21e)
 ELF_RELOC(R_AARCH64_TLSIE_LD_GOTTPREL_PREL19,        0x21f)
+// Local exec TLS relocations
 ELF_RELOC(R_AARCH64_TLSLE_MOVW_TPREL_G2,             0x220)
 ELF_RELOC(R_AARCH64_TLSLE_MOVW_TPREL_G1,             0x221)
 ELF_RELOC(R_AARCH64_TLSLE_MOVW_TPREL_G1_NC,          0x222)
@@ -108,6 +112,7 @@ ELF_RELOC(R_AARCH64_TLSLE_LDST32_TPREL_LO12,         0x22c)
 ELF_RELOC(R_AARCH64_TLSLE_LDST32_TPREL_LO12_NC,      0x22d)
 ELF_RELOC(R_AARCH64_TLSLE_LDST64_TPREL_LO12,         0x22e)
 ELF_RELOC(R_AARCH64_TLSLE_LDST64_TPREL_LO12_NC,      0x22f)
+// TLS descriptor relocations
 ELF_RELOC(R_AARCH64_TLSDESC_LD_PREL19,               0x230)
 ELF_RELOC(R_AARCH64_TLSDESC_ADR_PREL21,              0x231)
 ELF_RELOC(R_AARCH64_TLSDESC_ADR_PAGE21,              0x232)
@@ -123,7 +128,7 @@ ELF_RELOC(R_AARCH64_TLSLE_LDST128_TPREL_LO12_NC,     0x23b)
 ELF_RELOC(R_AARCH64_TLSLD_LDST128_DTPREL_LO12,       0x23c)
 ELF_RELOC(R_AARCH64_TLSLD_LDST128_DTPREL_LO12_NC,    0x23d)
 ELF_RELOC(R_AARCH64_AUTH_ABS64,                      0x244)
-// Dynamic relocations start
+// Dynamic relocations
 ELF_RELOC(R_AARCH64_COPY,                            0x400)
 ELF_RELOC(R_AARCH64_GLOB_DAT,                        0x401)
 ELF_RELOC(R_AARCH64_JUMP_SLOT,                       0x402)
@@ -136,8 +141,8 @@ ELF_RELOC(R_AARCH64_TLS_DTPREL64,                    0x405)
 ELF_RELOC(R_AARCH64_TLS_TPREL64,                     0x406)
 ELF_RELOC(R_AARCH64_TLSDESC,                         0x407)
 ELF_RELOC(R_AARCH64_IRELATIVE,                       0x408)
-ELF_RELOC(R_AARCH64_AUTH_RELATIVE,                   0x411)
 
+// ELF32
 // ELF_RELOC(R_AARCH64_P32_NONE,                         0)
 ELF_RELOC(R_AARCH64_P32_ABS32,                       0x001)
 ELF_RELOC(R_AARCH64_P32_ABS16,                       0x002)
@@ -216,7 +221,7 @@ ELF_RELOC(R_AARCH64_P32_TLSDESC_ADR_PAGE21,          0x07c)
 ELF_RELOC(R_AARCH64_P32_TLSDESC_LD32_LO12,           0x07d)
 ELF_RELOC(R_AARCH64_P32_TLSDESC_ADD_LO12,            0x07e)
 ELF_RELOC(R_AARCH64_P32_TLSDESC_CALL,                0x07f)
-// Dynamic relocations start
+// Dynamic relocations
 ELF_RELOC(R_AARCH64_P32_COPY,                        0x0b4)
 ELF_RELOC(R_AARCH64_P32_GLOB_DAT,                    0x0b5)
 ELF_RELOC(R_AARCH64_P32_JUMP_SLOT,                   0x0b6)

Copy link
Contributor

@kovdan01 kovdan01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The relocation is defined in https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst. The relocation is present in the spec (just not in the aaelf64 where you've tried to find it in but in pauthabielf64), we already use it in downstream and are submitting related patches to upstream - see, for example, #72714. Please don't delete it.

I suppose that @smithp35 can add something in terms of how Arm spec is organized, but basically I'm absolutely sure that the relocation is definitely needed. I see that there are other minor changes like changing comments in AArch64.def and adding a link to the aaelf64 spec - this might be useful, and, for example, a comment with a link to pauthabielf64 could be added.

Also tagging @asl

@artagnon artagnon changed the title ELFRelocs/AArch64: strip R_AARCH64_AUTH_RELATIVE, per doc ELFRelocs/AArch64: update canonical reference URL (NFC) Mar 28, 2024
@artagnon
Copy link
Contributor Author

Hi @kovdan01. Thanks for the review: now updated (with new commit message).

Update the URL of the reference to be used for AArch64.def, and add some
comments. The canonical aaelf64 document can be found at:

  https://github.com/ARM-software/abi-aa/blob/main/aaelf64/aaelf64.rst
@kovdan01
Copy link
Contributor

Looks reasonable as for me, but I want to see other opinions on this. AFAIK, @smithp35 will be available next week.

Also, please do not override old history via force-pushes. While the PR is pretty trivial, it's still nice to see history of the changes.

@MaskRay
Copy link
Member

MaskRay commented Mar 28, 2024

Looks reasonable as for me, but I want to see other opinions on this. AFAIK, @smithp35 will be available next week.

Also, please do not override old history via force-pushes. While the PR is pretty trivial, it's still nice to see history of the changes.

The update looks good me, but the official sign-off should probably be from an ABI maintainer like @smithp35

Copy link
Collaborator

@smithp35 smithp35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small correction to use Static AArch64 relocations to match https://github.com/ARM-software/abi-aa/blob/main/aaelf64/aaelf64.rst#static-aarch64-relocations otherwise LGTM.

It is true that the full definition of the AUTH relocations are in the PAuthABI. They should be mentioned in aaelf64 too as I recently merged that change. Currently checking aaelf64 it looks like there's a couple of mistakes that I've created ARM-software/abi-aa#254 to fix.

Given that PAuthABI is needed to understand the relocations I think it is fine to have this as the canonical reference.

llvm/include/llvm/BinaryFormat/ELFRelocs/AArch64.def Outdated Show resolved Hide resolved
Copy link
Collaborator

@smithp35 smithp35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks for the update

@artagnon artagnon merged commit 918542d into llvm:main Apr 4, 2024
3 of 4 checks passed
@artagnon artagnon deleted the aaelf64-def branch April 4, 2024 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants