[PATCH v2 00/26] kbuild: yet another series of cleanups (modpost and LTO)

[PATCH v2 00/26] kbuild: yet another series of cleanups (modpost and LTO)

[Masahiro Yamada]

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

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 (26):
  modpost: use bool type where appropriate
  modpost: change mod->gpl_compatible to bool type
  modpost: import include/linux/list.h
  modpost: traverse modules in order
  modpost: add sym_add_unresolved() helper
  modpost: traverse unresolved symbols in order
  modpost: use doubly linked list for dump_lists
  modpost: traverse the namespace_list in order
  modpost: dump Module.symvers in the same order of modules.order
  modpost: move static EXPORT_SYMBOL check to check_exports()
  modpost: make multiple export error
  modpost: make sym_add_exported() always allocate a new symbol
  modpost: split new_symbol() to symbol allocation and hash table
    addition
  modpost: mitigate false-negatives for static EXPORT_SYMBOL checks
  kbuild: record symbol versions in *.cmd files
  kbuild: generate a list of objects in vmlinux
  modpost: extract symbol versions from *.cmd files
  modpost: generate linker script to collect symbol versions
  kbuild: embed symbol versions at final link of vmlinux or modules
  kbuild: stop merging *.symversions
  genksyms: adjust the output format for .cmd files
  kbuild: do not create *.prelink.o for Clang LTO or IBT
  kbuild: make built-in.a rule robust against too long argument error
  kbuild: make *.mod rule robust against too long argument error
  modpost: simplify the ->is_static initialization
  modpost: use hlist for hash table implementation

 .gitignore                  |   1 +
 Makefile                    |   1 +
 scripts/Kbuild.include      |   4 +
 scripts/Makefile.build      | 118 +++------
 scripts/Makefile.lib        |   7 -
 scripts/Makefile.modfinal   |   6 +-
 scripts/Makefile.modpost    |  10 +-
 scripts/genksyms/genksyms.c |  17 +-
 scripts/link-vmlinux.sh     |  34 +--
 scripts/mod/file2alias.c    |   2 -
 scripts/mod/list.h          | 265 +++++++++++++++++++
 scripts/mod/modpost.c       | 501 ++++++++++++++++++++++--------------
 scripts/mod/modpost.h       |  24 +-
 scripts/mod/sumversion.c    |   8 +-
 14 files changed, 650 insertions(+), 348 deletions(-)
 create mode 100644 scripts/mod/list.h

-- 
2.32.0

[PATCH v2 01/26] modpost: use bool type where appropriate

[Masahiro Yamada]

Use 'bool' to clarify that the valid value is true or false.

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

Changes in v2:
  - postpone mod->gpl_compatible change
  - change is_static_library() as well

 scripts/mod/modpost.c    | 56 ++++++++++++++++++++--------------------
 scripts/mod/modpost.h    | 10 +++----
 scripts/mod/sumversion.c |  8 +++---
 3 files changed, 36 insertions(+), 38 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 689a34229809..a6035ab78cd8 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -23,20 +23,20 @@
 #include "../../include/linux/license.h"
 
 /* Are we using CONFIG_MODVERSIONS? */
-static int modversions;
+static bool modversions;
 /* Is CONFIG_MODULE_SRCVERSION_ALL set? */
-static int all_versions;
+static bool all_versions;
 /* If we are modposting external module set to 1 */
-static int external_module;
+static bool external_module;
 /* Only warn about unresolved symbols */
-static int warn_unresolved;
+static bool warn_unresolved;
 /* How a symbol is exported */
 static int sec_mismatch_count;
-static int sec_mismatch_warn_only = true;
+static bool sec_mismatch_warn_only = true;
 /* ignore missing files */
-static int ignore_missing_files;
+static bool ignore_missing_files;
 /* If set to 1, only warn (instead of error) about missing ns imports */
-static int allow_missing_ns_imports;
+static bool allow_missing_ns_imports;
 
 static bool error_occurred;
 
@@ -202,11 +202,11 @@ static struct module *new_module(const char *modname)
 struct symbol {
 	struct symbol *next;
 	struct module *module;
-	unsigned int crc;
-	int crc_valid;
 	char *namespace;
-	unsigned int weak:1;
-	unsigned int is_static:1;  /* 1 if symbol is not global */
+	unsigned int crc;
+	bool crc_valid;
+	bool weak;
+	bool is_static;		/* true if symbol is not global */
 	enum export  export;       /* Type of export */
 	char name[];
 };
@@ -230,7 +230,7 @@ static inline unsigned int tdb_hash(const char *name)
  * Allocate a new symbols for use in the hash of exported symbols or
  * the list of unresolved symbols per module
  **/
-static struct symbol *alloc_symbol(const char *name, unsigned int weak,
+static struct symbol *alloc_symbol(const char *name, bool weak,
 				   struct symbol *next)
 {
 	struct symbol *s = NOFAIL(malloc(sizeof(*s) + strlen(name) + 1));
@@ -239,7 +239,7 @@ static struct symbol *alloc_symbol(const char *name, unsigned int weak,
 	strcpy(s->name, name);
 	s->weak = weak;
 	s->next = next;
-	s->is_static = 1;
+	s->is_static = true;
 	return s;
 }
 
@@ -250,7 +250,7 @@ static struct symbol *new_symbol(const char *name, struct module *module,
 	unsigned int hash;
 
 	hash = tdb_hash(name) % SYMBOL_HASH_SIZE;
-	symbolhash[hash] = alloc_symbol(name, 0, symbolhash[hash]);
+	symbolhash[hash] = alloc_symbol(name, false, symbolhash[hash]);
 
 	return symbolhash[hash];
 }
@@ -424,7 +424,7 @@ static void sym_set_crc(const char *name, unsigned int crc)
 		return;
 
 	s->crc = crc;
-	s->crc_valid = 1;
+	s->crc_valid = true;
 }
 
 static void *grab_file(const char *filename, size_t *size)
@@ -721,9 +721,9 @@ static void handle_symbol(struct module *mod, struct elf_info *info,
 			sym_add_exported(name, mod, export);
 		}
 		if (strcmp(symname, "init_module") == 0)
-			mod->has_init = 1;
+			mod->has_init = true;
 		if (strcmp(symname, "cleanup_module") == 0)
-			mod->has_cleanup = 1;
+			mod->has_cleanup = true;
 		break;
 	}
 }
@@ -2058,7 +2058,7 @@ static void read_symbols(const char *modname)
 						       sym->st_name));
 
 			if (s)
-				s->is_static = 0;
+				s->is_static = false;
 		}
 	}
 
@@ -2078,7 +2078,7 @@ static void read_symbols(const char *modname)
 	 * the automatic versioning doesn't pick it up, but it's really
 	 * important anyhow */
 	if (modversions)
-		mod->unres = alloc_symbol("module_layout", 0, mod->unres);
+		mod->unres = alloc_symbol("module_layout", false, mod->unres);
 }
 
 static void read_symbols_from_files(const char *filename)
@@ -2310,7 +2310,7 @@ static void add_depends(struct buffer *b, struct module *mod)
 		if (s->module->seen)
 			continue;
 
-		s->module->seen = 1;
+		s->module->seen = true;
 		p = strrchr(s->module->name, '/');
 		if (p)
 			p++;
@@ -2427,10 +2427,10 @@ static void read_dump(const char *fname)
 		mod = find_module(modname);
 		if (!mod) {
 			mod = new_module(modname);
-			mod->from_dump = 1;
+			mod->from_dump = true;
 		}
 		s = sym_add_exported(symname, mod, export_no(export));
-		s->is_static = 0;
+		s->is_static = false;
 		sym_set_crc(symname, crc);
 		sym_update_namespace(symname, namespace);
 	}
