On Tue, Mar 08, 2022 at 09:51:18PM -0800, Fangrui Song 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}

                                                         ^ ld.{bfd,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.
> 
> DT_RELASZ is in practice bounded by an uint32_t. Dividing x by 24 can be
> implemented as (uint32_t)(x*0xaaaaaaab) >> 4.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1581
> Reported-by: Nathan Chancellor <nat...@kernel.org>
> Signed-off-by: Fangrui Song <mask...@google.com>

Thank you a lot for the fix! I tested this with several different
configurations with tip of tree LLVM and they all booted in QEMU. I also
tested one configuration with GCC 5.4 (we support GCC 5.x as a minimum)
and it booted as well.

Tested-by: Nathan Chancellor <nat...@kernel.org>

> ---
>  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
>  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,3f
> diff --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
> 

Reply via email to