Re: modpost warning by default on missing Module.symvers

[William McVicker <willmcvicker@google.com>]

On 01/10/2023, Masahiro Yamada wrote:
> On Tue, Jan 10, 2023 at 5:45 AM William McVicker
> <willmcvicker@google.com> wrote:
> >
> > Hi Masahiro,
> >
> > I recently noticed that in commit 4475dff55c54 ("kbuild: fix false-positive
> > modpost warning when all symbols are trimmed") [1] you modified the modpost
> > behavior to always warn (by passing `-w`) when there are missing Module.symver
> > files in order to allow module builds to continue building with warnings
> > instead of errors. I'm curious why you decided to not continue to rely on
> > KBUILD_MODPOST_WARN to enable/disable that functionality?
> >
> > I personally find it useful to keep these types of warnings as errors in order
> > to catch missing dependencies at build time (ideally by the CI build) instead
> > of at runtime when a module fails to load due to a missing symbol dependency.
> >
> > Let me know your thoughts on this and I'll try to come up with a solution to
> > factor in any concerns you have.
> >
> > Thanks,
> > Will
> >
> > [1] https://lore.kernel.org/all/20210325185412.2352951-3-masahiroy@kernel.org/
> 
> 
> 
> Good point.
> 
> I think we can always require KBUILD_MODPOST_WARN=1 explicitly.
> 
> Skipping unresolved symbols is not a good idea.
> Users can proceed if they want,
> but they should be aware of what they are doing, at least.
> 
> 
> How about something like this?
> 
> 
> 
> 
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index 43343e13c542..34baef239816 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -121,16 +121,14 @@ modpost-args += -e $(addprefix -i ,
> $(KBUILD_EXTRA_SYMBOLS))
> 
>  endif # ($(KBUILD_EXTMOD),)
> 
> -ifneq ($(missing-input),)
> -modpost-args += -w
> -endif
> -
>  quiet_cmd_modpost = MODPOST $@
>        cmd_modpost = \
>         $(if $(missing-input), \
>                 echo >&2 "WARNING: $(missing-input) is missing."; \
>                 echo >&2 "         Modules may not have dependencies
> or modversions."; \
> -               echo >&2 "         You may get many unresolved symbol
> warnings.";) \
> +               echo >&2 "         You may get many unresolved symbol
> errors.";) \
> +               echo >&2 "         You can set KBUILD_MODPOST_WARN=1
> to turn errors into warning"; \
> +               echo >&2 "         if you know what you are doing."; \
>         $(MODPOST) $(modpost-args)
> 
>  targets += $(output-symdump)
> 
> 
> 
> 
> 
> 
> 
> 
> 
> --
> Best Regards
> 
> Masahiro Yamada

That looks good to me! You do mention in [1] that there's a case where
unresolved symbols are expected. Can you clarify that? Why would you want to
build a kernel or module with unresolved symbols?

[1] https://lore.kernel.org/all/20230104140459.1147626-1-masahiroy@kernel.org/

Thanks,
Will