[PATCH v3 00/15] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS)

[Masahiro Yamada]

This is the third batch of cleanups in this development cycle.

Major changes in v3:

- Generate symbol CRCs as C code, and remove CONFIG_MODULE_REL_CRCS.

Major changes in v2:

- V1 did not work with CONFIG_MODULE_REL_CRCS.
I fixed this for v2.

- Reflect some review comments in v1

- Refactor the code more

- Avoid too long argument error


Masahiro Yamada (15):
modpost: mitigate false-negatives for static EXPORT_SYMBOL checks
modpost: change the license of EXPORT_SYMBOL to bool type
modpost: merge add_{intree_flag,retpoline,staging_flag} to add_header
modpost: move *.mod.c generation to write_mod_c_files()
kbuild: generate a list of objects in vmlinux
kbuild: record symbol versions in *.cmd files
modpost: extract symbol versions from *.cmd files
kbuild: link symbol CRCs at final link, removing
CONFIG_MODULE_REL_CRCS
kbuild: stop merging *.symversions
genksyms: adjust the output format to modpost
kbuild: do not create *.prelink.o for Clang LTO or IBT
modpost: simplify the ->is_static initialization
modpost: use hlist for hash table implementation
kbuild: make built-in.a rule robust against too long argument error
kbuild: make *.mod rule robust against too long argument error

arch/powerpc/Kconfig | 1 -
arch/s390/Kconfig | 1 -
arch/um/Kconfig | 1 -
include/asm-generic/export.h | 22 +-
include/linux/export.h | 30 +--
include/linux/symversion.h | 13 +
init/Kconfig | 4 -
kernel/module.c | 10 +-
scripts/Kbuild.include | 4 +
scripts/Makefile.build | 118 +++------
scripts/Makefile.lib | 7 -
scripts/Makefile.modfinal | 5 +-
scripts/Makefile.modpost | 9 +-
scripts/genksyms/genksyms.c | 18 +-
scripts/link-vmlinux.sh | 46 ++--
scripts/mod/file2alias.c | 2 -
scripts/mod/list.h | 52 ++++
scripts/mod/modpost.c | 449 ++++++++++++++++++++---------------
scripts/mod/modpost.h | 2 +
19 files changed, 402 insertions(+), 392 deletions(-)
create mode 100644 include/linux/symversion.h

--
2.32.0

[Masahiro Yamada]

Now modpost reads symbol versions from .*.cmd files.

These merged *.symversions are not used any more.

Signed-off-by: Masahiro Yamada <masa...@kernel.org>
---

(no changes since v1)

scripts/Makefile.build | 21 ++-------------------
scripts/link-vmlinux.sh | 15 ---------------
2 files changed, 2 insertions(+), 34 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index ddd9080fc028..dff9220135c4 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -390,17 +390,6 @@ $(obj)/%.asn1.c $(obj)/%.asn1.h: $(src)/%.asn1 $(objtree)/scripts/asn1_compiler
$(subdir-builtin): $(obj)/%/built-in.a: $(obj)/% ;
$(subdir-modorder): $(obj)/%/modules.order: $(obj)/% ;

-# combine symversions for later processing
-ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y)
- cmd_update_lto_symversions = \
- rm -f $@.symversions \
- $(foreach n, $(filter-out FORCE,$^), \
- $(if $(shell test -s $(n).symversions && echo y), \
- ; cat $(n).symversions >> $@.symversions))
-else
- cmd_update_lto_symversions = echo >/dev/null
-endif
-
#
# Rule to compile a set of .o files into one .a file (without symbol table)
#
@@ -408,11 +397,8 @@ endif
quiet_cmd_ar_builtin = AR $@
cmd_ar_builtin = rm -f $@; $(AR) cDPrST $@ $(real-prereqs)

-quiet_cmd_ar_and_symver = AR $@
- cmd_ar_and_symver = $(cmd_update_lto_symversions); $(cmd_ar_builtin)
-
$(obj)/built-in.a: $(real-obj-y) FORCE
- $(call if_changed,ar_and_symver)
+ $(call if_changed,ar_builtin)

#
# Rule to create modules.order file
@@ -432,16 +418,13 @@ $(obj)/modules.order: $(obj-m) FORCE
#
# Rule to compile a set of .o files into one .a file (with symbol table)
#
-quiet_cmd_ar_lib = AR $@
- cmd_ar_lib = $(cmd_update_lto_symversions); $(cmd_ar)

$(obj)/lib.a: $(lib-y) FORCE
- $(call if_changed,ar_lib)
+ $(call if_changed,ar)

ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
quiet_cmd_link_multi-m = AR [M] $@
cmd_link_multi-m = \
- $(cmd_update_lto_symversions); \
rm -f $@; \
$(AR) cDPrsT $@ @$(patsubst %.o,%.mod,$@)
else
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 2742b7dd089a..07333181938b 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -56,20 +56,6 @@ gen_initcalls()
> .tmp_initcalls.lds
}

-# If CONFIG_LTO_CLANG is selected, collect generated symbol versions into
-# .tmp_symversions.lds
-gen_symversions()
-{
- info GEN .tmp_symversions.lds
- rm -f .tmp_symversions.lds
-
- for o in ${KBUILD_VMLINUX_OBJS} ${KBUILD_VMLINUX_LIBS}; do
- if [ -f ${o}.symversions ]; then
- cat ${o}.symversions >> .tmp_symversions.lds
- fi
- done
-}
-
# Link of vmlinux.o used for section mismatch analysis
# ${1} output file
modpost_link()
@@ -299,7 +285,6 @@ cleanup()
rm -f .btf.*
rm -f .tmp_System.map
rm -f .tmp_initcalls.lds
- rm -f .tmp_symversions.lds
rm -f .tmp_vmlinux*
rm -f System.map
rm -f vmlinux
--
2.32.0

[Masahiro Yamada]

When CONFIG_MODVERSIONS=y, the output from genksyms is saved in
separate *.symversions files, and will be used much later when
CONFIG_LTO_CLANG=y because it is impossible to update LLVM bit code
here.

This approach is not robust because:

- *.symversions may or may not exist. If *.symversions does not
exist, we never know if it is missing for legitimate reason
(i.e. no EXPORT_SYMBOL) or something bad has happened (for
example, the user accidentally deleted it). Once it occurs,
it is not self-healing because *.symversions is generated
as a side effect.

- stale (i.e. invalid) *.symversions might be picked up if an
object is generated in a non-ordinary way, and corresponding
*.symversions (, which was generated by old builds) just happen
to exist.

A more robust approach is to save symbol versions in *.cmd files
because:

- *.cmd always exists (if the object is generated by if_changed
rule or friends). Even if the user accidentally deletes it,
it will be regenerated in the next build.

- *.cmd is always re-generated when the object is updated. This
avoid stale version information being picked up.

I will remove *.symversions later.

Signed-off-by: Masahiro Yamada <masa...@kernel.org>
Tested-by: Nicolas Schier <nic...@fjasle.eu>
Reviewed-by: Nicolas Schier <nic...@fjasle.eu>
---

(no changes since v2)

Changes in v2:
- Fix CONFIG_MODULE_REL_CRCS=y case

scripts/Makefile.build | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index f6a506318795..a1023868775f 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -171,10 +171,17 @@ ifdef CONFIG_MODVERSIONS

# Generate .o.symversions files for each .o with exported symbols, and link these
# to the kernel and/or modules at the end.
+
+genksyms_format_rel_crc := [^_]*__crc_\([^ ]*\) = \.; LONG(\([^)]*\)).*
+genksyms_format_normal := __crc_\(.*\) = \(.*\);
+genksyms_format := $(if $(CONFIG_MODULE_REL_CRCS),$(genksyms_format_rel_crc),$(genksyms_format_normal))
+
gen_symversions = \
if $(NM) $@ 2>/dev/null | grep -q __ksymtab; then \
$(call cmd_gensymtypes_$(1),$(KBUILD_SYMTYPES),$(@:.o=.symtypes)) \
> $@.symversions; \
+ sed -n 's/$(genksyms_format)/$(pound)SYMVER \1 \2/p' $@.symversions \
+ >> $(dot-target).cmd; \
else \
rm -f $@.symversions; \
fi
--
2.32.0