@@ -2508,7 +2508,7 @@ int main(int argc, char **argv)
 	while ((opt = getopt(argc, argv, "ei:mnT:o:awENd:")) != -1) {
 		switch (opt) {
 		case 'e':
-			external_module = 1;
+			external_module = true;
 			break;
 		case 'i':
 			*dump_read_iter =
@@ -2517,28 +2517,28 @@ int main(int argc, char **argv)
 			dump_read_iter = &(*dump_read_iter)->next;
 			break;
 		case 'm':
-			modversions = 1;
+			modversions = true;
 			break;
 		case 'n':
-			ignore_missing_files = 1;
+			ignore_missing_files = true;
 			break;
 		case 'o':
 			dump_write = optarg;
 			break;
 		case 'a':
-			all_versions = 1;
+			all_versions = true;
 			break;
 		case 'T':
 			files_source = optarg;
 			break;
 		case 'w':
-			warn_unresolved = 1;
+			warn_unresolved = true;
 			break;
 		case 'E':
 			sec_mismatch_warn_only = false;
 			break;
 		case 'N':
-			allow_missing_ns_imports = 1;
+			allow_missing_ns_imports = true;
 			break;
 		case 'd':
 			missing_namespace_deps = optarg;
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 96d6b3a16ca2..7ccfcc8899c1 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -1,4 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0 */
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdarg.h>
@@ -111,11 +112,10 @@ struct module {
 	struct module *next;
 	int gpl_compatible;
 	struct symbol *unres;
-	int from_dump;  /* 1 if module was loaded from *.symvers */
-	int is_vmlinux;
-	int seen;
-	int has_init;
-	int has_cleanup;
+	bool from_dump;		/* true if module was loaded from *.symvers */
+	bool is_vmlinux;
+	bool seen;
+	bool has_init, has_cleanup;
 	struct buffer dev_table_buf;
 	char	     srcversion[25];
 	// Missing namespace dependencies
diff --git a/scripts/mod/sumversion.c b/scripts/mod/sumversion.c
index 79bb9eaa65ac..6bf9caca0968 100644
--- a/scripts/mod/sumversion.c
+++ b/scripts/mod/sumversion.c
@@ -290,13 +290,11 @@ static int parse_file(const char *fname, struct md4_ctx *md)
 	return 1;
 }
 /* Check whether the file is a static library or not */
-static int is_static_library(const char *objfile)
+static bool is_static_library(const char *objfile)
 {
 	int len = strlen(objfile);
-	if (objfile[len - 2] == '.' && objfile[len - 1] == 'a')
-		return 1;
-	else
-		return 0;
+
+	return objfile[len - 2] == '.' && objfile[len - 1] == 'a';
 }
 
 /* We have dir/file.o.  Open dir/.file.o.cmd, look for source_ and deps_ line
-- 
2.32.0

[PATCH v2 02/26] modpost: change mod->gpl_compatible to bool type

[Masahiro Yamada]

Currently, mod->gpl_compatible is tristate; it is set to -1 by default,
then to 1 or 0 when MODULE_LICENSE() is found.

Maybe, -1 was chosen to represent the 'unknown' license, but it is not
useful.

The current code:

    if (!mod->gpl_compatible)
            check_for_gpl_usage(exp->export, basename, exp->name);

... only cares whether gpl_compatible is zero or not.

Change it to a bool type with the initial value 'true', which has no
functional change.

The default value should be 'true' instead of 'false'.

Since commit 1d6cd3929360 ("modpost: turn missing MODULE_LICENSE() into
error"), unknown module license is an error.

The error message, "missing MODULE_LICENSE()" is enough to explain the
issue. It is not sensible to show another message, "GPL-incompatible
module ... uses GPL-only symbol".

Add comments to explain this.

While I was here, I renamed gpl_compatible to is_gpl_compatible for
clarification, and also slightly refactored the code.

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

Changes in v2:
  - Split as a separate commit
  - More code refactoring
  - Add a comment
  - Rename it to ->is_gpl_compatible (Nick)

 scripts/mod/modpost.c | 17 +++++++++++------
 scripts/mod/modpost.h |  2 +-
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index a6035ab78cd8..f2d6c152cd3f 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -187,7 +187,14 @@ static struct module *new_module(const char *modname)
 	/* add to list */
 	strcpy(mod->name, modname);
 	mod->is_vmlinux = (strcmp(modname, "vmlinux") == 0);
-	mod->gpl_compatible = -1;
+
+	/*
+	 * Set mod->is_gpl_compatible to true by default to skip
+	 * check_for_gpl_usage() when MODULE_LICENSE() is missing.
+	 * If it is the case, modpost will exit with error anyway.
+	 */
+	mod->is_gpl_compatible = true;
+
 	mod->next = modules;
 	modules = mod;
 
@@ -2012,10 +2019,8 @@ static void read_symbols(const char *modname)
 		if (!license)
 			error("missing MODULE_LICENSE() in %s\n", modname);
 		while (license) {
-			if (license_is_gpl_compatible(license))
-				mod->gpl_compatible = 1;
-			else {
-				mod->gpl_compatible = 0;
+			if (!license_is_gpl_compatible(license)) {
+				mod->is_gpl_compatible = false;
 				break;
 			}
 			license = get_next_modinfo(&info, "license", license);
@@ -2183,7 +2188,7 @@ static void check_exports(struct module *mod)
 			add_namespace(&mod->missing_namespaces, exp->namespace);
 		}
 
-		if (!mod->gpl_compatible)
+		if (!mod->is_gpl_compatible)
 			check_for_gpl_usage(exp->export, basename, exp->name);
 	}
 }
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 7ccfcc8899c1..f21a3782885b 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -110,7 +110,7 @@ buf_write(struct buffer *buf, const char *s, int len);
 
 struct module {
 	struct module *next;
-	int gpl_compatible;
+	bool is_gpl_compatible;
 	struct symbol *unres;
 	bool from_dump;		/* true if module was loaded from *.symvers */
 	bool is_vmlinux;
-- 
2.32.0

[PATCH v2 03/26] modpost: import include/linux/list.h

[Masahiro Yamada]

Import include/linux/list.h to use convenient list macros in modpost.

I dropped kernel-space code such as {WRITE,READ}_ONCE etc. and unneeded
macros.

I also imported container_of() from include/linux/container_of.h and
type definitions from include/linux/types.h.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---

Changes in v2:
  - Drop hlist

 scripts/mod/list.h | 213 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 213 insertions(+)
 create mode 100644 scripts/mod/list.h

diff --git a/scripts/mod/list.h b/scripts/mod/list.h
new file mode 100644
index 000000000000..a924a6c4aa4d
--- /dev/null
+++ b/scripts/mod/list.h
@@ -0,0 +1,213 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef LIST_H
+#define LIST_H
+
+#include <stdbool.h>
+#include <stddef.h>
+
+/* Are two types/vars the same type (ignoring qualifiers)? */
+#define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
+
+/**
+ * container_of - cast a member of a structure out to the containing structure
+ * @ptr:	the pointer to the member.
+ * @type:	the type of the container struct this is embedded in.
+ * @member:	the name of the member within the struct.
+ *
+ */
+#define container_of(ptr, type, member) ({				\
+	void *__mptr = (void *)(ptr);					\
+	_Static_assert(__same_type(*(ptr), ((type *)0)->member) ||	\
+		      __same_type(*(ptr), void),			\
+		      "pointer type mismatch in container_of()");	\
+	((type *)(__mptr - offsetof(type, member))); })
+
+#define LIST_POISON1  ((void *) 0x100)
+#define LIST_POISON2  ((void *) 0x122)
+
+/*
+ * Circular doubly linked list implementation.
+ *
+ * Some of the internal functions ("__xxx") are useful when
+ * manipulating whole lists rather than single entries, as
+ * sometimes we already know the next/prev entries and we can
+ * generate better code by using them directly rather than
+ * using the generic single-entry routines.
+ */
+
+struct list_head {
+	struct list_head *next, *prev;
+};
+
+#define LIST_HEAD_INIT(name) { &(name), &(name) }
+
+#define LIST_HEAD(name) \
+	struct list_head name = LIST_HEAD_INIT(name)
+
+/**
+ * INIT_LIST_HEAD - Initialize a list_head structure
+ * @list: list_head structure to be initialized.
+ *
+ * Initializes the list_head to point to itself.  If it is a list header,
+ * the result is an empty list.
+ */
+static inline void INIT_LIST_HEAD(struct list_head *list)
+{
+	list->next = list;
+	list->prev = list;
+}
+
+/*
+ * Insert a new entry between two known consecutive entries.
+ *
+ * This is only for internal list manipulation where we know
+ * the prev/next entries already!
+ */
+static inline void __list_add(struct list_head *new,
+			      struct list_head *prev,
+			      struct list_head *next)
+{
+	next->prev = new;
+	new->next = next;
+	new->prev = prev;
+	prev->next = new;
+}
+
+/**
+ * list_add - add a new entry
+ * @new: new entry to be added
+ * @head: list head to add it after
+ *
+ * Insert a new entry after the specified head.
+ * This is good for implementing stacks.
+ */
+static inline void list_add(struct list_head *new, struct list_head *head)
+{
+	__list_add(new, head, head->next);
+}
+
+/**
+ * list_add_tail - add a new entry
+ * @new: new entry to be added
+ * @head: list head to add it before
+ *
+ * Insert a new entry before the specified head.
+ * This is useful for implementing queues.
+ */
+static inline void list_add_tail(struct list_head *new, struct list_head *head)
+{
+	__list_add(new, head->prev, head);
+}
+
+/*
+ * Delete a list entry by making the prev/next entries
+ * point to each other.
+ *
+ * This is only for internal list manipulation where we know
+ * the prev/next entries already!
+ */
+static inline void __list_del(struct list_head *prev, struct list_head *next)
+{
+	next->prev = prev;
+	prev->next = next;
+}
+
+static inline void __list_del_entry(struct list_head *entry)
+{
+	__list_del(entry->prev, entry->next);
+}
+
+/**
+ * list_del - deletes entry from list.
+ * @entry: the element to delete from the list.
+ * Note: list_empty() on entry does not return true after this, the entry is
+ * in an undefined state.
+ */
+static inline void list_del(struct list_head *entry)
+{
+	__list_del_entry(entry);
+	entry->next = LIST_POISON1;
+	entry->prev = LIST_POISON2;
+}
+
+/**
+ * list_is_head - tests whether @list is the list @head
+ * @list: the entry to test
+ * @head: the head of the list
+ */
+static inline int list_is_head(const struct list_head *list, const struct list_head *head)
+{
+	return list == head;
+}
+
+/**
+ * list_empty - tests whether a list is empty
+ * @head: the list to test.
+ */
+static inline int list_empty(const struct list_head *head)
+{
+	return head->next == head;
+}
+
+/**
+ * list_entry - get the struct for this entry
+ * @ptr:	the &struct list_head pointer.
+ * @type:	the type of the struct this is embedded in.
+ * @member:	the name of the list_head within the struct.
+ */
+#define list_entry(ptr, type, member) \
+	container_of(ptr, type, member)
+
+/**
+ * list_first_entry - get the first element from a list
+ * @ptr:	the list head to take the element from.
+ * @type:	the type of the struct this is embedded in.
+ * @member:	the name of the list_head within the struct.
+ *
+ * Note, that list is expected to be not empty.
+ */
+#define list_first_entry(ptr, type, member) \
+	list_entry((ptr)->next, type, member)
+
+/**
+ * list_next_entry - get the next element in list
+ * @pos:	the type * to cursor
+ * @member:	the name of the list_head within the struct.
+ */
+#define list_next_entry(pos, member) \
+	list_entry((pos)->member.next, typeof(*(pos)), member)
+
+/**
+ * list_entry_is_head - test if the entry points to the head of the list
+ * @pos:	the type * to cursor
+ * @head:	the head for your list.
+ * @member:	the name of the list_head within the struct.
+ */
+#define list_entry_is_head(pos, head, member)				\
+	(&pos->member == (head))
+
+/**
+ * list_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 list_head within the struct.
+ */
+#define list_for_each_entry(pos, head, member)				\
+	for (pos = list_first_entry(head, typeof(*pos), member);	\
+	     !list_entry_is_head(pos, head, member);			\
+	     pos = list_next_entry(pos, member))
+
+/**
+ * list_for_each_entry_safe - iterate over list of given type. Safe against removal of list entry
+ * @pos:	the type * to use as a loop cursor.
+ * @n:		another type * to use as temporary storage
+ * @head:	the head for your list.
+ * @member:	the name of the list_head within the struct.
+ */
+#define list_for_each_entry_safe(pos, n, head, member)			\
+	for (pos = list_first_entry(head, typeof(*pos), member),	\
+		n = list_next_entry(pos, member);			\
+	     !list_entry_is_head(pos, head, member);			\
+	     pos = n, n = list_next_entry(n, member))
+
+#endif /* LIST_H */
-- 
2.32.0

[PATCH v2 04/26] modpost: traverse modules in order

[Masahiro Yamada]

Currently, modpost manages modules in a singly linked list; it adds
a new node to the head, and traverses the list from new to old.

It works, but the error messages are shown in the reverse order.

If you have a Makefile like this:

  obj-m += foo.o bar.o

then, modpost shows error messages in bar.o, foo.o, in this order.

Use a doubly linked list to keep the order in modules.order; use
list_add_tail() for the node addition and list_for_each_entry() for
the list traverse.

Now that the kernel's list macros have been imported to modpost, I will
use them actively going forward.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---

(no changes since v1)

 scripts/mod/modpost.c | 17 ++++++++---------
 scripts/mod/modpost.h |  3 ++-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index f2d6c152cd3f..553e5cf88fee 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -165,16 +165,17 @@ char *get_line(char **stringp)
 }
 
 /* A list of all modules we processed */
