diff mbox series

[v4,09/39] x86/vdso: Enable sframe generation in VDSO

Message ID c93fbd53f01c2c86f747af27040ac11f16dee032.1737511963.git.jpoimboe@kernel.org (mailing list archive)
State New
Headers show
Series unwind, perf: sframe user space unwinding | expand

Commit Message

Josh Poimboeuf Jan. 22, 2025, 2:31 a.m. UTC
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/Kconfig                          |  3 +++
 arch/x86/entry/vdso/Makefile          | 10 +++++++---
 arch/x86/entry/vdso/vdso-layout.lds.S |  3 +++
 arch/x86/include/asm/dwarf2.h         |  5 ++++-
 4 files changed, 17 insertions(+), 4 deletions(-)

Comments

Jens Remus Jan. 24, 2025, 4 p.m. UTC | #1
On 22.01.2025 03:31, Josh Poimboeuf wrote:
> Enable sframe generation in the VDSO library so kernel and user space
> can unwind through it.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

...

> diff --git a/arch/x86/include/asm/dwarf2.h b/arch/x86/include/asm/dwarf2.h
> index b195b3c8677e..1c354f648505 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.
>   	 */
> -

Nit: Deleted blank line you introduced in "[PATCH v4 05/39] x86/asm:
Avoid emitting DWARF CFI for non-VDSO".

> +#ifdef __x86_64__

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

AFAIK the kernel has a minimum binutils requirement of 2.25 [1]
and assembler option "--gsframe" as well as directive
".cfi_sections .sframe" were introduced with 2.40.

> +	.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 Jan. 24, 2025, 4:30 p.m. UTC | #2
On 22.01.2025 03:31, Josh Poimboeuf wrote:
> Enable sframe generation in the VDSO library so kernel and user space
> can unwind through it.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

> diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile

> @@ -47,13 +47,17 @@ quiet_cmd_vdso2c = VDSO2C  $@
>   $(obj)/vdso-image-%.c: $(obj)/vdso%.so.dbg $(obj)/vdso%.so $(obj)/vdso2c FORCE
>   	$(call if_changed,vdso2c)
>   
> +#ifdef CONFIG_AS_SFRAME
> +SFRAME_CFLAGS := -Wa$(comma)-gsframe
> +#endif
> +

You probably erroneously mixed up C preprocessor and Makefile syntax? :-)

ifeq ($(CONFIG_AS_SFRAME),y)
        SFRAME_CFLAGS := -Wa,--gsframe
endif

$(comma) does not appear to be required in this context.

Regards,
Jens
Josh Poimboeuf Jan. 24, 2025, 4:43 p.m. UTC | #3
On Fri, Jan 24, 2025 at 05:00:27PM +0100, Jens Remus wrote:
> On 22.01.2025 03:31, Josh Poimboeuf wrote:
> > diff --git a/arch/x86/include/asm/dwarf2.h b/arch/x86/include/asm/dwarf2.h
> > index b195b3c8677e..1c354f648505 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.
> >   	 */
> > -
> 
> Nit: Deleted blank line you introduced in "[PATCH v4 05/39] x86/asm:
> Avoid emitting DWARF CFI for non-VDSO".

Indeed.

> > +#ifdef __x86_64__
> 
> #if defined(__x86_64__) && defined(CONFIG_AS_SFRAME)
> 
> AFAIK the kernel has a minimum binutils requirement of 2.25 [1]
> and assembler option "--gsframe" as well as directive
> ".cfi_sections .sframe" were introduced with 2.40.

