diff mbox

x86/kbuild: enable modversions for symbols exported from asm

Message ID CA+55aFxCKgTrh1gS-cMyhBa0QoLW2DL2+DYxOAcA-Bd15H15vg@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Linus Torvalds Nov. 29, 2016, 5:10 p.m. UTC 
On Tue, Nov 29, 2016 at 9:05 AM, Adam Borowski <kilobyte@angband.pl> wrote:
>
> Thus, if it's indeed binutils, you'll see the breakage as soon as Fedora
> recovers from the freeze.

So quite frankly, I don't want to make our kernel sources worse due to
broken shit tools getting something wrong that we shouldn't even care
about.

How about this stupid patch? It weakens modversions, but that may be
ok for Debian, and a better alternative than just saying "we don't
support it at all".

            Linus
kernel/module.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Linus Torvalds Nov. 29, 2016, 5:14 p.m. UTC | #1 
On Tue, Nov 29, 2016 at 9:10 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> So quite frankly, I don't want to make our kernel sources worse due to
> broken shit tools getting something wrong that we shouldn't even care
> about.

And yes, I'm on binutils 2.26 (with no issues), so it could be that
it's 2.27 that triggers this.

We could make the pr_warn_once() mention "broken binutils?" so that
people know why the warning happens.

           Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Marek Nov. 29, 2016, 9:23 p.m. UTC | #2 
Dne 29.11.2016 v 18:10 Linus Torvalds napsal(a):
> How about this stupid patch? It weakens modversions, but that may be
> ok for Debian, and a better alternative than just saying "we don't
> support it at all".
[...]
> -	pr_warn("%s: no symbol version for %s\n", mod->name, symname);
> -	return 0;
> +	/* Broken toolchain. Warn once, then let it go.. */
> +	pr_warn_once("%s: no symbol version for %s\n", mod->name, symname);
> +	return 1;

Fine with me, if it goes with the revert of the "depends on BROKEN."

Michal

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Dec. 1, 2016, 1:58 p.m. UTC | #3 
On Tuesday, November 29, 2016 9:14:46 AM CET Linus Torvalds wrote:
> On Tue, Nov 29, 2016 at 9:10 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > So quite frankly, I don't want to make our kernel sources worse due to
> > broken shit tools getting something wrong that we shouldn't even care
> > about.
> 
> And yes, I'm on binutils 2.26 (with no issues), so it could be that
> it's 2.27 that triggers this.
> 
> We could make the pr_warn_once() mention "broken binutils?" so that
> people know why the warning happens.

I've tried to get to the bottom of this, but couldn't find anything
related to the toolchain version. I've tried binutils 2.23, 2.24, 2.26
and 2.27, and also gcc-7.0, gcc-5.4.1 and gcc-4.9.3, but with today's
linux-next, I always get

WARNING: EXPORT symbol "mcount" [arch/x86/entry/built-in.ko] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "mcount" [arch/x86/built-in.ko] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "cmpxchg8b_emu" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "empty_zero_page" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "mcount" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "cmpxchg8b_emu" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "empty_zero_page" [vmlinux] version generation failed, symbol will not be versioned.
WARNING: EXPORT symbol "mcount" [vmlinux] version generation failed, symbol will not be versioned.

Out of 12 randconfig builds that had CONFIG_MODVERSIONS enabled, all 12
had this problem, though not always with all the symbols.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Marek Dec. 1, 2016, 4:21 p.m. UTC | #4 
On 2016-12-01 14:58, Arnd Bergmann wrote:
> On Tuesday, November 29, 2016 9:14:46 AM CET Linus Torvalds wrote:
>> On Tue, Nov 29, 2016 at 9:10 AM, Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>>>
>>> So quite frankly, I don't want to make our kernel sources worse due to
>>> broken shit tools getting something wrong that we shouldn't even care
>>> about.
>>
>> And yes, I'm on binutils 2.26 (with no issues), so it could be that
>> it's 2.27 that triggers this.
>>
>> We could make the pr_warn_once() mention "broken binutils?" so that
>> people know why the warning happens.
> 
> I've tried to get to the bottom of this, but couldn't find anything
> related to the toolchain version. I've tried binutils 2.23, 2.24, 2.26
> and 2.27, and also gcc-7.0, gcc-5.4.1 and gcc-4.9.3, but with today's
> linux-next, I always get
> 
> WARNING: EXPORT symbol "mcount" [arch/x86/entry/built-in.ko] version generation failed, symbol will not be versioned.
> WARNING: EXPORT symbol "mcount" [arch/x86/built-in.ko] version generation failed, symbol will not be versioned.
> WARNING: EXPORT symbol "cmpxchg8b_emu" [vmlinux] version generation failed, symbol will not be versioned.
> WARNING: EXPORT symbol "empty_zero_page" [vmlinux] version generation failed, symbol will not be versioned.
> WARNING: EXPORT symbol "mcount" [vmlinux] version generation failed, symbol will not be versioned.
> WARNING: EXPORT symbol "cmpxchg8b_emu" [vmlinux] version generation failed, symbol will not be versioned.
> WARNING: EXPORT symbol "empty_zero_page" [vmlinux] version generation failed, symbol will not be versioned.
> WARNING: EXPORT symbol "mcount" [vmlinux] version generation failed, symbol will not be versioned.
> 
> Out of 12 randconfig builds that had CONFIG_MODVERSIONS enabled, all 12
> had this problem, though not always with all the symbols.

That we are not generating modversions for the asm exports is a known
problem, independent of the toolchain. The problem with some toolchains
(presumably, because that's the next thing to blame if the source and
the .config is identical) is that the CRCs do not appear as 0 in the
___kcrctab/___kcrctab_gpl section.

Michal

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Torvalds Dec. 1, 2016, 6:26 p.m. UTC | #5 
On Thu, Dec 1, 2016 at 5:58 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> WARNING: EXPORT symbol "mcount" [arch/x86/entry/built-in.ko] version generation failed, symbol will not be versioned.
> WARNING: EXPORT symbol "mcount" [arch/x86/built-in.ko] version generation failed, symbol will not be versioned.
> WARNING: EXPORT symbol "cmpxchg8b_emu" [vmlinux] version generation failed, symbol will not be versioned.
> WARNING: EXPORT symbol "empty_zero_page" [vmlinux] version generation failed, symbol will not be versioned.
> WARNING: EXPORT symbol "mcount" [vmlinux] version generation failed, symbol will not be versioned.
> WARNING: EXPORT symbol "cmpxchg8b_emu" [vmlinux] version generation failed, symbol will not be versioned.
> WARNING: EXPORT symbol "empty_zero_page" [vmlinux] version generation failed, symbol will not be versioned.
> WARNING: EXPORT symbol "mcount" [vmlinux] version generation failed, symbol will not be versioned.
>
> Out of 12 randconfig builds that had CONFIG_MODVERSIONS enabled, all 12
> had this problem, though not always with all the symbols.

Well, the good news is that pretty fundamentally, if it's just the asm
symbls, those really don't have ABI's that change (or if they change,
it's such a fundamental change that everything else will likely have
changed too and we don't need to worry about one asm symbol crc - the
change will be caught by other symbols).

So I think the whole "we don't really care" approach should work fine.
The "let's make every symbol always be versioned" may just be too much
pain for no real gain.

            Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/kernel/module.c b/kernel/module.c
index f57dd63186e6..0e54d5bf0097 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1301,8 +1301,9 @@  static int check_version(Elf_Shdr *sechdrs,
 		goto bad_version;
 	}
 
-	pr_warn("%s: no symbol version for %s\n", mod->name, symname);
-	return 0;
+	/* Broken toolchain. Warn once, then let it go.. */
+	pr_warn_once("%s: no symbol version for %s\n", mod->name, symname);
+	return 1;
 
 bad_version:
 	pr_warn("%s: disagrees about version of symbol %s\n",