-static struct module *modules;
+LIST_HEAD(modules);
 
 static struct module *find_module(const char *modname)
 {
 	struct module *mod;
 
-	for (mod = modules; mod; mod = mod->next)
+	list_for_each_entry(mod, &modules, list) {
 		if (strcmp(mod->name, modname) == 0)
-			break;
-	return mod;
+			return mod;
+	}
+	return NULL;
 }
 
 static struct module *new_module(const char *modname)
@@ -184,7 +185,6 @@ static struct module *new_module(const char *modname)
 	mod = NOFAIL(malloc(sizeof(*mod) + strlen(modname) + 1));
 	memset(mod, 0, sizeof(*mod));
 
-	/* add to list */
 	strcpy(mod->name, modname);
 	mod->is_vmlinux = (strcmp(modname, "vmlinux") == 0);
 
@@ -195,8 +195,7 @@ static struct module *new_module(const char *modname)
 	 */
 	mod->is_gpl_compatible = true;
 
-	mod->next = modules;
-	modules = mod;
+	list_add_tail(&mod->list, &modules);
 
 	return mod;
 }
@@ -2477,7 +2476,7 @@ static void write_namespace_deps_files(const char *fname)
 	struct namespace_list *ns;
 	struct buffer ns_deps_buf = {};
 