True, I'll change it to just '#ifdef CONFIG_AS_SFRAME' since that's what
really matters (and 32-bit doesn't support it anyway).

> > +	.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
Josh Poimboeuf Jan. 24, 2025, 4:53 p.m. UTC | #4
On Fri, Jan 24, 2025 at 08:43:35AM -0800, Josh Poimboeuf wrote:
> On Fri, Jan 24, 2025 at 05:00:27PM +0100, Jens Remus wrote:
> > On 22.01.2025 03:31, Josh Poimboeuf wrote:
> > > +#ifdef __x86_64__
> > 
> > #if defined(__x86_64__) && defined(CONFIG_AS_SFRAME)
> > 
> > AFAIK the kernel has a minimum binutils requirement of 2.25 [1]
> > and assembler option "--gsframe" as well as directive
> > ".cfi_sections .sframe" were introduced with 2.40.
> 
> True, I'll change it to just '#ifdef CONFIG_AS_SFRAME' since that's what
> really matters (and 32-bit doesn't support it anyway).

Actually, just CONFIG_AS_CFAME didn't work as 64-bit still compiles some
32-bit asm files.  I'll go with your suggestion.
Josh Poimboeuf Jan. 24, 2025, 4:56 p.m. UTC | #5
On Fri, Jan 24, 2025 at 05:30:36PM +0100, Jens Remus wrote:
> On 22.01.2025 03:31, Josh Poimboeuf wrote:
> > Enable sframe generation in the VDSO library so kernel and user space
> > can unwind through it.
> > 
> > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> 
> > diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
> 
> > @@ -47,13 +47,17 @@ quiet_cmd_vdso2c = VDSO2C  $@
> >   $(obj)/vdso-image-%.c: $(obj)/vdso%.so.dbg $(obj)/vdso%.so $(obj)/vdso2c FORCE
> >   	$(call if_changed,vdso2c)
> > +#ifdef CONFIG_AS_SFRAME
> > +SFRAME_CFLAGS := -Wa$(comma)-gsframe
> > +#endif
> > +
> 
> You probably erroneously mixed up C preprocessor and Makefile syntax? :-)
> 
> ifeq ($(CONFIG_AS_SFRAME),y)
>        SFRAME_CFLAGS := -Wa,--gsframe
> endif
> 
> $(comma) does not appear to be required in this context.

Yeah, before it was in a Makefile macro which needed the $(comma) to
distinguish it from the macro arguments.
diff mbox series

Patch

diff --git a/arch/Kconfig b/arch/Kconfig
index 6682b2a53e34..65228c78fef0 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -435,6 +435,9 @@  config HAVE_HARDLOCKUP_DETECTOR_ARCH
 	  It uses the same command line parameters, and sysctl interface,
 	  as the generic hardlockup detectors.
 
+config AS_SFRAME
+	def_bool $(as-instr,.cfi_sections .sframe\n.cfi_startproc\n.cfi_endproc)
+
 config HAVE_PERF_REGS
 	bool
 	help
diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index c9216ac4fb1e..478de89029d1 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -47,13 +47,17 @@  quiet_cmd_vdso2c = VDSO2C  $@
 $(obj)/vdso-image-%.c: $(obj)/vdso%.so.dbg $(obj)/vdso%.so $(obj)/vdso2c FORCE
 	$(call if_changed,vdso2c)
 
+#ifdef CONFIG_AS_SFRAME
+SFRAME_CFLAGS := -Wa$(comma)-gsframe
+#endif
+
 #
 # 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 +67,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.
@@ -104,7 +108,7 @@  $(obj)/%-x32.o: $(obj)/%.o FORCE
 
 targets += vdsox32.lds $(vobjx32s-y)
 
-$(obj)/%.so: OBJCOPYFLAGS := -S --remove-section __ex_table
+$(obj)/%.so: OBJCOPYFLAGS := -g --remove-section __ex_table
 $(obj)/%.so: $(obj)/%.so.dbg FORCE
 	$(call if_changed,objcopy)
 
diff --git a/arch/x86/entry/vdso/vdso-layout.lds.S b/arch/x86/entry/vdso/vdso-layout.lds.S
index 506c9800a5aa..4dcde4747b07 100644
--- a/arch/x86/entry/vdso/vdso-layout.lds.S
+++ b/arch/x86/entry/vdso/vdso-layout.lds.S
@@ -63,6 +63,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
@@ -91,6 +92,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
@@ -102,4 +104,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 b195b3c8677e..1c354f648505 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