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