Enable sframe generation in the VDSO library so kernel and user space
can unwind through it.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 arch/x86/entry/vdso/Makefile          | 6 ++++--
 arch/x86/entry/vdso/vdso-layout.lds.S | 3 +++
 arch/x86/include/asm/dwarf2.h         | 5 ++++-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index c9216ac4fb1e..75ae9e093a2d 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -47,13 +47,15 @@ quiet_cmd_vdso2c = VDSO2C  $@
 $(obj)/vdso-image-%.c: $(obj)/vdso%.so.dbg $(obj)/vdso%.so $(obj)/vdso2c FORCE
 	$(call if_changed,vdso2c)
 
+SFRAME_CFLAGS := $(call as-option,-Wa$(comma)-gsframe,)
+
 #
 # Don't omit frame pointers for ease of userspace debugging, but do
 # optimize sibling calls.
 #
 CFL := $(PROFILING) -mcmodel=small -fPIC -O2 -fasynchronous-unwind-tables -m64 \
        $(filter -g%,$(KBUILD_CFLAGS)) -fno-stack-protector \
-       -fno-omit-frame-pointer -foptimize-sibling-calls \
+       -fno-omit-frame-pointer $(SFRAME_CFLAGS) -foptimize-sibling-calls \
        -DDISABLE_BRANCH_PROFILING -DBUILD_VDSO
 
 ifdef CONFIG_MITIGATION_RETPOLINE
@@ -63,7 +65,7 @@ endif
 endif
 
 $(vobjs): KBUILD_CFLAGS := $(filter-out $(PADDING_CFLAGS) $(CC_FLAGS_LTO) $(CC_FLAGS_CFI) $(RANDSTRUCT_CFLAGS) $(GCC_PLUGINS_CFLAGS) $(RETPOLINE_CFLAGS),$(KBUILD_CFLAGS)) $(CFL)
-$(vobjs): KBUILD_AFLAGS += -DBUILD_VDSO
+$(vobjs): KBUILD_AFLAGS += -DBUILD_VDSO $(SFRAME_CFLAGS)
 
 #
 # vDSO code runs in userspace and -pg doesn't help with profiling anyway.
diff --git a/arch/x86/entry/vdso/vdso-layout.lds.S b/arch/x86/entry/vdso/vdso-layout.lds.S
index a42c7d4a33da..f6cd6654bb9e 100644
--- a/arch/x86/entry/vdso/vdso-layout.lds.S
+++ b/arch/x86/entry/vdso/vdso-layout.lds.S
@@ -69,6 +69,7 @@ SECTIONS
 	.eh_frame_hdr	: { *(.eh_frame_hdr) }		:text	:eh_frame_hdr
 	.eh_frame	: { KEEP (*(.eh_frame)) }	:text
 
+	.sframe		: { *(.sframe) }		:text	:sframe
 
 	/*
 	 * Text is well-separated from actual data: there's plenty of
@@ -97,6 +98,7 @@ SECTIONS
  * Very old versions of ld do not recognize this name token; use the constant.
  */
 #define PT_GNU_EH_FRAME	0x6474e550
+#define PT_GNU_SFRAME	0x6474e554
 
 /*
  * We must supply the ELF program headers explicitly to get just one
@@ -108,4 +110,5 @@ PHDRS
 	dynamic		PT_DYNAMIC	FLAGS(4);		/* PF_R */
 	note		PT_NOTE		FLAGS(4);		/* PF_R */
 	eh_frame_hdr	PT_GNU_EH_FRAME;
+	sframe		PT_GNU_SFRAME;
 }
diff --git a/arch/x86/include/asm/dwarf2.h b/arch/x86/include/asm/dwarf2.h
index b1aa3fcd5bca..1a49492817a1 100644
--- a/arch/x86/include/asm/dwarf2.h
+++ b/arch/x86/include/asm/dwarf2.h
@@ -12,8 +12,11 @@
 	 * For the vDSO, emit both runtime unwind information and debug
 	 * symbols for the .dbg file.
 	 */
-
+#ifdef __x86_64__
+	.cfi_sections .eh_frame, .debug_frame, .sframe
+#else
 	.cfi_sections .eh_frame, .debug_frame
+#endif
 
 #define CFI_STARTPROC		.cfi_startproc
 #define CFI_ENDPROC		.cfi_endproc
-- 
2.47.0

On 28.10.2024 22:47, Josh Poimboeuf wrote:
> Enable sframe generation in the VDSO library so kernel and user space
> can unwind through it.

...

Applying similar changes to s390 and using the current binutils release without SFrame support on s390 results in build errors.

AFAIK the kernel has a minimum binutils requirement of 2.25 [1] and the assembler option "--gsframe was introduced with 2.40.

> diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
> index c9216ac4fb1e..75ae9e093a2d 100644
> --- a/arch/x86/entry/vdso/Makefile
> +++ b/arch/x86/entry/vdso/Makefile
> @@ -47,13 +47,15 @@ quiet_cmd_vdso2c = VDSO2C  $@
>   $(obj)/vdso-image-%.c: $(obj)/vdso%.so.dbg $(obj)/vdso%.so $(obj)/vdso2c FORCE
>   	$(call if_changed,vdso2c)
>   
> +SFRAME_CFLAGS := $(call as-option,-Wa$(comma)-gsframe,)

I have the impression this test might not work as expected. On s390 the assembler accepts option --gsframe and only generates an error when the assembler code actually contains any CFI directives:

$ cat test-nocfi.a
         la      %r1,42
$ as --gsframe test-nocfi.a
$ echo $?
0

$ cat test-cfi.a
         .cfi_startproc
         .cfi_endproc
$ as --gsframe test-cfi.a
test-cfi.a: Assembler messages:
test-cfi.a: Error: .sframe not supported for target
$ echo $?
1

But the following assembler code triggers an error:

$ cat test-sframe.a
         .cfi_sections .sframe
         .cfi_startproc
         .cfi_endproc
$ as test-sframe.a
test-sframe.a: Assembler messages:
test-sframe.a: Error: .sframe not supported for target
$ echo $?
1

Maybe the following would be an alternative test in the Makefile?

SFRAME_CFLAGS := $(call as-instr,.cfi_sections .sframe\n.cfi_startproc\n.cfi_endproc,-DCONFIG_AS_SFRAME=1)
ifneq ($(SFRAME_CFLAGS),)
        SFRAME_CFLAGS += -Wa,--gsframe
endif

> diff --git a/arch/x86/include/asm/dwarf2.h b/arch/x86/include/asm/dwarf2.h
> index b1aa3fcd5bca..1a49492817a1 100644
> --- a/arch/x86/include/asm/dwarf2.h
> +++ b/arch/x86/include/asm/dwarf2.h
> @@ -12,8 +12,11 @@
>   	 * For the vDSO, emit both runtime unwind information and debug
>   	 * symbols for the .dbg file.
>   	 */
> -
> +#ifdef __x86_64__

#if defined(__x86_64__) && defined(CONFIG_AS_SFRAME)

> +	.cfi_sections .eh_frame, .debug_frame, .sframe
> +#else
>   	.cfi_sections .eh_frame, .debug_frame
> +#endif
>   
>   #define CFI_STARTPROC		.cfi_startproc
>   #define CFI_ENDPROC		.cfi_endproc

[1]: https://docs.kernel.org/process/changes.html

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303) and z/VSE Support
+49-7031-16-1128 Office
jremus@de.ibm.com

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/

On Wed, Oct 30, 2024 at 07:20:08PM +0100, Jens Remus wrote:
> > diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
> > index c9216ac4fb1e..75ae9e093a2d 100644
> > --- a/arch/x86/entry/vdso/Makefile
> > +++ b/arch/x86/entry/vdso/Makefile
> > @@ -47,13 +47,15 @@ quiet_cmd_vdso2c = VDSO2C  $@
> >   $(obj)/vdso-image-%.c: $(obj)/vdso%.so.dbg $(obj)/vdso%.so $(obj)/vdso2c FORCE
> >   	$(call if_changed,vdso2c)
> > +SFRAME_CFLAGS := $(call as-option,-Wa$(comma)-gsframe,)
> 
> I have the impression this test might not work as expected.

I suspect it works fine on x86-64, since that was the first arch
supported, so --gsframe being supported also means x86-64 is supported.

But yeah, other arches (and x86-32) are a different story.

> Maybe the following would be an alternative test in the Makefile?
> 
> SFRAME_CFLAGS := $(call as-instr,.cfi_sections .sframe\n.cfi_startproc\n.cfi_endproc,-DCONFIG_AS_SFRAME=1)
> ifneq ($(SFRAME_CFLAGS),)
>        SFRAME_CFLAGS += -Wa,--gsframe
> endif

Looks good, though the ifneq isn't needed:

SFRAME_CFLAGS := $(call as-instr, \
		   .cfi_sections .sframe\n.cfi_startproc\n.cfi_endproc, \
		   Wa$(comma)--gsframe -DCONFIG_AS_SFRAME=1)


Though, if multiple arches are going to be using that, maybe it should
just be a config option:


diff --git a/arch/Kconfig b/arch/Kconfig
index 33449485eafd..676dd45a7255 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -446,6 +446,9 @@ config HAVE_UNWIND_USER_SFRAME
 	bool
 	select UNWIND_USER
 
+config AS_SFRAME
+	def_bool $(as-instr,.cfi_sections .sframe\n.cfi_startproc\n.cfi_endproc)
+
 config HAVE_PERF_CALLCHAIN_DEFERRED
 	bool