LinuxLists.cc - linux-next: build failure after merge of the origin tree

2021-09-07 23:10:47

[permalink] [raw]

Subject: Re: linux-next: build failure after merge of the origin tree

Hi all,

On Wed, 8 Sep 2021 09:07:20 +1000 Stephen Rothwell <[email protected]> wrote:
>
> Building Linus' tree, today's linux-next build (mips
> allmodconfig) failed like this:
>
> In file included from /kisskb/src/include/linux/compiler_types.h:65:0,
> from <command-line>:0:
> include/linux/compiler_attributes.h:29:29: error: "__GCC4_has_attribute___no_sanitize_coverage__" is not defined [-Werror=undef]
> # define __has_attribute(x) __GCC4_has_attribute_##x
> ^
> include/linux/compiler-gcc.h:125:29: note: in expansion of macro '__has_attribute'
> #if defined(CONFIG_KCOV) && __has_attribute(__no_sanitize_coverage__)
> ^
> cc1: all warnings being treated as errors
>
> Caused by commit
>
> 540540d06e9d ("kcov: add __no_sanitize_coverage to fix noinstr for all architectures")
>
> This ia a gcc 4.9 build, so presumably this?
>
> diff -ru a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> --- a/include/linux/compiler_attributes.h 2021-09-08 09:03:35.169225661 +1000
> +++ b/include/linux/compiler_attributes.h 2021-09-08 09:05:47.790907780 +1000
> @@ -36,6 +36,7 @@
> # define __GCC4_has_attribute___no_profile_instrument_function__ 0
> # define __GCC4_has_attribute___nonstring__ 0
> # define __GCC4_has_attribute___no_sanitize_address__ 1
> +# define __GCC4_has_attribute___no_sanitize_coverage__ 0
> # define __GCC4_has_attribute___no_sanitize_undefined__ 1
> # define __GCC4_has_attribute___fallthrough__ 0
> #endif

Just to be clear, I haven't tested the above in any way.

--
Cheers,
Stephen Rothwell

Attachments:

2021-09-07 23:40:56

by Miguel Ojeda

[permalink] [raw]

Subject: Re: linux-next: build failure after merge of the origin tree

On Wed, Sep 8, 2021 at 1:17 AM Marco Elver <[email protected]> wrote:
>
> I see this in next-20210907: "Compiler Attributes: fix
> __has_attribute(__no_sanitize_coverage__) for GCC 4"
> Which does the same fix.
>
> Not sure what happened to it.
>
> I would have also expected this to be merged as a fix into mainline by
> now? Miguel?

Yes, sorry, I have some things in my queues -- I will send them in a few hours.

Cheers,
Miguel

2021-09-07 23:52:40

by Marco Elver

[permalink] [raw]

Subject: Re: linux-next: build failure after merge of the origin tree

On Wed, 8 Sept 2021 at 01:09, Stephen Rothwell <[email protected]> wrote:
> Hi all,
>
> On Wed, 8 Sep 2021 09:07:20 +1000 Stephen Rothwell <[email protected]> wrote:
> >
> > Building Linus' tree, today's linux-next build (mips
> > allmodconfig) failed like this:
> >
> > In file included from /kisskb/src/include/linux/compiler_types.h:65:0,
> > from <command-line>:0:
> > include/linux/compiler_attributes.h:29:29: error: "__GCC4_has_attribute___no_sanitize_coverage__" is not defined [-Werror=undef]
> > # define __has_attribute(x) __GCC4_has_attribute_##x
> > ^
> > include/linux/compiler-gcc.h:125:29: note: in expansion of macro '__has_attribute'
> > #if defined(CONFIG_KCOV) && __has_attribute(__no_sanitize_coverage__)
> > ^
> > cc1: all warnings being treated as errors
> >
> > Caused by commit
> >
> > 540540d06e9d ("kcov: add __no_sanitize_coverage to fix noinstr for all architectures")
> >
> > This ia a gcc 4.9 build, so presumably this?
> >
> > diff -ru a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> > --- a/include/linux/compiler_attributes.h 2021-09-08 09:03:35.169225661 +1000
> > +++ b/include/linux/compiler_attributes.h 2021-09-08 09:05:47.790907780 +1000
> > @@ -36,6 +36,7 @@
> > # define __GCC4_has_attribute___no_profile_instrument_function__ 0
> > # define __GCC4_has_attribute___nonstring__ 0
> > # define __GCC4_has_attribute___no_sanitize_address__ 1
> > +# define __GCC4_has_attribute___no_sanitize_coverage__ 0
> > # define __GCC4_has_attribute___no_sanitize_undefined__ 1
> > # define __GCC4_has_attribute___fallthrough__ 0
> > #endif
>
> Just to be clear, I haven't tested the above in any way.