[Masahiro Yamada]

->is_static is set to true at allocation, then to false if the symbol
comes from the dump file.

It is simpler to use !mod->from_dump as the init value.

Signed-off-by: Masahiro Yamada <masa...@kernel.org>

---

(no changes since v2)

Changes in v2:

- New patch

scripts/mod/modpost.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index e07542a90fc6..4edd5b223f49 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -242,7 +242,7 @@ static struct symbol *alloc_symbol(const char *name)

memset(s, 0, sizeof(*s));
strcpy(s->name, name);
- s->is_static = true;
+
return s;
}

@@ -376,6 +376,7 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,

s = alloc_symbol(name);
s->module = mod;
+ s->is_static = !mod->from_dump;
s->is_gpl_only = gpl_only;
list_add_tail(&s->list, &mod->exported_symbols);
hash_add_symbol(s);
@@ -2523,7 +2524,6 @@ static void read_dump(const char *fname)
mod->from_dump = true;
}
s = sym_add_exported(symname, mod, gpl_only);
- s->is_static = false;
sym_set_crc(s, crc);
sym_update_namespace(symname, namespace);
}
--
2.32.0

[Masahiro Yamada]

Make genksyms output symbol versions in the format modpost expects,
so the 'sed' is unneeded.

This commit makes *.symversions completely unneeded.

I will keep *.symversions in .gitignore and 'make clean' for a while.
Otherwise, some people might be upset with 'git status'.

Signed-off-by: Masahiro Yamada <masa...@kernel.org>
---

(no changes since v2)

Changes in v2:
- New patch

scripts/Makefile.build | 6 ------
scripts/genksyms/genksyms.c | 3 +--
2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index dff9220135c4..461998a2ad2b 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -165,16 +165,10 @@ ifdef CONFIG_MODVERSIONS
# o modpost will extract versions from that file and create *.c files that will
# be compiled and linked to the kernel and/or modules.

-genksyms_format := __crc_\(.*\) = \(.*\);
-

gen_symversions = \
if $(NM) $@ 2>/dev/null | grep -q __ksymtab; then \
$(call cmd_gensymtypes_$(1),$(KBUILD_SYMTYPES),$(@:.o=.symtypes)) \

- > $@.symversions; \
- sed -n 's/$(genksyms_format)/$(pound)SYMVER \1 \2/p' $@.symversions \
>> $(dot-target).cmd; \
- else \
- rm -f $@.symversions; \
fi

cmd_gen_symversions_c = $(call gen_symversions,c)
diff --git a/scripts/genksyms/genksyms.c b/scripts/genksyms/genksyms.c
index 6e6933ae7911..f5dfdb9d80e9 100644
--- a/scripts/genksyms/genksyms.c
+++ b/scripts/genksyms/genksyms.c
@@ -680,8 +680,7 @@ void export_symbol(const char *name)
if (flag_dump_defs)
fputs(">\n", debugfile);

- /* Used as a linker script. */
- printf("__crc_%s = 0x%08lx;\n", name, crc);
+ printf("#SYMVER %s 0x%08lx\n", name, crc);
}
}

--
2.32.0

[Masahiro Yamada]

Like built-in.a, the command length of the *.mod rule scales with
the depth of the directory times the number of objects in the Makefile.

Add $(obj)/ by the shell command (awk) instead of by Make's builtin
function.

In-tree modules still have some room to the limit (ARG_MAX=2097152),
but this is more future-proof for big modules in a deep directory.

For example, you can build i915 as a module (CONFIG_DRM_I915=m) and
compare drivers/gpu/drm/i915/.i915.mod.cmd with/without this commit.

The issue is more critical for external modules because the M= path
can be very long as Jeff Johnson reported before [1].

[1] https://lore.kernel.org/linux-kbuild/4c02050c4e95e4cb...@codeaurora.org/

Signed-off-by: Masahiro Yamada <masa...@kernel.org>
---

(no changes since v2)

Changes in v2:
- New patch

scripts/Makefile.build | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index cea48762299c..e7b3f329d443 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -266,8 +266,8 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
$(call if_changed_rule,cc_o_c)
$(call cmd,force_checksrc)

-cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) | \
- $(AWK) -v RS='( |\n)' '!x[$$0]++' > $@
+cmd_mod = echo $(call real-search, $*.o, .o, -objs -y -m) | \
+ $(AWK) -v RS='( |\n)' '!x[$$0]++ { print("$(obj)/"$$0) }' > $@

$(obj)/%.mod: FORCE
$(call if_changed,mod)
--
2.32.0

[Masahiro Yamada]

Kbuild runs at the top of objtree instead of changing the working
directory to subdirectories. I think this design is nice overall but
some commands have a scapability issue.

The build command of built-in.a is one of them whose length scales with:

O(D * N)

Here, D is the length of the directory path (i.e. $(obj)/ prefix),
N is the number of objects in the Makefile, O() is the big O notation.

The deeper directory the Makefile directory is located, the more easily
it will hit the too long argument error.

We can make it better. Trim the $(obj)/ by Make's builtin function, and
restore it by a shell command (sed).

With this, the command length scales with:

O(D + N)

In-tree modules still have some room to the limit (ARG_MAX=2097152),
but this is more future-proof for big modules in a deep directory.

For example, you can build i915 as builtin (CONFIG_DRM_I915=y) and
compare drivers/gpu/drm/i915/.built-in.a.cmd with/without this commit.

Signed-off-by: Masahiro Yamada <masa...@kernel.org>
---

(no changes since v2)

Changes in v2:
- New patch

scripts/Makefile.build | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 0436ff94800e..cea48762299c 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -370,7 +370,10 @@ $(subdir-modorder): $(obj)/%/modules.order: $(obj)/% ;
#

quiet_cmd_ar_builtin = AR $@
- cmd_ar_builtin = rm -f $@; $(AR) cDPrST $@ $(real-prereqs)
+ cmd_ar_builtin = rm -f $@; \
+ echo $(patsubst $(obj)/%,%,$(real-prereqs)) | \
+ sed -E 's:([^ ]+):$(obj)/\1:g' | \
+ xargs $(AR) cDPrST $@

$(obj)/built-in.a: $(real-obj-y) FORCE
$(call if_changed,ar_builtin)
--
2.32.0

[Masahiro Yamada]

Import hlist macros from include/linux/list.h to implement the hash
table in a more generic way.

While I was here, I increased the hash table size from 1024 to 8192
to decrease the hash collision.

I moved ARRAY_SIZE() from file2alias.c to modpost.h because it is needed
in modpost.c as well.

Signed-off-by: Masahiro Yamada <masa...@kernel.org>
---

(no changes since v2)

Changes in v2:

- Move to the end of the series because this is now optional

scripts/mod/file2alias.c | 2 --
scripts/mod/list.h | 52 ++++++++++++++++++++++++++++++++++++++++
scripts/mod/modpost.c | 39 +++++++++++++++---------------
scripts/mod/modpost.h | 2 ++
4 files changed, 73 insertions(+), 22 deletions(-)

diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 5258247d78ac..e8a9c6816fec 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -734,8 +734,6 @@ static int do_vio_entry(const char *filename, void *symval,
return 1;
}

-#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
-
static void do_input(char *alias,
kernel_ulong_t *arr, unsigned int min, unsigned int max)
{
diff --git a/scripts/mod/list.h b/scripts/mod/list.h
index a924a6c4aa4d..c60dbaa70d6b 100644
--- a/scripts/mod/list.h
+++ b/scripts/mod/list.h
@@ -210,4 +210,56 @@ static inline int list_empty(const struct list_head *head)
!list_entry_is_head(pos, head, member); \
pos = n, n = list_next_entry(n, member))

