[PAC][lld] Fix reloc against adrp imm in PAC PLT header #113429
Conversation
The PAC PLT header contains adrp instruction which immediate should be filled. In https://reviews.llvm.org/D62609, the adrp instruction address was calculated incorrectly. This patch resolves the issue. The test is already present in test/ELF/aarch64-feature-pac.s
| @llvm/pr-subscribers-lld-elf Author: Daniil Kovalev (kovdan01) ChangesThe PAC PLT header contains adrp instruction which immediate should be filled. In https://reviews.llvm.org/D62609, the adrp instruction address was calculated incorrectly. This patch resolves the issue. The test is already present in test/ELF/aarch64-feature-pac.s Full diff: https://github.com/llvm/llvm-project/pull/113429.diff 1 Files Affected:
diff --git a/lld/ELF/Arch/AArch64.cpp b/lld/ELF/Arch/AArch64.cpp index f4f867d019136e..28e0fce6a6f499 100644 --- a/lld/ELF/Arch/AArch64.cpp +++ b/lld/ELF/Arch/AArch64.cpp @@ -1048,7 +1048,7 @@ void AArch64BtiPac::writePltHeader(uint8_t *buf) const { memcpy(buf, pltData, sizeof(pltData)); relocateNoSym(buf + 4, R_AARCH64_ADR_PREL_PG_HI21, - getAArch64Page(got + 16) - getAArch64Page(plt + 8)); + getAArch64Page(got + 16) - getAArch64Page(plt + 4)); relocateNoSym(buf + 8, R_AARCH64_LDST64_ABS_LO12_NC, got + 16); relocateNoSym(buf + 12, R_AARCH64_ADD_ABS_LO12_NC, got + 16); if (!btiHeader) |
| @llvm/pr-subscribers-lld Author: Daniil Kovalev (kovdan01) ChangesThe PAC PLT header contains adrp instruction which immediate should be filled. In https://reviews.llvm.org/D62609, the adrp instruction address was calculated incorrectly. This patch resolves the issue. The test is already present in test/ELF/aarch64-feature-pac.s Full diff: https://github.com/llvm/llvm-project/pull/113429.diff 1 Files Affected:
diff --git a/lld/ELF/Arch/AArch64.cpp b/lld/ELF/Arch/AArch64.cpp index f4f867d019136e..28e0fce6a6f499 100644 --- a/lld/ELF/Arch/AArch64.cpp +++ b/lld/ELF/Arch/AArch64.cpp @@ -1048,7 +1048,7 @@ void AArch64BtiPac::writePltHeader(uint8_t *buf) const { memcpy(buf, pltData, sizeof(pltData)); relocateNoSym(buf + 4, R_AARCH64_ADR_PREL_PG_HI21, - getAArch64Page(got + 16) - getAArch64Page(plt + 8)); + getAArch64Page(got + 16) - getAArch64Page(plt + 4)); relocateNoSym(buf + 8, R_AARCH64_LDST64_ABS_LO12_NC, got + 16); relocateNoSym(buf + 12, R_AARCH64_ADD_ABS_LO12_NC, got + 16); if (!btiHeader) |
smithp35 left a comment
smithp35 left a comment There was a problem hiding this comment.
LGTM.
This was discussed in Discord under https://discord.com/channels/636084430946959380/1133112394701348895/1298121731428585564
I think the existing incorrect value of 8 is always going to round down to the same page [1] as 4 by getAArch64Page, but it is worth getting this right in the source code.
[1] PLT is 16-byte aligned. I don't think there's a placement of the PLT such that PLT + 4 or PLT +8 can cross a 4KiB page boundary.
The PAC PLT header contains adrp instruction which immediate should be filled. In https://reviews.llvm.org/D62609, the adrp instruction address was calculated incorrectly. This patch resolves the issue. The test is already present in test/ELF/aarch64-feature-pac.s
4 participants
The PAC PLT header contains adrp instruction which immediate should be filled. In https://reviews.llvm.org/D62609, the adrp instruction address was calculated incorrectly. This patch resolves the issue.
The test is already present in test/ELF/aarch64-feature-pac.s