Re: [PATCH] kbuild: use objcopy to generate asm-offsets

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 03/09/2024 01:45, Michael Ellerman wrote:
Vegard Nossum <vegard.nossum@xxxxxxxxxx> writes:
Remove the sed script and compile the C source listing structs and
offsets to an object file (instead of assembly code) that embeds C source
directly. Then extract the C source using objcopy.


I threw some builders at this and hit a few errors:

Thanks, I also got the ones from kernel test robot and figured something
was going a bit wrong.

There are several issues: clang wants - instead of /dev/stdout as an
argument to objcopy, and then gcc has some bugs that prevent the numbers
from appearing correctly on some architectures, also I had an extra # in
the COMMENT() macro which only resulted in an error on some
architectures. I've attached a tentative v2 that fixes these issues, but
I'm still trying to figure out why m68k is giving me slightly different
output for include/generated/asm-offsets.h and why the arc assembler
fails.

In the end I'm wondering if this patch is really worth it, given all the
failures and little workarounds :-| If I can sort out the last few
failures I'll submit it as an RFC.

Full list here, but note there are some unrelated pre-existing failures:
   http://kisskb.ellerman.id.au/kisskb/head/259bba3447faaf5e5b12ae41a26a62978d4c1965/

Thanks again,


Vegard
From 290fcf01dde9c83dcc2db875b3e4a03053cd3a74 Mon Sep 17 00:00:00 2001
From: Vegard Nossum <vegard.nossum@xxxxxxxxxx>
Date: Tue, 27 Aug 2024 19:44:44 +0200
Subject: [PATCH v2] kbuild: use objcopy to generate asm-offsets

In order to give assembly code access to C structs without having to
hardcode member offsets, the kernel compiles a C source file listing all
the structs and offsets that are needed in assembly code. Using some
C preprocessor trickery and a sed script, the compiled assembly code is
turned back into C preprocessor code that in turn can be used by the
asssembly code.

This sed script is very hard to read and understand.

Remove the sed script and compile the C source listing structs and
offsets to an object file (instead of assembly code) that embeds C source
directly. Then extract the C source using objcopy.

The resulting code is more readable and less fragile.

Note to reviewers: The 'objcopy ... /dev/stdout | cat' bit is needed to
force the correct ordering of the objcopy lines vs. the surrounding echo
commands; without it, objcopy will open /dev/stdout (which refers to a
temporary file created by kbuild) and reset the file offset to 0. In
other words, the pipe ensures that objcopy doesn't overwrite the lines
that already exist in /dev/stdout.

v2:
- fixed changelog typos (thanks to Johannes Berg)
- support LLVM's objcopy by testing for $(LLVM) and using - for /dev/stdout
- attempt to work around build failures on arc, mips64el, and sh4 by
  using %n0 instead of %c0 in the inline assembly for generating the header
- fix filechk_offsets indentation
- fix extraneous double quotes in COMMENT()

This version of the patch adds about 7 lines overall, but this is mostly
due to the extra explanations inserted as code comments.

Signed-off-by: Vegard Nossum <vegard.nossum@xxxxxxxxxx>
---
 Kbuild                       | 10 +++++-----
 arch/arm/mach-at91/Makefile  |  4 ++--
 arch/arm/mach-omap2/Makefile |  4 ++--
 arch/arm64/kvm/Makefile      |  9 +++++----
 arch/x86/kvm/Makefile        |  4 ++--
 arch/x86/um/Makefile         |  6 +++---
 drivers/memory/Makefile      |  4 ++--
 include/linux/kbuild.h       | 20 +++++++++++++++----
 samples/bpf/Makefile         |  4 ++--
 scripts/Makefile.lib         | 38 +++++++++++++++---------------------
 scripts/mod/Makefile         |  4 ++--
 11 files changed, 57 insertions(+), 50 deletions(-)

diff --git a/Kbuild b/Kbuild
index 464b34a08f51e..412b77007deb1 100644
--- a/Kbuild
+++ b/Kbuild
@@ -9,9 +9,9 @@
 
 bounds-file := include/generated/bounds.h
 