+/*
+ * Double linked lists with a single pointer list head.
+ * Mostly useful for hash tables where the two pointer list head is
+ * too wasteful.
+ * You lose the ability to access the tail in O(1).
+ */
+
+struct hlist_head {
+ struct hlist_node *first;
+};
+
+struct hlist_node {
+ struct hlist_node *next, **pprev;
+};
+
+/**
+ * hlist_add_head - add a new entry at the beginning of the hlist
+ * @n: new entry to be added
+ * @h: hlist head to add it after
+ *
+ * Insert a new entry after the specified head.
+ * This is good for implementing stacks.
+ */
+static inline void hlist_add_head(struct hlist_node *n, struct hlist_head *h)
+{
+ struct hlist_node *first = h->first;
+
+ n->next = first;
+ if (first)
+ first->pprev = &n->next;
+ h->first = n;
+ n->pprev = &h->first;
+}
+
+#define hlist_entry(ptr, type, member) container_of(ptr, type, member)
+
+#define hlist_entry_safe(ptr, type, member) \
+ ({ typeof(ptr) ____ptr = (ptr); \
+ ____ptr ? hlist_entry(____ptr, type, member) : NULL; \
+ })
+
+/**
+ * hlist_for_each_entry - iterate over list of given type
+ * @pos: the type * to use as a loop cursor.
+ * @head: the head for your list.
+ * @member: the name of the hlist_node within the struct.
+ */
+#define hlist_for_each_entry(pos, head, member) \
+ for (pos = hlist_entry_safe((head)->first, typeof(*(pos)), member);\
+ pos; \
+ pos = hlist_entry_safe((pos)->member.next, typeof(*(pos)), member))
+
#endif /* LIST_H */
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 4edd5b223f49..7f7e0818940f 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -199,13 +199,8 @@ static struct module *new_module(const char *modname)
return mod;
}

-/* A hash of all exported symbols,
- * struct symbol is also used for lists of unresolved symbols */
-
-#define SYMBOL_HASH_SIZE 1024
-
struct symbol {
- struct symbol *next;
+ struct hlist_node hash_node; /* link to the hash table */
struct list_head list; /* link to module::exported_symbols or module::unresolved_symbols */
struct module *module;
char *namespace;
@@ -217,8 +212,6 @@ struct symbol {
char name[];
};

-static struct symbol *symbolhash[SYMBOL_HASH_SIZE];
-
/* This is based on the hash algorithm from gdbm, via tdb */
static inline unsigned int tdb_hash(const char *name)
{
@@ -232,6 +225,21 @@ static inline unsigned int tdb_hash(const char *name)
return (1103515243 * value + 12345);
}

+/* useful hash macros */
+#define hash_head(table, key) (&(table)[tdb_hash(key) % ARRAY_SIZE(table)])
+
+#define hash_add_symbol(sym, table) hlist_add_head(&(sym)->hash_node, \
+ hash_head(table, (sym)->name))
+
+#define hash_for_matched_symbol(sym, table, key) \
+ hlist_for_each_entry(sym, hash_head(table, key), hash_node) \
+ if (!strcmp(sym->name, key))
+
+#define HASHTABLE_DECLARE(name, size) struct hlist_head name[size]
+
+/* hash table of all exported symbols */
+HASHTABLE_DECLARE(exported_symbols, 8192);
+
/**
* Allocate a new symbols for use in the hash of exported symbols or
* the list of unresolved symbols per module
@@ -246,15 +254,6 @@ static struct symbol *alloc_symbol(const char *name)
return s;
}

-/* For the hash of exported symbols */
-static void hash_add_symbol(struct symbol *sym)
-{
- unsigned int hash;
-
- hash = tdb_hash(sym->name) % SYMBOL_HASH_SIZE;
- sym->next = symbolhash[hash];
- symbolhash[hash] = sym;
-}

static void sym_add_unresolved(const char *name, struct module *mod, bool weak)
{
@@ -274,8 +273,8 @@ static struct symbol *sym_find_with_module(const char *name, struct module *mod)
if (name[0] == '.')
name++;

- for (s = symbolhash[tdb_hash(name) % SYMBOL_HASH_SIZE]; s; s = s->next) {
- if (strcmp(s->name, name) == 0 && (!mod || s->module == mod))
+ hash_for_matched_symbol(s, exported_symbols, name) {
+ if (!mod || s->module == mod)
return s;
}
return NULL;
@@ -379,7 +378,7 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,

s->is_static = !mod->from_dump;
s->is_gpl_only = gpl_only;
list_add_tail(&s->list, &mod->exported_symbols);

- hash_add_symbol(s);
+ hash_add_symbol(s, exported_symbols);

return s;
}
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 2e8c897e0953..0cd8eec6f59b 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -14,6 +14,8 @@
#include "list.h"
#include "elfconfig.h"

+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
+
/* On BSD-alike OSes elf.h defines these according to host's word size */
#undef ELF_ST_BIND
#undef ELF_ST_TYPE
--
2.32.0

[Masahiro Yamada]

When CONFIG_LTO_CLANG=y, additional intermediate *.prelink.o is created
for each module. Also, objtool is postponed until LLVM bitcode is
converted to ELF.

CONFIG_X86_KERNEL_IBT works in a similar way to postpone objtool until
objects are merged together.

This commit stops generating *.prelink.o, so the build flow will look
the same with/without LTO.

The following figures show how the LTO build currently works, and
how this commit is changing it.

Current build flow
==================

[1] single-object module

[+objtool]
$(CC) $(LD) $(LD)
foo.c --------------------> foo.o -----> foo.prelink.o -----> foo.ko
(LLVM bitcode) (ELF) |
|
foo.mod.o --/

[2] multi-object module
[+objtool]
$(CC) $(AR) $(LD) $(LD)
foo1.c -----> foo1.o -----> foo.o -----> foo.prelink.o -----> foo.ko
| (archive) (ELF) |
foo2.c -----> foo2.o --/ |
(LLVM bitcode) foo.mod.o --/

One confusion is foo.o in multi-object module is an archive despite of
its suffix.

New build flow
==============

[1] single-object module

Since there is only one object, we do not need to have the LLVM
bitcode stage. Use $(CC)+$(LD) to generate an ELF object in one
build rule. When LTO is disabled, $(LD) is unneeded because $(CC)
produces an ELF object.

$(CC)+$(LD)[+objtool] $(LD)
foo.c ------------------------> foo.o -------> foo.ko
(ELF) |
|
foo.mod.o --/

[2] multi-object module

Previously, $(AR) was used to combine LLVM bitcode into an archive,
but there was no technical reason to do so.
This commit just uses $(LD) to combine and convert them into a single
ELF object.

[+objtool]
$(CC) $(LD) $(LD)
foo1.c -------> foo1.o -------> foo.o -------> foo.ko
| (ELF) |
foo2.c -------> foo2.o ---/ |
(LLVM bitcode) foo.mod.o --/

Signed-off-by: Masahiro Yamada <masa...@kernel.org>
---

(no changes since v2)

Changes in v2:

- replace the chain of $(if ...) with $(and )

scripts/Kbuild.include | 4 +++
scripts/Makefile.build | 58 ++++++++++++---------------------------
scripts/Makefile.lib | 7 -----
scripts/Makefile.modfinal | 5 ++--
scripts/Makefile.modpost | 9 ++----
scripts/mod/modpost.c | 7 -----
6 files changed, 25 insertions(+), 65 deletions(-)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 3514c2149e9d..455a0a6ce12d 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -15,6 +15,10 @@ pound := \#
# Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
dot-target = $(dir $@).$(notdir $@)

+###
+# Name of target with a '.tmp_' as filename prefix. foo/bar.o => foo/.tmp_bar.o
+tmp-target = $(dir $@).tmp_$(notdir $@)
+
###
# The temporary file to save gcc -MMD generated dependencies must not
# contain a comma
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 461998a2ad2b..0436ff94800e 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -88,10 +88,6 @@ endif
targets-for-modules := $(foreach x, o mod $(if $(CONFIG_TRIM_UNUSED_KSYMS), usyms), \
$(patsubst %.o, %.$x, $(filter %.o, $(obj-m))))

-ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
-targets-for-modules += $(patsubst %.o, %.prelink.o, $(filter %.o, $(obj-m)))
-endif
-
ifdef need-modorder
targets-for-modules += $(obj)/modules.order
endif
@@ -152,8 +148,16 @@ $(obj)/%.ll: $(src)/%.c FORCE
# The C file is compiled and updated dependency information is generated.
# (See cmd_cc_o_c + relevant part of rule_cc_o_c)

+is-single-obj-m = $(and $(part-of-module),$(filter $@, $(obj-m)),y)
+
+ifdef CONFIG_LTO_CLANG
+cmd_ld_single = $(if $(is-single-obj-m), ; $(LD) $(ld_flags) -r -o $(tmp-target) $@; mv $(tmp-target) $@)
+endif
+
quiet_cmd_cc_o_c = CC $(quiet_modtag) $@
- cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $< $(cmd_objtool)
+ cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $< \
+ $(cmd_ld_single) \
+ $(cmd_objtool)

ifdef CONFIG_MODVERSIONS
# When module versioning is enabled the following steps are executed:
@@ -224,21 +228,16 @@ cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(o

endif # CONFIG_STACK_VALIDATION

-ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
-
-# Skip objtool for LLVM bitcode
-$(obj)/%.o: objtool-enabled :=
-
-else

# 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
# 'OBJECT_FILES_NON_STANDARD_foo.o := 'y': skip objtool checking for a file
# 'OBJECT_FILES_NON_STANDARD_foo.o := 'n': override directory skip for a file

-$(obj)/%.o: objtool-enabled = $(if $(filter-out y%, \
- $(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n),y)
+is-standard-object = $(if $(filter-out y%, $(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n),y)

-endif
+delay-objtool := $(or $(CONFIG_LTO_CLANG),$(CONFIG_X86_KERNEL_IBT))
+
+$(obj)/%.o: objtool-enabled = $(if $(is-standard-object),$(if $(delay-objtool),$(is-single-obj-m),y))

ifdef CONFIG_TRIM_UNUSED_KSYMS
cmd_gen_ksymdeps = \
@@ -267,24 +266,6 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
$(call if_changed_rule,cc_o_c)
$(call cmd,force_checksrc)

-ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
-# Module .o files may contain LLVM bitcode, compile them into native code
-# before ELF processing
-quiet_cmd_cc_prelink_modules = LD [M] $@
- cmd_cc_prelink_modules = \
- $(LD) $(ld_flags) -r -o $@ \
- --whole-archive $(filter-out FORCE,$^) \
- $(cmd_objtool)
-
-# objtool was skipped for LLVM bitcode, run it now that we have compiled
-# modules into native code
-$(obj)/%.prelink.o: objtool-enabled = y
-$(obj)/%.prelink.o: part-of-module := y
-
-$(obj)/%.prelink.o: $(obj)/%.o FORCE
- $(call if_changed,cc_prelink_modules)
-endif
-

cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) | \

$(AWK) -v RS='( |\n)' '!x[$$0]++' > $@

@@ -294,7 +275,7 @@ $(obj)/%.mod: FORCE
# List module undefined symbols
cmd_undefined_syms = $(NM) $< | sed -n 's/^ *U //p' > $@

-$(obj)/%.usyms: $(obj)/%$(mod-prelink-ext).o FORCE
+$(obj)/%.usyms: $(obj)/%.o FORCE
$(call if_changed,undefined_syms)

quiet_cmd_cc_lst_c = MKLST $@
@@ -416,16 +397,11 @@ $(obj)/modules.order: $(obj-m) FORCE
$(obj)/lib.a: $(lib-y) FORCE
$(call if_changed,ar)

-ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
-quiet_cmd_link_multi-m = AR [M] $@
-cmd_link_multi-m = \
- rm -f $@; \
- $(AR) cDPrsT $@ @$(patsubst %.o,%.mod,$@)
-else
quiet_cmd_link_multi-m = LD [M] $@
- cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ @$(patsubst %.o,%.mod,$@)
-endif
+ cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ @$(patsubst %.o,%.mod,$@) $(cmd_objtool)

+$(multi-obj-m): objtool-enabled := $(delay-objtool)
+$(multi-obj-m): part-of-module := y
$(multi-obj-m): %.o: %.mod FORCE
$(call if_changed,link_multi-m)
$(call multi_depend, $(multi-obj-m), .o, -objs -y -m)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 0453a1904646..f75138385449 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -225,13 +225,6 @@ dtc_cpp_flags = -Wp,-MMD,$(depfile).pre.tmp -nostdinc \
$(addprefix -I,$(DTC_INCLUDE)) \
-undef -D__DTS__

-ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
-# With CONFIG_LTO_CLANG, .o files in modules might be LLVM bitcode, so we
-# need to run LTO to compile them into native code (.lto.o) before further
-# processing.
-mod-prelink-ext := .prelink
-endif
-
# Useful for describing the dependency of composite objects
# Usage:
# $(call multi_depend, multi_used_targets, suffix_to_remove, suffix_to_add)
diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index 7f39599e9fae..35100e981f4a 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -9,7 +9,7 @@ __modfinal:
include include/config/auto.conf
include $(srctree)/scripts/Kbuild.include

-# for c_flags and mod-prelink-ext
+# for c_flags
include $(srctree)/scripts/Makefile.lib

# find all modules listed in modules.order
@@ -54,9 +54,8 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \
$(cmd); \
printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)

-
# Re-generate module BTFs if either module's .ko or vmlinux changed
-$(modules): %.ko: %$(mod-prelink-ext).o %.mod.o scripts/module.lds $(if $(KBUILD_BUILTIN),vmlinux) FORCE
+$(modules): %.ko: %.o %.mod.o scripts/module.lds $(if $(KBUILD_BUILTIN),vmlinux) FORCE
+$(call if_changed_except,ld_ko_o,vmlinux)
ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+$(if $(newer-prereqs),$(call cmd,btf_ko))
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 48585c4d04ad..f2ce411acd59 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -41,9 +41,6 @@ __modpost:
include include/config/auto.conf
include $(srctree)/scripts/Kbuild.include

-# for mod-prelink-ext
-include $(srctree)/scripts/Makefile.lib
-
MODPOST = scripts/mod/modpost \
$(if $(CONFIG_MODVERSIONS),-m) \
$(if $(CONFIG_MODULE_SRCVERSION_ALL),-a) \
@@ -118,8 +115,6 @@ $(input-symdump):
@echo >&2 ' Modules may not have dependencies or modversions.'
@echo >&2 ' You may get many unresolved symbol warnings.'

-modules := $(sort $(shell cat $(MODORDER)))
-
# KBUILD_MODPOST_WARN can be set to avoid error out in case of undefined symbols
ifneq ($(KBUILD_MODPOST_WARN)$(filter-out $(existing-input-symdump), $(input-symdump)),)
MODPOST += -w
@@ -128,9 +123,9 @@ endif
# Read out modules.order to pass in modpost.
# Otherwise, allmodconfig would fail with "Argument list too long".
quiet_cmd_modpost = MODPOST $@
- cmd_modpost = sed 's/\.ko$$/$(mod-prelink-ext)\.o/' $< | $(MODPOST) -T -
+ cmd_modpost = sed 's/ko$$/o/' $< | $(MODPOST) -T -

-$(output-symdump): $(MODORDER) $(input-symdump) $(modules:.ko=$(mod-prelink-ext).o) FORCE
+$(output-symdump): $(MODORDER) $(input-symdump) FORCE
$(call if_changed,modpost)

targets += $(output-symdump)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index be7d8adc5d31..e07542a90fc6 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1911,10 +1911,6 @@ static char *remove_dot(char *s)
size_t m = strspn(s + n + 1, "0123456789");
if (m && (s[n + m] == '.' || s[n + m] == 0))
s[n] = 0;
-
- /* strip trailing .prelink */
- if (strends(s, ".prelink"))
- s[strlen(s) - 8] = '\0';
}
return s;
}
@@ -2034,9 +2030,6 @@ static void read_symbols(const char *modname)
/* strip trailing .o */
tmp = NOFAIL(strdup(modname));
tmp[strlen(tmp) - 2] = '\0';
- /* strip trailing .prelink */
- if (strends(tmp, ".prelink"))
- tmp[strlen(tmp) - 8] = '\0';
mod = new_module(tmp);
free(tmp);
}
--
2.32.0

[Masahiro Yamada]

Currently, CONFIG_MODVERSIONS needs extra link to embed the symbol
versions into ELF objects. Then, modpost extracts the version CRCs
from them.

The following figures show how it currently works, and how I am trying
to change it.

Current implementation
======================
|----------|
embed CRC -------------------------->| final |
$(CC) $(LD) / |---------| | link for |
*.c ------> *.o -------> *.o -->| modpost | | vmlinux |
/ | |-- *.mod.c -->| or |
genksyms / |---------| | module |
*.c ------> *.symversions |----------|

Genksyms outputs the calculated CRCs in the form of linker script
(*.symversions), which is used by $(LD) to update the object.

If CONFIG_LTO_CLANG=y, the build process becomes much more complex.
Embedding the CRCs is postponed until the LLVM bitcode is converted
into ELF, creating another intermediate *.prelink.o.

However, this complexity is unneeded. There is no reason why we must
embed version CRCs in objects so early.

There is final link stage for vmlinux (scripts/link-vmlinux.sh) and
modules (scripts/Makefile.modfinal). We can link CRCs at the very last
moment.

New implementation
==================
|----------|
--------------------------------------->| final |
$(CC) / |---------| | link for |
*.c ------> *.o ---->| | | vmlinux |
| modpost |--- .vmlinux-symver.c -->| or |
genksyms | |--- *.mod.c ------------>| module |
*.c ------> *.cmd -->|---------| |----------|

Pass the symbol versions to modpost as separate text data, which are
available in *.cmd files.

This commit changes modpost to extract CRCs from *.cmd files instead of
from ELF objects.

Signed-off-by: Masahiro Yamada <masa...@kernel.org>
---

(no changes since v2)

Changes in v2:

- Simplify the implementation (parse .cmd files after ELF)

scripts/mod/modpost.c | 177 ++++++++++++++++++++++++++++++------------
1 file changed, 129 insertions(+), 48 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 78a7107fcc40..92ee1f454e29 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -383,19 +383,10 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,
return s;
}

-static void sym_set_crc(const char *name, unsigned int crc)
+static void sym_set_crc(struct symbol *sym, unsigned int crc)
{
- struct symbol *s = find_symbol(name);
-
- /*
- * Ignore stand-alone __crc_*, which might be auto-generated symbols
- * such as __*_veneer in ARM ELF.
- */
- if (!s)
- return;
-
- s->crc = crc;
- s->crc_valid = true;
+ sym->crc = crc;
+ sym->crc_valid = true;
}

static void *grab_file(const char *filename, size_t *size)
@@ -618,33 +609,6 @@ static int ignore_undef_symbol(struct elf_info *info, const char *symname)
return 0;
}

-static void handle_modversion(const struct module *mod,
- const struct elf_info *info,
- const Elf_Sym *sym, const char *symname)
-{
- unsigned int crc;
-
- if (sym->st_shndx == SHN_UNDEF) {
- warn("EXPORT symbol \"%s\" [%s%s] version generation failed, symbol will not be versioned.\n"
- "Is \"%s\" prototyped in <asm/asm-prototypes.h>?\n",
- symname, mod->name, mod->is_vmlinux ? "" : ".ko",
- symname);
-
- return;
- }
-
- if (sym->st_shndx == SHN_ABS) {
- crc = sym->st_value;
- } else {
- unsigned int *crcp;
-
- /* symbol points to the CRC in the ELF object */
- crcp = sym_get_data(info, sym);
- crc = TO_NATIVE(*crcp);
- }
- sym_set_crc(symname, crc);
-}
-
static void handle_symbol(struct module *mod, struct elf_info *info,
const Elf_Sym *sym, const char *symname)
{
@@ -1955,6 +1919,102 @@ static char *remove_dot(char *s)
return s;
}

+/*
+ * The CRCs are recorded in .*.cmd files in the form of:
+ * #SYMVER <name> <crc>
+ */
+static void extract_crcs_for_object(const char *object, struct module *mod)
+{
+ char cmd_file[PATH_MAX];
+ char *buf, *p;
+ const char *base;
+ int dirlen, ret;
+
+ base = strrchr(object, '/');
+ if (base) {
+ base++;
+ dirlen = base - object;
+ } else {
+ dirlen = 0;
+ base = object;
+ }
+
+ ret = snprintf(cmd_file, sizeof(cmd_file), "%.*s.%s.cmd",
+ dirlen, object, base);
+ if (ret >= sizeof(cmd_file)) {
+ error("%s: too long path was truncated\n", cmd_file);
+ return;
+ }
+
+ buf = read_text_file(cmd_file);
+ p = buf;
+
+ while ((p = strstr(p, "\n#SYMVER "))) {
+ char *name;
+ size_t namelen;
+ unsigned int crc;
+ struct symbol *sym;
+
+ name = p + strlen("\n#SYMVER ");
+
+ p = strchr(name, ' ');
+ if (!p)
+ break;
+
+ namelen = p - name;
+ p++;
+
+ if (!isdigit(*p))
+ continue; /* skip this line */
+
+ crc = strtol(p, &p, 0);
+ if (*p != '\n')
+ continue; /* skip this line */
+
+ name[namelen] = '\0';
+
+ sym = sym_find_with_module(name, mod);
+ if (!sym) {
+ warn("Skip the version for unexported symbol \"%s\" [%s%s]\n",
+ name, mod->name, mod->is_vmlinux ? "" : ".ko");
+ continue;
+ }
+ sym_set_crc(sym, crc);
+ }
+
+ free(buf);
+}
+
+/*
+ * The symbol versions (CRC) are recorded in the .*.cmd files.
+ * Parse them to retrieve CRCs for the current module.
+ */
+static void mod_set_crcs(struct module *mod)
+{
+ char objlist[PATH_MAX];
+ char *buf, *p, *obj;
+ int ret;
+
+ if (mod->is_vmlinux) {
+ strcpy(objlist, ".vmlinux.objs");
+ } else {
+ /* objects for a module are listed in the *.mod file. */
+ ret = snprintf(objlist, sizeof(objlist), "%s.mod", mod->name);
+ if (ret >= sizeof(objlist)) {
+ error("%s: too long path was truncated\n", objlist);
+ return;
+ }
+ }
+
+ buf = read_text_file(objlist);
+ p = buf;
+
+ while ((obj = strsep(&p, "\n")) && obj[0])
+ extract_crcs_for_object(obj, mod);
+
+ free(buf);
+}
+

static void read_symbols(const char *modname)

{
const char *symname;
@@ -2015,9 +2075,6 @@ static void read_symbols(const char *modname)
if (strstarts(symname, "__kstrtabns_"))
sym_update_namespace(symname + strlen("__kstrtabns_"),
sym_get_data(&info, sym));
- if (strstarts(symname, "__crc_"))
- handle_modversion(mod, &info, sym,
- symname + strlen("__crc_"));
}

// check for static EXPORT_SYMBOL_* functions && global vars
@@ -2046,12 +2103,17 @@ static void read_symbols(const char *modname)

parse_elf_finish(&info);

- /* Our trick to get versioning for module struct etc. - it's
- * never passed as an argument to an exported function, so
- * the automatic versioning doesn't pick it up, but it's really
- * important anyhow */
- if (modversions)
+ if (modversions) {
+ /*
+ * Our trick to get versioning for module struct etc. - it's
+ * never passed as an argument to an exported function, so
+ * the automatic versioning doesn't pick it up, but it's really
+ * important anyhow
+ */
sym_add_unresolved("module_layout", mod, false);
+
+ mod_set_crcs(mod);
+ }
}

static void read_symbols_from_files(const char *filename)
@@ -2214,6 +2276,23 @@ static void add_header(struct buffer *b, struct module *mod)
buf_printf(b, "\nMODULE_INFO(staging, \"Y\");\n");
}

