Re: [PATCH RFC] kbuild: Prevent compiler mismatch with external modules

[Masahiro Yamada <masahiroy@kernel.org>]

On Fri, Jan 29, 2021 at 7:08 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> On Thu, Jan 28, 2021 at 01:45:51PM -0800, Linus Torvalds wrote:
> > On Thu, Jan 28, 2021 at 1:34 PM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > >
> > > On Thu, Jan 28, 2021 at 01:23:11PM -0800, Linus Torvalds wrote:
> > > > THAT workaround is long gone, but I didn't check what other ones we
> > > > might have now. But the gcc version checks we _do_ have are not
> > > > necessarily about major versions at all (ie I trivially found checks
> > > > for 4.9, 4.9.2, 5.1, 7.2 and 9.1).
> > >
> > > Then maybe the check should be same major.minor?
> >
> > Well, how many of them are actually about things that generate
> > incompatible object code?
> >
> > The main one I can think of is the KASAN ABI version checks, but
> > honestly, I think that's irrelevant. I really hope no distros enable
> > KASAN in user kernels.
> >
> > Another version check I looked at was the one that just checks whether
> > the compiler natively supports __builtin_mul_overflow() or not - it
> > doesn't generate incompatible object code, it just takes advantage of
> > a compiler feature if one exists. You can mix and match those kinds of
> > things well enough.
> >
> > So I'd really like to hear actual hard technical reasons with
> > examples, for why you'd want to add this test in the first place.
>
> Unfortunately I don't have technical reasons beyond what we've already
> discussed, found from code inspection.
>
> This patch was born from a discussion where wildly different opinions
> were expressed about whether such a mismatch scenario (or even external
> modules in general!) would be supported at all.
>
> So I guess the goal is to clarify (in the code base) to what extent
> compiler mismatches are supported/allowed/encouraged.  Because they
> definitely happen in the real world, but a lot of people seem to be
> sticking their head in the sand about it.
>
> If we decide it's not a cut-and-dry makefile check, then the policy
> should at least be documented.
>
> I'd prefer the warning though, since nobody's going to read the docs.
>
> > No hand-waving "different compiler versions don't work together".
> > Because that's simply not true.
> >
> > > And convert it to a strongly worded warning/disclaimer?
> >
> > A warning might be better for the simple reason that it wouldn't cause
> > people to just fix it with "make oldconfig".
> >
> > Maybe you haven't looked at people who compile external modules, but
> > they always have various "this is how to work around issues with
> > version XYZ". That "make oldconfig" would simply just become the
> > workaround for any build errors.
> >
> > And a warning might be more palatable even if different compiler
> > version work fine together. Just a heads up on "it looks like you
> > might be mixing compiler versions" is a valid note, and isn't
> > necessarily wrong. Even when they work well together, maybe you want
> > to have people at least _aware_ of it.
>
> Sounds reasonable.
>
> --
> Josh
>

[1]

First, let me explain how Kbuild works w.r.t the compiler version
check.

When working on the kernel tree, Kbuild automatically detects
the compiler upgrade (this is done by comparing the output
of '$(CC) --version'), and invokes Kconfig to sync the configuration.
So, the .config is updated even if you do not explicitly
do "make oldconfig".


When working on external modules, in contrast,
Kbuild does not attempt to update anything in the kernel tree.
This makes sense since the build tree, /lib/modules/$(uname -r)/build/
is read-only.
You cannot manually run Kconfig either because the config targets
are hidden for external modules.

$ make M=../qemu-build/extmod  oldconfig
make: *** No rule to make target 'oldconfig'.  Stop.



[2]

As for this patch, it is wrong to do this check in the Makefile
parse stage.

"make M=...  clean"
"make M=...  help"

etc. will fail.
Such targets do not require the compiler in the first place.

This check must be done before starting building something,


Also, this patch is not applicable.
gcc-version.sh and clang-version.sh do not exist.
See linux-next.



[3]
Peterz already pointed out asm-goto as an example of ABI mismatch.

I remember a trouble reported in the past due
to the mismatch of -mstack-protector-guard-offset.

https://bugzilla.kernel.org/show_bug.cgi?id=201891

This has already been fixed,
and it will no longer happen though.





--
Best Regards




Masahiro Yamada