-targets := kernel/bounds.s
+targets := kernel/bounds.o
 
-$(bounds-file): kernel/bounds.s FORCE
+$(bounds-file): kernel/bounds.o FORCE
 	$(call filechk,offsets,__LINUX_BOUNDS_H__)
 
 # Generate timeconst.h
@@ -27,11 +27,11 @@ $(timeconst-file): kernel/time/timeconst.bc FORCE
 
 offsets-file := include/generated/asm-offsets.h
 
-targets += arch/$(SRCARCH)/kernel/asm-offsets.s
+targets += arch/$(SRCARCH)/kernel/asm-offsets.o
 
-arch/$(SRCARCH)/kernel/asm-offsets.s: $(timeconst-file) $(bounds-file)
+arch/$(SRCARCH)/kernel/asm-offsets.o: $(timeconst-file) $(bounds-file)
 
-$(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s FORCE
+$(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.o FORCE
 	$(call filechk,offsets,__ASM_OFFSETS_H__)
 
 # Check for missing system calls
diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile
index 794bd12ab0a8e..4d4be0000fd98 100644
--- a/arch/arm/mach-at91/Makefile
+++ b/arch/arm/mach-at91/Makefile
@@ -18,10 +18,10 @@ ifeq ($(CONFIG_PM_DEBUG),y)
 CFLAGS_pm.o += -DDEBUG
 endif
 
-$(obj)/pm_data-offsets.h: $(obj)/pm_data-offsets.s FORCE
+$(obj)/pm_data-offsets.h: $(obj)/pm_data-offsets.o FORCE
 	$(call filechk,offsets,__PM_DATA_OFFSETS_H__)
 
 $(obj)/pm_suspend.o: $(obj)/pm_data-offsets.h
 
-targets += pm_data-offsets.s
+targets += pm_data-offsets.o
 clean-files += pm_data-offsets.h
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index daf21127c82f1..991ffe6871d1d 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -217,12 +217,12 @@ obj-y					+= omap_phy_internal.o
 
 obj-$(CONFIG_MACH_OMAP2_TUSB6010)	+= usb-tusb6010.o
 
-$(obj)/pm-asm-offsets.h: $(obj)/pm-asm-offsets.s FORCE
+$(obj)/pm-asm-offsets.h: $(obj)/pm-asm-offsets.o FORCE
 	$(call filechk,offsets,__TI_PM_ASM_OFFSETS_H__)
 
 $(obj)/sleep33xx.o $(obj)/sleep43xx.o: $(obj)/pm-asm-offsets.h
 
-targets += pm-asm-offsets.s
+targets += pm-asm-offsets.o
 clean-files += pm-asm-offsets.h
 
 obj-$(CONFIG_OMAP_IOMMU)		+= omap-iommu.o
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 86a629aaf0a13..ee699a683d82c 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -28,17 +28,18 @@ kvm-y += arm.o mmu.o mmio.o psci.o hypercalls.o pvtime.o \
 kvm-$(CONFIG_HW_PERF_EVENTS)  += pmu-emul.o pmu.o
 kvm-$(CONFIG_ARM64_PTR_AUTH)  += pauth.o
 
-always-y := hyp_constants.h hyp-constants.s
+always-y := hyp_constants.h hyp-constants.o
 
 define rule_gen_hyp_constants
 	$(call filechk,offsets,__HYP_CONSTANTS_H__)
 endef
 
 CFLAGS_hyp-constants.o = -I $(src)/hyp/include
-$(obj)/hyp-constants.s: $(src)/hyp/hyp-constants.c FORCE
-	$(call if_changed_dep,cc_s_c)
 
-$(obj)/hyp_constants.h: $(obj)/hyp-constants.s FORCE
+$(obj)/hyp-constants.o: $(src)/hyp/hyp-constants.c FORCE
+	$(call if_changed_dep,cc_o_c)
+
+$(obj)/hyp_constants.h: $(obj)/hyp-constants.o FORCE
 	$(call if_changed_rule,gen_hyp_constants)
 
 obj-kvm := $(addprefix $(obj)/, $(kvm-y))
diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index 5494669a055a6..3b561c7e7c4f9 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -42,8 +42,8 @@ $(obj)/svm/vmenter.o: $(obj)/kvm-asm-offsets.h
 AFLAGS_vmx/vmenter.o    := -iquote $(obj)
 $(obj)/vmx/vmenter.o: $(obj)/kvm-asm-offsets.h
 
-$(obj)/kvm-asm-offsets.h: $(obj)/kvm-asm-offsets.s FORCE
+$(obj)/kvm-asm-offsets.h: $(obj)/kvm-asm-offsets.o FORCE
 	$(call filechk,offsets,__KVM_ASM_OFFSETS_H__)
 
-targets += kvm-asm-offsets.s
+targets += kvm-asm-offsets.o
 clean-files += kvm-asm-offsets.h
diff --git a/arch/x86/um/Makefile b/arch/x86/um/Makefile
index 36e67fc97c22f..6563503d4b25d 100644
--- a/arch/x86/um/Makefile
+++ b/arch/x86/um/Makefile
@@ -38,11 +38,11 @@ subarch-$(CONFIG_MODULES) += ../kernel/module.o
 
 USER_OBJS := bugs_$(BITS).o ptrace_user.o fault.o
 
-$(obj)/user-offsets.s: c_flags = -Wp,-MD,$(depfile) $(USER_CFLAGS) \
+$(obj)/user-offsets.o: c_flags = -Wp,-MD,$(depfile) $(USER_CFLAGS) \
 	-Iarch/x86/include/generated
-targets += user-offsets.s
+targets += user-offsets.o
 
-include/generated/user_constants.h: $(obj)/user-offsets.s FORCE
+include/generated/user_constants.h: $(obj)/user-offsets.o FORCE
 	$(call filechk,offsets,__USER_CONSTANT_H__)
 
 UNPROFILE_OBJS := stub_segv.o
diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
index d2e6ca9abbe02..efae95f315a12 100644
--- a/drivers/memory/Makefile
+++ b/drivers/memory/Makefile
@@ -34,8 +34,8 @@ ti-emif-sram-objs		:= ti-emif-pm.o ti-emif-sram-pm.o
 
 $(obj)/ti-emif-sram-pm.o: $(obj)/ti-emif-asm-offsets.h
 
-$(obj)/ti-emif-asm-offsets.h: $(obj)/emif-asm-offsets.s FORCE
+$(obj)/ti-emif-asm-offsets.h: $(obj)/emif-asm-offsets.o FORCE
 	$(call filechk,offsets,__TI_EMIF_ASM_OFFSETS_H__)
 
-targets += emif-asm-offsets.s
+targets += emif-asm-offsets.o
 clean-files += ti-emif-asm-offsets.h
diff --git a/include/linux/kbuild.h b/include/linux/kbuild.h
index e7be517aaaf68..bd7e09a5ebe1f 100644
--- a/include/linux/kbuild.h
+++ b/include/linux/kbuild.h
@@ -2,15 +2,27 @@
 #ifndef __LINUX_KBUILD_H
 #define __LINUX_KBUILD_H
 
-#define DEFINE(sym, val) \
-	asm volatile("\n.ascii \"->" #sym " %0 " #val "\"" : : "i" (val))
+#define _LINE(x, ...) \
+	asm volatile( \
+		".pushsection \".data.kbuild\"; "\
+		".ascii \"" x "\\n\"; "\
+		".popsection" : : __VA_ARGS__)
 
-#define BLANK() asm volatile("\n.ascii \"->\"" : : )
+/*
+ * NOTE: We use %n0 and -(val) to work around a bug in some compilers
+ * that output a leading pound symbol ('#') even when using the %c
+ * modifier.
+ */
+#define DEFINE(sym, val) \
+	_LINE("#define " #sym " %n0 /* " #val " */", "i" (-(unsigned long) (val)))
 
 #define OFFSET(sym, str, mem) \
 	DEFINE(sym, offsetof(struct str, mem))
 
+#define BLANK() \
+	_LINE("")
+
 #define COMMENT(x) \
-	asm volatile("\n.ascii \"->#" x "\"")
+	_LINE("/* " x " */")
 
 #endif
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 3e003dd6bea09..a5d86ac8f5f57 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -270,10 +270,10 @@ $(LIBBPF_OUTPUT) $(BPFTOOL_OUTPUT):
 	$(call msg,MKDIR,$@)
 	$(Q)mkdir -p $@
 
-$(obj)/syscall_nrs.h:	$(obj)/syscall_nrs.s FORCE
+$(obj)/syscall_nrs.h:	$(obj)/syscall_nrs.o FORCE
 	$(call filechk,offsets,__SYSCALL_NRS_H__)
 
-targets += syscall_nrs.s
+targets += syscall_nrs.o
 clean-files += syscall_nrs.h
 
 FORCE:
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 207325eaf1d1c..1ad0599147d0a 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -583,29 +583,23 @@ quiet_cmd_zstd22_with_size = ZSTD22  $@
 # ASM offsets
 # ---------------------------------------------------------------------------
 
-# Default sed regexp - multiline due to syntax constraints
-#
-# Use [:space:] because LLVM's integrated assembler inserts <tab> around
-# the .ascii directive whereas GCC keeps the <space> as-is.
-define sed-offsets
-	's:^[[:space:]]*\.ascii[[:space:]]*"\(.*\)".*:\1:; \
-	/^->/{s:->#\(.*\):/* \1 */:; \
-	s:^->\([^ ]*\) [\$$#]*\([^ ]*\) \(.*\):#define \1 \2 /* \3 */:; \
-	s:->::; p;}'
-endef
-
 # Use filechk to avoid rebuilds when a header changes, but the resulting file
 # does not
+#
+# binutils objcopy doesn't support opening "-" as stdout, so we use /dev/stdout
+# instead. LLVM's objcopy is the opposite; it supports "-" but not /dev/stdout.
+# Opening /dev/stdout will start overwriting at file offset 0; pipe to cat to
+# ensure /dev/stdout refers to a pipe and not the temporary file.
 define filechk_offsets
-	 echo "#ifndef $2"; \
-	 echo "#define $2"; \
-	 echo "/*"; \
-	 echo " * DO NOT MODIFY."; \
-	 echo " *"; \
-	 echo " * This file was generated by Kbuild"; \
-	 echo " */"; \
-	 echo ""; \
-	 sed -ne $(sed-offsets) < $<; \
-	 echo ""; \
-	 echo "#endif"
+	echo "#ifndef $2"; \
+	echo "#define $2"; \
+	echo "/*"; \
+	echo " * DO NOT MODIFY."; \
+	echo " *"; \
+	echo " * This file was generated by Kbuild"; \
+	echo " */"; \
+	echo ""; \
+	$(OBJCOPY) -j .data.kbuild -O binary $< $(if $(LLVM),-,/dev/stdout) | cat; \
+	echo ""; \
+	echo "#endif"
 endef
diff --git a/scripts/mod/Makefile b/scripts/mod/Makefile
index c729bc936bae1..3c3f5e16a30a2 100644
--- a/scripts/mod/Makefile
+++ b/scripts/mod/Makefile
@@ -8,10 +8,10 @@ modpost-objs	:= modpost.o file2alias.o sumversion.o symsearch.o
 
 devicetable-offsets-file := devicetable-offsets.h
 
-$(obj)/$(devicetable-offsets-file): $(obj)/devicetable-offsets.s FORCE
+$(obj)/$(devicetable-offsets-file): $(obj)/devicetable-offsets.o FORCE
 	$(call filechk,offsets,__DEVICETABLE_OFFSETS_H__)
 
-targets += $(devicetable-offsets-file) devicetable-offsets.s
+targets += $(devicetable-offsets-file) devicetable-offsets.o
 
 # dependencies on generated files need to be listed explicitly
 
-- 
2.34.1


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux