[PATCH 1/3] kbuild: generate Module.symvers only when vmlinux exists

[PATCH 1/3] kbuild: generate Module.symvers only when vmlinux exists

[Masahiro Yamada]

The external module build shows the following warning if Module.symvers
is missing in the kernel tree.

  WARNING: Symbol version dump "Module.symvers" is missing.
           Modules may not have dependencies or modversions.

I think this is an important heads-up because the resulting modules may
not work as expected. This happens when you did not build the entire
kernel tree, for example, you might have prepared the minimal setups
for external modules by 'make defconfig && make modules_preapre'.

A problem is that 'make modules' creates Module.symvers even without
vmlinux. In this case, that warning is suppressed since Module.symvers
already exists in spite of its incomplete content.

The incomplete (i.e. invalid) Module.symvers should not be created.

This commit changes the second pass of modpost to dump symbols into
modules-only.symvers. The final Module.symvers is created by
concatenating vmlinux.symvers and modules-only.symvers if both exist.

Module.symvers is supposed to contain all the symbols from both vmlinux
and modules. It might be a bit confusing, and I am not quite sure if it
is an official interface, but presumably it is difficult to rename it
because some projects (e.g. kmod) parse it.

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

 .gitignore               |  1 +
 Documentation/dontdiff   |  1 +
 Makefile                 |  2 +-
 scripts/Makefile.modpost | 15 ++++++++++++++-
 scripts/mod/modpost.c    | 15 +--------------
 5 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/.gitignore b/.gitignore
index 3adea59847ce..df8d3146a43f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -57,6 +57,7 @@ modules.order
 /tags
 /TAGS
 /linux
+/modules-only.symvers
 /vmlinux
 /vmlinux.32
 /vmlinux.map
diff --git a/Documentation/dontdiff b/Documentation/dontdiff
index ac42ad8d430d..910b30a2a7d9 100644
--- a/Documentation/dontdiff
+++ b/Documentation/dontdiff
@@ -178,6 +178,7 @@ mktables
 mktree
 mkutf8data
 modpost
+modules-only.symvers
 modules.builtin
 modules.builtin.modinfo
 modules.nsdeps
diff --git a/Makefile b/Makefile
index 2b161f5a5a66..ed8bd815e8a3 100644
--- a/Makefile
+++ b/Makefile
@@ -1532,7 +1532,7 @@ endif # CONFIG_MODULES
 # make distclean Remove editor backup files, patch leftover files and the like
 
 # Directories & files removed with 'make clean'
-CLEAN_FILES += include/ksym vmlinux.symvers \
+CLEAN_FILES += include/ksym vmlinux.symvers modules-only.symvers \
 	       modules.builtin modules.builtin.modinfo modules.nsdeps \
 	       compile_commands.json .thinlto-cache
 
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index df57e259fac3..3f5b09a09aef 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -68,7 +68,20 @@ else
 ifeq ($(KBUILD_EXTMOD),)
 
 input-symdump := vmlinux.symvers
-output-symdump := Module.symvers
+output-symdump := modules-only.symvers
+
+quiet_cmd_cat = GEN     $@
+      cmd_cat = cat $(real-prereqs) > $@
+
+ifneq ($(wildcard vmlinux.symvers),)
+
+__modpost: Module.symvers
+Module.symvers: vmlinux.symvers modules-only.symvers FORCE
+	$(call if_changed,cat)
+
+targets += Module.symvers
+
+endif
 
 else
 
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 24725e50c7b4..10c3fba26f03 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2423,19 +2423,6 @@ static void read_dump(const char *fname)
 	fatal("parse error in symbol dump file\n");
 }
 