+static void check_symversions(struct module *mod)
+{
+ struct symbol *sym;
+
+ if (!modversions)
+ return;
+
+ list_for_each_entry(sym, &mod->exported_symbols, list) {
+ if (!sym->crc_valid) {
+ warn("EXPORT symbol \"%s\" [%s%s] version generation failed, symbol will not be versioned.\n"
+ "Is \"%s\" prototyped in <asm/asm-prototypes.h>?\n",
+ sym->name, mod->name, mod->is_vmlinux ? "" : ".ko",
+ sym->name);
+ }
+ }
+}
+
/**
* Record CRCs for unresolved symbols
**/
@@ -2429,7 +2508,7 @@ static void read_dump(const char *fname)

}
s = sym_add_exported(symname, mod, gpl_only);

s->is_static = false;
- sym_set_crc(symname, crc);
+ sym_set_crc(s, crc);
sym_update_namespace(symname, namespace);
}
free(buf);
@@ -2554,6 +2633,8 @@ int main(int argc, char **argv)
if (mod->from_dump)
continue;

+ check_symversions(mod);
+
if (!mod->is_vmlinux)
write_mod_c_file(mod);
}
--
2.32.0

[Masahiro Yamada]

include/{linux,asm-generic}/export.h defines a weak symbol, __crc_*
as a placeholder.

Genksyms writes the version CRCs into the linker script, which will be
used for filling the __crc_* symbols. The linker script format depends
on CONFIG_MODULE_REL_CRCS. If it is enabled, __crc_* holds the offset
to the reference of CRC.

We are ready to get rid of this complexity.

Now that modpost parses text files (.*.cmd) to collect all the CRCs,
it can generate C code that will be linked to the vmlinux or modules.

Generate a new C file, .vmlinux-symver.c, which contains the CRCs of
symbols exported by vmlinux. It is compiled and linked to vmlinux in
scripts/link-vmlinux.sh.

Put the CRCs of symbols exported by modules into the existing *.mod.c
files. No additional build step is needed for modules. As usual,
*.mod.c are compiled and linked to *.ko in scripts/Makefile.modfinal.

Please note we no longer use the linker magic. The new C implementation
works in the same way, whether CONFIG_RELOCATABLE is enabled or not.
CONFIG_MODULE_REL_CRCS is no longer needed.

Previously, Kbuild invoked additional $(LD) to update the CRCs in
objects, but this step is unneeded too.

Signed-off-by: Masahiro Yamada <masa...@kernel.org>
---

Changes in v3:
- New patch

arch/powerpc/Kconfig | 1 -
arch/s390/Kconfig | 1 -
arch/um/Kconfig | 1 -

include/asm-generic/export.h | 22 ++++++++--------------
include/linux/export.h | 30 ++++++++----------------------
include/linux/symversion.h | 13 +++++++++++++
init/Kconfig | 4 ----
kernel/module.c | 10 +---------
scripts/Makefile.build | 27 ++++-----------------------
scripts/genksyms/genksyms.c | 17 ++++-------------
scripts/link-vmlinux.sh | 20 +++++++++++++-------
scripts/mod/modpost.c | 31 +++++++++++++++++++++++++++----
12 files changed, 78 insertions(+), 99 deletions(-)
create mode 100644 include/linux/symversion.h

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 174edabb74fa..a4e8dd889e29 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -566,7 +566,6 @@ config RELOCATABLE
bool "Build a relocatable kernel"
depends on PPC64 || (FLATMEM && (44x || FSL_BOOKE))
select NONSTATIC_KERNEL
- select MODULE_REL_CRCS if MODVERSIONS
help
This builds a kernel image that is capable of running at the
location the kernel is loaded at. For ppc32, there is no any
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 77b5a03de13a..aa5848004c76 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -567,7 +567,6 @@ endchoice

config RELOCATABLE
bool "Build a relocatable kernel"
- select MODULE_REL_CRCS if MODVERSIONS
default y
help
This builds a kernel image that retains relocation information
diff --git a/arch/um/Kconfig b/arch/um/Kconfig
index 4d398b80aea8..e8983d098e73 100644
--- a/arch/um/Kconfig
+++ b/arch/um/Kconfig
@@ -106,7 +106,6 @@ config LD_SCRIPT_DYN
bool
default y
depends on !LD_SCRIPT_STATIC
- select MODULE_REL_CRCS if MODVERSIONS

config LD_SCRIPT_DYN_RPATH
bool "set rpath in the binary" if EXPERT
diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
index 07a36a874dca..51ce72ce80fa 100644
--- a/include/asm-generic/export.h
+++ b/include/asm-generic/export.h
@@ -2,6 +2,14 @@
#ifndef __ASM_GENERIC_EXPORT_H
#define __ASM_GENERIC_EXPORT_H

+/*
+ * This comment block is used by fixdep. Please do not remove.
+ *
+ * When CONFIG_MODVERSIONS is changed from n to y, all source files having
+ * EXPORT_SYMBOL variants must be re-compiled because genksyms is run as a
+ * side effect of the .o build rule.
+ */
+
#ifndef KSYM_FUNC
#define KSYM_FUNC(x) x
#endif
@@ -12,9 +20,6 @@
#else
#define KSYM_ALIGN 4
#endif
-#ifndef KCRC_ALIGN
-#define KCRC_ALIGN 4
-#endif

.macro __put, val, name
#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
@@ -43,17 +48,6 @@ __ksymtab_\name:
__kstrtab_\name:
.asciz "\name"
.previous
-#ifdef CONFIG_MODVERSIONS
- .section ___kcrctab\sec+\name,"a"
- .balign KCRC_ALIGN
-#if defined(CONFIG_MODULE_REL_CRCS)
- .long __crc_\name - .
-#else
- .long __crc_\name
-#endif
- .weak __crc_\name
- .previous
-#endif
#endif
.endm

diff --git a/include/linux/export.h b/include/linux/export.h
index 27d848712b90..6c8e24e723bd 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -11,6 +11,14 @@
* hackers place grumpy comments in header files.
*/

+/*
+ * This comment block is used by fixdep. Please do not remove.
+ *
+ * When CONFIG_MODVERSIONS is changed from n to y, all source files having
+ * EXPORT_SYMBOL variants must be re-compiled because genksyms is run as a
+ * side effect of the .o build rule.
+ */
+
#ifndef __ASSEMBLY__
#ifdef MODULE
extern struct module __this_module;
@@ -19,26 +27,6 @@ extern struct module __this_module;
#define THIS_MODULE ((struct module *)0)
#endif

