On 1/15/24 15:18, Richard Henderson wrote:
> On 1/16/24 10:15, Vineet Gupta wrote:
>> When testing gcc testsuite against QEMU v8.2 we found some additional
>> failures vs. v8.1.2.
>>
>> | FAIL: gcc.dg/cleanup-10.c execution test
>> | FAIL: gcc.dg/cleanup-11.c execution test
>> | FAIL: gcc.dg/cleanup-8.c execution test
>> | FAIL: gcc.dg/cleanup-9.c execution test
>>
>> All of these tests involve unwinding off signal stack and v8.2 did
>> introduce a vdso with sigreturn trampoline and associated unwinding
>> info. It seems that info is not correct and making it similar to
>> to one in the linux kernel fixes the above failures.
> So.. you didn't actually determine what might be off in the unwind info?

Not yet. I just tried what kernel had and that worked.

>
>> +    .cfi_startproc
>> +    .cfi_signal_frame
>> +       raw_syscall __NR_rt_sigreturn
>> +       .cfi_endproc
> No, this is wrong.  It indicates that the unwind info is present and trivial.

Ok it seems the issue is really subtle.

With 8.2 trunk, the NOP needed before signal trampoline seems to be be
factored into the unwind info for sigrestorer.

    0000003c 0000000000000098 00000000 CIE
      Version:               3
      Augmentation:          "zRS"
      Code alignment factor: 1
      Data alignment factor: -4
      Return address column: 64
      Augmentation data:     1b
      DW_CFA_def_cfa: r2 (sp) ofs 832
      DW_CFA_offset_extended: r64 at cfa-528
      DW_CFA_offset: r1 (ra) at cfa-520
      DW_CFA_offset: r2 (sp) at cfa-512
    ...
      DW_CFA_offset: r63 (ft11) at cfa-24
      DW_CFA_nop
      DW_CFA_nop

    000000d8 0000000000000010 000000a0 FDE cie=0000003c
    pc=000000000000066c..0000000000000678
                                                                                
                                                     
    ^^^    <--- NOP included
      DW_CFA_nop
      DW_CFA_nop
      DW_CFA_nop

    0000000000000664 <__vdso_flush_icache>:
     664:    00000513              li    a0,0
     668:    00008067              ret
     66c:    00000013              nop                 <--- this NOP

    0000000000000670 <__vdso_rt_sigreturn>:
     670:    08b00893              li    a7,139
     674:    00000073              ecall


This is due to the .cfi_startproc bracketing. If we move the nop out of
the .cfi_{start,end}proc, things start to work as well.

    diff --git a/linux-user/riscv/vdso.S b/linux-user/riscv/vdso.S
    index 4b4e34aeea51..8c9f1038cb8c 100644
    --- a/linux-user/riscv/vdso.S
    +++ b/linux-user/riscv/vdso.S
    @@ -92,6 +92,8 @@ endf __vdso_flush_icache
     
            .cfi_endproc
     
    +       nop
    +
     /*
      * Start the unwind info at least one instruction before the signal
      * trampoline, because the unwinder will assume we are returning
    @@ -178,8 +180,6 @@ endf __vdso_flush_icache
            .cfi_offset     62, B_FR + 30 * sizeof_freg
            .cfi_offset     63, B_FR + 31 * sizeof_freg     /* f31 */
     
    -       nop
    -
     __vdso_rt_sigreturn:
            raw_syscall __NR_rt_sigreturn
     endf __vdso_rt_sigreturn


This changes the cfi info slightly as follows:

000000d8 0000000000000010 000000a0 FDE cie=0000003c
pc=0000000000000670..0000000000000678  <-- excludes nop
  DW_CFA_nop
  DW_CFA_nop
  DW_CFA_nop


0000000000000664 <__vdso_flush_icache>:
 664:    00000513              li    a0,0
 668:    00008067              ret
 66c:    00000013              nop

0000000000000670 <__vdso_rt_sigreturn>:
 670:    08b00893              li    a7,139
 674:    00000073              ecall

I concur this is still not 100% explanation of why things are going off,
but I have exact same nop quirk for glibc ARC sigrestorer.
Would an updated patch along those lines be more palatable.

Thx,
-Vineet

Reply via email to