-	for (mod = modules; mod; mod = mod->next) {
+	list_for_each_entry(mod, &modules, list) {
 
 		if (mod->from_dump || !mod->missing_namespaces)
 			continue;
@@ -2568,7 +2567,7 @@ int main(int argc, char **argv)
 	if (files_source)
 		read_symbols_from_files(files_source);
 
-	for (mod = modules; mod; mod = mod->next) {
+	list_for_each_entry(mod, &modules, list) {
 		char fname[PATH_MAX];
 		int ret;
 
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index f21a3782885b..d0a8ab60f413 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -11,6 +11,7 @@
 #include <unistd.h>
 #include <elf.h>
 
+#include "list.h"
 #include "elfconfig.h"
 
 /* On BSD-alike OSes elf.h defines these according to host's word size */
@@ -109,7 +110,7 @@ void
 buf_write(struct buffer *buf, const char *s, int len);
 
 struct module {
-	struct module *next;
+	struct list_head list;
 	bool is_gpl_compatible;
 	struct symbol *unres;
 	bool from_dump;		/* true if module was loaded from *.symvers */
-- 
2.32.0

[PATCH v2 05/26] modpost: add sym_add_unresolved() helper

[Masahiro Yamada]

Add a small helper, sym_add_unresolved() to ease the further
refactoring.

Remove the 'weak' argument from alloc_symbol() because it is sensible
only for unresolved symbols.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---

(no changes since v1)

 scripts/mod/modpost.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 553e5cf88fee..abcdb0677775 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -236,14 +236,12 @@ static inline unsigned int tdb_hash(const char *name)
  * Allocate a new symbols for use in the hash of exported symbols or
  * the list of unresolved symbols per module
  **/
-static struct symbol *alloc_symbol(const char *name, bool weak,
-				   struct symbol *next)
+static struct symbol *alloc_symbol(const char *name, struct symbol *next)
 {
 	struct symbol *s = NOFAIL(malloc(sizeof(*s) + strlen(name) + 1));
 
 	memset(s, 0, sizeof(*s));
 	strcpy(s->name, name);
-	s->weak = weak;
 	s->next = next;
 	s->is_static = true;
 	return s;
@@ -256,11 +254,17 @@ static struct symbol *new_symbol(const char *name, struct module *module,
 	unsigned int hash;
 
 	hash = tdb_hash(name) % SYMBOL_HASH_SIZE;
-	symbolhash[hash] = alloc_symbol(name, false, symbolhash[hash]);
+	symbolhash[hash] = alloc_symbol(name, symbolhash[hash]);
 
 	return symbolhash[hash];
 }
 
+static void sym_add_unresolved(const char *name, struct module *mod, bool weak)
+{
+	mod->unres = alloc_symbol(name, mod->unres);
+	mod->unres->weak = weak;
+}
+
 static struct symbol *find_symbol(const char *name)
 {
 	struct symbol *s;
@@ -712,9 +716,8 @@ static void handle_symbol(struct module *mod, struct elf_info *info,
 			}
 		}
 
-		mod->unres = alloc_symbol(symname,
-					  ELF_ST_BIND(sym->st_info) == STB_WEAK,
-					  mod->unres);
+		sym_add_unresolved(symname, mod,
+				   ELF_ST_BIND(sym->st_info) == STB_WEAK);
 		break;
 	default:
 		/* All exported symbols */
@@ -2082,7 +2085,7 @@ static void read_symbols(const char *modname)
 	 * the automatic versioning doesn't pick it up, but it's really
 	 * important anyhow */
 	if (modversions)
-		mod->unres = alloc_symbol("module_layout", false, mod->unres);
+		sym_add_unresolved("module_layout", mod, false);
 }
 
 static void read_symbols_from_files(const char *filename)
-- 
2.32.0

[PATCH v2 06/26] modpost: traverse unresolved symbols in order

[Masahiro Yamada]

Currently, modpost manages unresolved in a singly linked list; it adds
a new node to the head, and traverses the list from new to old.

Use a doubly linked list to keep the order in the symbol table in the
ELF file.

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

(no changes since v1)

 scripts/mod/modpost.c | 20 ++++++++++++++------
 scripts/mod/modpost.h |  2 +-
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index abcdb0677775..c7dda4cfa497 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -185,6 +185,8 @@ static struct module *new_module(const char *modname)
 	mod = NOFAIL(malloc(sizeof(*mod) + strlen(modname) + 1));
 	memset(mod, 0, sizeof(*mod));
 
+	INIT_LIST_HEAD(&mod->unresolved_symbols);
+
 	strcpy(mod->name, modname);
 	mod->is_vmlinux = (strcmp(modname, "vmlinux") == 0);
 
@@ -207,6 +209,7 @@ static struct module *new_module(const char *modname)
 
 struct symbol {
 	struct symbol *next;
+	struct list_head list;	/* link to module::unresolved_symbols */
 	struct module *module;
 	char *namespace;
 	unsigned int crc;
@@ -261,8 +264,12 @@ static struct symbol *new_symbol(const char *name, struct module *module,
 
 static void sym_add_unresolved(const char *name, struct module *mod, bool weak)
 {
-	mod->unres = alloc_symbol(name, mod->unres);
-	mod->unres->weak = weak;
+	struct symbol *sym;
+
+	sym = alloc_symbol(name, NULL);
+	sym->weak = weak;
+
+	list_add_tail(&sym->list, &mod->unresolved_symbols);
 }
 
 static struct symbol *find_symbol(const char *name)
@@ -2156,7 +2163,7 @@ static void check_exports(struct module *mod)
 {
 	struct symbol *s, *exp;
 
-	for (s = mod->unres; s; s = s->next) {
+	list_for_each_entry(s, &mod->unresolved_symbols, list) {
 		const char *basename;
 		exp = find_symbol(s->name);
 		if (!exp) {
@@ -2277,7 +2284,7 @@ static void add_versions(struct buffer *b, struct module *mod)
 	buf_printf(b, "static const struct modversion_info ____versions[]\n");
 	buf_printf(b, "__used __section(\"__versions\") = {\n");
 
-	for (s = mod->unres; s; s = s->next) {
+	list_for_each_entry(s, &mod->unresolved_symbols, list) {
 		if (!s->module)
 			continue;
 		if (!s->crc_valid) {
@@ -2303,13 +2310,14 @@ static void add_depends(struct buffer *b, struct module *mod)
 	int first = 1;
 
 	/* Clear ->seen flag of modules that own symbols needed by this. */
-	for (s = mod->unres; s; s = s->next)
+	list_for_each_entry(s, &mod->unresolved_symbols, list) {
 		if (s->module)
 			s->module->seen = s->module->is_vmlinux;
+	}
 
 	buf_printf(b, "\n");
 	buf_printf(b, "MODULE_INFO(depends, \"");
-	for (s = mod->unres; s; s = s->next) {
+	list_for_each_entry(s, &mod->unresolved_symbols, list) {
 		const char *p;
 		if (!s->module)
 			continue;
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index d0a8ab60f413..65296eca20a1 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -111,8 +111,8 @@ buf_write(struct buffer *buf, const char *s, int len);
 
 struct module {
 	struct list_head list;
+	struct list_head unresolved_symbols;
 	bool is_gpl_compatible;
-	struct symbol *unres;
 	bool from_dump;		/* true if module was loaded from *.symvers */
 	bool is_vmlinux;
 	bool seen;
-- 
2.32.0

[PATCH v2 07/26] modpost: use doubly linked list for dump_lists

[Masahiro Yamada]

This looks easier to understand (just because this is a pattern in
the kernel code). No functional change is intended.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---

(no changes since v1)

 scripts/mod/modpost.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index c7dda4cfa497..5e99493b0a82 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2505,7 +2505,7 @@ static void write_namespace_deps_files(const char *fname)
 }
 
 struct dump_list {
-	struct dump_list *next;
+	struct list_head list;
 	const char *file;
 };
 
@@ -2517,8 +2517,8 @@ int main(int argc, char **argv)
 	char *dump_write = NULL, *files_source = NULL;
 	int opt;
 	int n;
-	struct dump_list *dump_read_start = NULL;
-	struct dump_list **dump_read_iter = &dump_read_start;
+	LIST_HEAD(dump_lists);
+	struct dump_list *dl, *dl2;
 
 	while ((opt = getopt(argc, argv, "ei:mnT:o:awENd:")) != -1) {
 		switch (opt) {
@@ -2526,10 +2526,9 @@ int main(int argc, char **argv)
 			external_module = true;
 			break;
 		case 'i':
-			*dump_read_iter =
-				NOFAIL(calloc(1, sizeof(**dump_read_iter)));
-			(*dump_read_iter)->file = optarg;
-			dump_read_iter = &(*dump_read_iter)->next;
+			dl = NOFAIL(malloc(sizeof(*dl)));
+			dl->file = optarg;
+			list_add_tail(&dl->list, &dump_lists);
 			break;
 		case 'm':
 			modversions = true;
@@ -2563,13 +2562,10 @@ int main(int argc, char **argv)
 		}
 	}
 
-	while (dump_read_start) {
-		struct dump_list *tmp;
-
-		read_dump(dump_read_start->file);
-		tmp = dump_read_start->next;
-		free(dump_read_start);
-		dump_read_start = tmp;
+	list_for_each_entry_safe(dl, dl2, &dump_lists, list) {
+		read_dump(dl->file);
+		list_del(&dl->list);
+		free(dl);
 	}
 
 	while (optind < argc)
-- 
2.32.0

[PATCH v2 08/26] modpost: traverse the namespace_list in order

[Masahiro Yamada]

Use the doubly linked list to traverse the list in the added order.
This makes the code more consistent.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---

(no changes since v1)

 scripts/mod/modpost.c | 33 +++++++++++++++------------------
 scripts/mod/modpost.h |  4 ++--
 2 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 5e99493b0a82..d0cf94e1e984 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -186,6 +186,8 @@ static struct module *new_module(const char *modname)
 	memset(mod, 0, sizeof(*mod));
 
 	INIT_LIST_HEAD(&mod->unresolved_symbols);
+	INIT_LIST_HEAD(&mod->missing_namespaces);
+	INIT_LIST_HEAD(&mod->imported_namespaces);
 
 	strcpy(mod->name, modname);
 	mod->is_vmlinux = (strcmp(modname, "vmlinux") == 0);
@@ -288,39 +290,34 @@ static struct symbol *find_symbol(const char *name)
 }
 
 struct namespace_list {
-	struct namespace_list *next;
+	struct list_head list;
 	char namespace[];
 };
 
-static bool contains_namespace(struct namespace_list *list,
-			       const char *namespace)
+static bool contains_namespace(struct list_head *head, const char *namespace)
 {
-	for (; list; list = list->next)
+	struct namespace_list *list;
+
+	list_for_each_entry(list, head, list) {
 		if (!strcmp(list->namespace, namespace))
 			return true;
+	}
 
 	return false;
 }
 
-static void add_namespace(struct namespace_list **list, const char *namespace)
+static void add_namespace(struct list_head *head, const char *namespace)
 {
 	struct namespace_list *ns_entry;
 
-	if (!contains_namespace(*list, namespace)) {
-		ns_entry = NOFAIL(malloc(sizeof(struct namespace_list) +
+	if (!contains_namespace(head, namespace)) {
+		ns_entry = NOFAIL(malloc(sizeof(*ns_entry) +
 					 strlen(namespace) + 1));
 		strcpy(ns_entry->namespace, namespace);
-		ns_entry->next = *list;
-		*list = ns_entry;
+		list_add_tail(&ns_entry->list, head);
 	}
 }
 
-static bool module_imports_namespace(struct module *module,
-				     const char *namespace)
-{
-	return contains_namespace(module->imported_namespaces, namespace);
-}
-
 static const struct {
 	const char *str;
 	enum export export;
@@ -2190,7 +2187,7 @@ static void check_exports(struct module *mod)
 			basename = mod->name;
 
 		if (exp->namespace &&
-		    !module_imports_namespace(mod, exp->namespace)) {
+		    !contains_namespace(&mod->imported_namespaces, exp->namespace)) {
 			modpost_log(allow_missing_ns_imports ? LOG_WARN : LOG_ERROR,
 				    "module %s uses symbol %s from namespace %s, but does not import it.\n",
 				    basename, exp->name, exp->namespace);
@@ -2489,12 +2486,12 @@ static void write_namespace_deps_files(const char *fname)
 
 	list_for_each_entry(mod, &modules, list) {
 
-		if (mod->from_dump || !mod->missing_namespaces)
+		if (mod->from_dump || list_empty(&mod->missing_namespaces))
 			continue;
 
 		buf_printf(&ns_deps_buf, "%s.ko:", mod->name);
 
-		for (ns = mod->missing_namespaces; ns; ns = ns->next)
+		list_for_each_entry(ns, &mod->missing_namespaces, list)
 			buf_printf(&ns_deps_buf, " %s", ns->namespace);
 
 		buf_printf(&ns_deps_buf, "\n");
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 65296eca20a1..2a8c1ad0305e 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -120,9 +120,9 @@ struct module {
 	struct buffer dev_table_buf;
 	char	     srcversion[25];
 	// Missing namespace dependencies
-	struct namespace_list *missing_namespaces;
+	struct list_head missing_namespaces;
 	// Actual imported namespaces
-	struct namespace_list *imported_namespaces;
+	struct list_head imported_namespaces;
 	char name[];
 };
 
-- 
2.32.0

[PATCH v2 09/26] modpost: dump Module.symvers in the same order of modules.order

[Masahiro Yamada]

modpost dumps the exported symbols into Module.symvers, but currently
in random order because it iterates in the hash table.

Add a linked list of exported symbols in struct module, so we can
iterate on symbols per module.

This commit makes Module.symvers much more readable; the outer loop in
write_dump() iterates over the modules in the order of modules.order,
and the inner loop dumps symbols in each module.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---

(no changes since v1)

 scripts/mod/modpost.c | 29 +++++++++++++----------------
 scripts/mod/modpost.h |  1 +
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index d0cf94e1e984..cd49ef7b5953 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -185,6 +185,7 @@ static struct module *new_module(const char *modname)
 	mod = NOFAIL(malloc(sizeof(*mod) + strlen(modname) + 1));
 	memset(mod, 0, sizeof(*mod));
 
+	INIT_LIST_HEAD(&mod->exported_symbols);
 	INIT_LIST_HEAD(&mod->unresolved_symbols);
 	INIT_LIST_HEAD(&mod->missing_namespaces);
 	INIT_LIST_HEAD(&mod->imported_namespaces);
@@ -211,7 +212,7 @@ static struct module *new_module(const char *modname)
 
 struct symbol {
 	struct symbol *next;
-	struct list_head list;	/* link to module::unresolved_symbols */
+	struct list_head list;	/* link to module::exported_symbols or module::unresolved_symbols */
 	struct module *module;
 	char *namespace;
 	unsigned int crc;
@@ -413,6 +414,7 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,
 
 	if (!s) {
 		s = new_symbol(name, mod, export);
+		list_add_tail(&s->list, &mod->exported_symbols);
 	} else if (!external_module || s->module->is_vmlinux ||
 		   s->module == mod) {
 		warn("%s: '%s' exported twice. Previous export was in %s%s\n",
@@ -2456,22 +2458,17 @@ static void read_dump(const char *fname)
 static void write_dump(const char *fname)
 {
 	struct buffer buf = { };
-	struct symbol *symbol;
-	const char *namespace;
-	int n;
+	struct module *mod;
+	struct symbol *sym;
 
-	for (n = 0; n < SYMBOL_HASH_SIZE ; n++) {
-		symbol = symbolhash[n];
-		while (symbol) {
-			if (!symbol->module->from_dump) {
-				namespace = symbol->namespace;
-				buf_printf(&buf, "0x%08x\t%s\t%s\t%s\t%s\n",
-					   symbol->crc, symbol->name,
-					   symbol->module->name,
-					   export_str(symbol->export),
-					   namespace ? namespace : "");
-			}
-			symbol = symbol->next;
+	list_for_each_entry(mod, &modules, list) {
+		if (mod->from_dump)
+			continue;
+		list_for_each_entry(sym, &mod->exported_symbols, list) {
+			buf_printf(&buf, "0x%08x\t%s\t%s\t%s\t%s\n",
+				   sym->crc, sym->name, mod->name,
+				   export_str(sym->export),
+				   sym->namespace ?: "");
 		}
 	}
 	write_buf(&buf, fname);
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 2a8c1ad0305e..1c2d6498d764 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -111,6 +111,7 @@ buf_write(struct buffer *buf, const char *s, int len);
 
 struct module {
 	struct list_head list;
+	struct list_head exported_symbols;
 	struct list_head unresolved_symbols;
 	bool is_gpl_compatible;
 	bool from_dump;		/* true if module was loaded from *.symvers */
-- 
2.32.0

[PATCH v2 10/26] modpost: move static EXPORT_SYMBOL check to check_exports()

[Masahiro Yamada]

Now that struct module has a list of symbols it exports, this check
can go into check_exports(). The code becomes shorter.

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

(no changes since v1)

 scripts/mod/modpost.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index cd49ef7b5953..4ce8d164b8e0 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2199,6 +2199,12 @@ static void check_exports(struct module *mod)
 		if (!mod->is_gpl_compatible)
 			check_for_gpl_usage(exp->export, basename, exp->name);
 	}
+
+	list_for_each_entry(s, &mod->exported_symbols, list) {
+		if (s->is_static)
+			error("\"%s\" [%s] is a static %s\n",
+			      s->name, mod->name, export_str(s->export));
+	}
 }
 
 static void check_modname_len(struct module *mod)
@@ -2510,7 +2516,6 @@ int main(int argc, char **argv)
 	char *missing_namespace_deps = NULL;
 	char *dump_write = NULL, *files_source = NULL;
 	int opt;
-	int n;
 	LIST_HEAD(dump_lists);
 	struct dump_list *dl, *dl2;
 
@@ -2606,16 +2611,6 @@ int main(int argc, char **argv)
 	if (sec_mismatch_count && !sec_mismatch_warn_only)
 		error("Section mismatches detected.\n"
 		      "Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.\n");
-	for (n = 0; n < SYMBOL_HASH_SIZE; n++) {
-		struct symbol *s;
-
-		for (s = symbolhash[n]; s; s = s->next) {
-			if (s->is_static)
-				error("\"%s\" [%s] is a static %s\n",
-				      s->name, s->module->name,
-				      export_str(s->export));
-		}
-	}
 
 	if (nr_unresolved > MAX_UNRESOLVED_REPORTS)
 		warn("suppressed %u unresolved symbol warnings because there were too many)\n",
-- 
2.32.0

[PATCH v2 11/26] modpost: make multiple export error

[Masahiro Yamada]

This is currently a warning, but I think modpost should stop building
in this case.

If the same symbol is exported multiple times and we let it keep going,
the sanity check becomes difficult.

Only the legitimate case is that an external module overrides the
corresponding in-tree module to provide a different implementation
with the same interface.

Also, there exists an upstream example that exploits this feature.

  $ make M=tools/testing/nvdimm

... builds tools/testing/nvdimm/libnvdimm.ko. This is a mocked module
that overrides the symbols from drivers/nvdimm/libnvdimm.ko.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---

Changes in v2:
  - Add more commit description

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

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 4ce8d164b8e0..1f01fc942f94 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -417,9 +417,9 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,
 		list_add_tail(&s->list, &mod->exported_symbols);
 	} else if (!external_module || s->module->is_vmlinux ||
 		   s->module == mod) {
-		warn("%s: '%s' exported twice. Previous export was in %s%s\n",
-		     mod->name, name, s->module->name,
-		     s->module->is_vmlinux ? "" : ".ko");
+		error("%s: '%s' exported twice. Previous export was in %s%s\n",
+		      mod->name, name, s->module->name,
+		      s->module->is_vmlinux ? "" : ".ko");
 		return s;
 	}
 
-- 
2.32.0

[PATCH v2 12/26] modpost: make sym_add_exported() always allocate a new symbol

[Masahiro Yamada]

Currently, sym_add_exported() does not allocate a symbol if the same
name symbol already exists in the hash table.

This does not reflect the real use cases. You can let an external
module override the in-tree one. In this case, the external module
will export the same name symbols as the in-tree one. However,
modpost simply ignores those symbols, then Module.symvers for the
external module loses its symbols.

sym_add_exported() should allocate a new symbol.

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

(no changes since v1)

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

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 1f01fc942f94..c9b75697d0fc 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -412,19 +412,17 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,
 {
 	struct symbol *s = find_symbol(name);
 
-	if (!s) {
-		s = new_symbol(name, mod, export);
-		list_add_tail(&s->list, &mod->exported_symbols);
-	} else if (!external_module || s->module->is_vmlinux ||
-		   s->module == mod) {
+	if (s && (!external_module || s->module->is_vmlinux || s->module == mod)) {
 		error("%s: '%s' exported twice. Previous export was in %s%s\n",
 		      mod->name, name, s->module->name,
 		      s->module->is_vmlinux ? "" : ".ko");
-		return s;
 	}
 
+	s = new_symbol(name, mod, export);
 	s->module = mod;
 	s->export    = export;
+	list_add_tail(&s->list, &mod->exported_symbols);
+
 	return s;
 }
 
-- 
2.32.0

[PATCH v2 13/26] modpost: split new_symbol() to symbol allocation and hash table addition

[Masahiro Yamada]

new_symbol() does two things; allocate a new symbol and register it
to the hash table.

Using a separate function for each is easier to understand.

Replace new_symbol() with hash_add_symbol(). Remove the second parameter
of alloc_symbol().

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

Changes in v2:
  - New patch

 scripts/mod/modpost.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index c9b75697d0fc..b9f359d10968 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -242,34 +242,31 @@ static inline unsigned int tdb_hash(const char *name)
  * Allocate a new symbols for use in the hash of exported symbols or
  * the list of unresolved symbols per module
  **/
-static struct symbol *alloc_symbol(const char *name, struct symbol *next)
+static struct symbol *alloc_symbol(const char *name)
 {
 	struct symbol *s = NOFAIL(malloc(sizeof(*s) + strlen(name) + 1));
 
 	memset(s, 0, sizeof(*s));
 	strcpy(s->name, name);
-	s->next = next;
 	s->is_static = true;
 	return s;
 }
 
 /* For the hash of exported symbols */
-static struct symbol *new_symbol(const char *name, struct module *module,
-				 enum export export)
+static void hash_add_symbol(struct symbol *sym)
 {
 	unsigned int hash;
 
-	hash = tdb_hash(name) % SYMBOL_HASH_SIZE;
-	symbolhash[hash] = alloc_symbol(name, symbolhash[hash]);
-
-	return symbolhash[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)
 {
 	struct symbol *sym;
 
-	sym = alloc_symbol(name, NULL);
+	sym = alloc_symbol(name);
 	sym->weak = weak;
 
 	list_add_tail(&sym->list, &mod->unresolved_symbols);
@@ -418,10 +415,11 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,
 		      s->module->is_vmlinux ? "" : ".ko");
 	}
 
-	s = new_symbol(name, mod, export);
+	s = alloc_symbol(name);
 	s->module = mod;
 	s->export    = export;
 	list_add_tail(&s->list, &mod->exported_symbols);
+	hash_add_symbol(s);
 
 	return s;
 }
-- 
2.32.0

[PATCH v2 14/26] modpost: mitigate false-negatives for static EXPORT_SYMBOL checks

[Masahiro Yamada]

The 'static' specifier and EXPORT_SYMBOL() are an odd combination.

Since commit 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL*
functions"), modpost tries to detect it, but there are false negatives.

Here is the sample code.

[Sample 1]

  Makefile:

    obj-m += mymod1.o mymod2.o

  mymod1.c:

    #include <linux/export.h>
    #include <linux/module.h>
    static void foo(void) {}
    EXPORT_SYMBOL(foo);
    MODULE_LICENSE("GPL");

  mymod2.c:

    #include <linux/module.h>
    void foo(void) {}
    MODULE_LICENSE("GPL");

mymod1 exports the static symbol 'foo', but modpost cannot catch it
because it is confused by the same name symbol in another module, mymod2.
(Without mymod2, modpost can detect the error in mymod1)

find_symbol() returns the first symbol found in the hash table with the
given name. This hash table is global, so it may return a symbol from
an unrelated module. So, a global symbol in a module may unset the
'is_static' flag of a different module.

To mitigate this issue, add sym_find_with_module(), which receives the
module pointer as the second argument. If non-NULL pointer is passed, it
returns the symbol in the specified module. If NULL is passed, it is
equivalent to find_module().

Please note there are still false positives in the composite module,
like below (or when both are built-in). I have no idea how to do this
correctly.

[Sample 2]  (not fixed by this commit)

  Makefile:
    obj-m += mymod.o
    mymod-objs := mymod1.o mymod2.o

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

Changes in v2:
  - Rename the new func to sym_find_with_module()

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

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index b9f359d10968..375b9463cb8a 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -272,7 +272,7 @@ static void sym_add_unresolved(const char *name, struct module *mod, bool weak)
 	list_add_tail(&sym->list, &mod->unresolved_symbols);
 }
 
-static struct symbol *find_symbol(const char *name)
+static struct symbol *sym_find_with_module(const char *name, struct module *mod)
 {
 	struct symbol *s;
 
@@ -281,12 +281,17 @@ static struct symbol *find_symbol(const char *name)
 		name++;
 
 	for (s = symbolhash[tdb_hash(name) % SYMBOL_HASH_SIZE]; s; s = s->next) {
-		if (strcmp(s->name, name) == 0)
+		if (strcmp(s->name, name) == 0 && (!mod || s->module == mod))
 			return s;
 	}
 	return NULL;
 }
 
+static struct symbol *find_symbol(const char *name)
+{
+	return sym_find_with_module(name, NULL);
+}
+
 struct namespace_list {
 	struct list_head list;
 	char namespace[];
@@ -2063,8 +2068,9 @@ static void read_symbols(const char *modname)
 
 		if (bind == STB_GLOBAL || bind == STB_WEAK) {
 			struct symbol *s =
-				find_symbol(remove_dot(info.strtab +
-						       sym->st_name));
+				sym_find_with_module(remove_dot(info.strtab +
+								sym->st_name),
+						     mod);
 
 			if (s)
 				s->is_static = false;
-- 
2.32.0

[PATCH v2 15/26] kbuild: record symbol versions in *.cmd files

[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 <masahiroy@kernel.org>
Tested-by: Nicolas Schier <nicolas@fjasle.eu>
Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>
---

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

[PATCH v2 16/26] kbuild: generate a list of objects in vmlinux

[Masahiro Yamada]

A *.mod file lists the member objects of a module, but vmlinux does
not have such a file to list out all the member objects.

Generate this list to allow modpost to know all the member objects.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>
---

Changes in v2:
  - Move '> .vmlinux.objs' to the outside of the loop (Nicolas)
  - Clean up .vmlinux.objs explicitly

 scripts/link-vmlinux.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 20f44504a644..eceb3ee7ec06 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -311,6 +311,7 @@ cleanup()
 	rm -f vmlinux.map
 	rm -f vmlinux.o
 	rm -f .vmlinux.d
+	rm -f .vmlinux.objs
 }
 
 # Use "make V=1" to debug this script
@@ -342,6 +343,16 @@ ${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init need-builtin=1
 modpost_link vmlinux.o
 objtool_link vmlinux.o
 
+# Generate the list of objects in vmlinux
+for f in ${KBUILD_VMLINUX_OBJS} ${KBUILD_VMLINUX_LIBS}; do
+	case ${f} in
+	*.a)
+		${AR} t ${f} ;;
+	*)
+		echo ${f} ;;
+	esac
+done > .vmlinux.objs
+
 # modpost vmlinux.o to check for section mismatches
 ${MAKE} -f "${srctree}/scripts/Makefile.modpost" MODPOST_VMLINUX=1
 
-- 
2.32.0

[PATCH v2 17/26] modpost: extract symbol versions from *.cmd files

[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        |---------|              |------------|
       $(CC)        $(LD)          |         |              | final link |
  *.c ------> *.o --------> *.o -->| modpost |-- *.o ------>| for        |
                      /            |         |-- *.mod.c -->| vmlinux    |
     genksyms        /             |         |              | or 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 embed CRCs at the very
last moment.

New implementation
==================

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

We can pass the symbol versions to modpost as separate text data.
(The previous commit output the versions in *.cmd files.)

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

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

Changes in v2:
  - Simplify the implementation (parse .cmd files after ELF)

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

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 375b9463cb8a..d8df0f8d3def 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -429,19 +429,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)
@@ -664,33 +655,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)
 {
@@ -1997,6 +1961,111 @@ 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_from_cmdfile(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) {
+			error("invalid\n");
+			goto out;
+		}
+
+		namelen = p - name;
+
+		p++;
+
+		if (!isdigit(*p)) {
+			error("invalid\n");
+			goto out;
+		}
+
+		crc = strtol(p, &p, 0);
+
+		if (*p != '\n') {
+			error("invalid\n");
+			goto out;
+		}
+
+		name[namelen] = '\0';
+
+		sym = sym_find_with_module(name, mod);
+		if (!sym) {
+			warn("Skip the version for unexported symbol \"%s\" [%s%s]",
+			     name, mod->name, mod->is_vmlinux ? "" : ".ko");
+			continue;
+		}
+		sym_set_crc(sym, crc);
+	}
+
+out:
+	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_from_cmdfile(obj, mod);
+
+	free(buf);
+}
+
 static void read_symbols(const char *modname)
 {
 	const char *symname;
@@ -2057,9 +2126,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
@@ -2088,12 +2154,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)
@@ -2453,7 +2524,7 @@ static void read_dump(const char *fname)
 		}
 		s = sym_add_exported(symname, mod, export_no(export));
 		s->is_static = false;
-		sym_set_crc(symname, crc);
+		sym_set_crc(s, crc);
 		sym_update_namespace(symname, namespace);
 	}
 	free(buf);
@@ -2483,6 +2554,20 @@ static void write_dump(const char *fname)
 	free(buf.p);
 }
 
+static void check_symversions(struct module *mod)
+{
+	struct symbol *sym;
+
+	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);
+		}
+	}
+}
+
 static void write_namespace_deps_files(const char *fname)
 {
 	struct module *mod;
@@ -2579,6 +2664,9 @@ int main(int argc, char **argv)
 		char fname[PATH_MAX];
 		int ret;
 
+		if (modversions && !mod->from_dump)
+			check_symversions(mod);
+
 		if (mod->is_vmlinux || mod->from_dump)
 			continue;
 
-- 
2.32.0

[PATCH v2 18/26] modpost: generate linker script to collect symbol versions

[Masahiro Yamada]

Merge version CRCs per vmlinux or per module.

These linker scripts will be fed to the final link stage.

Add the new option, -r,  to modpost. This is needed to output the
linker scripts in a different format when CONFIG_MODULE_REL_CRCS=y.

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

Changes in v2:
  - Fix CONFIG_MODULE_REL_CRCS=y case

 .gitignore               |  1 +
 Makefile                 |  1 +
 scripts/Makefile.modpost |  1 +
 scripts/mod/modpost.c    | 37 ++++++++++++++++++++++++++++++++++---
 4 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/.gitignore b/.gitignore
index 265959544978..f9dad6b917e6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -42,6 +42,7 @@
 *.so.dbg
 *.su
 *.symtypes
+*.symver.lds
 *.symversions
 *.tab.[ch]
 *.tar
diff --git a/Makefile b/Makefile
index 7a82bbc505f8..79a69ffd5379 100644
--- a/Makefile
+++ b/Makefile
@@ -1867,6 +1867,7 @@ clean: $(clean-dirs)
 		-o -name '*.c.[012]*.*' \
 		-o -name '*.ll' \
 		-o -name '*.gcno' \
+		-o -name '*.symver.lds' \
 		-o -name '*.*.symversions' \) -type f -print | xargs rm -f
 
 # Generate tags for editors
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 48585c4d04ad..fecd721537c9 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -46,6 +46,7 @@ include $(srctree)/scripts/Makefile.lib
 
 MODPOST = scripts/mod/modpost								\
 	$(if $(CONFIG_MODVERSIONS),-m)							\
+	$(if $(CONFIG_MODULE_REL_CRCS),-r)						\
 	$(if $(CONFIG_MODULE_SRCVERSION_ALL),-a)					\
 	$(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E)					\
 	-o $@
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index d8df0f8d3def..935f57f73e40 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -24,6 +24,8 @@
 
 /* Are we using CONFIG_MODVERSIONS? */
 static bool modversions;
+/* Is CONFIG_MODULE_REL_CRCS enabled? */
+static bool modversions_rel_crcs;
 /* Is CONFIG_MODULE_SRCVERSION_ALL set? */
 static bool all_versions;
 /* If we are modposting external module set to 1 */
@@ -2554,9 +2556,21 @@ static void write_dump(const char *fname)
 	free(buf.p);
 }
 
-static void check_symversions(struct module *mod)
+static void write_symversions_lds(struct module *mod)
 {
+	struct buffer buf = { };
 	struct symbol *sym;
+	char lds_file[PATH_MAX];
+	int ret;
+
+	ret = snprintf(lds_file, sizeof(lds_file), "%s.symver.lds", mod->name);
+	if (ret >= sizeof(lds_file)) {
+		error("%s: too long path was truncated\n", lds_file);
+		return;
+	}
+
+	if (modversions_rel_crcs)
+		buf_printf(&buf, "SECTIONS { .rodata : ALIGN(4) {\n");
 
 	list_for_each_entry(sym, &mod->exported_symbols, list) {
 		if (!sym->crc_valid) {
@@ -2564,8 +2578,22 @@ static void check_symversions(struct module *mod)
 			     "Is \"%s\" prototyped in <asm/asm-prototypes.h>?\n",
 			     sym->name, mod->name, mod->is_vmlinux ? "" : ".ko",
 			     sym->name);
+			continue;
 		}
+
+		if (modversions_rel_crcs)
+			buf_printf(&buf, "__crc_%s = .; LONG(0x%08x);\n",
+				   sym->name, sym->crc);
+		else
+			buf_printf(&buf, "__crc_%s = 0x%08x;\n",
+				   sym->name, sym->crc);
 	}
+
+	if (modversions_rel_crcs)
+		buf_printf(&buf, "} }\n");
+
+	write_if_changed(&buf, lds_file);
+	free(buf.p);
 }
 
 static void write_namespace_deps_files(const char *fname)
@@ -2606,7 +2634,7 @@ int main(int argc, char **argv)
 	LIST_HEAD(dump_lists);
 	struct dump_list *dl, *dl2;
 
-	while ((opt = getopt(argc, argv, "ei:mnT:o:awENd:")) != -1) {
+	while ((opt = getopt(argc, argv, "ei:mrnT:o:awENd:")) != -1) {
 		switch (opt) {
 		case 'e':
 			external_module = true;
@@ -2619,6 +2647,9 @@ int main(int argc, char **argv)
 		case 'm':
 			modversions = true;
 			break;
+		case 'r':
+			modversions_rel_crcs = true;
+			break;
 		case 'n':
 			ignore_missing_files = true;
 			break;
@@ -2665,7 +2696,7 @@ int main(int argc, char **argv)
 		int ret;
 
 		if (modversions && !mod->from_dump)
-			check_symversions(mod);
+			write_symversions_lds(mod);
 
 		if (mod->is_vmlinux || mod->from_dump)
 			continue;
-- 
2.32.0

[PATCH v2 19/26] kbuild: embed symbol versions at final link of vmlinux or modules

[Masahiro Yamada]

Do not update objects with version CRCs while the directory descending.

Do it at the final link stage.

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

(no changes since v1)

 scripts/Makefile.build    | 22 +++-------------------
 scripts/Makefile.modfinal |  3 ++-
 scripts/link-vmlinux.sh   |  4 +++-
 3 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index a1023868775f..cec17b28de42 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -162,15 +162,9 @@ 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
-
-# Generate .o.symversions files for each .o with exported symbols, and link these
-# to the kernel and/or modules at the end.
+#   genksyms on the preprocessed source and dump them into the .cmd file.
+# o modpost will extract versions from the .cmd file and create linker
+#   scripts used to link the kernel and/or modules.
 
 genksyms_format_rel_crc := [^_]*__crc_\([^ ]*\) = \.; LONG(\([^)]*\)).*
 genksyms_format_normal := __crc_\(.*\) = \(.*\);
@@ -188,12 +182,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 +261,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 +269,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 +282,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/Makefile.modfinal b/scripts/Makefile.modfinal
index 7f39599e9fae..d429e3f9ae1d 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -34,6 +34,7 @@ quiet_cmd_ld_ko_o = LD [M]  $@
       cmd_ld_ko_o +=							\
 	$(LD) -r $(KBUILD_LDFLAGS)					\
 		$(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE)		\
+		$(addprefix -T, $(filter %.symver.lds, $(real-prereqs)))\
 		-T scripts/module.lds -o $@ $(filter %.o, $^);		\
 	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
 
@@ -56,7 +57,7 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check),      \
 
 
 # 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: %$(mod-prelink-ext).o %.mod.o $(if $(CONFIG_MODVERSIONS), %.symver.lds) 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/link-vmlinux.sh b/scripts/link-vmlinux.sh
index eceb3ee7ec06..8da5c0182665 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -90,7 +90,6 @@ modpost_link()
 
 		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
@@ -196,6 +195,9 @@ vmlinux_link()
 	fi
 
 	ldflags="${ldflags} ${wl}--script=${objtree}/${KBUILD_LDS}"
+	if is_enabled CONFIG_MODVERSIONS; then
+		ldflags="${ldflags} ${wl}--script=vmlinux.symver.lds"
+	fi
 
 	# The kallsyms linking does not need debug symbols included.
 	if [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
-- 
2.32.0

[PATCH v2 20/26] kbuild: stop merging *.symversions

[Masahiro Yamada]

Now modpost takes care of all of these.

Clean up the Makefile and script.

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

(no changes since v1)

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

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index cec17b28de42..10092efb61ac 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -393,17 +393,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)
 #
@@ -411,11 +400,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
@@ -435,16 +421,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 8da5c0182665..77e343835743 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()
@@ -88,10 +74,6 @@ modpost_link()
 		gen_initcalls
 		lds="-T .tmp_initcalls.lds"
 
-		if is_enabled CONFIG_MODVERSIONS; then
-			gen_symversions
-		fi
-
 		# This might take a while, so indicate that we're doing
 		# an LTO link
 		info LTO ${1}
@@ -306,7 +288,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

[PATCH v2 21/26] genksyms: adjust the output format for .cmd files

[Masahiro Yamada]

genksyms output symbol versions in the linker script format.
The output format depends on CONFIG_MODULE_REL_CRCS.

Now, symbol versions are passed to modpost as plain text data,
we can simplify the genksyms code.

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

Changes in v2:
  - New patch

 scripts/Makefile.build      |  9 ---------
 scripts/genksyms/genksyms.c | 17 ++++-------------
 2 files changed, 4 insertions(+), 22 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 10092efb61ac..1f480e4ff70a 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)
 
@@ -166,18 +165,10 @@ ifdef CONFIG_MODVERSIONS
 # o modpost will extract versions from the .cmd file and create linker
 #   scripts used to link the kernel and/or modules.
 
-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
 
 cmd_gen_symversions_c =	$(call gen_symversions,c)
diff --git a/scripts/genksyms/genksyms.c b/scripts/genksyms/genksyms.c
index 4827c5abe5b7..ec5ad4405483 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;
@@ -680,11 +680,7 @@ void export_symbol(const char *name)
 		if (flag_dump_defs)
 			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("#SYMVER %s 0x%08lx\n", name, crc);
 	}
 }
 
@@ -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;
-- 
2.32.0

[PATCH v2 22/26] kbuild: do not create *.prelink.o for Clang LTO or IBT

[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. Of course, only $(CC) is used when LTO is disabled.

           $(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 not a 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 <masahiroy@kernel.org>
---

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 1f480e4ff70a..cc8f362f58e2 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 d429e3f9ae1d..51d384a0e4f9 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
@@ -55,9 +55,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 $(if $(CONFIG_MODVERSIONS), %.symver.lds) scripts/module.lds $(if $(KBUILD_BUILTIN),vmlinux) FORCE
+$(modules): %.ko: %.o %.mod.o $(if $(CONFIG_MODVERSIONS), %.symver.lds) 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 fecd721537c9..01e1c8b88a67 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_REL_CRCS),-r)						\
@@ -119,8 +116,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
@@ -129,9 +124,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 935f57f73e40..1f2f53688d40 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1955,10 +1955,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;
 }
@@ -2087,9 +2083,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

[PATCH v2 23/26] kbuild: make built-in.a rule robust against too long argument error

[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 <masahiroy@kernel.org>
---

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 cc8f362f58e2..a9236add4864 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

[PATCH v2 24/26] kbuild: make *.mod rule robust against too long argument error

[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/4c02050c4e95e4cb8cc04282695f8404@codeaurora.org/

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

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 a9236add4864..b0718076f50b 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

[PATCH v2 25/26] modpost: simplify the ->is_static initialization

[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 <masahiroy@kernel.org>
---

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 1f2f53688d40..85fcac84d2d1 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -250,7 +250,7 @@ static struct symbol *alloc_symbol(const char *name)
 
 	memset(s, 0, sizeof(*s));
 	strcpy(s->name, name);
-	s->is_static = true;
+
 	return s;
 }
 
@@ -424,6 +424,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->export    = export;
 	list_add_tail(&s->list, &mod->exported_symbols);
 	hash_add_symbol(s);
@@ -2518,7 +2519,6 @@ static void read_dump(const char *fname)
 			mod->from_dump = true;
 		}
 		s = sym_add_exported(symname, mod, export_no(export));
-		s->is_static = false;
 		sym_set_crc(s, crc);
 		sym_update_namespace(symname, namespace);
 	}
-- 
2.32.0

[PATCH v2 26/26] modpost: use hlist for hash table implementation

[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 <masahiroy@kernel.org>
---

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 85fcac84d2d1..ebd544717b91 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -207,13 +207,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;
@@ -225,8 +220,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)
 {
@@ -240,6 +233,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
@@ -254,15 +262,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)
 {
@@ -282,8 +281,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;
@@ -427,7 +426,7 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,
 	s->is_static = !mod->from_dump;
 	s->export    = export;
 	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 1c2d6498d764..180eb142375e 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

Re: [PATCH v2 00/26] kbuild: yet another series of cleanups (modpost and LTO)

[Masahiro Yamada]

On Sun, May 1, 2022 at 5:42 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
>
> This is the third batch of cleanups in this development cycle.
>
> 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
>


This series is available at:

git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git
lto-cleanup-v2







>
> Masahiro Yamada (26):
>   modpost: use bool type where appropriate
>   modpost: change mod->gpl_compatible to bool type
>   modpost: import include/linux/list.h
>   modpost: traverse modules in order
>   modpost: add sym_add_unresolved() helper
>   modpost: traverse unresolved symbols in order
>   modpost: use doubly linked list for dump_lists
>   modpost: traverse the namespace_list in order
>   modpost: dump Module.symvers in the same order of modules.order
>   modpost: move static EXPORT_SYMBOL check to check_exports()
>   modpost: make multiple export error
>   modpost: make sym_add_exported() always allocate a new symbol
>   modpost: split new_symbol() to symbol allocation and hash table
>     addition
>   modpost: mitigate false-negatives for static EXPORT_SYMBOL checks
>   kbuild: record symbol versions in *.cmd files
>   kbuild: generate a list of objects in vmlinux
>   modpost: extract symbol versions from *.cmd files
>   modpost: generate linker script to collect symbol versions
>   kbuild: embed symbol versions at final link of vmlinux or modules
>   kbuild: stop merging *.symversions
>   genksyms: adjust the output format for .cmd files
>   kbuild: do not create *.prelink.o for Clang LTO or IBT
>   kbuild: make built-in.a rule robust against too long argument error
>   kbuild: make *.mod rule robust against too long argument error
>   modpost: simplify the ->is_static initialization
>   modpost: use hlist for hash table implementation
>
>  .gitignore                  |   1 +
>  Makefile                    |   1 +
>  scripts/Kbuild.include      |   4 +
>  scripts/Makefile.build      | 118 +++------
>  scripts/Makefile.lib        |   7 -
>  scripts/Makefile.modfinal   |   6 +-
>  scripts/Makefile.modpost    |  10 +-
>  scripts/genksyms/genksyms.c |  17 +-
>  scripts/link-vmlinux.sh     |  34 +--
>  scripts/mod/file2alias.c    |   2 -
>  scripts/mod/list.h          | 265 +++++++++++++++++++
>  scripts/mod/modpost.c       | 501 ++++++++++++++++++++++--------------
>  scripts/mod/modpost.h       |  24 +-
>  scripts/mod/sumversion.c    |   8 +-
>  14 files changed, 650 insertions(+), 348 deletions(-)
>  create mode 100644 scripts/mod/list.h
>
> --
> 2.32.0
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 19/26] kbuild: embed symbol versions at final link of vmlinux or modules

[Masahiro Yamada]

On Sun, May 1, 2022 at 5:42 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Do not update objects with version CRCs while the directory descending.
>
> Do it at the final link stage.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>


As 0-day bot reported, this does not work for CONFIG_MODULE_REL_CRCS.

I will send v3.


> ---
>
> (no changes since v1)
>
>  scripts/Makefile.build    | 22 +++-------------------
>  scripts/Makefile.modfinal |  3 ++-
>  scripts/link-vmlinux.sh   |  4 +++-
>  3 files changed, 8 insertions(+), 21 deletions(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index a1023868775f..cec17b28de42 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -162,15 +162,9 @@ 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
> -
> -# Generate .o.symversions files for each .o with exported symbols, and link these
> -# to the kernel and/or modules at the end.
> +#   genksyms on the preprocessed source and dump them into the .cmd file.
> +# o modpost will extract versions from the .cmd file and create linker
> +#   scripts used to link the kernel and/or modules.
>
>  genksyms_format_rel_crc := [^_]*__crc_\([^ ]*\) = \.; LONG(\([^)]*\)).*
>  genksyms_format_normal := __crc_\(.*\) = \(.*\);
> @@ -188,12 +182,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 +261,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 +269,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 +282,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/Makefile.modfinal b/scripts/Makefile.modfinal
> index 7f39599e9fae..d429e3f9ae1d 100644
> --- a/scripts/Makefile.modfinal
> +++ b/scripts/Makefile.modfinal
> @@ -34,6 +34,7 @@ quiet_cmd_ld_ko_o = LD [M]  $@
>        cmd_ld_ko_o +=                                                   \
>         $(LD) -r $(KBUILD_LDFLAGS)                                      \
>                 $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE)              \
> +               $(addprefix -T, $(filter %.symver.lds, $(real-prereqs)))\
>                 -T scripts/module.lds -o $@ $(filter %.o, $^);          \
>         $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
>
> @@ -56,7 +57,7 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check),      \
>
>
>  # 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: %$(mod-prelink-ext).o %.mod.o $(if $(CONFIG_MODVERSIONS), %.symver.lds) 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/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index eceb3ee7ec06..8da5c0182665 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -90,7 +90,6 @@ modpost_link()
>
>                 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
> @@ -196,6 +195,9 @@ vmlinux_link()
>         fi
>
>         ldflags="${ldflags} ${wl}--script=${objtree}/${KBUILD_LDS}"
> +       if is_enabled CONFIG_MODVERSIONS; then
> +               ldflags="${ldflags} ${wl}--script=vmlinux.symver.lds"
> +       fi
>
>         # The kallsyms linking does not need debug symbols included.
>         if [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
> --
> 2.32.0
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 01/26] modpost: use bool type where appropriate

[Nick Desaulniers]

On Sun, May 1, 2022 at 1:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index 96d6b3a16ca2..7ccfcc8899c1 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -1,4 +1,5 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
> +#include <stdbool.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <stdarg.h>
> @@ -111,11 +112,10 @@ struct module {
>         struct module *next;
>         int gpl_compatible;
>         struct symbol *unres;
> -       int from_dump;  /* 1 if module was loaded from *.symvers */
> -       int is_vmlinux;
> -       int seen;
> -       int has_init;
> -       int has_cleanup;
> +       bool from_dump;         /* true if module was loaded from *.symvers */
> +       bool is_vmlinux;
> +       bool seen;
> +       bool has_init, has_cleanup;

Consider keeping these on separate lines. Either way:
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 02/26] modpost: change mod->gpl_compatible to bool type

[Nick Desaulniers]

On Sun, May 1, 2022 at 1:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Currently, mod->gpl_compatible is tristate; it is set to -1 by default,
> then to 1 or 0 when MODULE_LICENSE() is found.
>
> Maybe, -1 was chosen to represent the 'unknown' license, but it is not
> useful.
>
> The current code:
>
>     if (!mod->gpl_compatible)
>             check_for_gpl_usage(exp->export, basename, exp->name);
>
> ... only cares whether gpl_compatible is zero or not.
>
> Change it to a bool type with the initial value 'true', which has no
> functional change.
>
> The default value should be 'true' instead of 'false'.
>
> Since commit 1d6cd3929360 ("modpost: turn missing MODULE_LICENSE() into
> error"), unknown module license is an error.
>
> The error message, "missing MODULE_LICENSE()" is enough to explain the
> issue. It is not sensible to show another message, "GPL-incompatible
> module ... uses GPL-only symbol".
>
> Add comments to explain this.
>
> While I was here, I renamed gpl_compatible to is_gpl_compatible for
> clarification, and also slightly refactored the code.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 06/26] modpost: traverse unresolved symbols in order

[Nick Desaulniers]

On Sun, May 1, 2022 at 1:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Currently, modpost manages unresolved in a singly linked list; it adds
> a new node to the head, and traverses the list from new to old.
>
> Use a doubly linked list to keep the order in the symbol table in the
> ELF file.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> (no changes since v1)

Technically, you added a comment. :P
https://lore.kernel.org/linux-kbuild/20220424190811.1678416-11-masahiroy@kernel.org/

>
>  scripts/mod/modpost.c | 20 ++++++++++++++------
>  scripts/mod/modpost.h |  2 +-
>  2 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index abcdb0677775..c7dda4cfa497 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -185,6 +185,8 @@ static struct module *new_module(const char *modname)
>         mod = NOFAIL(malloc(sizeof(*mod) + strlen(modname) + 1));
>         memset(mod, 0, sizeof(*mod));
>
> +       INIT_LIST_HEAD(&mod->unresolved_symbols);
> +
>         strcpy(mod->name, modname);
>         mod->is_vmlinux = (strcmp(modname, "vmlinux") == 0);
>
> @@ -207,6 +209,7 @@ static struct module *new_module(const char *modname)
>
>  struct symbol {
>         struct symbol *next;
> +       struct list_head list;  /* link to module::unresolved_symbols */

Thanks. ;)
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 10/26] modpost: move static EXPORT_SYMBOL check to check_exports()

[Nick Desaulniers]

On Sun, May 1, 2022 at 1:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Now that struct module has a list of symbols it exports, this check
> can go into check_exports(). The code becomes shorter.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
> (no changes since v1)
>
>  scripts/mod/modpost.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index cd49ef7b5953..4ce8d164b8e0 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -2199,6 +2199,12 @@ static void check_exports(struct module *mod)
>                 if (!mod->is_gpl_compatible)
>                         check_for_gpl_usage(exp->export, basename, exp->name);
>         }
> +
> +       list_for_each_entry(s, &mod->exported_symbols, list) {
> +               if (s->is_static)
> +                       error("\"%s\" [%s] is a static %s\n",
> +                             s->name, mod->name, export_str(s->export));
> +       }
>  }
>
>  static void check_modname_len(struct module *mod)
> @@ -2510,7 +2516,6 @@ int main(int argc, char **argv)
>         char *missing_namespace_deps = NULL;
>         char *dump_write = NULL, *files_source = NULL;
>         int opt;
> -       int n;
>         LIST_HEAD(dump_lists);
>         struct dump_list *dl, *dl2;
>
> @@ -2606,16 +2611,6 @@ int main(int argc, char **argv)
>         if (sec_mismatch_count && !sec_mismatch_warn_only)
>                 error("Section mismatches detected.\n"
>                       "Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.\n");
> -       for (n = 0; n < SYMBOL_HASH_SIZE; n++) {
> -               struct symbol *s;
> -
> -               for (s = symbolhash[n]; s; s = s->next) {
> -                       if (s->is_static)
> -                               error("\"%s\" [%s] is a static %s\n",
> -                                     s->name, s->module->name,
> -                                     export_str(s->export));
> -               }
> -       }
>
>         if (nr_unresolved > MAX_UNRESOLVED_REPORTS)
>                 warn("suppressed %u unresolved symbol warnings because there were too many)\n",
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 12/26] modpost: make sym_add_exported() always allocate a new symbol

[Nick Desaulniers]

On Sun, May 1, 2022 at 1:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Currently, sym_add_exported() does not allocate a symbol if the same
> name symbol already exists in the hash table.
>
> This does not reflect the real use cases. You can let an external
> module override the in-tree one. In this case, the external module
> will export the same name symbols as the in-tree one. However,
> modpost simply ignores those symbols, then Module.symvers for the
> external module loses its symbols.
>
> sym_add_exported() should allocate a new symbol.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
> (no changes since v1)
>
>  scripts/mod/modpost.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 1f01fc942f94..c9b75697d0fc 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -412,19 +412,17 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,
>  {
>         struct symbol *s = find_symbol(name);
>
> -       if (!s) {
> -               s = new_symbol(name, mod, export);
> -               list_add_tail(&s->list, &mod->exported_symbols);
> -       } else if (!external_module || s->module->is_vmlinux ||
> -                  s->module == mod) {
> +       if (s && (!external_module || s->module->is_vmlinux || s->module == mod)) {
>                 error("%s: '%s' exported twice. Previous export was in %s%s\n",
>                       mod->name, name, s->module->name,
>                       s->module->is_vmlinux ? "" : ".ko");
> -               return s;
>         }
>
> +       s = new_symbol(name, mod, export);
>         s->module = mod;
>         s->export    = export;
> +       list_add_tail(&s->list, &mod->exported_symbols);
> +
>         return s;
>  }
>
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 13/26] modpost: split new_symbol() to symbol allocation and hash table addition

[Nick Desaulniers]

On Sun, May 1, 2022 at 1:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> new_symbol() does two things; allocate a new symbol and register it
> to the hash table.
>
> Using a separate function for each is easier to understand.
>
> Replace new_symbol() with hash_add_symbol(). Remove the second parameter
> of alloc_symbol().
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> Changes in v2:
>   - New patch
>
>  scripts/mod/modpost.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index c9b75697d0fc..b9f359d10968 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -242,34 +242,31 @@ static inline unsigned int tdb_hash(const char *name)
>   * Allocate a new symbols for use in the hash of exported symbols or
>   * the list of unresolved symbols per module
>   **/
> -static struct symbol *alloc_symbol(const char *name, struct symbol *next)
> +static struct symbol *alloc_symbol(const char *name)
>  {
>         struct symbol *s = NOFAIL(malloc(sizeof(*s) + strlen(name) + 1));
>
>         memset(s, 0, sizeof(*s));
>         strcpy(s->name, name);
> -       s->next = next;
>         s->is_static = true;
>         return s;
>  }
>
>  /* For the hash of exported symbols */
> -static struct symbol *new_symbol(const char *name, struct module *module,
> -                                enum export export)

`module` was also previously unused! Yuck. Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> +static void hash_add_symbol(struct symbol *sym)
>  {
>         unsigned int hash;
>
> -       hash = tdb_hash(name) % SYMBOL_HASH_SIZE;
> -       symbolhash[hash] = alloc_symbol(name, symbolhash[hash]);
> -
> -       return symbolhash[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)
>  {
>         struct symbol *sym;
>
> -       sym = alloc_symbol(name, NULL);
> +       sym = alloc_symbol(name);
>         sym->weak = weak;
>
>         list_add_tail(&sym->list, &mod->unresolved_symbols);
> @@ -418,10 +415,11 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,
>                       s->module->is_vmlinux ? "" : ".ko");
>         }
>
> -       s = new_symbol(name, mod, export);
> +       s = alloc_symbol(name);
>         s->module = mod;
>         s->export    = export;
>         list_add_tail(&s->list, &mod->exported_symbols);
> +       hash_add_symbol(s);
>
>         return s;
>  }
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 01/26] modpost: use bool type where appropriate

[Masahiro Yamada]

On Wed, May 4, 2022 at 6:43 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Sun, May 1, 2022 at 1:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> > index 96d6b3a16ca2..7ccfcc8899c1 100644
> > --- a/scripts/mod/modpost.h
> > +++ b/scripts/mod/modpost.h
> > @@ -1,4 +1,5 @@
> >  /* SPDX-License-Identifier: GPL-2.0 */
> > +#include <stdbool.h>
> >  #include <stdio.h>
> >  #include <stdlib.h>
> >  #include <stdarg.h>
> > @@ -111,11 +112,10 @@ struct module {
> >         struct module *next;
> >         int gpl_compatible;
> >         struct symbol *unres;
> > -       int from_dump;  /* 1 if module was loaded from *.symvers */
> > -       int is_vmlinux;
> > -       int seen;
> > -       int has_init;
> > -       int has_cleanup;
> > +       bool from_dump;         /* true if module was loaded from *.symvers */
> > +       bool is_vmlinux;
> > +       bool seen;
> > +       bool has_init, has_cleanup;
>
> Consider keeping these on separate lines. Either way:

OK, I will keep them on separate lines.




> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> --
> Thanks,
> ~Nick Desaulniers



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 21/26] genksyms: adjust the output format for .cmd files

[Nicolas Schier]

On sø. 01. mai 2022 kl. 17.40 Masahiro Yamada wrote:
> genksyms output symbol versions in the linker script format.

output -> outputs ?

> The output format depends on CONFIG_MODULE_REL_CRCS.

Looking at the patch itself, I think the sentence above should be 
inverted, as all rel_crc special handling is removed.  Or did I get it 
wrong?

Kind regards,
Nicolas

> 
> Now, symbol versions are passed to modpost as plain text data,
> we can simplify the genksyms code.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
> Changes in v2:
>   - New patch
> 
>  scripts/Makefile.build      |  9 ---------
>  scripts/genksyms/genksyms.c | 17 ++++-------------
>  2 files changed, 4 insertions(+), 22 deletions(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 10092efb61ac..1f480e4ff70a 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)
>  
> @@ -166,18 +165,10 @@ ifdef CONFIG_MODVERSIONS
>  # o modpost will extract versions from the .cmd file and create linker
>  #   scripts used to link the kernel and/or modules.
>  
> -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
>  
>  cmd_gen_symversions_c =	$(call gen_symversions,c)
> diff --git a/scripts/genksyms/genksyms.c b/scripts/genksyms/genksyms.c
> index 4827c5abe5b7..ec5ad4405483 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;
> @@ -680,11 +680,7 @@ void export_symbol(const char *name)
>  		if (flag_dump_defs)
>  			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("#SYMVER %s 0x%08lx\n", name, crc);
>  	}
>  }
>  
> @@ -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;
> -- 
> 2.32.0

-- 
epost|xmpp: nicolas@fjasle.eu          irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb  c82b 7d97 0932 55a0 ce7f
     -- frykten for herren er opphav til kunnskap --

Re: [PATCH v2 00/26] kbuild: yet another series of cleanups (modpost and LTO)

[Masahiro Yamada]

On Sun, May 1, 2022 at 5:42 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
>
> This is the third batch of cleanups in this development cycle.
>
> 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 (26):
>   modpost: use bool type where appropriate
>   modpost: change mod->gpl_compatible to bool type
>   modpost: import include/linux/list.h
>   modpost: traverse modules in order
>   modpost: add sym_add_unresolved() helper
>   modpost: traverse unresolved symbols in order
>   modpost: use doubly linked list for dump_lists
>   modpost: traverse the namespace_list in order
>   modpost: dump Module.symvers in the same order of modules.order
>   modpost: move static EXPORT_SYMBOL check to check_exports()
>   modpost: make multiple export error
>   modpost: make sym_add_exported() always allocate a new symbol
>   modpost: split new_symbol() to symbol allocation and hash table
>     addition
>   modpost: mitigate false-negatives for static EXPORT_SYMBOL checks
>   kbuild: record symbol versions in *.cmd files
>   kbuild: generate a list of objects in vmlinux
>   modpost: extract symbol versions from *.cmd files
>   modpost: generate linker script to collect symbol versions
>   kbuild: embed symbol versions at final link of vmlinux or modules
>   kbuild: stop merging *.symversions
>   genksyms: adjust the output format for .cmd files
>   kbuild: do not create *.prelink.o for Clang LTO or IBT
>   kbuild: make built-in.a rule robust against too long argument error
>   kbuild: make *.mod rule robust against too long argument error
>   modpost: simplify the ->is_static initialization
>   modpost: use hlist for hash table implementation

Applied 01-13 to linux-kbuild
with Nick's reviewed-by.

I will send v3 for the rest.

>  .gitignore                  |   1 +
>  Makefile                    |   1 +
>  scripts/Kbuild.include      |   4 +
>  scripts/Makefile.build      | 118 +++------
>  scripts/Makefile.lib        |   7 -
>  scripts/Makefile.modfinal   |   6 +-
>  scripts/Makefile.modpost    |  10 +-
>  scripts/genksyms/genksyms.c |  17 +-
>  scripts/link-vmlinux.sh     |  34 +--
>  scripts/mod/file2alias.c    |   2 -
>  scripts/mod/list.h          | 265 +++++++++++++++++++
>  scripts/mod/modpost.c       | 501 ++++++++++++++++++++++--------------
>  scripts/mod/modpost.h       |  24 +-
>  scripts/mod/sumversion.c    |   8 +-
>  14 files changed, 650 insertions(+), 348 deletions(-)
>  create mode 100644 scripts/mod/list.h
>
> --
> 2.32.0
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 21/26] genksyms: adjust the output format for .cmd files

[Masahiro Yamada]

On Thu, May 5, 2022 at 5:23 AM Nicolas Schier <nicolas@fjasle.eu> wrote:
>
> On sø. 01. mai 2022 kl. 17.40 Masahiro Yamada wrote:
> > genksyms output symbol versions in the linker script format.
>
> output -> outputs ?
>
> > The output format depends on CONFIG_MODULE_REL_CRCS.
>
> Looking at the patch itself, I think the sentence above should be
> inverted, as all rel_crc special handling is removed.  Or did I get it
> wrong?

I admit this commit description is unclear.

In v3, the patch is simpler and the commit message is clearer.

Thanks.



-- 
Best Regards
Masahiro Yamada