-/* For normal builds always dump all symbols.
- * For external modules only dump symbols
- * that are not read from kernel Module.symvers.
- **/
-static int dump_sym(struct symbol *sym)
-{
-	if (!external_module)
-		return 1;
-	if (sym->module->from_dump)
-		return 0;
-	return 1;
-}
-
 static void write_dump(const char *fname)
 {
 	struct buffer buf = { };
@@ -2446,7 +2433,7 @@ static void write_dump(const char *fname)
 	for (n = 0; n < SYMBOL_HASH_SIZE ; n++) {
 		symbol = symbolhash[n];
 		while (symbol) {
-			if (dump_sym(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,
-- 
2.27.0

[PATCH 2/3] kbuild: do not set -w for vmlinux.o modpost

[Masahiro Yamada]

The -w option is meaningless for the first pass of modpost (vmlinux.o).

We know there are unresolved symbols in vmlinux.o, hence we skip
check_exports() and other checks when mod->is_vmlinux is set.

See the following part in the for-loop.

    if (mod->is_vmlinux || mod->from_dump)
            continue;

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

 scripts/Makefile.modpost | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 3f5b09a09aef..b3e08fb1fd56 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -32,8 +32,6 @@
 # Step 4 is solely used to allow module versioning in external modules,
 # where the CRC of each module is retrieved from the Module.symvers file.
 
-# KBUILD_MODPOST_WARN can be set to avoid error out in case of undefined
-# symbols in the final module linking stage
 # KBUILD_MODPOST_NOFINAL can be set to skip the final link of modules.
 # This is solely useful to speed up test compiles
 
@@ -50,7 +48,6 @@ MODPOST = scripts/mod/modpost								\
 	$(if $(CONFIG_MODVERSIONS),-m)							\
 	$(if $(CONFIG_MODULE_SRCVERSION_ALL),-a)					\
 	$(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E)					\
-	$(if $(KBUILD_MODPOST_WARN),-w) \
 	-o $@
 
 ifdef MODPOST_VMLINUX
@@ -136,6 +133,11 @@ endif
 
 modules := $(sort $(shell cat $(MODORDER)))
 
+# KBUILD_MODPOST_WARN can be set to avoid error out in case of undefined symbols
+ifneq ($(KBUILD_MODPOST_WARN),)
+MODPOST += -w
+endif
+
 # Read out modules.order to pass in modpost.
 # Otherwise, allmodconfig would fail with "Argument list too long".
 quiet_cmd_modpost = MODPOST $@
-- 
2.27.0

[PATCH 3/3] kbuild: fix false-positive modpost warning when all symbols are trimmed

[Masahiro Yamada]

Nathan reports that the mips defconfig emits the following warning:

  WARNING: modpost: Symbol info of vmlinux is missing. Unresolved symbol check will be entirely skipped.

This false-positive happens under the following combination:

 - CONFIG_TRIM_UNUSED_KSYMS=y
 - CONFIG_MODULES=y
 - No CONFIG option is set to 'm'

Commit a0590473c5e6 ("nfs: fix PNFS_FLEXFILE_LAYOUT Kconfig default")
turned the last 'm' into 'y' for the mips defconfig, and uncovered
this issue.

In this case, the module feature itself is enabled, but we have no
module. As a result, CONFIG_TRIM_UNUSED_KSYMS drops all the instances
of EXPORT_SYMBOL. Then, modpost wrongly assumes vmlinux is missing
because vmlinux.symvers is empty. (Or, you can create a module that
does not use any symbol of vmlinux).

The current behavior is to entirely suppress the unresolved symbol
warnings when vmlinux is missing just because there are too many.
I found the origin of this code in the historical git tree. [1]

If this is a matter of noisiness, I think modpost can display the
first 10 warnings, and the number of suppressed warnings at the end.

You will get a bit noisier logs when you run 'make modules' without
vmlinux, but such warnings are better to show because you never know
the resulting modules are actually loadable or not.

This commit changes as follows:

 - If any of input *.symver files is missing, pass -w option to let
   the module build keep going with warnings instead of errors.

 - If there are too many (10+) unresolved symbol warnings, show only
   the first 10, and also the number of suppressed warnings.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=1cc0e0529569bf6a94f6d49770aa6d4b599d2c46

Reported-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/Makefile.modpost |  7 +++++--
 scripts/mod/modpost.c    | 25 ++++++++++++-------------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index b3e08fb1fd56..c383ba33d837 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -98,9 +98,11 @@ output-symdump := $(KBUILD_EXTMOD)/Module.symvers
 
 endif
 
+existing-input-symdump := $(wildcard $(input-symdump))
+
 # modpost options for modules (both in-kernel and external)
 MODPOST += \
-	$(addprefix -i ,$(wildcard $(input-symdump))) \
+	$(addprefix -i ,$(existing-input-symdump)) \
 	$(if $(KBUILD_NSDEPS),-d $(MODULES_NSDEPS)) \
 	$(if $(CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS)$(KBUILD_NSDEPS),-N)
 
@@ -114,6 +116,7 @@ VPATH :=
 $(input-symdump):
 	@echo >&2 'WARNING: Symbol version dump "$@" is missing.'
 	@echo >&2 '         Modules may not have dependencies or modversions.'
+	@echo >&2 '         You may get many unresolved symbol warnings.'
 
 ifdef CONFIG_LTO_CLANG
 # With CONFIG_LTO_CLANG, .o files might be LLVM bitcode, so we need to run
@@ -134,7 +137,7 @@ endif
 modules := $(sort $(shell cat $(MODORDER)))
 
 # KBUILD_MODPOST_WARN can be set to avoid error out in case of undefined symbols
-ifneq ($(KBUILD_MODPOST_WARN),)
+ifneq ($(KBUILD_MODPOST_WARN)$(filter-out $(existing-input-symdump), $(input-symdump)),)
 MODPOST += -w
 endif
 
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 10c3fba26f03..7c6bec78fa34 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -23,8 +23,6 @@
 
 /* Are we using CONFIG_MODVERSIONS? */
 static int modversions = 0;
-/* Warn about undefined symbols? (do so if we have vmlinux) */
-static int have_vmlinux = 0;
 /* Is CONFIG_MODULE_SRCVERSION_ALL set? */
 static int all_versions = 0;
 /* If we are modposting external module set to 1 */
@@ -41,6 +39,13 @@ static int allow_missing_ns_imports;
 
 static bool error_occurred;
 
+/*
+ * Cut off the warnings when there are too many. This typically occurs when
+ * vmlinux is missing. ('make modules' without building vmlinux.)
+ */
+#define MAX_UNRESOLVED_REPORTS	10
+static unsigned int nr_unresolved;
+
 enum export {
 	export_plain,
 	export_gpl,
@@ -177,9 +182,6 @@ static struct module *new_module(const char *modname)
 	mod->next = modules;
 	modules = mod;
 
-	if (mod->is_vmlinux)
-		have_vmlinux = 1;
-
 	return mod;
 }
 
@@ -2141,7 +2143,7 @@ static void check_exports(struct module *mod)
 		const char *basename;
 		exp = find_symbol(s->name);
 		if (!exp || exp->module == mod) {
-			if (have_vmlinux && !s->weak)
+			if (!s->weak && nr_unresolved++ < MAX_UNRESOLVED_REPORTS)
 				modpost_log(warn_unresolved ? LOG_WARN : LOG_ERROR,
 					    "\"%s\" [%s.ko] undefined!\n",
 					    s->name, mod->name);
@@ -2545,13 +2547,6 @@ int main(int argc, char **argv)
 	if (files_source)
 		read_symbols_from_files(files_source);
 
-	/*
-	 * When there's no vmlinux, don't print warnings about
-	 * unresolved symbols (since there'll be too many ;)
-	 */
-	if (!have_vmlinux)
-		warn("Symbol info of vmlinux is missing. Unresolved symbol check will be entirely skipped.\n");
-
 	for (mod = modules; mod; mod = mod->next) {
 		char fname[PATH_MAX];
 
@@ -2595,6 +2590,10 @@ int main(int argc, char **argv)
 		}
 	}
 
+	if (nr_unresolved > MAX_UNRESOLVED_REPORTS)
+		warn("suppressed %u unresolved symbol warnings because there were too many)\n",
+		     nr_unresolved - MAX_UNRESOLVED_REPORTS);
+
 	free(buf.p);
 
 	return error_occurred ? 1 : 0;
-- 
2.27.0

Re: [PATCH 3/3] kbuild: fix false-positive modpost warning when all symbols are trimmed

[Nathan Chancellor]

On Fri, Mar 26, 2021 at 03:54:11AM +0900, Masahiro Yamada wrote:
> Nathan reports that the mips defconfig emits the following warning:
> 
>   WARNING: modpost: Symbol info of vmlinux is missing. Unresolved symbol check will be entirely skipped.
> 
> This false-positive happens under the following combination:
> 
>  - CONFIG_TRIM_UNUSED_KSYMS=y
>  - CONFIG_MODULES=y
>  - No CONFIG option is set to 'm'
> 
> Commit a0590473c5e6 ("nfs: fix PNFS_FLEXFILE_LAYOUT Kconfig default")
> turned the last 'm' into 'y' for the mips defconfig, and uncovered
> this issue.
> 
> In this case, the module feature itself is enabled, but we have no
> module. As a result, CONFIG_TRIM_UNUSED_KSYMS drops all the instances
> of EXPORT_SYMBOL. Then, modpost wrongly assumes vmlinux is missing
> because vmlinux.symvers is empty. (Or, you can create a module that
> does not use any symbol of vmlinux).
> 
> The current behavior is to entirely suppress the unresolved symbol
> warnings when vmlinux is missing just because there are too many.
> I found the origin of this code in the historical git tree. [1]
> 
> If this is a matter of noisiness, I think modpost can display the
> first 10 warnings, and the number of suppressed warnings at the end.
> 
> You will get a bit noisier logs when you run 'make modules' without
> vmlinux, but such warnings are better to show because you never know
> the resulting modules are actually loadable or not.
> 
> This commit changes as follows:
> 
>  - If any of input *.symver files is missing, pass -w option to let
>    the module build keep going with warnings instead of errors.
> 
>  - If there are too many (10+) unresolved symbol warnings, show only
>    the first 10, and also the number of suppressed warnings.
> 
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=1cc0e0529569bf6a94f6d49770aa6d4b599d2c46
> 
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Thank you for the patch, I do not see that warning anymore.

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

> ---
> 
>  scripts/Makefile.modpost |  7 +++++--
>  scripts/mod/modpost.c    | 25 ++++++++++++-------------
>  2 files changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index b3e08fb1fd56..c383ba33d837 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -98,9 +98,11 @@ output-symdump := $(KBUILD_EXTMOD)/Module.symvers
>  
>  endif
>  
> +existing-input-symdump := $(wildcard $(input-symdump))
> +
>  # modpost options for modules (both in-kernel and external)
>  MODPOST += \
> -	$(addprefix -i ,$(wildcard $(input-symdump))) \
> +	$(addprefix -i ,$(existing-input-symdump)) \
>  	$(if $(KBUILD_NSDEPS),-d $(MODULES_NSDEPS)) \
>  	$(if $(CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS)$(KBUILD_NSDEPS),-N)
>  
> @@ -114,6 +116,7 @@ VPATH :=
>  $(input-symdump):
>  	@echo >&2 'WARNING: Symbol version dump "$@" is missing.'
>  	@echo >&2 '         Modules may not have dependencies or modversions.'
> +	@echo >&2 '         You may get many unresolved symbol warnings.'
>  
>  ifdef CONFIG_LTO_CLANG
>  # With CONFIG_LTO_CLANG, .o files might be LLVM bitcode, so we need to run
> @@ -134,7 +137,7 @@ endif
>  modules := $(sort $(shell cat $(MODORDER)))
>  
>  # KBUILD_MODPOST_WARN can be set to avoid error out in case of undefined symbols
> -ifneq ($(KBUILD_MODPOST_WARN),)
> +ifneq ($(KBUILD_MODPOST_WARN)$(filter-out $(existing-input-symdump), $(input-symdump)),)
>  MODPOST += -w
>  endif
>  
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 10c3fba26f03..7c6bec78fa34 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -23,8 +23,6 @@
>  
>  /* Are we using CONFIG_MODVERSIONS? */
>  static int modversions = 0;
> -/* Warn about undefined symbols? (do so if we have vmlinux) */
> -static int have_vmlinux = 0;
>  /* Is CONFIG_MODULE_SRCVERSION_ALL set? */
>  static int all_versions = 0;
>  /* If we are modposting external module set to 1 */
> @@ -41,6 +39,13 @@ static int allow_missing_ns_imports;
>  
>  static bool error_occurred;
>  
> +/*
> + * Cut off the warnings when there are too many. This typically occurs when
> + * vmlinux is missing. ('make modules' without building vmlinux.)
> + */
> +#define MAX_UNRESOLVED_REPORTS	10
> +static unsigned int nr_unresolved;
> +
>  enum export {
>  	export_plain,
>  	export_gpl,
> @@ -177,9 +182,6 @@ static struct module *new_module(const char *modname)
>  	mod->next = modules;
>  	modules = mod;
>  
> -	if (mod->is_vmlinux)
> -		have_vmlinux = 1;
> -
>  	return mod;
>  }
>  
> @@ -2141,7 +2143,7 @@ static void check_exports(struct module *mod)
>  		const char *basename;
>  		exp = find_symbol(s->name);
>  		if (!exp || exp->module == mod) {
> -			if (have_vmlinux && !s->weak)
> +			if (!s->weak && nr_unresolved++ < MAX_UNRESOLVED_REPORTS)
>  				modpost_log(warn_unresolved ? LOG_WARN : LOG_ERROR,
>  					    "\"%s\" [%s.ko] undefined!\n",
>  					    s->name, mod->name);
> @@ -2545,13 +2547,6 @@ int main(int argc, char **argv)
>  	if (files_source)
>  		read_symbols_from_files(files_source);
>  
> -	/*
> -	 * When there's no vmlinux, don't print warnings about
> -	 * unresolved symbols (since there'll be too many ;)
> -	 */
> -	if (!have_vmlinux)
> -		warn("Symbol info of vmlinux is missing. Unresolved symbol check will be entirely skipped.\n");
> -
>  	for (mod = modules; mod; mod = mod->next) {
>  		char fname[PATH_MAX];
>  
> @@ -2595,6 +2590,10 @@ int main(int argc, char **argv)
>  		}
>  	}
>  
> +	if (nr_unresolved > MAX_UNRESOLVED_REPORTS)
> +		warn("suppressed %u unresolved symbol warnings because there were too many)\n",
> +		     nr_unresolved - MAX_UNRESOLVED_REPORTS);
> +
>  	free(buf.p);
>  
>  	return error_occurred ? 1 : 0;
> -- 
> 2.27.0
>