Re: [PATCH] powerpc: Replace ppc64 DT_RELACOUNT usage with DT_RELASZ/24
flat view
From: Nick Desaulniers <ndesaulniers@google.com>
Date: 2022-03-10 19:48:57
Also in:
lkml, llvm
On Tue, Mar 8, 2022 at 9:53 PM Fangrui Song [off-list ref] wrote:
DT_RELACOUNT is an ELF dynamic tag inherited from SunOS indicating the
number of R_*_RELATIVE relocations. It is optional but {ld.lld,ld.lld}
-z combreloc always creates it (if non-zero) to slightly speed up glibc
ld.so relocation resolving by avoiding R_*R_PPC64_RELATIVE type
comparison. The tag is otherwise nearly unused in the wild and I'd
recommend that software avoids using it.
lld>=14.0.0 (since commit da0e5b885b25cf4ded0fa89b965dc6979ac02ca9)
underestimates DT_RELACOUNT for ppc64 when position-independent long
branch thunks are used. Correcting it needs non-trivial arch-specific
complexity which I'd prefer to avoid. Since our code always compares the
relocation type with R_PPC64_RELATIVE, replacing every occurrence of
DT_RELACOUNT with DT_RELASZ/sizeof(Elf64_Rela)=DT_RELASZ/24 is a correct
alternative.checking that sizeof(Elf64_Rela) == 24, yep: https://godbolt.org/z/bb4aKbo5T
DT_RELASZ is in practice bounded by an uint32_t. Dividing x by 24 can be implemented as (uint32_t)(x*0xaaaaaaab) >> 4.
Yep: https://godbolt.org/z/x9445ePPv
quoted hunk ↗ jump to hunk
Link: https://github.com/ClangBuiltLinux/linux/issues/1581 Reported-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Fangrui Song <redacted> --- arch/powerpc/boot/crt0.S | 28 +++++++++++++++++----------- arch/powerpc/kernel/reloc_64.S | 15 +++++++++------ 2 files changed, 26 insertions(+), 17 deletions(-)diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S index feadee18e271..1c96ebe7ef1a 100644 --- a/arch/powerpc/boot/crt0.S +++ b/arch/powerpc/boot/crt0.S@@ -8,7 +8,7 @@ #include "ppc_asm.h" RELA = 7 -RELACOUNT = 0x6ffffff9 +RELASZ = 8 .data /* A procedure descriptor used when booting this as a COFF file.@@ -65,7 +65,7 @@ p_base: mflr r10 /* r10 now points to runtime addr of p_base */ subf r11,r11,r12 /* runtime - linktime offset */ /* The dynamic section contains a series of tagged entries. - * We need the RELA and RELACOUNT entries. */ + * We need the RELA and RELASZ entries. */ li r9,0 li r0,0 9: lwz r8,0(r12) /* get tag */@@ -75,18 +75,21 @@ p_base: mflr r10 /* r10 now points to runtime addr of p_base */ bne 11f lwz r9,4(r12) /* get RELA pointer in r9 */ b 12f -11: addis r8,r8,(-RELACOUNT)@ha - cmpwi r8,RELACOUNT@l +11: cmpwi r8,RELASZ bne 12f - lwz r0,4(r12) /* get RELACOUNT value in r0 */ + lwz r0,4(r12) /* get RELASZ / 24 in r0 */ + lis r8,0xaaaa + ori r8,r8,0xaaab + mulhwu r0,r0,r8 + srwi r0,r0,4
Both translation units include arch/powerpc/include/asm/ppc_asm.h, which happens to define LOAD_REG_IMMEDIATE. Let's reuse that rather than open code lis+ori?
quoted hunk ↗ jump to hunk
12: addi r12,r12,8 b 9b /* The relocation section contains a list of relocations. * We now do the R_PPC_RELATIVE ones, which point to words * which need to be initialized with addend + offset. - * The R_PPC_RELATIVE ones come first and there are RELACOUNT - * of them. */ + * The R_PPC_RELATIVE ones come first and there are at most + * RELASZ/24 of them. */ 10: /* skip relocation if we don't have both */ cmpwi r0,0 beq 3f@@ -160,14 +163,17 @@ p_base: mflr r10 /* r10 now points to runtime addr of p_base */ bne 10f ld r13,8(r11) /* get RELA pointer in r13 */ b 11f -10: addis r12,r12,(-RELACOUNT)@ha - cmpdi r12,RELACOUNT@l +10: cmpdi r12,RELASZ bne 11f - ld r8,8(r11) /* get RELACOUNT value in r8 */ + ld r8,8(r11) /* get RELASZ / 24 in r8 */ + lis r0,0xaaaa + ori r0,r0,0xaaab + mulhwu r8,r8,r0 + srwi r8,r8,4 11: addi r11,r11,16 b 9b 12: - cmpdi r13,0 /* check we have both RELA and RELACOUNT */ + cmpdi r13,0 /* check we have both RELA and RELASZ */ cmpdi cr1,r8,0 beq 3f beq cr1,3fdiff --git a/arch/powerpc/kernel/reloc_64.S b/arch/powerpc/kernel/reloc_64.S index 02d4719bf43a..362be759609f 100644 --- a/arch/powerpc/kernel/reloc_64.S +++ b/arch/powerpc/kernel/reloc_64.S@@ -8,7 +8,7 @@ #include <asm/ppc_asm.h> RELA = 7 -RELACOUNT = 0x6ffffff9 +RELASZ = 8 R_PPC64_RELATIVE = 22 /*@@ -27,7 +27,7 @@ _GLOBAL(relocate) add r10,r10,r12 /* r10 has runtime addr of _stext */ /* - * Scan the dynamic section for the RELA and RELACOUNT entries. + * Scan the dynamic section for the RELA and RELASZ entries. */ li r7,0 li r8,0@@ -38,13 +38,16 @@ _GLOBAL(relocate) bne 2f ld r7,8(r11) /* get RELA pointer in r7 */ b 3f -2: addis r6,r6,(-RELACOUNT)@ha - cmpdi r6,RELACOUNT@l +2: cmpdi r6,RELASZ bne 3f - ld r8,8(r11) /* get RELACOUNT value in r8 */ + ld r8,8(r11) /* get RELA / 24 in r8 */ + lis r0,0xaaaa + ori r0,r0,0xaaab + mulhwu r8,r8,r0 + srwi r8,r8,4 3: addi r11,r11,16 b 1b -4: cmpdi r7,0 /* check we have both RELA and RELACOUNT */ +4: cmpdi r7,0 /* check we have both RELA and RELASZ */ cmpdi cr1,r8,0 beq 6f beq cr1,6f --2.35.1.616.g0bdcbb4464-goog
-- Thanks, ~Nick Desaulniers