Re: [PATCH 00/15] Implement MODVERSIONS for Rust

From: Masahiro Yamada
Date: Tue Jun 18 2024 - 12:29:08 EST

Next message: Paul E. McKenney: "Re: [PATCH v2 16/35] preempt,rcu: warn on PREEMPT_RCU=n, preempt=full"
Previous message: Boqun Feng: "Re: [PATCH v4 2/2] rust: add firmware abstractions"
In reply to: Sami Tolvanen: "[PATCH 08/15] gendwarfksyms: Expand subroutine_type"
Next in thread: Sami Tolvanen: "Re: [PATCH 00/15] Implement MODVERSIONS for Rust"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Tue, Jun 18, 2024 at 2:58 AM Sami Tolvanen <samitolvanen@xxxxxxxxxx> wrote:
>
> Hi folks,
>
> This series implements CONFIG_MODVERSIONS for Rust, an important
> feature for distributions like Android that want to ship Rust
> kernel modules, and depend on modversions to help ensure module ABI
> compatibility.
>
> There have been earlier proposals [1][2] that would allow Rust
> modules to coexist with modversions, but none that actually implement
> symbol versioning. Unlike C, Rust source code doesn't have sufficient
> information about the final ABI, as the compiler has considerable
> freedom in adjusting structure layout for improved performance [3],
> for example, which makes using a source code parser like genksyms
> a non-starter. Based on Matt's suggestion and previous feedback
> from maintainers, this series uses DWARF debugging information for
> computing versions. DWARF is an established and relatively stable
> format, which includes all the necessary ABI details, and adding a
> CONFIG_DEBUG_INFO dependency for Rust symbol versioning seems like a
> reasonable trade-off.
>
> The first 12 patches of this series add a small tool for computing
> symbol versions from DWARF, called gendwarfksyms. When passed a list
> of exported symbols, the tool generates an expanded type string
> for each symbol, and computes symbol CRCs similarly to genksyms.
> gendwarfksyms is written in C and uses libdw to process DWARF, mainly
> because of the existing support for C host tools that use elfutils
> (e.g., objtool).
>
> Another compatibility issue is fitting the extremely long mangled
> Rust symbol names into struct modversion_info, which can only hold
> 55-character names (on 64-bit systems). Previous proposals suggested
> changing the structure to support longer names, but the conclusion was
> that we cannot break userspace tools that parse the version table.
>
> The next two patches of the series implement support for hashed
> symbol names in struct modversion_info, where names longer than 55
> characters are hashed, and the hash is stored in the name field. To
> avoid breaking userspace tools, the binary hash is prefixed with a
> null-terminated string containing the name of the hash function. While
> userspace tools can later be updated to potentially produce more
> useful information about the long symbols, this allows them to
> continue working in the meantime.
>
> The final patch allows CONFIG_MODVERSIONS to be selected with Rust,
> provided that debugging information is also available.

I am surprised at someone who attempts to add another variant of genksyms.

I am also surprised at the tool being added under the tools/ directory.

At least, we can avoid the latter misfortune, though.

A patch attached (on top of your patch set).

Such a tool can be compiled in scripts/, or even better
in rust/Makefile if it is only used in rust/Makefile.

--
Best Regards
Masahiro Yamada
From dcd8a348af34bc2cd89ef62765a716b8d63d5b0d Mon Sep 17 00:00:00 2001
From: Masahiro Yamada <masahiroy@kernel.org>
Date: Tue, 18 Jun 2024 10:58:55 +0900
Subject: [PATCH] kbuild: move tools/gendwarfksyms to scripts/gendwarfksyms

There is no good reason to use the tool's fragile build system.
Move tools/gendwarfksyms/ to scripts/gendwarfksyms/.

