Date:   Fri, 2 Apr 2021 14:15:52 -0700
From:   Nathan Chancellor <nathan@...nel.org>
To:     Masahiro Yamada <masahiroy@...nel.org>
Cc:     linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
        Michal Marek <michal.lkml@...kovi.net>
Subject: Re: [PATCH 3/3] kbuild: fix false-positive modpost warning when all
 symbols are trimmed

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@...nel.org>
> Signed-off-by: Masahiro Yamada <masahiroy@...nel.org>

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

Tested-by: Nathan Chancellor <nathan@...nel.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
> 

Powered by blists - more mailing lists