Re: [PATCH 2/2] powerpc: Move cache line size to rtld_global_ro

[Florian Weimer <fweimer@redhat.com>]

* Joseph Myers:

>> Does this affect the statically linked test binaries built by
>> build-many-glibcs.py?
>
> Yes.  I tested build-many-glibcs.py? for powerpc-linux-gnu with current 
> default versions of everything (so binutils 2.35 branch in this case); 
> running the math/atest-exp binary left from a --keep=all build produces 
> that same assertion failure.

I can reproduce it:

(gdb) bt
#0  0x10007d74 in __libc_signal_restore_set (set=0xfffed708)
    at ../sysdeps/unix/sysv/linux/internal-signals.h:104
#1  raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:47
#2  0x10000268 in abort () at abort.c:79
#3  0x1001c7b8 in __malloc_assert (
    assertion=assertion@entry=0x10081ec4 "(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)", 
    file=file@entry=0x10081690 "malloc.c", line=line@entry=2394, 
    function=function@entry=0x100828b8 <__PRETTY_FUNCTION__.3> "sysmalloc") at malloc.c:298
#4  0x1001ee1c in sysmalloc (nb=nb@entry=64, 
    av=av@entry=0x100c03d4 <main_arena>) at malloc.c:2394
#5  0x10020614 in _int_malloc (av=av@entry=0x100c03d4 <main_arena>, 
    bytes=bytes@entry=53) at malloc.c:4169
#6  0x10021804 in __libc_malloc (bytes=53) at malloc.c:3078
#7  0x10060f44 in _dl_get_origin ()
    at ../sysdeps/unix/sysv/linux/dl-origin.c:49
#8  0x100302f4 in _dl_non_dynamic_init () at dl-support.c:311
#9  0x10031908 in __libc_init_first (argc=argc@entry=2, 
    argv=argv@entry=0xfffeecd4, envp=0xfffeece0) at init-first.c:72
#10 0x100024dc in generic_start_main (main=0x100003d4 <main>, 
    argc=argc@entry=2, argv=argv@entry=0xfffeecd4, 
    auxvec=auxvec@entry=0xfffeed9c, init=0x10002be0 <__libc_csu_init>, 
    fini=0x10002d58 <__libc_csu_fini>, rtld_fini=rtld_fini@entry=0x0, 
    stack_end=stack_end@entry=0xfffeecd0) at ../csu/libc-start.c:250
#11 0x10002774 in __libc_start_main (argc=2, argv=0xfffeecd4, 
    ev=<optimized out>, auxvec=0xfffeed9c, rtld_fini=0x0, 
    stinfo=0x1007f3e0, stack_on_entry=0xfffeecd0)
    at ../sysdeps/unix/sysv/linux/powerpc/libc-start.c:98
#12 0x00000000 in ?? ()

main_arena.top->mchunk_size gets overwritten during tcache_init:

#0  memset () at ../sysdeps/powerpc/powerpc32/memset.S:291
#1  0x10021660 in tcache_init () at malloc.c:3021
#2  0x10021af4 in __libc_malloc (bytes=bytes@entry=53) at malloc.c:3064
#3  0x10021d80 in malloc_hook_ini (sz=53, caller=<optimized out>) at hooks.c:32
#4  0x10021aa0 in __libc_malloc (bytes=53) at malloc.c:3053
#5  0x100614c4 in _dl_get_origin () at ../sysdeps/unix/sysv/linux/dl-origin.c:49
#6  0x10030874 in _dl_non_dynamic_init () at dl-support.c:311
#7  0x10031e88 in __libc_init_first (argc=argc@entry=2, 
    argv=argv@entry=0xfffeecd4, envp=0xfffeece0) at init-first.c:72
#8  0x100024dc in generic_start_main (main=0x100003d4 <main>, argc=argc@entry=2, 
    argv=argv@entry=0xfffeecd4, auxvec=auxvec@entry=0xfffeed9c, 
    init=0x10002be0 <__libc_csu_init>, fini=0x10002d58 <__libc_csu_fini>, 
    rtld_fini=rtld_fini@entry=0x0, stack_end=stack_end@entry=0xfffeecd0)
    at ../csu/libc-start.c:250
#9  0x10002774 in __libc_start_main (argc=2, argv=0xfffeecd4, 
    ev=<optimized out>, auxvec=0xfffeed9c, rtld_fini=0x0, stinfo=0x1007f960, 
    stack_on_entry=0xfffeecd0)
    at ../sysdeps/unix/sysv/linux/powerpc/libc-start.c:98
#10 0x00000000 in ?? ()

The memset goes wrong because it loads the cache line size as 1 here:

/* Load rtld_global_ro._dl_cache_line_size.  */
	__GLRO(rCLS, rGOT, _dl_cache_line_size,
	       RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET)

0x100259e4 <+576>:   lis     r8,4108
=> 0x100259e8 <+580>:   lwz     r8,3912(r8)

(gdb) print (void*)($r8 + 3912)
$24 = (void *) 0x100c0f48 <__libc_enable_secure_decided>

This is not the address that is seen by the debugger for
_dl_cache_line_size:

(gdb) print &_dl_cache_line_size 
$26 = (int *) 0x100c0f44 <_dl_cache_line_size>

The symbol table looks pretty reasonable:
  1461: 100c0f44     4 OBJECT  GLOBAL DEFAULT   23 _dl_cache_line_size
  1495: 100c0f40     4 OBJECT  GLOBAL DEFAULT   23 _dl_platform
  1512: 100c0f48     4 OBJECT  GLOBAL DEFAULT   23 __libc_enable_secure_decided
  2135: 100c0f4c     4 OBJECT  GLOBAL DEFAULT   23 __libc_argv

For some reason, we have relocations with displacements in
string/memset.o:

 244:   3d 00 00 00     lis     r8,0
                        246: R_PPC_ADDR16_HA    _dl_cache_line_size+0x4
 248:   81 08 00 00     lwz     r8,0(r8)
                        24a: R_PPC_ADDR16_LO    _dl_cache_line_size+0x4

This is due to the definition of __GLRO:

#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)
#endif  /* PIC */

And LOWORD is 4 on big-endian PowerPC:

/* The 32-bit words of a 64-bit dword are at these offsets in memory.  */
#if defined __LITTLE_ENDIAN__ || defined _LITTLE_ENDIAN
# define LOWORD 0
# define HIWORD 4
#else
# define LOWORD 4
# define HIWORD 0
#endif

I believe we should remove the “+LOWORD” part here:

diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h
index 2ba009e9..829eec26 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)
 #endif /* PIC */
 
 #endif /* __ASSEMBLER__ */

It fixes math/atest-exp for me.

Tulio, I believe you constructed this macro from
sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S, where it
is needed because we are loading the lower 32 bits of a 64-bit value.
It's not correct for loading a 32-bit quantity.

Technically, this bug is not a release blocker.  It's not a regression,
it's present in 2.31 as well.  I will file a bug and post a proper patch.

Thanks,
Florian