Re: [PATCH 10/17] nptl: Move pthread_mutexattr_gettype into libc

[Florian Weimer via Libc-alpha <libc-alpha@sourceware.org>]

* Andreas Schwab:

> # bad: [24f261f27fb8fd19ae294ff2a13bc5b7a0bafc91] nptl: Remove __h_errno_location from libpthread
> # good: [10624a97e8e47004985740cbb04060a84cfada76] powerpc: Add optimized strlen for POWER10
> git bisect start '24f261f27f' '10624a97e8'
> # bad: [241ac38c333ae2539182f214dc641d0956f6ff6d] nptl: Move pthread_mutexattr_setprotocol into libc
> git bisect bad 241ac38c333ae2539182f214dc641d0956f6ff6d
> # good: [d236322b6f342d13bbd3fe97cb72ca53cba1b428] nptl: Move pthread_mutexattr_getprioceiling into libc
> git bisect good d236322b6f342d13bbd3fe97cb72ca53cba1b428
> # good: [9b7ab14e112476c96e7b20fb23e6838b7012dfda] nptl: Move pthread_mutexattr_getrobust into libc
> git bisect good 9b7ab14e112476c96e7b20fb23e6838b7012dfda
> # bad: [506385d30ec67279b21929f117b292bbbe8f5e7b] nptl: Move pthread_mutexattr_init, __pthread_mutexattr_init into libc
> git bisect bad 506385d30ec67279b21929f117b292bbbe8f5e7b
> # bad: [2a23e899e255f9ce2b4024d4ec029ce57af518bd] nptl: Move pthread_mutexattr_gettype into libc
> git bisect bad 2a23e899e255f9ce2b4024d4ec029ce57af518bd
> # first bad commit: [2a23e899e255f9ce2b4024d4ec029ce57af518bd] nptl: Move pthread_mutexattr_gettype into libc

Thank you, and also to Dmitry for pointing out the gnulib aspect.

The crash happens on line 294:

291     static void
292     init_fatal_signal_set (void)
293     {
294       gl_once (fatal_signal_set_once, do_init_fatal_signal_set);
295     }

The disassembly looks like this:

Dump of assembler code for function init_fatal_signal_set:
   0x000000010006a2f0 <+0>:     addis   r2,r12,5
   0x000000010006a2f4 <+4>:     addi    r2,r2,-9456
   0x000000010006a2f8 <+8>:     nop
   0x000000010006a2fc <+12>:    ld      r9,-32512(r2)
   0x000000010006a300 <+16>:    cmpdi   r9,0
   0x000000010006a304 <+20>:    beq     0x10006a344 <init_fatal_signal_set+84>
   0x000000010006a308 <+24>:    mflr    r0
   0x000000010006a30c <+28>:    std     r0,16(r1)
   0x000000010006a310 <+32>:    stdu    r1,-32(r1)
   0x000000010006a314 <+36>:    addis   r4,r2,-5
   0x000000010006a318 <+40>:    nop
   0x000000010006a31c <+44>:    addi    r4,r4,9248
   0x000000010006a320 <+48>:    addi    r3,r2,-13364
   0x000000010006a324 <+52>:    bl      0x100006700 <00000027.plt_call.pthread_once>
=> 0x000000010006a328 <+56>:    ld      r2,24(r1)
   0x000000010006a32c <+60>:    cmpdi   r3,0
   0x000000010006a330 <+64>:    bne     0x10006a374 <init_fatal_signal_set+132>
   0x000000010006a334 <+68>:    addi    r1,r1,32
   0x000000010006a338 <+72>:    ld      r0,16(r1)
   0x000000010006a33c <+76>:    mtlr    r0
   0x000000010006a340 <+80>:    blr
   0x000000010006a344 <+84>:    nop
   0x000000010006a348 <+88>:    ld      r9,-32504(r2)
   0x000000010006a34c <+92>:    cmpdi   r9,0
   0x000000010006a350 <+96>:    bne     0x10006a308 <init_fatal_signal_set+24>
   0x000000010006a354 <+100>:   nop
   0x000000010006a358 <+104>:   lbz     r9,-13364(r2)
   0x000000010006a35c <+108>:   cmpwi   r9,0
   0x000000010006a360 <+112>:   bnelr
   0x000000010006a364 <+116>:   xxspltib vs0,255
   0x000000010006a368 <+120>:   addi    r9,r2,-13364
   0x000000010006a36c <+124>:   stxsibx vs0,0,r9
   0x000000010006a370 <+128>:   b       0x10006a228 <do_init_fatal_signal_set+8>
   0x000000010006a374 <+132>:   bl      0x100006340 <00000027.plt_call.abort@@GLIBC_2.17>
   0x000000010006a378 <+136>:   ld      r2,24(r1)
   0x000000010006a37c <+140>:   .long 0x0
   0x000000010006a380 <+144>:   .long 0x1000000
   0x000000010006a384 <+148>:   .long 0x80

The crash is at the bl instruction, it branches to address zero.  The
load at 0x10006a2fc loads the address of pthread_mutexattr_gettype,
which used to be zero but is not anymore.

The use of weak symbols with dynamic linking has always been iffy.  An
old discussion is here:

  Specify how undefined weak symbol should be resolved in executable
  <https://sourceware.org/legacy-ml/gnu-gabi/2016-q1/msg00004.html>

The other issue with weak symbols is that they do not care symbol
version information and may therefore bind to baseline versions
unexpectedly.

I will bring this to the gnulib list.

Thanks,
Florian