Add scripts/gendwarfksyms/.gitignore to ignore gendwarfksyms.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
Makefile | 6 ---
rust/Makefile | 2 +-
scripts/Makefile | 1 +
scripts/gendwarfksyms/.gitignore | 2 +
scripts/gendwarfksyms/Makefile | 10 ++++
{tools => scripts}/gendwarfksyms/cache.c | 0
{tools => scripts}/gendwarfksyms/crc32.c | 0
{tools => scripts}/gendwarfksyms/crc32.h | 0
.../gendwarfksyms/gendwarfksyms.c | 0
.../gendwarfksyms/gendwarfksyms.h | 0
{tools => scripts}/gendwarfksyms/symbols.c | 0
{tools => scripts}/gendwarfksyms/types.c | 0
tools/Makefile | 7 ++-
tools/gendwarfksyms/Build | 5 --
tools/gendwarfksyms/Makefile | 49 -------------------
15 files changed, 17 insertions(+), 65 deletions(-)
create mode 100644 scripts/gendwarfksyms/.gitignore
create mode 100644 scripts/gendwarfksyms/Makefile
rename {tools => scripts}/gendwarfksyms/cache.c (100%)
rename {tools => scripts}/gendwarfksyms/crc32.c (100%)
rename {tools => scripts}/gendwarfksyms/crc32.h (100%)
rename {tools => scripts}/gendwarfksyms/gendwarfksyms.c (100%)
rename {tools => scripts}/gendwarfksyms/gendwarfksyms.h (100%)
rename {tools => scripts}/gendwarfksyms/symbols.c (100%)
rename {tools => scripts}/gendwarfksyms/types.c (100%)
delete mode 100644 tools/gendwarfksyms/Build
delete mode 100644 tools/gendwarfksyms/Makefile

diff --git a/Makefile b/Makefile
index 1499b9352634..925a75b8ba7d 100644
--- a/Makefile
+++ b/Makefile
@@ -1344,12 +1344,6 @@ prepare: tools/bpf/resolve_btfids
endif
endif

-ifdef CONFIG_MODVERSIONS
-ifdef CONFIG_RUST
-prepare: tools/gendwarfksyms
-endif
-endif
-
PHONY += resolve_btfids_clean

resolve_btfids_O = $(abspath $(objtree))/tools/bpf/resolve_btfids
diff --git a/rust/Makefile b/rust/Makefile
index 385cdf5dc320..93bf3cf16d6f 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -426,7 +426,7 @@ ifdef CONFIG_MODVERSIONS
# debugging information. We can't use a source code parser like genksyms,
# because the source files don't have information about the final structure
# layout and emitted symbols.
-gendwarfksyms := $(objtree)/tools/gendwarfksyms/gendwarfksyms
+gendwarfksyms := scripts/gendwarfksyms/gendwarfksyms

cmd_gendwarfksyms = \
$(call rust_exports,$@,"%s %s\n",$$1$(comma)$$3) \
diff --git a/scripts/Makefile b/scripts/Makefile
index fe56eeef09dd..8bed26489dcc 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -55,6 +55,7 @@ targets += module.lds
subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
subdir-$(CONFIG_MODVERSIONS) += genksyms
subdir-$(CONFIG_SECURITY_SELINUX) += selinux
+subdir-$(if $(CONFIG_RUST),$(CONFIG_MODVERSIONS)) += gendwarfksyms

