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

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]

Hi Linus,

In Linus' tree, today's linux-next build (powerpc ppc64_defconfig) failed
like this:

arch/powerpc/platforms/pseries/hotplug-memory.c: In function 'pseries_remove_memblock':
arch/powerpc/platforms/pseries/hotplug-memory.c:103:17: error: unused variable 'pfn' [-Werror=unused-variable]

Caused by commit d760afd4d257 ("memory-hotplug: suppress "Trying to free
nonexistent resource <XXXXXXXXXXXXXXXX-YYYYYYYYYYYYYYYY>" warning").

I can't see what the point of the "pfn" variable is and this patch never
appeared in linux-next before being merged.  :-(

I have reverted that commit for today.

If this patch truly was authored yesterday (according the Author Date in
git), why was it merged yesterday while still under discussion?  And the
latest update to it still has this build problem ... did anyone even try
to build this for powerpc (since that architecture was obviously
affected)?  (Cross compilers available here
http://www.kernel.org/pub/tools/crosstool/).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

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

[Andrew Morton]

On Wed, 10 Oct 2012 10:21:50 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi Linus,
> 
> In Linus' tree, today's linux-next build (powerpc ppc64_defconfig) failed
> like this:
> 
> arch/powerpc/platforms/pseries/hotplug-memory.c: In function 'pseries_remove_memblock':
> arch/powerpc/platforms/pseries/hotplug-memory.c:103:17: error: unused variable 'pfn' [-Werror=unused-variable]
> 
> Caused by commit d760afd4d257 ("memory-hotplug: suppress "Trying to free
> nonexistent resource <XXXXXXXXXXXXXXXX-YYYYYYYYYYYYYYYY>" warning").
> 
> I can't see what the point of the "pfn" variable is

This:

--- a/arch/powerpc/platforms/pseries/hotplug-memory.c~a
+++ a/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -101,7 +101,7 @@ static int pseries_remove_memblock(unsig
 	sections_to_remove = (memblock_size >> PAGE_SHIFT) / PAGES_PER_SECTION;
 	for (i = 0; i < sections_to_remove; i++) {
 		unsigned long pfn = start_pfn + i * PAGES_PER_SECTION;
-		ret = __remove_pages(zone, start_pfn,  PAGES_PER_SECTION);
+		ret = __remove_pages(zone, pfn, PAGES_PER_SECTION);
 		if (ret)
 			return ret;
 	}

> and this patch never
> appeared in linux-next before being merged.  :-(

It was first sighted October 3.

> I have reverted that commit for today.
> 
> If this patch truly was authored yesterday (according the Author Date in
> git), why was it merged yesterday while still under discussion?  And the
> latest update to it still has this build problem ... did anyone even try
> to build this for powerpc (since that architecture was obviously
> affected)?

Apparently not - the ppc bit was a best-effort fixup for a patch which
addresses an x86 problem.

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

[Yasuaki Ishimatsu]

Hi Stephen,

2012/10/10 8:45, Andrew Morton wrote:
> On Wed, 10 Oct 2012 10:21:50 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
>> Hi Linus,
>>
>> In Linus' tree, today's linux-next build (powerpc ppc64_defconfig) failed
>> like this:
>>
>> arch/powerpc/platforms/pseries/hotplug-memory.c: In function 'pseries_remove_memblock':
>> arch/powerpc/platforms/pseries/hotplug-memory.c:103:17: error: unused variable 'pfn' [-Werror=unused-variable]
>>
>> Caused by commit d760afd4d257 ("memory-hotplug: suppress "Trying to free
>> nonexistent resource <XXXXXXXXXXXXXXXX-YYYYYYYYYYYYYYYY>" warning").
>>
>> I can't see what the point of the "pfn" variable is
>
> This:
>
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c~a
> +++ a/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -101,7 +101,7 @@ static int pseries_remove_memblock(unsig
>   	sections_to_remove = (memblock_size >> PAGE_SHIFT) / PAGES_PER_SECTION;
>   	for (i = 0; i < sections_to_remove; i++) {
>   		unsigned long pfn = start_pfn + i * PAGES_PER_SECTION;
> -		ret = __remove_pages(zone, start_pfn,  PAGES_PER_SECTION);
> +		ret = __remove_pages(zone, pfn, PAGES_PER_SECTION);
>   		if (ret)
>   			return ret;
>   	}

I believe the error to be fixed with this patch.
Could you try it?

Thanks,
Yasuaki Ishimatsu

>
>> and this patch never
>> appeared in linux-next before being merged.  :-(
>
> It was first sighted October 3.
>
>> I have reverted that commit for today.
>>
>> If this patch truly was authored yesterday (according the Author Date in
>> git), why was it merged yesterday while still under discussion?  And the
>> latest update to it still has this build problem ... did anyone even try
>> to build this for powerpc (since that architecture was obviously
>> affected)?
>
> Apparently not - the ppc bit was a best-effort fixup for a patch which
> addresses an x86 problem.
>

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1684 bytes --]

Hi,

On Wed, 10 Oct 2012 08:52:21 +0900 Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> wrote:
>
> 2012/10/10 8:45, Andrew Morton wrote:
> > On Wed, 10 Oct 2012 10:21:50 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> >> Hi Linus,
> >>
> >> In Linus' tree, today's linux-next build (powerpc ppc64_defconfig) failed
> >> like this:
> >>
> >> arch/powerpc/platforms/pseries/hotplug-memory.c: In function 'pseries_remove_memblock':
> >> arch/powerpc/platforms/pseries/hotplug-memory.c:103:17: error: unused variable 'pfn' [-Werror=unused-variable]
> >>
> >> Caused by commit d760afd4d257 ("memory-hotplug: suppress "Trying to free
> >> nonexistent resource <XXXXXXXXXXXXXXXX-YYYYYYYYYYYYYYYY>" warning").
> >>
> >> I can't see what the point of the "pfn" variable is
> >
> > This:
> >
> > --- a/arch/powerpc/platforms/pseries/hotplug-memory.c~a
> > +++ a/arch/powerpc/platforms/pseries/hotplug-memory.c
> > @@ -101,7 +101,7 @@ static int pseries_remove_memblock(unsig
> >   	sections_to_remove = (memblock_size >> PAGE_SHIFT) / PAGES_PER_SECTION;
> >   	for (i = 0; i < sections_to_remove; i++) {
> >   		unsigned long pfn = start_pfn + i * PAGES_PER_SECTION;
> > -		ret = __remove_pages(zone, start_pfn,  PAGES_PER_SECTION);
> > +		ret = __remove_pages(zone, pfn, PAGES_PER_SECTION);
> >   		if (ret)
> >   			return ret;
> >   	}
> 
> I believe the error to be fixed with this patch.
> Could you try it?

The certainly fixes the build problem.  I can't comment in the semantics
of the patch.

Tested-by: Stephen Rothwell <sfr@canb.auug.org.au>  (Build only)
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1673 bytes --]

Hi Andrew,

On Tue, 9 Oct 2012 16:45:14 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Wed, 10 Oct 2012 10:21:50 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
> > I can't see what the point of the "pfn" variable is
> 
> This:
> 
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c~a
> +++ a/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -101,7 +101,7 @@ static int pseries_remove_memblock(unsig
>  	sections_to_remove = (memblock_size >> PAGE_SHIFT) / PAGES_PER_SECTION;
>  	for (i = 0; i < sections_to_remove; i++) {
>  		unsigned long pfn = start_pfn + i * PAGES_PER_SECTION;
> -		ret = __remove_pages(zone, start_pfn,  PAGES_PER_SECTION);
> +		ret = __remove_pages(zone, pfn, PAGES_PER_SECTION);
>  		if (ret)
>  			return ret;
>  	}

Can we get that fix to Linus ASAP, please?

> > and this patch never
> > appeared in linux-next before being merged.  :-(
> 
> It was first sighted October 3.

Yeah, my mistake. But it never made it to linux-next.

> > I have reverted that commit for today.
> > 
> > If this patch truly was authored yesterday (according the Author Date in
> > git), why was it merged yesterday while still under discussion?  And the
> > latest update to it still has this build problem ... did anyone even try
> > to build this for powerpc (since that architecture was obviously
> > affected)?
> 
> Apparently not - the ppc bit was a best-effort fixup for a patch which
> addresses an x86 problem.

Right, and that is one of the reasons we have linux-next - to test for
cross architecture problems.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 5706 bytes --]

Hi all,

After merging the origin tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:

In file included from <command-line>:
include/linux/compiler_attributes.h:62:5: warning: "__has_attribute" is not defined, evaluates to 0 [-Wundef]
   62 | #if __has_attribute(__assume_aligned__)
      |     ^~~~~~~~~~~~~~~
include/linux/compiler_attributes.h:62:20: error: missing binary operator before token "("
   62 | #if __has_attribute(__assume_aligned__)
      |                    ^
include/linux/compiler_attributes.h:88:5: warning: "__has_attribute" is not defined, evaluates to 0 [-Wundef]
   88 | #if __has_attribute(__copy__)
      |     ^~~~~~~~~~~~~~~
include/linux/compiler_attributes.h:88:20: error: missing binary operator before token "("
   88 | #if __has_attribute(__copy__)
      |                    ^
include/linux/compiler_attributes.h:113:5: warning: "__has_attribute" is not defined, evaluates to 0 [-Wundef]
  113 | #if __has_attribute(__designated_init__)
      |     ^~~~~~~~~~~~~~~
include/linux/compiler_attributes.h:113:20: error: missing binary operator before token "("
  113 | #if __has_attribute(__designated_init__)
      |                    ^
include/linux/compiler_attributes.h:124:5: warning: "__has_attribute" is not defined, evaluates to 0 [-Wundef]
  124 | #if __has_attribute(__error__)
      |     ^~~~~~~~~~~~~~~
include/linux/compiler_attributes.h:124:20: error: missing binary operator before token "("
  124 | #if __has_attribute(__error__)
      |                    ^
include/linux/compiler_attributes.h:135:5: warning: "__has_attribute" is not defined, evaluates to 0 [-Wundef]
  135 | #if __has_attribute(__externally_visible__)
      |     ^~~~~~~~~~~~~~~
include/linux/compiler_attributes.h:135:20: error: missing binary operator before token "("
  135 | #if __has_attribute(__externally_visible__)
      |                    ^
In file included from <command-line>:
include/linux/compiler_attributes.h:171:5: warning: "__has_attribute" is not defined, evaluates to 0 [-Wundef]
  171 | #if __has_attribute(__no_caller_saved_registers__)
      |     ^~~~~~~~~~~~~~~
include/linux/compiler_attributes.h:171:20: error: missing binary operator before token "("
  171 | #if __has_attribute(__no_caller_saved_registers__)
      |                    ^
include/linux/compiler_attributes.h:182:5: warning: "__has_attribute" is not defined, evaluates to 0 [-Wundef]
  182 | #if __has_attribute(__noclone__)
      |     ^~~~~~~~~~~~~~~
include/linux/compiler_attributes.h:182:20: error: missing binary operator before token "("
  182 | #if __has_attribute(__noclone__)
      |                    ^
include/linux/compiler_attributes.h:199:5: warning: "__has_attribute" is not defined, evaluates to 0 [-Wundef]
  199 | #if __has_attribute(__fallthrough__)
      |     ^~~~~~~~~~~~~~~
include/linux/compiler_attributes.h:199:20: error: missing binary operator before token "("
  199 | #if __has_attribute(__fallthrough__)
      |                    ^
include/linux/compiler_attributes.h:226:5: warning: "__has_attribute" is not defined, evaluates to 0 [-Wundef]
  226 | #if __has_attribute(__nonstring__)
      |     ^~~~~~~~~~~~~~~
include/linux/compiler_attributes.h:226:20: error: missing binary operator before token "("
  226 | #if __has_attribute(__nonstring__)
      |                    ^
include/linux/compiler_attributes.h:238:5: warning: "__has_attribute" is not defined, evaluates to 0 [-Wundef]
  238 | #if __has_attribute(__no_profile_instrument_function__)
      |     ^~~~~~~~~~~~~~~
include/linux/compiler_attributes.h:238:20: error: missing binary operator before token "("
  238 | #if __has_attribute(__no_profile_instrument_function__)
      |                    ^
include/linux/compiler_attributes.h:296:5: warning: "__has_attribute" is not defined, evaluates to 0 [-Wundef]
  296 | #if __has_attribute(__warning__)
      |     ^~~~~~~~~~~~~~~
include/linux/compiler_attributes.h:296:20: error: missing binary operator before token "("
  296 | #if __has_attribute(__warning__)
      |                    ^
make[2]: *** [arch/powerpc/boot/Makefile:225: arch/powerpc/boot/crt0.o] Error 1

Exposed by commit

  6d2ef226f2f1 ("compiler_attributes.h: drop __has_attribute() support for gcc4")

Powerpc uses BOOTAS to build arch/powerpc/boot/crt0.o (and others)
which (with V=1) is

  gcc -Wp,-MD,arch/powerpc/boot/.crt0.o.d -D__ASSEMBLY__ -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc -include /home/sfr/next/next/include/linux/compiler_attributes.h -I/home/sfr/next/next/arch/powerpc/include -I./arch/powerpc/include/generated -I/home/sfr/next/next/include -I./include -I/home/sfr/next/next/arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi -I/home/sfr/next/next/include/uapi -I./include/generated/uapi -include /home/sfr/next/next/include/linux/compiler-version.h -include /home/sfr/next/next/include/linux/kconfig.h -m32 -isystem /usr/lib/gcc/powerpc64le-linux-gnu/10/include -mbig-endian -nostdinc -c -o arch/powerpc/boot/crt0.o /home/sfr/next/next/arch/powerpc/boot/crt0.S

I am not sure what is missing here to get __has_attribute defined.

I have reverted the above commit for today ( and reapplied the bits
from commits

  7ed012969bbc ("Compiler Attributes: fix __has_attribute(__no_sanitize_coverage__) for GCC 4")
  b83a908498d6 ("compiler_attributes.h: move __compiletime_{error|warning}")
).

I assume that this needs some work in the powerpc arch Makfile(s) ...
-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Linus Torvalds]

On Mon, Sep 13, 2021 at 5:09 PM Stephen Rothwell <sfr@canb.auug.org.au> 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

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

[Linus Torvalds]

On Mon, Sep 13, 2021 at 5:19 PM Linus Torvalds
<torvalds@linux-foundation.org> 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

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 893 bytes --]

Hi Linus,

On Mon, 13 Sep 2021 17:24:11 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
> On Mon, Sep 13, 2021 at 5:19 PM Linus Torvalds
> <torvalds@linux-foundation.org> 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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Linus Torvalds]

[-- Attachment #1: Type: text/plain, Size: 2449 bytes --]

On Mon, Sep 13, 2021 at 5:58 PM Stephen Rothwell <sfr@canb.auug.org.au> 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

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 1611 bytes --]

 arch/powerpc/boot/Makefile | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 6900d0ac2421..9bcf62d65509 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -32,28 +32,30 @@ else
     BOOTAR := $(AR)
 endif
 
-BOOTCFLAGS    := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
-		 -fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx \
-		 -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \
-		 -include $(srctree)/include/linux/compiler_attributes.h \
-		 $(LINUXINCLUDE)
+BOOTCOREFLAGS := $(LINUXINCLUDE)
 
 ifdef CONFIG_PPC64_BOOT_WRAPPER
-BOOTCFLAGS	+= -m64
+BOOTCOREFLAGS	+= -m64
 else
-BOOTCFLAGS	+= -m32
+BOOTCOREFLAGS	+= -m32
 endif
 
-BOOTCFLAGS	+= -isystem $(shell $(BOOTCC) -print-file-name=include)
+BOOTCOREFLAGS	+= -isystem $(shell $(BOOTCC) -print-file-name=include)
 
 ifdef CONFIG_CPU_BIG_ENDIAN
-BOOTCFLAGS	+= -mbig-endian
+BOOTCOREFLAGS	+= -mbig-endian
 else
-BOOTCFLAGS	+= -mlittle-endian
-BOOTCFLAGS	+= $(call cc-option,-mabi=elfv2)
+BOOTCOREFLAGS	+= -mlittle-endian
+BOOTCOREFLAGS	+= $(call cc-option,-mabi=elfv2)
 endif
 
-BOOTAFLAGS	:= -D__ASSEMBLY__ $(BOOTCFLAGS) -nostdinc
+BOOTCFLAGS    := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
+		 -fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx \
+		 -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \
+		 -include $(srctree)/include/linux/compiler_attributes.h \
+		 $(BOOTCOREFLAGS)
+
+BOOTAFLAGS	:= -D__ASSEMBLY__ $(BOOTCOREFLAGS) -nostdinc
 
 BOOTARFLAGS	:= -crD
 

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

[Linus Torvalds]

On Mon, Sep 13, 2021 at 6:29 PM Linus Torvalds
<torvalds@linux-foundation.org> 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

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

[Linus Torvalds]

On Mon, Sep 13, 2021 at 6:37 PM Linus Torvalds
<torvalds@linux-foundation.org> 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

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2904 bytes --]

Hi Linus,

On Mon, 13 Sep 2021 18:29:26 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
> On Mon, Sep 13, 2021 at 5:58 PM Stephen Rothwell <sfr@canb.auug.org.au> 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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Linus Torvalds]

On Mon, Sep 13, 2021 at 7:08 PM Stephen Rothwell <sfr@canb.auug.org.au> 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

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

[Michael Ellerman]

Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Mon, Sep 13, 2021 at 7:08 PM Stephen Rothwell <sfr@canb.auug.org.au> 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".

I'll have a look and get back to you before tomorrow.

cheers

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

[Michael Ellerman]

Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Mon, Sep 13, 2021 at 7:08 PM Stephen Rothwell <sfr@canb.auug.org.au> 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".

I ended up doing a more minimal version of your change.

I sent it separately, or it's here:

  https://lore.kernel.org/lkml/20210914121723.3756817-1-mpe@ellerman.id.au/


cheers

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 280 bytes --]

Hi all,

On Tue, 14 Sep 2021 12:08:18 +1000 Stephen Rothwell <sfr@canb.auug.org.au> 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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]