[PATCH] kernel: use the gnu89 standard explicitly

[Sasha Levin]

gcc5 changes the default standard to c11, rather than gnu89, which
makes kernel build unhappy.

Explicitly define the kernel standard to be gnu89 which should
keep everything working exactly like it was before gcc5.

Signed-off-by: Sasha Levin <***@oracle.com>
---
Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index dd7e1cb..43f31cb 100644
--- a/Makefile
+++ b/Makefile
@@ -642,7 +642,8 @@ ifdef CONFIG_READABLE_ASM
# partial inlining inlines only parts of functions
KBUILD_CFLAGS += $(call cc-option,-fno-reorder-blocks,) \
$(call cc-option,-fno-ipa-cp-clone,) \
- $(call cc-option,-fno-partial-inlining)
+ $(call cc-option,-fno-partial-inlining,) \
+ $(call cc-option,-std=gnu89)
endif

ifneq ($(CONFIG_FRAME_WARN),0)

[Linus Torvalds]

Post by Sasha Levin
gcc5 changes the default standard to c11, rather than gnu89, which
makes kernel build unhappy.

Hmm. Just how unhappy does it make it? Maybe we can instead try to
make the build happier with some minimal changes? Or is it really
painful?

Linus

[Joe Perches]

Post by Linus Torvalds

Post by Sasha Levin
gcc5 changes the default standard to c11, rather than gnu89, which
makes kernel build unhappy.

Hmm. Just how unhappy does it make it? Maybe we can instead try to
make the build happier with some minimal changes? Or is it really
painful?

I think it'd be moderately painful.

There are ~1000 uses of variables like "<some_type> new" in the kernel.

[Joe Perches]

Post by Joe Perches
I think it'd be moderately painful.
There are ~1000 uses of variables like "<some_type> new" in the kernel.

So? That's not a C11 violation last I saw. The whole "new" keyword is still
just a C++ problem AFAIK.
And if that is some stupid gcc5 thing, maybe just turning off that
particular gcc bug is possible?

My mistake, false premise.

Use of the <some_type> new is not an issue with
gcc 4.9 -std=gnu11.

[Aaro Koskinen]

Hi,

Post by Linus Torvalds

Post by Sasha Levin
gcc5 changes the default standard to c11, rather than gnu89, which
makes kernel build unhappy.

(I guess the default will be gnu11 instead of c11.)

Post by Linus Torvalds
Hmm. Just how unhappy does it make it? Maybe we can instead try to
make the build happier with some minimal changes? Or is it really
painful?

Here's one example how it fails: http://marc.info/?l=gcc&m=141349914632010&w=2

It's probably arch-independent as on SPARC I saw a similar failure
(I didn't test any others).

A.

[Linus Torvalds]

Post by Aaro Koskinen
Here's one example how it fails: http://marc.info/?l=gcc&m=141349914632010&w=2

Ok, that just looks like a gnu11 bug, then. Not being able to
initialize structures because some sub-structure has a volatile member
is just pure BS.

Has anybody reported this as a gcc bug? That email may be on the gcc
list, but I'm not seeing anybody acknowledge it as a bug..

I cannot imagine that anybody sane claims that this is *wanted*
behavior from "gnu11".

Linus

[Kirill A. Shutemov]

Post by Linus Torvalds

Post by Aaro Koskinen
Here's one example how it fails: http://marc.info/?l=gcc&m=141349914632010&w=2

Ok, that just looks like a gnu11 bug, then. Not being able to
initialize structures because some sub-structure has a volatile member
is just pure BS.
Has anybody reported this as a gcc bug? That email may be on the gcc
list, but I'm not seeing anybody acknowledge it as a bug..
I cannot imagine that anybody sane claims that this is *wanted*
behavior from "gnu11".

IIUC, it's nothing to do with volatile. C11 and above reads

(rwlock_t) { .raw_lock = { 0 }, }

as compound literal (which is not constant) rather than constant
initalizer plus a cast.

[Al Viro]

Post by Kirill A. Shutemov

Post by Linus Torvalds

Post by Aaro Koskinen
Here's one example how it fails: http://marc.info/?l=gcc&m=141349914632010&w=2

Ok, that just looks like a gnu11 bug, then. Not being able to
initialize structures because some sub-structure has a volatile member
is just pure BS.
Has anybody reported this as a gcc bug? That email may be on the gcc
list, but I'm not seeing anybody acknowledge it as a bug..
I cannot imagine that anybody sane claims that this is *wanted*
behavior from "gnu11".

IIUC, it's nothing to do with volatile. C11 and above reads
(rwlock_t) { .raw_lock = { 0 }, }
as compound literal (which is not constant) rather than constant
initalizer plus a cast.

Ah... They hadn't even pulled it into gnu99; IIRC, they even tried to
remove it in gnu89, but Linus' complaints had stopped that attempt.

[Kirill A. Shutemov]

Post by Kirill A. Shutemov

Post by Linus Torvalds

Post by Aaro Koskinen
Here's one example how it fails: http://marc.info/?l=gcc&m=141349914632010&w=2

Ok, that just looks like a gnu11 bug, then. Not being able to
initialize structures because some sub-structure has a volatile member
is just pure BS.
Has anybody reported this as a gcc bug? That email may be on the gcc
list, but I'm not seeing anybody acknowledge it as a bug..
I cannot imagine that anybody sane claims that this is *wanted*
behavior from "gnu11".

IIUC, it's nothing to do with volatile. C11 and above reads

s/C11/C99/

Post by Kirill A. Shutemov
(rwlock_t) { .raw_lock = { 0 }, }
as compound literal (which is not constant) rather than constant
initalizer plus a cast.

In some places we can just drop the cast, but it doesn't work everywhere.
I don't see a way to get pre-c99 semantics here.

[Al Viro]

Post by Kirill A. Shutemov
s/C11/C99/

Post by Kirill A. Shutemov
(rwlock_t) { .raw_lock = { 0 }, }
as compound literal (which is not constant) rather than constant
initalizer plus a cast.

In some places we can just drop the cast, but it doesn't work everywhere.
I don't see a way to get pre-c99 semantics here.

I have a moderately bitrotten patchset taking care of that; might make sense
to resurrect it...

[Linus Torvalds]

On Sun, Oct 19, 2014 at 4:10 PM, Kirill A. Shutemov

Post by Kirill A. Shutemov
IIUC, it's nothing to do with volatile. C11 and above reads
(rwlock_t) { .raw_lock = { 0 }, }
as compound literal (which is not constant) rather than constant
initalizer plus a cast.

Ahh. I thought it was the volatile, just because of the "constant"
part and the fact that it looked like the test-case was minimal. But
apparently that was just a red herring.

So it's just the fact that C11 hasn't got the useful GNU extension of
a cast-with-initializer, and then that extension got removed (probably
just an oversight) because people thought that the "standard" C11
initializers were good enough.

And it sounds like this will get fixed (perhaps already is) in gcc anyway.

So maybe we need to do that "--std=gnu89" as a temporary workaround,
but not as long long-term "newer versions are broken".

That doesn't sound too bad. My main objection to the patch was a
"let's try to fix this properly instead", but if the breakage is just
a temporary problem, there's no issue with "properly". Although it
does look like the *placement* of the --std=gnu89 was incorrect in
that original patch, so it needs updating regardless.

AndrewP, mind explaing the other difference you mentioned (ie wrt
"extern inline")? I thought we had already long since ended up
following the gcc semantics (ie use "static inline" if we don't have
an external version somehwre), what exactly changed?

Thanks,

Linus

[Sasha Levin]

Post by Linus Torvalds
On Sun, Oct 19, 2014 at 4:10 PM, Kirill A. Shutemov

Post by Kirill A. Shutemov
IIUC, it's nothing to do with volatile. C11 and above reads
(rwlock_t) { .raw_lock = { 0 }, }
as compound literal (which is not constant) rather than constant
initalizer plus a cast.

Ahh. I thought it was the volatile, just because of the "constant"
part and the fact that it looked like the test-case was minimal. But
apparently that was just a red herring.
So it's just the fact that C11 hasn't got the useful GNU extension of
a cast-with-initializer, and then that extension got removed (probably
just an oversight) because people thought that the "standard" C11
initializers were good enough.
And it sounds like this will get fixed (perhaps already is) in gcc anyway.

It was already fixed in gcc (I've reported it here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63567)

Post by Linus Torvalds
So maybe we need to do that "--std=gnu89" as a temporary workaround,
but not as long long-term "newer versions are broken".

Agreed, I could send patches to fix the build issues resulting from c11,
but I'm afraid that it'll need real good testing to uncover things that don't
show up during the build.

Post by Linus Torvalds
That doesn't sound too bad. My main objection to the patch was a
"let's try to fix this properly instead", but if the breakage is just
a temporary problem, there's no issue with "properly". Although it
does look like the *placement* of the --std=gnu89 was incorrect in
that original patch, so it needs updating regardless.

Apologies. It seemed like the right place to me and it worked fine when
I tested it.

Post by Linus Torvalds
AndrewP, mind explaing the other difference you mentioned (ie wrt
"extern inline")? I thought we had already long since ended up
following the gcc semantics (ie use "static inline" if we don't have
an external version somehwre), what exactly changed?

(Stolen from gcc changelog:)

gnu89: extern inline: Will not generate an out-of-line version, but
might call one.
gnu99: extern inline: like GNU "inline", externally visible code is
emitted.

(So what happens is that with gnu99 you end up with multiple definitions
of the symbol since it was externed from multiple compilation units).


Thanks,
Sasha

[Linus Torvalds]

Post by Sasha Levin

Post by Linus Torvalds
AndrewP, mind explaing the other difference you mentioned (ie wrt
"extern inline")? I thought we had already long since ended up
following the gcc semantics (ie use "static inline" if we don't have
an external version somehwre), what exactly changed?

(Stolen from gcc changelog:)
gnu89: extern inline: Will not generate an out-of-line version, but
might call one.
gnu99: extern inline: like GNU "inline", externally visible code is
emitted.
(So what happens is that with gnu99 you end up with multiple definitions
of the symbol since it was externed from multiple compilation units).

Oh Christ. So this got broken yet again, even *after* they had
documented the old behavior?

Originally, gcc documented that "extern inline" is a good replacement
for a macro. Then, that changed, and "static inline" became the
replacement for a macro, and "extern inline" was to mean that *if* it
gets inlined, that definition is used, but otherwise there's supposed
to be an external non-inlined copy somewhere else (so the inline
definition of the function is basically entirely ignored when not
inlining for one reason or another).

So now we have a *third* semantic of "extern inline", and one that
seems to be entirely inappropriate to *ever* be used in a header file
due to duplicate symbol problems. What a mess.

Maybe we should just specify "gnu89" to avoid these kinds of insane
semantic changes.

Linus

[Al Viro]

Post by Linus Torvalds
So now we have a *third* semantic of "extern inline", and one that
seems to be entirely inappropriate to *ever* be used in a header file
due to duplicate symbol problems. What a mess.
Maybe we should just specify "gnu89" to avoid these kinds of insane
semantic changes.

Umm... does anything except alpha really use extern inline? The instances
outside of arch/alpha seem to be mostly cargo-culting; lib/mpi might be
another one, but other than that...

[p***@gmail.com]

Post by Linus Torvalds

Post by Aaro Koskinen
Here's one example how it fails: http://marc.info/?l=gcc&m=141349914632010&w=2

Ok, that just looks like a gnu11 bug, then. Not being able to
initialize structures because some sub-structure has a volatile member
is just pure BS.
Has anybody reported this as a gcc bug? That email may be on the gcc
list, but I'm not seeing anybody acknowledge it as a bug..

Yes it was reported and both problems relating to this extension has been added to gnu99 and gnu11. Though there are other issues with the kernel dealing with extern inline have different semantics between gnu89 and gnu99/11.

Thanks,
Andrew

Post by Linus Torvalds
I cannot imagine that anybody sane claims that this is *wanted*
behavior from "gnu11".
Linus

[Sasha Levin]

Post by Linus Torvalds

Post by Sasha Levin
gcc5 changes the default standard to c11, rather than gnu89, which
makes kernel build unhappy.

Hmm. Just how unhappy does it make it? Maybe we can instead try to
make the build happier with some minimal changes? Or is it really
painful?

The build breakage is easy to fix. I'm worried about the stuff that won't show
upon build. Different behaviours that go unnoticed and maybe issues inside
gcc's code generation that weren't uncovered until now.

Also, unless we specify exactly what standard we want what would happen is that
a significantly different C standard would be used depending on the compiler
version. It is already the case with clang but putting in a 3rd one in the
mix is too much IMO.

I'll send a few other patches to make it at least compile under c11, but I still
think that we should plug this hole by sticking with gnu89 for now.

Thanks,
Sasha

[Kirill A. Shutemov]

Post by Sasha Levin
gcc5 changes the default standard to c11, rather than gnu89, which
makes kernel build unhappy.
Explicitly define the kernel standard to be gnu89 which should
keep everything working exactly like it was before gcc5.
---
Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index dd7e1cb..43f31cb 100644
--- a/Makefile
+++ b/Makefile
@@ -642,7 +642,8 @@ ifdef CONFIG_READABLE_ASM
# partial inlining inlines only parts of functions
KBUILD_CFLAGS += $(call cc-option,-fno-reorder-blocks,) \
$(call cc-option,-fno-ipa-cp-clone,) \
- $(call cc-option,-fno-partial-inlining)
+ $(call cc-option,-fno-partial-inlining,) \
+ $(call cc-option,-std=gnu89)
endif

Hm. Why is that under #ifdef CONFIG_READABLE_ASM ?