# Let clean descend into subdirs
subdir- += basic dtc gdb kconfig mod
diff --git a/scripts/gendwarfksyms/.gitignore b/scripts/gendwarfksyms/.gitignore
new file mode 100644
index 000000000000..ab8c763b3afe
--- /dev/null
+++ b/scripts/gendwarfksyms/.gitignore
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+/gendwarfksyms
diff --git a/scripts/gendwarfksyms/Makefile b/scripts/gendwarfksyms/Makefile
new file mode 100644
index 000000000000..88039d97bdd8
--- /dev/null
+++ b/scripts/gendwarfksyms/Makefile
@@ -0,0 +1,10 @@
+hostprogs-always-y += gendwarfksyms
+
+gendwarfksyms-objs += gendwarfksyms.o
+gendwarfksyms-objs += cache.o
+gendwarfksyms-objs += crc32.o
+gendwarfksyms-objs += symbols.o
+gendwarfksyms-objs += types.o
+
+HOST_EXTRACFLAGS := -I $(srctree)/tools/include
+HOSTLDLIBS_gendwarfksyms := -ldw -lelf
diff --git a/tools/gendwarfksyms/cache.c b/scripts/gendwarfksyms/cache.c
similarity index 100%
rename from tools/gendwarfksyms/cache.c
rename to scripts/gendwarfksyms/cache.c
diff --git a/tools/gendwarfksyms/crc32.c b/scripts/gendwarfksyms/crc32.c
similarity index 100%
rename from tools/gendwarfksyms/crc32.c
rename to scripts/gendwarfksyms/crc32.c
diff --git a/tools/gendwarfksyms/crc32.h b/scripts/gendwarfksyms/crc32.h
similarity index 100%
rename from tools/gendwarfksyms/crc32.h
rename to scripts/gendwarfksyms/crc32.h
diff --git a/tools/gendwarfksyms/gendwarfksyms.c b/scripts/gendwarfksyms/gendwarfksyms.c
similarity index 100%
rename from tools/gendwarfksyms/gendwarfksyms.c
rename to scripts/gendwarfksyms/gendwarfksyms.c
diff --git a/tools/gendwarfksyms/gendwarfksyms.h b/scripts/gendwarfksyms/gendwarfksyms.h
similarity index 100%
rename from tools/gendwarfksyms/gendwarfksyms.h
rename to scripts/gendwarfksyms/gendwarfksyms.h
diff --git a/tools/gendwarfksyms/symbols.c b/scripts/gendwarfksyms/symbols.c
similarity index 100%
rename from tools/gendwarfksyms/symbols.c
rename to scripts/gendwarfksyms/symbols.c
diff --git a/tools/gendwarfksyms/types.c b/scripts/gendwarfksyms/types.c
similarity index 100%
rename from tools/gendwarfksyms/types.c
rename to scripts/gendwarfksyms/types.c
diff --git a/tools/Makefile b/tools/Makefile
index 60806ff753b7..fb0f84e492cb 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -17,7 +17,6 @@ help:
@echo ' firewire - the userspace part of nosy, an IEEE-1394 traffic sniffer'
@echo ' firmware - Firmware tools'
@echo ' freefall - laptop accelerometer program for disk protection'
- @echo ' gendwarfksyms - generates symbol versions from DWARF'
@echo ' gpio - GPIO tools'
@echo ' hv - tools used when in Hyper-V clients'
@echo ' iio - IIO tools'
@@ -69,7 +68,7 @@ acpi: FORCE
cpupower: FORCE
$(call descend,power/$@)

-counter firewire hv guest bootconfig spi usb virtio mm bpf iio gendwarfksyms gpio objtool leds wmi pci firmware debugging tracing: FORCE
+counter firewire hv guest bootconfig spi usb virtio mm bpf iio gpio objtool leds wmi pci firmware debugging tracing: FORCE
$(call descend,$@)

bpf/%: FORCE
@@ -155,7 +154,7 @@ freefall_install:
kvm_stat_install:
$(call descend,kvm/$(@:_install=),install)

-install: acpi_install counter_install cpupower_install gendwarfksyms_install \
+install: acpi_install counter_install cpupower_install \
gpio_install hv_install firewire_install iio_install \
perf_install selftests_install turbostat_install usb_install \
virtio_install mm_install bpf_install x86_energy_perf_policy_install \
@@ -169,7 +168,7 @@ acpi_clean:
cpupower_clean:
$(call descend,power/cpupower,clean)

-counter_clean hv_clean firewire_clean bootconfig_clean spi_clean usb_clean virtio_clean mm_clean wmi_clean bpf_clean iio_clean gendwarfksyms_clean gpio_clean objtool_clean leds_clean pci_clean firmware_clean debugging_clean tracing_clean:
+counter_clean hv_clean firewire_clean bootconfig_clean spi_clean usb_clean virtio_clean mm_clean wmi_clean bpf_clean iio_clean gpio_clean objtool_clean leds_clean pci_clean firmware_clean debugging_clean tracing_clean:
$(call descend,$(@:_clean=),clean)

libapi_clean:
diff --git a/tools/gendwarfksyms/Build b/tools/gendwarfksyms/Build
deleted file mode 100644
index 220a4aa9b380..000000000000
--- a/tools/gendwarfksyms/Build
+++ /dev/null
@@ -1,5 +0,0 @@
-gendwarfksyms-y += gendwarfksyms.o
-gendwarfksyms-y += cache.o
-gendwarfksyms-y += crc32.o
-gendwarfksyms-y += symbols.o
-gendwarfksyms-y += types.o
diff --git a/tools/gendwarfksyms/Makefile b/tools/gendwarfksyms/Makefile
deleted file mode 100644
index 1138ebd8bd0f..000000000000
--- a/tools/gendwarfksyms/Makefile
+++ /dev/null
@@ -1,49 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-include ../scripts/Makefile.include
-include ../scripts/Makefile.arch
-
-ifeq ($(srctree),)
-srctree := $(patsubst %/,%,$(dir $(CURDIR)))
-srctree := $(patsubst %/,%,$(dir $(srctree)))
-endif
-
-GENDWARFKSYMS := $(OUTPUT)gendwarfksyms
-GENDWARFKSYMS_IN := $(GENDWARFKSYMS)-in.o
-
-LIBDW_FLAGS := $(shell $(HOSTPKG_CONFIG) libdw --cflags 2>/dev/null)
-LIBDW_LIBS := $(shell $(HOSTPKG_CONFIG) libdw --libs 2>/dev/null || echo -ldw -lelf)
-
-all: $(GENDWARFKSYMS)
-
-INCLUDES := -I$(srctree)/tools/include
-
-WARNINGS := -Wall -Wno-unused-value
-GENDWARFKSYMS_CFLAGS := -Werror $(WARNINGS) $(KBUILD_HOSTCFLAGS) -g $(INCLUDES) $(LIBDW_FLAGS)
-GENDWARFKSYMS_LDFLAGS := $(LIBDW_LIBS) $(KBUILD_HOSTLDFLAGS)
-
-# Always want host compilation.
-HOST_OVERRIDES := CC="$(HOSTCC)" LD="$(HOSTLD)" AR="$(HOSTAR)"
-
-ifeq ($(V),1)
- Q =
-else
- Q = @
-endif
-
-export srctree OUTPUT
-include $(srctree)/tools/build/Makefile.include
-
-$(GENDWARFKSYMS_IN): FORCE
- $(Q)$(MAKE) $(build)=gendwarfksyms $(HOST_OVERRIDES) CFLAGS="$(GENDWARFKSYMS_CFLAGS)" \
- LDFLAGS="$(GENDWARFKSYMS_LDFLAGS)"
-
-
-$(GENDWARFKSYMS): $(GENDWARFKSYMS_IN)
- $(QUIET_LINK)$(HOSTCC) $(GENDWARFKSYMS_IN) $(GENDWARFKSYMS_LDFLAGS) -o $@
-
-clean:
- $(call QUIET_CLEAN, gendwarfksyms) $(RM) $(GENDWARFKSYMS)
-
-FORCE:
-
-.PHONY: clean FORCE
--
2.43.0

Next message: Paul E. McKenney: "Re: [PATCH v2 16/35] preempt,rcu: warn on PREEMPT_RCU=n, preempt=full"
Previous message: Boqun Feng: "Re: [PATCH v4 2/2] rust: add firmware abstractions"
In reply to: Sami Tolvanen: "[PATCH 08/15] gendwarfksyms: Expand subroutine_type"
Next in thread: Sami Tolvanen: "Re: [PATCH 00/15] Implement MODVERSIONS for Rust"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]