[BUILD FAILURE] nfs4state.c fails to compile with gcc 4.5.4

[BUILD FAILURE] nfs4state.c fails to compile with gcc 4.5.4

[Steven Rostedt]

Just letting you know that this commit:

fcd8843c406 ('NFSv4: Replace closed stateids with the "invalid special
stateid"') causes gcc 4.5.4 to fail the build with this error:

  CC [M]  fs/nfs/nfs4state.o
/work/git/linux-trace.git/fs/nfs/nfs4state.c:75:4: error: field name not in record or union initializer
/work/git/linux-trace.git/fs/nfs/nfs4state.c:75:4: error: (near initialization for 'invalid_stateid.<anonymous>.data')
/work/git/linux-trace.git/fs/nfs/nfs4state.c:75:4: warning: overflow in implicit constant conversion
/work/git/linux-trace.git/fs/nfs/nfs4state.c:76:4: error: field name not in record or union initializer
/work/git/linux-trace.git/fs/nfs/nfs4state.c:76:4: error: (near initialization for 'invalid_stateid.<anonymous>.data')
/work/git/linux-trace.git/fs/nfs/nfs4state.c:76:4: warning: braces around scalar initializer
/work/git/linux-trace.git/fs/nfs/nfs4state.c:76:4: warning: (near initialization for 'invalid_stateid.<anonymous>.data[1]')
make[3]: *** [/work/git/linux-trace.git/scripts/Makefile.build:310: fs/nfs/nfs4state.o] Error 1
make[2]: *** [/work/git/linux-trace.git/scripts/Makefile.build:569: fs/nfs] Error 2
make[1]: *** [/work/git/linux-trace.git/Makefile:1012: fs] Error 2
make[1]: Leaving directory '/work/git/nobackup/bxtest/trace'

Seems it can't handle the initialization of an anonymous struct within
an anonymous union.

I switched to gcc 4.6.3 and it compiles fine.

-- Steve

Re: [BUILD FAILURE] nfs4state.c fails to compile with gcc 4.5.4

[Linus Torvalds]

On Wed, Nov 29, 2017 at 6:58 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Seems it can't handle the initialization of an anonymous struct within
> an anonymous union.

I think we've seen this before, and I think there was some trick to
make it work with older gcc versions.

Maybe the unnamed entry that gets initialized had to be declared
first, and the initializer needed an extra set of braces?

Something like that.

But maybe the answer is just to say "4.5.4 is really old".

Doing some log digging gives me

    ee9d3429c0e4 ("net/sched/sch_red.c: work around gcc-4.4.4 anon
union initializer issue")
    aa4bf44dc851 ("userns: use union in {g,u}idmap struct")
    6c09ffd02725 ("irqchip/gic-v4: Fix building with ancient gcc")
    c848c49a62b3 ("drivers/media/cec/cec-adap.c: fix build with gcc-4.4.4")

but they aren't necessarily _exactly_ the same issue.

            Linus

Re: [BUILD FAILURE] nfs4state.c fails to compile with gcc 4.5.4

[Andrew Morton]

On Wed, 29 Nov 2017 19:27:45 -0800 Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Wed, Nov 29, 2017 at 6:58 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > Seems it can't handle the initialization of an anonymous struct within
> > an anonymous union.
> 
> I think we've seen this before, and I think there was some trick to
> make it work with older gcc versions.
> 
> Maybe the unnamed entry that gets initialized had to be declared
> first, and the initializer needed an extra set of braces?
> 
> Something like that.
> 
> But maybe the answer is just to say "4.5.4 is really old".
> 
> Doing some log digging gives me
> 
>     ee9d3429c0e4 ("net/sched/sch_red.c: work around gcc-4.4.4 anon
> union initializer issue")
>     aa4bf44dc851 ("userns: use union in {g,u}idmap struct")
>     6c09ffd02725 ("irqchip/gic-v4: Fix building with ancient gcc")
>     c848c49a62b3 ("drivers/media/cec/cec-adap.c: fix build with gcc-4.4.4")
> 
> but they aren't necessarily _exactly_ the same issue.

Yeah, I'm the world expert at working around this gcc bug but the nfs4
one had me stumped when I looked at it, so I shelved it for revisiting
later.

It would be nice to fix it, given that this is the only (known) site in
the kernel which rules out these compiler versions.

Re: [BUILD FAILURE] nfs4state.c fails to compile with gcc 4.5.4

[Linus Torvalds]

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

On Wed, Nov 29, 2017 at 7:39 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
>
> Yeah, I'm the world expert at working around this gcc bug but the nfs4
> one had me stumped when I looked at it, so I shelved it for revisiting
> later.

Does the attached patch fix it? I don't have old compilers.

           Linus

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

 fs/nfs/nfs4state.c   | 3 +--
 include/linux/nfs4.h | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 54fd56d715a8..9db5d0d57fcb 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -71,8 +71,7 @@ const nfs4_stateid zero_stateid = {
 };
 const nfs4_stateid invalid_stateid = {
 	{
-		.seqid = cpu_to_be32(0xffffffffU),
-		.other = { 0 },
+		{ .seqid = cpu_to_be32(0xffffffffU), .other = { 0 }, },
 	},
 	.type = NFS4_INVALID_STATEID_TYPE,
 };
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 47adac640191..e394ba0cd6c5 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -53,11 +53,11 @@ typedef struct { char data[NFS4_VERIFIER_SIZE]; } nfs4_verifier;
 
 struct nfs4_stateid_struct {
 	union {
-		char data[NFS4_STATEID_SIZE];
 		struct {
 			__be32 seqid;
 			char other[NFS4_STATEID_OTHER_SIZE];
 		} __attribute__ ((packed));
+		char data[NFS4_STATEID_SIZE];
 	};
 
 	enum {

Re: [BUILD FAILURE] nfs4state.c fails to compile with gcc 4.5.4

[Andrew Morton]

On Wed, 29 Nov 2017 19:44:33 -0800 Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Wed, Nov 29, 2017 at 7:39 PM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
> >
> > Yeah, I'm the world expert at working around this gcc bug but the nfs4
> > one had me stumped when I looked at it, so I shelved it for revisiting
> > later.
> 
> Does the attached patch fix it? I don't have old compilers.


yup, thanks.

Re: [BUILD FAILURE] nfs4state.c fails to compile with gcc 4.5.4

[Steven Rostedt]

On Wed, 29 Nov 2017 19:44:33 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:

I keep older compilers around to test ftrace before -mfentry was
introduced.

I wonder if I should just add a config to force the use without
-mfentry even if the compiler supports it.

>  fs/nfs/nfs4state.c   | 3 +--
>  include/linux/nfs4.h | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index 54fd56d715a8..9db5d0d57fcb 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -71,8 +71,7 @@ const nfs4_stateid zero_stateid = {
>  };
>  const nfs4_stateid invalid_stateid = {
>  	{
> -		.seqid = cpu_to_be32(0xffffffffU),
> -		.other = { 0 },
> +		{ .seqid = cpu_to_be32(0xffffffffU), .other = { 0 }, },

I tried this and it failed.

>  	},
>  	.type = NFS4_INVALID_STATEID_TYPE,
>  };
> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> index 47adac640191..e394ba0cd6c5 100644
> --- a/include/linux/nfs4.h
> +++ b/include/linux/nfs4.h
> @@ -53,11 +53,11 @@ typedef struct { char data[NFS4_VERIFIER_SIZE]; } nfs4_verifier;
>  
>  struct nfs4_stateid_struct {
>  	union {
> -		char data[NFS4_STATEID_SIZE];
>  		struct {
>  			__be32 seqid;
>  			char other[NFS4_STATEID_OTHER_SIZE];
>  		} __attribute__ ((packed));
> +		char data[NFS4_STATEID_SIZE];

But I didn't try this. This looks to be the key.

This works for me.

Tested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

>  	};
>  
>  	enum {

RE: [BUILD FAILURE] nfs4state.c fails to compile with gcc 4.5.4

[David Laight]

From: Steven Rostedt
> Sent: 30 November 2017 02:59
> Subject: [BUILD FAILURE] nfs4state.c fails to compile with gcc 4.5.4

FWIW I can't build the current kernel with gcc 4.7.3-1ubuntu1
under ubuntu 13.04.

The compiler generates some object files that binutils (objdump/ld) 2.23.2
says are invalid.

I've not looked at the hexdump to look for anything 'funny'.

	David

Re: [BUILD FAILURE] nfs4state.c fails to compile with gcc 4.5.4

[Trond Myklebust]

On Wed, 2017-11-29 at 23:05 -0500, Steven Rostedt wrote:
> On Wed, 29 Nov 2017 19:44:33 -0800
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> I keep older compilers around to test ftrace before -mfentry was
> introduced.
> 
> I wonder if I should just add a config to force the use without
> -mfentry even if the compiler supports it.
> 
> >  fs/nfs/nfs4state.c   | 3 +--
> >  include/linux/nfs4.h | 2 +-
> >  2 files changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> > index 54fd56d715a8..9db5d0d57fcb 100644
> > --- a/fs/nfs/nfs4state.c
> > +++ b/fs/nfs/nfs4state.c
> > @@ -71,8 +71,7 @@ const nfs4_stateid zero_stateid = {
> >  };
> >  const nfs4_stateid invalid_stateid = {
> >  	{
> > -		.seqid = cpu_to_be32(0xffffffffU),
> > -		.other = { 0 },
> > +		{ .seqid = cpu_to_be32(0xffffffffU), .other = { 0
> > }, },
> 
> I tried this and it failed.
> 
> >  	},
> >  	.type = NFS4_INVALID_STATEID_TYPE,
> >  };
> > diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> > index 47adac640191..e394ba0cd6c5 100644
> > --- a/include/linux/nfs4.h
> > +++ b/include/linux/nfs4.h
> > @@ -53,11 +53,11 @@ typedef struct { char data[NFS4_VERIFIER_SIZE];
> > } nfs4_verifier;
> >  
> >  struct nfs4_stateid_struct {
> >  	union {
> > -		char data[NFS4_STATEID_SIZE];
> >  		struct {
> >  			__be32 seqid;
> >  			char other[NFS4_STATEID_OTHER_SIZE];
> >  		} __attribute__ ((packed));
> > +		char data[NFS4_STATEID_SIZE];
> 
> But I didn't try this. This looks to be the key.
> 
> This works for me.
> 
> Tested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> 
> -- Steve
> 
> >  	};
> >  
> >  	enum {
> 
> 

We already have a fix queued up for this issue. It was reported and the
patch hit linux-nfs+linux-kernel almost 2 weeks ago. Anna?

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com

Re: [BUILD FAILURE] nfs4state.c fails to compile with gcc 4.5.4

[Arnd Bergmann]

On Thu, Nov 30, 2017 at 4:39 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Wed, 29 Nov 2017 19:27:45 -0800 Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>> On Wed, Nov 29, 2017 at 6:58 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
>> >
>> > Seems it can't handle the initialization of an anonymous struct within
>> > an anonymous union.
>>
>> I think we've seen this before, and I think there was some trick to
>> make it work with older gcc versions.
>>
>> Maybe the unnamed entry that gets initialized had to be declared
>> first, and the initializer needed an extra set of braces?
>>
>> Something like that.
>>
>> But maybe the answer is just to say "4.5.4 is really old".
>>
>> Doing some log digging gives me
>>
>>     ee9d3429c0e4 ("net/sched/sch_red.c: work around gcc-4.4.4 anon
>> union initializer issue")
>>     aa4bf44dc851 ("userns: use union in {g,u}idmap struct")
>>     6c09ffd02725 ("irqchip/gic-v4: Fix building with ancient gcc")
>>     c848c49a62b3 ("drivers/media/cec/cec-adap.c: fix build with gcc-4.4.4")
>>
>> but they aren't necessarily _exactly_ the same issue.
>
> Yeah, I'm the world expert at working around this gcc bug but the nfs4
> one had me stumped when I looked at it, so I shelved it for revisiting
> later.
>
> It would be nice to fix it, given that this is the only (known) site in
> the kernel which rules out these compiler versions.

There are more in drivers/irqchip/irq-gic-v4.c and virt/kvm/arm/vgic/, but
they are not built on x86. I've fixed it before, but it seems to be coming
back each time a new GIC variant gets added.

If anyone cares, I can find the remaining patches I have for this and
send them again.

      Arnd

Re: [BUILD FAILURE] nfs4state.c fails to compile with gcc 4.5.4

[Anna Schumaker]

On 11/30/2017 06:46 AM, Trond Myklebust wrote:
> On Wed, 2017-11-29 at 23:05 -0500, Steven Rostedt wrote:
>> On Wed, 29 Nov 2017 19:44:33 -0800
>> Linus Torvalds <torvalds@linux-foundation.org> wrote:
>>
>> I keep older compilers around to test ftrace before -mfentry was
>> introduced.
>>
>> I wonder if I should just add a config to force the use without
>> -mfentry even if the compiler supports it.
>>
>>>  fs/nfs/nfs4state.c   | 3 +--
>>>  include/linux/nfs4.h | 2 +-
>>>  2 files changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
>>> index 54fd56d715a8..9db5d0d57fcb 100644
>>> --- a/fs/nfs/nfs4state.c
>>> +++ b/fs/nfs/nfs4state.c
>>> @@ -71,8 +71,7 @@ const nfs4_stateid zero_stateid = {
>>>  };
>>>  const nfs4_stateid invalid_stateid = {
>>>  	{
>>> -		.seqid = cpu_to_be32(0xffffffffU),
>>> -		.other = { 0 },
>>> +		{ .seqid = cpu_to_be32(0xffffffffU), .other = { 0
>>> }, },
>>
>> I tried this and it failed.
>>
>>>  	},
>>>  	.type = NFS4_INVALID_STATEID_TYPE,
>>>  };
>>> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
>>> index 47adac640191..e394ba0cd6c5 100644
>>> --- a/include/linux/nfs4.h
>>> +++ b/include/linux/nfs4.h
>>> @@ -53,11 +53,11 @@ typedef struct { char data[NFS4_VERIFIER_SIZE];
>>> } nfs4_verifier;
>>>  
>>>  struct nfs4_stateid_struct {
>>>  	union {
>>> -		char data[NFS4_STATEID_SIZE];
>>>  		struct {
>>>  			__be32 seqid;
>>>  			char other[NFS4_STATEID_OTHER_SIZE];
>>>  		} __attribute__ ((packed));
>>> +		char data[NFS4_STATEID_SIZE];
>>
>> But I didn't try this. This looks to be the key.
>>
>> This works for me.
>>
>> Tested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>>
>> -- Steve
>>
>>>  	};
>>>  
>>>  	enum {
>>
>>
> 
> We already have a fix queued up for this issue. It was reported and the
> patch hit linux-nfs+linux-kernel almost 2 weeks ago. Anna?

I have it queued up, and I was planning to send it this week with some other fixes.

Anna
> 

Re: [BUILD FAILURE] nfs4state.c fails to compile with gcc 4.5.4

[Marc Zyngier]

On 30/11/17 12:28, Arnd Bergmann wrote:
> On Thu, Nov 30, 2017 at 4:39 AM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
>> On Wed, 29 Nov 2017 19:27:45 -0800 Linus Torvalds <torvalds@linux-foundation.org> wrote:
>>
>>> On Wed, Nov 29, 2017 at 6:58 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
>>>>
>>>> Seems it can't handle the initialization of an anonymous struct within
>>>> an anonymous union.
>>>
>>> I think we've seen this before, and I think there was some trick to
>>> make it work with older gcc versions.
>>>
>>> Maybe the unnamed entry that gets initialized had to be declared
>>> first, and the initializer needed an extra set of braces?
>>>
>>> Something like that.
>>>
>>> But maybe the answer is just to say "4.5.4 is really old".
>>>
>>> Doing some log digging gives me
>>>
>>>     ee9d3429c0e4 ("net/sched/sch_red.c: work around gcc-4.4.4 anon
>>> union initializer issue")
>>>     aa4bf44dc851 ("userns: use union in {g,u}idmap struct")
>>>     6c09ffd02725 ("irqchip/gic-v4: Fix building with ancient gcc")
>>>     c848c49a62b3 ("drivers/media/cec/cec-adap.c: fix build with gcc-4.4.4")
>>>
>>> but they aren't necessarily _exactly_ the same issue.
>>
>> Yeah, I'm the world expert at working around this gcc bug but the nfs4
>> one had me stumped when I looked at it, so I shelved it for revisiting
>> later.
>>
>> It would be nice to fix it, given that this is the only (known) site in
>> the kernel which rules out these compiler versions.
> 
> There are more in drivers/irqchip/irq-gic-v4.c and virt/kvm/arm/vgic/, but
> they are not built on x86. I've fixed it before, but it seems to be coming
> back each time a new GIC variant gets added.
> 
> If anyone cares, I can find the remaining patches I have for this and
> send them again.

If you have patches for either GIC or the vgic, I'll happily take them.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...