-#ifdef CONFIG_MODVERSIONS
-/* Mark the CRC weak since genksyms apparently decides not to
- * generate a checksums for some symbols */
-#if defined(CONFIG_MODULE_REL_CRCS)
-#define __CRC_SYMBOL(sym, sec) \
- asm(" .section \"___kcrctab" sec "+" #sym "\", \"a\" \n" \
- " .weak __crc_" #sym " \n" \
- " .long __crc_" #sym " - . \n" \
- " .previous \n")
-#else
-#define __CRC_SYMBOL(sym, sec) \
- asm(" .section \"___kcrctab" sec "+" #sym "\", \"a\" \n" \
- " .weak __crc_" #sym " \n" \
- " .long __crc_" #sym " \n" \
- " .previous \n")
-#endif
-#else
-#define __CRC_SYMBOL(sym, sec)
-#endif
-
#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
#include <linux/compiler.h>
/*
@@ -85,7 +73,6 @@ struct kernel_symbol {
/*
* For every exported symbol, do the following:
*
- * - If applicable, place a CRC entry in the __kcrctab section.
* - Put the name of the symbol and namespace (empty string "" for none) in
* __ksymtab_strings.
* - Place a struct kernel_symbol entry in the __ksymtab section.
@@ -98,7 +85,6 @@ struct kernel_symbol {
extern typeof(sym) sym; \
extern const char __kstrtab_##sym[]; \
extern const char __kstrtabns_##sym[]; \
- __CRC_SYMBOL(sym, sec); \
asm(" .section \"__ksymtab_strings\",\"aMS\",%progbits,1 \n" \
"__kstrtab_" #sym ": \n" \
" .asciz \"" #sym "\" \n" \
diff --git a/include/linux/symversion.h b/include/linux/symversion.h
new file mode 100644
index 000000000000..09971145710a
--- /dev/null
+++ b/include/linux/symversion.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* This header is used by C files generated by modpost. */
+
+#ifndef __LINUX_SYMVERSION_H__
+#define __LINUX_SYMVERSION_H__
+
+#include <linux/compiler.h>
+#include <linux/types.h>
+
+#define SYMBOL_CRC(sym, crc, sec) \
+ u32 __section("___kcrctab" sec "+" #sym) __crc_##sym = crc
+
+#endif /* __LINUX_SYMVERSION_H__ */
diff --git a/init/Kconfig b/init/Kconfig
index ddcbefe535e9..f5b14318dfcb 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2136,10 +2136,6 @@ config ASM_MODVERSIONS
assembly. This can be enabled only when the target architecture
supports it.

-config MODULE_REL_CRCS
- bool
- depends on MODVERSIONS
-
config MODULE_SRCVERSION_ALL
bool "Source checksum for all modules"
help
diff --git a/kernel/module.c b/kernel/module.c
index 6cea788fd965..c9e2342da28e 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1231,11 +1231,6 @@ static int try_to_force_load(struct module *mod, const char *reason)

#ifdef CONFIG_MODVERSIONS

-static u32 resolve_rel_crc(const s32 *crc)
-{
- return *(u32 *)((void *)crc + *crc);
-}
-
static int check_version(const struct load_info *info,
const char *symname,
struct module *mod,
@@ -1264,10 +1259,7 @@ static int check_version(const struct load_info *info,
if (strcmp(versions[i].name, symname) != 0)
continue;

- if (IS_ENABLED(CONFIG_MODULE_REL_CRCS))
- crcval = resolve_rel_crc(crc);
- else
- crcval = *crc;
+ crcval = *crc;
if (versions[i].crc == crcval)
return 1;
pr_debug("Found checksum %X vs module %lX\n",
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index a1023868775f..ddd9080fc028 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -128,7 +128,6 @@ $(obj)/%.i: $(src)/%.c FORCE

genksyms = scripts/genksyms/genksyms \
$(if $(1), -T $(2)) \
- $(if $(CONFIG_MODULE_REL_CRCS), -R) \
$(if $(KBUILD_PRESERVE), -p) \
-r $(or $(wildcard $(2:.symtypes=.symref)), /dev/null)

@@ -162,19 +161,11 @@ ifdef CONFIG_MODVERSIONS
# o if <file>.o doesn't contain a __ksymtab version, i.e. does
# not export symbols, it's done.
# o otherwise, we calculate symbol versions using the good old
-# genksyms on the preprocessed source and postprocess them in a way
-# that they are usable as a linker script
-# o generate .tmp_<file>.o from <file>.o using the linker to
-# replace the unresolved symbols __crc_exported_symbol with
-# the actual value of the checksum generated by genksyms
-# o remove .tmp_<file>.o to <file>.o
+# genksyms on the preprocessed source and dump them into the .cmd file.
+# o modpost will extract versions from that file and create *.c files that will
+# be compiled and linked to the kernel and/or modules.

-# Generate .o.symversions files for each .o with exported symbols, and link these
-# to the kernel and/or modules at the end.
-
-genksyms_format_rel_crc := [^_]*__crc_\([^ ]*\) = \.; LONG(\([^)]*\)).*
-genksyms_format_normal := __crc_\(.*\) = \(.*\);
-genksyms_format := $(if $(CONFIG_MODULE_REL_CRCS),$(genksyms_format_rel_crc),$(genksyms_format_normal))
+genksyms_format := __crc_\(.*\) = \(.*\);

gen_symversions = \
if $(NM) $@ 2>/dev/null | grep -q __ksymtab; then \

@@ -188,12 +179,6 @@ gen_symversions = \

cmd_gen_symversions_c = $(call gen_symversions,c)

-cmd_modversions = \
- if [ -r $@.symversions ]; then \
- $(LD) $(KBUILD_LDFLAGS) -r -o $(@D)/.tmp_$(@F) $@ \
- -T $@.symversions; \
- mv -f $(@D)/.tmp_$(@F) $@; \
- fi
endif

ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
@@ -273,7 +258,6 @@ define rule_cc_o_c
$(call cmd,checkdoc)
$(call cmd,gen_objtooldep)
$(call cmd,gen_symversions_c)
- $(if $(CONFIG_LTO_CLANG),,$(call cmd,modversions))
$(call cmd,record_mcount)
endef

@@ -282,7 +266,6 @@ define rule_as_o_S
$(call cmd,gen_ksymdeps)
$(call cmd,gen_objtooldep)
$(call cmd,gen_symversions_S)
- $(call cmd,modversions)
endef

# Built-in and composite module parts
@@ -296,8 +279,6 @@ ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
quiet_cmd_cc_prelink_modules = LD [M] $@
cmd_cc_prelink_modules = \
$(LD) $(ld_flags) -r -o $@ \
- $(shell [ -s $(@:.prelink.o=.o.symversions) ] && \
- echo -T $(@:.prelink.o=.o.symversions)) \
--whole-archive $(filter-out FORCE,$^) \
$(cmd_objtool)

diff --git a/scripts/genksyms/genksyms.c b/scripts/genksyms/genksyms.c
index 4827c5abe5b7..6e6933ae7911 100644
--- a/scripts/genksyms/genksyms.c
+++ b/scripts/genksyms/genksyms.c
@@ -33,7 +33,7 @@ char *cur_filename;
int in_source_file;

static int flag_debug, flag_dump_defs, flag_reference, flag_dump_types,
- flag_preserve, flag_warnings, flag_rel_crcs;
+ flag_preserve, flag_warnings;

static int errors;
static int nsyms;
@@ -681,10 +681,7 @@ void export_symbol(const char *name)
fputs(">\n", debugfile);

/* Used as a linker script. */

- printf(!flag_rel_crcs ? "__crc_%s = 0x%08lx;\n" :
- "SECTIONS { .rodata : ALIGN(4) { "
- "__crc_%s = .; LONG(0x%08lx); } }\n",
- name, crc);
+ printf("__crc_%s = 0x%08lx;\n", name, crc);
}
}

@@ -733,7 +730,6 @@ static void genksyms_usage(void)
" -q, --quiet Disable warnings (default)\n"
" -h, --help Print this message\n"
" -V, --version Print the release version\n"
- " -R, --relative-crc Emit section relative symbol CRCs\n"
#else /* __GNU_LIBRARY__ */
" -s Select symbol prefix\n"
" -d Increment the debug level (repeatable)\n"
@@ -745,7 +741,6 @@ static void genksyms_usage(void)
" -q Disable warnings (default)\n"
" -h Print this message\n"
" -V Print the release version\n"
- " -R Emit section relative symbol CRCs\n"
#endif /* __GNU_LIBRARY__ */
, stderr);
}
@@ -766,14 +761,13 @@ int main(int argc, char **argv)
{"preserve", 0, 0, 'p'},
{"version", 0, 0, 'V'},
{"help", 0, 0, 'h'},
- {"relative-crc", 0, 0, 'R'},
{0, 0, 0, 0}
};

- while ((o = getopt_long(argc, argv, "s:dwqVDr:T:phR",
+ while ((o = getopt_long(argc, argv, "s:dwqVDr:T:ph",
&long_opts[0], NULL)) != EOF)
#else /* __GNU_LIBRARY__ */
- while ((o = getopt(argc, argv, "s:dwqVDr:T:phR")) != EOF)
+ while ((o = getopt(argc, argv, "s:dwqVDr:T:ph")) != EOF)
#endif /* __GNU_LIBRARY__ */
switch (o) {
case 'd':
@@ -813,9 +807,6 @@ int main(int argc, char **argv)
case 'h':
genksyms_usage();
return 0;
- case 'R':
- flag_rel_crcs = 1;
- break;
default:
genksyms_usage();
return 1;
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index eceb3ee7ec06..2742b7dd089a 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -88,11 +88,6 @@ modpost_link()
gen_initcalls
lds="-T .tmp_initcalls.lds"

- if is_enabled CONFIG_MODVERSIONS; then
- gen_symversions
- lds="${lds} -T .tmp_symversions.lds"
- fi
-
# This might take a while, so indicate that we're doing
# an LTO link
info LTO ${1}
@@ -277,7 +272,7 @@ kallsyms_step()
kallsymso=${kallsyms_vmlinux}.o
kallsyms_S=${kallsyms_vmlinux}.S

- vmlinux_link ${kallsyms_vmlinux} "${kallsymso_prev}" ${btf_vmlinux_bin_o}
+ vmlinux_link ${kallsyms_vmlinux} "${kallsymso_prev}" ${vmlinux_symver_o} ${btf_vmlinux_bin_o}
kallsyms ${kallsyms_vmlinux} ${kallsyms_S}

info AS ${kallsyms_S}
@@ -312,6 +307,7 @@ cleanup()
rm -f vmlinux.o
rm -f .vmlinux.d
rm -f .vmlinux.objs
+ rm -f .vmlinux-symver.c
}

# Use "make V=1" to debug this script
@@ -373,6 +369,16 @@ if is_enabled CONFIG_DEBUG_INFO_BTF; then
fi
fi

+vmlinux_symver_o=
+if is_enabled CONFIG_MODVERSIONS; then
+ vmlinux_symver_o=.vmlinux-symver.o
+ info CC ${vmlinux_symver_o}
+ ${CC} ${NOSTDINC_FLAGS} ${LINUXINCLUDE} \
+ ${KBUILD_CPPFLAGS} ${KBUILD_CFLAGS} \
+ ${KBUILD_CFLAGS_KERNEL} ${CFLAGS_KERNEL} \
+ -c -o ${vmlinux_symver_o} ${vmlinux_symver_o%.o}.c
+fi
+
kallsymso=""
kallsymso_prev=""
kallsyms_vmlinux=""
@@ -413,7 +419,7 @@ if is_enabled CONFIG_KALLSYMS; then
fi
fi

-vmlinux_link vmlinux "${kallsymso}" ${btf_vmlinux_bin_o}
+vmlinux_link vmlinux "${kallsymso}" ${vmlinux_symver_o} ${btf_vmlinux_bin_o}

# fill in BTF IDs
if is_enabled CONFIG_DEBUG_INFO_BTF && is_enabled CONFIG_BPF; then
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 92ee1f454e29..be7d8adc5d31 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2276,20 +2276,30 @@ static void add_header(struct buffer *b, struct module *mod)

buf_printf(b, "\nMODULE_INFO(staging, \"Y\");\n");
}

-static void check_symversions(struct module *mod)
+/* Record CRCs for exported symbols */
+static void add_exported_symversions(struct buffer *buf, struct module *mod)
{
struct symbol *sym;

if (!modversions)
return;

+ buf_printf(buf,
+ "\n"
+ "#include <linux/symversion.h>\n"
+ "\n");

+
list_for_each_entry(sym, &mod->exported_symbols, list) {

if (!sym->crc_valid) {

warn("EXPORT symbol \"%s\" [%s%s] version generation failed, symbol will not be versioned.\n"

"Is \"%s\" prototyped in <asm/asm-prototypes.h>?\n",

sym->name, mod->name, mod->is_vmlinux ? "" : ".ko",

sym->name);
+ continue;
}
+
+ buf_printf(buf, "SYMBOL_CRC(%s, 0x%08x, \"%s\");\n",
+ sym->name, sym->crc, sym->is_gpl_only ? "_gpl" : "");
}
}

@@ -2426,6 +2436,18 @@ static void write_if_changed(struct buffer *b, const char *fname)
write_buf(b, fname);
}

+static void write_vmlinux_symver_c_file(struct module *mod)
+{
+ struct buffer buf = { };

+
+ if (!modversions)
+ return;
+

+ add_exported_symversions(&buf, mod);
+ write_if_changed(&buf, ".vmlinux-symver.c");
+ free(buf.p);
+}
+
/* do sanity checks, and generate *.mod.c file */
static void write_mod_c_file(struct module *mod)
{
@@ -2437,6 +2459,7 @@ static void write_mod_c_file(struct module *mod)
check_exports(mod);

add_header(&buf, mod);
+ add_exported_symversions(&buf, mod);
add_versions(&buf, mod);
add_depends(&buf, mod);
add_moddevtable(&buf, mod);
@@ -2633,9 +2656,9 @@ int main(int argc, char **argv)
if (mod->from_dump)
continue;

- check_symversions(mod);
-
- if (!mod->is_vmlinux)
+ if (mod->is_vmlinux)
+ write_vmlinux_symver_c_file(mod);
+ else
write_mod_c_file(mod);
}

--
2.32.0

[Masahiro Yamada]

On Thu, May 5, 2022 at 4:24 PM Masahiro Yamada <masa...@kernel.org> wrote:
>
>
> This is the third batch of cleanups in this development cycle.

This series is available at:
git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git
lto-cleanup-v3








--
Best Regards
Masahiro Yamada

[Nicolas Schier]

Reviewed-by: Nicolas Schier <nic...@fjasle.eu>

[Nicolas Schier]

Reviewed-by: Nicolas Schier <nic...@fjasle.eu>

[Nicolas Schier]

Reviewed-by: Nicolas Schier <nic...@fjasle.eu>

[Nicolas Schier]

Reviewed-by: Nicolas Schier <nic...@fjasle.eu>

[Nicolas Schier]

Reviewed-by: Nicolas Schier <nic...@fjasle.eu>

[Nick Desaulniers]

On Thu, May 5, 2022 at 12:25 AM Masahiro Yamada <masa...@kernel.org> wrote:
>
> Kbuild runs at the top of objtree instead of changing the working
> directory to subdirectories. I think this design is nice overall but
> some commands have a scapability issue.

s/scapability/scalability/


--
Thanks,
~Nick Desaulniers

[Nicolas Schier]

Thanks! At work, some colleagues of mine stumbled over that problem, too.

Reviewed-by: Nicolas Schier <nic...@fjasle.eu>

[Nathan Chancellor]

Hi Masahiro,

I merged this series into mainline and tested an Arch Linux
x86_64 configuration and Fedora aarch64 configuration with ThinLTO and
saw no new warnings or issues. Modules loaded just fine in QEMU for Arch
Linux and I did not notice any boot issues or warnings.

Tested-by: Nathan Chancellor <nat...@kernel.org>

Cheers,
Nathan

[Masahiro Yamada]

On Thu, May 5, 2022 at 4:24 PM Masahiro Yamada <masa...@kernel.org> wrote:
>
>

Only 03-06 were applied.

I will send v4 for the rest.
(I rewrote the static EXPORT checks).

>
> arch/powerpc/Kconfig | 1 -
> arch/s390/Kconfig | 1 -
> arch/um/Kconfig | 1 -
> include/asm-generic/export.h | 22 +-
> include/linux/export.h | 30 +--
> include/linux/symversion.h | 13 +
> init/Kconfig | 4 -
> kernel/module.c | 10 +-
> scripts/Kbuild.include | 4 +
> scripts/Makefile.build | 118 +++------
> scripts/Makefile.lib | 7 -
> scripts/Makefile.modfinal | 5 +-
> scripts/Makefile.modpost | 9 +-
> scripts/genksyms/genksyms.c | 18 +-
> scripts/link-vmlinux.sh | 46 ++--
> scripts/mod/file2alias.c | 2 -
> scripts/mod/list.h | 52 ++++
> scripts/mod/modpost.c | 449 ++++++++++++++++++++---------------
> scripts/mod/modpost.h | 2 +
> 19 files changed, 402 insertions(+), 392 deletions(-)
> create mode 100644 include/linux/symversion.h
>
> --
> 2.32.0
>

> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-li...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20220505072244.1155033-1-masahiroy%40kernel.org.