[PATCH] powerpc: Fix incorrect cache line size load in memset (bug 26332)

[Carlos O'Donell]

On 8/3/20 4:51 AM, Florian Weimer wrote:
> __GLRO loaded the word after the requested variable on big-endian
> PowerPC, where LOWORD is 4.  This can cause the memset implement
> go wrong because the masking with the cache line size produces
> wrong results, particularly if the loaded value happens to be 1.
> 
> The __GLRO macro is not used in any place where loading the lower
> 32-bit word of a 64-bit value is desired, so the +4 offset is always
> wrong.

Agreed.

I reviewed sysdeps/powerpc/powerpc32/sysdep.h, and:
sysdeps/powerpc/powerpc32/power4/multiarch/init-arch.h
sysdeps/powerpc/powerpc32/sysdep.h
sysdeps/powerpc/powerpc32/memset.S
sysdeps/powerpc/powerpc32/a2/memcpy.S

I see only 32-bit word accesses and so the +4 offset is wrong. This
was likely just a copy-and-paste that came from similar code which
accesses the 64-bit HWCAP/HWCAP2 e.g. uint64_t _dl_hwcap.

The 64-bit sysdeps/powerpc/powerpc64/sysdep.h is different and doesn't
have this problem for obvious reasons (it can load a 64-bit value).

> Fixes commit 18363b4f010da9ba459b13310b113ac0647c2fcc
> ("powerpc: Move cache line size to rtld_global_ro") and bug 26332.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  sysdeps/powerpc/powerpc32/sysdep.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h
> index 2ba009e919..829eec266a 100644
> --- a/sysdeps/powerpc/powerpc32/sysdep.h
> +++ b/sysdeps/powerpc/powerpc32/sysdep.h
> @@ -179,8 +179,8 @@ GOT_LABEL:			;					      \
>  #else
>  /* Position-dependent code does not require access to the GOT.  */
>  # define __GLRO(rOUT, rGOT, member, offset)				\
> -	lis     rOUT,(member+LOWORD)@ha;					\
> -	lwz     rOUT,(member+LOWORD)@l(rOUT)
> +	lis     rOUT,(member)@ha;					\
> +	lwz     rOUT,(member)@l(rOUT)

OK.

>  #endif	/* PIC */
>  
>  #endif	/* __ASSEMBLER__ */
> 


-- 
Cheers,
Carlos.