Re: [PATCH] LoongArch: vDSO: Emit GNU_EH_FRAME correctly

[Xi Ruoyao <xry111@xry111.site>]

On Thu, 2026-02-26 at 17:22 +0800, Huacai Chen wrote:
> Hi, Ruoyao,
> 
> On Wed, Feb 25, 2026 at 6:46 PM Xi Ruoyao <xry111@xry111.site> wrote:
> > 
> > With -fno-asynchronous-unwind-tables and --no-eh-frame-hdr (the default
> > of the linker), the GNU_EH_FRAME segment (specified by vdso.lds.S) is
> > empty.  This is not valid, as the current DWARF specification mandates
> > the first byte of the EH frame to be the version number 1.  It causes
> > some unwinders to complain, for example the ClickHouse query profiler
> > spams the log with messages:
> > 
> >     clickhouse-server[365854]: libunwind: unsupported .eh_frame_hdr
> >     version: 127 at 7ffffffb0000
> > 
> > Here "127" is just the byte located at the p_vaddr (0, i.e. the
> > beginning of the vDSO) of the empty GNU_EH_FRAME segment.
> > Cross-checking with /proc/365854/maps has also proven 7ffffffb0000 is
> > the start of vDSO in the process VM image.
> > 
> > In LoongArch the -fno-asynchronous-unwind-tables option seems just a
> > MIPS legacy, and MIPS only uses this option to satisfy the MIPS-specific
> > "genvdso" program, per the commit cfd75c2db17e ("MIPS: VDSO: Explicitly
> > use -fno-asynchronous-unwind-tables").  IIUC it indicates some inherent
> > limitation of the MIPS ELF ABI and has nothing to do with LoongArch.  So
> > we can simply flip it over to -fasynchronous-unwind-tables and pass
> > --eh-frame-hdr for linking the vDSO, allowing the profilers to unwind the
> > stack for statistics even if the sample point is taken when the PC is in
> > the vDSO.
> > 
> > However simply adjusting the options above would exploit an issue: when
> > the libgcc unwinder saw the invalid GNU_EH_FRAME segment, it silently
> > falled back to a machine-specific routine to match the code pattern of
> > rt_sigreturn and extract the registers saved in the sigframe if the code
> > pattern is matched.  As unwinding from signal handlers is vital for
> > libgcc to support pthread cancellation etc., the fall-back routine had
> > been silently keeping the LoongArch Linux systems functioning since
> > Linux 5.19.  But when we start to emit GNU_EH_FRAME with the correct
> > format, fall-back routine will no longer be used and libgcc will fail
> > to unwind the sigframe, and unwinding from signal handlers will no
> > longer work, causing dozens of glibc test failures.  To make it possible
> > to unwind from signal handlers again, it's necessary to code the unwind
> > info in __vdso_rt_sigreturn via .cfi_* directives.
> > 
> > The offsets in the .cfi_* directives depend on the layout of struct
> > sigframe, notably the offset of sigcontect in the sigframe.  To use the
> > offset in the assembly file, factor out struct sigframe into a header to
> > allow asm-offsets.c to output the offset for assembly.
> > 
> > To work around a long-term issue in the libgcc unwinder (the pc is
> > unconditionally substracted by 1: doing so is technically incorrect for
> > a signal frame), a nop instruction is included with the two real
> > instructions in __vdso_rt_sigreturn in the same FDE PC range.  The same
> > hack has been used on x86 for a long time.
> > 
> > Fixes: c6b99bed6b8f ("LoongArch: Add VDSO and VSYSCALL support")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> > ---
> >  arch/loongarch/include/asm/sigframe.h | 22 ++++++++++++++
> >  arch/loongarch/kernel/asm-offsets.c   |  2 ++
> >  arch/loongarch/kernel/signal.c        |  6 +---
> >  arch/loongarch/vdso/Makefile          |  4 +--
> >  arch/loongarch/vdso/sigreturn.S       | 44 ++++++++++++++++++++++++---
> >  5 files changed, 67 insertions(+), 11 deletions(-)
> >  create mode 100644 arch/loongarch/include/asm/sigframe.h
> > 
> > diff --git a/arch/loongarch/include/asm/sigframe.h b/arch/loongarch/include/asm/sigframe.h
> > new file mode 100644
> > index 000000000000..6889bcf5dc88
> > --- /dev/null
> > +++ b/arch/loongarch/include/asm/sigframe.h
> > @@ -0,0 +1,22 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Separated from arch/loongarch/kernel/signal.c:
> > + *
> > + * Author: Hanlu Li <lihanlu@loongson.cn>
> > + *         Huacai Chen <chenhuacai@loongson.cn>
> > + * Copyright (C) 2020-2022 Loongson Technology Corporation Limited
> > + *
> > + * Derived from MIPS:
> > + * Copyright (C) 1991, 1992  Linus Torvalds
> > + * Copyright (C) 1994 - 2000  Ralf Baechle
> > + * Copyright (C) 1999, 2000 Silicon Graphics, Inc.
> > + * Copyright (C) 2014, Imagination Technologies Ltd.
> > + */
> I think we don't need to copy so many lines here, they are enough in signal.c.

Will remove them in V2.

> > +
> > +#include <uapi/asm/ucontext.h>
> Is it a requirement that the UAPI header should be the first?

I don't think so, and it seems the uapi/ component isn't needed here (as
Kbuild passes -Iarch/$(ARCH)/include/uapi).  I'd remove the uapi/
component and move it after siginfo.h following the alphabetical order.

> > +#include <asm/siginfo.h>

> > +       /*
> > +        * HACK: The dwarf2 unwind routine will subtract 1 from the return
> > +        * address to get an address in the middle of the persumed call
> > +        * instruction.  While in libgcc there exists a logic to avoid
> > +        * subtracting 1 for the signal frame (a frame with the 'S'
> > +        * augmentation that we've already added via .cfi_signal_frame),
> > +        * unfortunately it doesn't really work: the check of signal frame
> > +        * is at libgcc/unwind-dw2:1008 in GCC 15.2.0, but the flag it
> > +        * checks will only get updated by the extract_cie_info call at line
> > +        * 1025.  So include a nop before the real start to make up for it.
> > +        * This is also the reason we don't use SYM_FUNC_START.
> > +        */
> > +       nop
> This hack is out of my knowledge, can "nop" be after SYM_START()?

I guess we can do it if:

diff --git a/arch/loongarch/kernel/signal.c b/arch/loongarch/kernel/signal.c
index e297d54ea638..b2c7f5754818 100644
--- a/arch/loongarch/kernel/signal.c
+++ b/arch/loongarch/kernel/signal.c
@@ -1009,7 +1009,7 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs)
 
 	rseq_signal_deliver(ksig, regs);
 
-	ret = setup_rt_frame(vdso + current->thread.vdso->offset_sigreturn, ksig, regs, oldset);
+	ret = setup_rt_frame(vdso + current->thread.vdso->offset_sigreturn + 4, ksig, regs, oldset);
 
 	signal_setup_done(ret, ksig, 0);
 }

I.e. the FDE must include one instruction before what we set up as the
return address of the signal handler.  But I don't think this would be
prettier.
> 

-- 
Xi Ruoyao <xry111@xry111.site>