I see this in next-20210907: "Compiler Attributes: fix
__has_attribute(__no_sanitize_coverage__) for GCC 4"
Which does the same fix.

Not sure what happened to it.

I would have also expected this to be merged as a fix into mainline by
now? Miguel?

Thanks,
-- Marco

2021-09-14 01:29:19

[permalink] [raw]

Subject: Re: linux-next: build failure after merge of the origin tree

On Mon, Sep 13, 2021 at 5:19 PM Linus Torvalds
<[email protected]> wrote:
>
> What version of gcc is this? Are you maybe on gcc-4.9 and we just
> didn't check that properly?

Hmm. That version check works for me (tested by just arbitrarily
making min-tool-version return version 15 for gcc ;)

So you got far enough that I don't believe you have gcc-4.

I have no idea why it then complains about removal of the GCC4 macros.

Can somebody hit me with the clue-bat?

Linus

2021-09-14 01:30:15

[permalink] [raw]

Subject: Re: linux-next: build failure after merge of the origin tree

On Mon, Sep 13, 2021 at 5:09 PM Stephen Rothwell <[email protected]> wrote:
>
> gcc -Wp,-MD,arch/powerpc/boot/.crt0.o.d

Ok, so it's not the funky "clang reports gcc-4" that caused tool breakage.

What version of gcc is this? Are you maybe on gcc-4.9 and we just
didn't check that properly?

Linus

2021-09-14 01:33:35

[permalink] [raw]

Subject: Re: linux-next: build failure after merge of the origin tree

Hi Linus,

On Mon, 13 Sep 2021 17:24:11 -0700 Linus Torvalds <[email protected]> wrote:
>
> On Mon, Sep 13, 2021 at 5:19 PM Linus Torvalds
> <[email protected]> wrote:
> >
> > What version of gcc is this? Are you maybe on gcc-4.9 and we just
> > didn't check that properly?
>
> Hmm. That version check works for me (tested by just arbitrarily
> making min-tool-version return version 15 for gcc ;)
>
> So you got far enough that I don't believe you have gcc-4.

$ gcc --version
gcc (Debian 10.2.1-6) 10.2.1 20210110
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

> I have no idea why it then complains about removal of the GCC4 macros.

Me neither :-(

--
Cheers,
Stephen Rothwell

Attachments:

2021-09-14 01:34:39

[permalink] [raw]

Subject: Re: linux-next: build failure after merge of the origin tree

On Mon, Sep 13, 2021 at 5:58 PM Stephen Rothwell <[email protected]> wrote:
>
> > I have no idea why it then complains about removal of the GCC4 macros.
>
> Me neither :-(

Ooh.

So I'm looking at gcc sources, just to see if "maybe this thing is
somehow conditional".

And bingo.

In cpp_init_special_builtins(), gcc does

if (b->value == BT_HAS_ATTRIBUTE
&& (CPP_OPTION (pfile, lang) == CLK_ASM
|| pfile->cb.has_attribute == NULL))
continue;

which basically says that if we're pre-processing an ASM file, the
magical pre-processor symbol for __has_attribute is not defined.

I'm not sure what that 'pfile->cb.has_attribute == NULL' thing means,
but the libcpp/ChangeLog file also mentions this:

(cpp_init_special_builtins): Don't initialize __has_attribute
or __has_cpp_attribute if CLK_ASM or pfile->cb.has_attribute is NULL.

So this is a very very special magical thing: if building an *.S file,
__has_attribute magically goes away.

And sure enough, that's exactly what is going on. It's during that
build of arch/powerpc/boot/crt0.S, and the reason this hits on powerpc
is that in arch/powerpc/boot/Makefile we have

-include $(srctree)/include/linux/compiler_attributes.h

as part of BOOTCFLAGS, and then it does

BOOTAFLAGS := -D__ASSEMBLY__ $(BOOTCFLAGS) -nostdinc

to also include that header file when building ASM files.

And our old GCC4 code silently hid this all, and made it work, because
for a *.S file you'd then (completely illogically) get those fake
gcc-4 attribute macros.

Now, do I know *why* that ppc Makefile it does that? No. Neither do I
know why the gcc people decided to just make ASM preprocessor so
special.

But at least I understand how the odd error happens.

This was too damn subtle. When you have to go read the compiler
sources to figure things like this out, you know you are too deep.

The fix should be pretty simple: remove almost all of BOOTCFLAGS from
BOOTAFLAGS.

But sadly, "almost all" isn't "all". There's the include path stuff,
there's the ABI and endianness, and there's the bit size ones.

So I think the fix is either

(a) remove that

-include $(srctree)/include/linux/compiler_attributes.h

thing entirely, and add it as required to the C files.

OR

(b) something like this ENTIRELY UNTESTED ATTACHED patch

I will leave it to the powerpc people to make the right choice.

Linus

Attachments:

patch.diff (1.57 kB)

2021-09-14 01:41:00

[permalink] [raw]

Subject: Re: linux-next: build failure after merge of the origin tree

On Mon, Sep 13, 2021 at 6:29 PM Linus Torvalds
<[email protected]> wrote:
>
> Now, do I know *why* that ppc Makefile it does that? No.

Well, that is simple enough to find out..

git show 77433830ed164

just tells us.

Of course, that also points to scripts/Makefile.lib, which doesn't
have this problem, because it keeps c_flags and a_flags nicely
separated.

Anyway, that just makes me think that something like that patch in my
previous email is the way to go, but I would like to stress (again)
how little testing it had: exactly none.

So please consider that nothing more than a hand-wavy "something like this".

Linus

2021-09-14 02:06:19

[permalink] [raw]

Subject: Re: linux-next: build failure after merge of the origin tree

On Mon, Sep 13, 2021 at 6:37 PM Linus Torvalds
<[email protected]> wrote:
>
> Anyway, that just makes me think that something like that patch in my
> previous email is the way to go, but I would like to stress (again)
> how little testing it had: exactly none.
>
> So please consider that nothing more than a hand-wavy "something like this".

The alternative would be to just add a

#ifndef __ASSEMBLY__
...
#endif

around the whole thing. I could do that without asking for help from
the powerpc people.

But it really does seem kind of wrong to include a "compiler
attributes" header file to compile a *.S file. It's not like any of
those attributes are valid in asm anyway.

I did just verify that the patch I sent out seems to cross-compile ok.
At least for the power64 defconfig.

So that's _some_ testing, and implies that the patch isn't complete garbage.

Linus

2021-09-14 02:10:27

[permalink] [raw]

Subject: Re: linux-next: build failure after merge of the origin tree

Hi Linus,

On Mon, 13 Sep 2021 18:29:26 -0700 Linus Torvalds <[email protected]> wrote:
>
> On Mon, Sep 13, 2021 at 5:58 PM Stephen Rothwell <[email protected]> wrote:
> >
> > > I have no idea why it then complains about removal of the GCC4 macros.
> >
> > Me neither :-(
>
> Ooh.
>
> So I'm looking at gcc sources, just to see if "maybe this thing is
> somehow conditional".
>
> And bingo.
>
> In cpp_init_special_builtins(), gcc does
>
> if (b->value == BT_HAS_ATTRIBUTE
> && (CPP_OPTION (pfile, lang) == CLK_ASM
> || pfile->cb.has_attribute == NULL))
> continue;
>
> which basically says that if we're pre-processing an ASM file, the
> magical pre-processor symbol for __has_attribute is not defined.
>
> I'm not sure what that 'pfile->cb.has_attribute == NULL' thing means,
> but the libcpp/ChangeLog file also mentions this:
>
> (cpp_init_special_builtins): Don't initialize __has_attribute
> or __has_cpp_attribute if CLK_ASM or pfile->cb.has_attribute is NULL.
>
> So this is a very very special magical thing: if building an *.S file,
> __has_attribute magically goes away.
>
> And sure enough, that's exactly what is going on. It's during that
> build of arch/powerpc/boot/crt0.S, and the reason this hits on powerpc
> is that in arch/powerpc/boot/Makefile we have
>
> -include $(srctree)/include/linux/compiler_attributes.h
>
> as part of BOOTCFLAGS, and then it does
>
> BOOTAFLAGS := -D__ASSEMBLY__ $(BOOTCFLAGS) -nostdinc
>
> to also include that header file when building ASM files.
>
> And our old GCC4 code silently hid this all, and made it work, because
> for a *.S file you'd then (completely illogically) get those fake
> gcc-4 attribute macros.
>
> Now, do I know *why* that ppc Makefile it does that? No. Neither do I
> know why the gcc people decided to just make ASM preprocessor so
> special.
>
> But at least I understand how the odd error happens.

Its good to know there is a reason :-)

> This was too damn subtle. When you have to go read the compiler
> sources to figure things like this out, you know you are too deep.
>
> The fix should be pretty simple: remove almost all of BOOTCFLAGS from
> BOOTAFLAGS.
>
> But sadly, "almost all" isn't "all". There's the include path stuff,
> there's the ABI and endianness, and there's the bit size ones.
>
> So I think the fix is either
>
> (a) remove that
>
> -include $(srctree)/include/linux/compiler_attributes.h
>
> thing entirely, and add it as required to the C files.
>
> OR
>
> (b) something like this ENTIRELY UNTESTED ATTACHED patch
>
> I will leave it to the powerpc people to make the right choice.

That patch works for me - for the ppc64_defconfig build at least.

--
Cheers,
Stephen Rothwell

Attachments:

2021-09-14 02:14:59

[permalink] [raw]

Subject: Re: linux-next: build failure after merge of the origin tree

On Mon, Sep 13, 2021 at 7:08 PM Stephen Rothwell <[email protected]> wrote:
>
> That patch works for me - for the ppc64_defconfig build at least.

Yeah, I just tested the allmodconfig case too, although I suspect it's
essentially the same wrt the boot *.S files, so it probably doesn't
matter.

I'd like to have Michael or somebody who can actually run some tests
on the end result ack that patch (or - even better - come up with
something cleaner) before committing it.

Because yeah, the build failure is annoying and I apologize, but I'd
rather have the build fail overnight than commit something that builds
but then is subtle buggy for some reason.

But if I don't get any other comments by the time I'm up again
tomorrow, I'll just commit it as "fixes the build".

Linus

2021-09-14 02:43:34

[permalink] [raw]

Subject: Re: linux-next: build failure after merge of the origin tree

Hi all,

On Tue, 14 Sep 2021 12:08:18 +1000 Stephen Rothwell <[email protected]> wrote:
>
> That patch works for me - for the ppc64_defconfig build at least.

also allnoconfig, 64bit allnoconfig, pseries_le_defconfig and ppc44x_defconfig
--
Cheers,
Stephen Rothwell

Attachments: