Build failure with GCC 15 (defaults to -std=gnu23)
Hi,
postgres-17.1 fails to build with upcoming GCC 15 which defaults to
-std=gnu23 as follows:
```
In file included from ../../src/include/postgres_fe.h:25,
from checksum_helper.c:17:
../../src/include/c.h:456:23: error: two or more data types in declaration specifiers
456 | typedef unsigned char bool;
| ^~~~
../../src/include/c.h:456:1: warning: useless type name in empty declaration
456 | typedef unsigned char bool;
| ^~~~~~~
```
src/include/c.h does attempt to check for whether bool is already defined but
the check doesn't work.
There may be more issues afterwards but I haven't tried papering over
the above issue. It should be possible to reproduce w/
CFLAGS="... -std=gnu23" or CFLAGS="... -std=c23" on older GCC or Clang.
thanks,
sam
Sam James <sam@gentoo.org> writes:
postgres-17.1 fails to build with upcoming GCC 15 which defaults to
-std=gnu23 as follows:
I do not think we claim to support C23 yet.
Having said that, I can reproduce this on gcc 14 using -std=gnu23.
It appears that configure is deciding that <stdbool.h> is not
conformant to C99 because it doesn't declare "bool" as a macro.
Did C23 really remove that !?
regards, tom lane
On Mon, Nov 18, 2024 at 7:12 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Sam James <sam@gentoo.org> writes:
postgres-17.1 fails to build with upcoming GCC 15 which defaults to
-std=gnu23 as follows:I do not think we claim to support C23 yet.
Having said that, I can reproduce this on gcc 14 using -std=gnu23.
It appears that configure is deciding that <stdbool.h> is not
conformant to C99 because it doesn't declare "bool" as a macro.
Did C23 really remove that !?
Yes, seems to be a general pattern: features introduced as keyword
_Xxx with a library macro xxx -> _Xxx (usually where xxx is already a
keyword in C++ but the C committee was afraid to unleash a new keyword
directly on the world, I guess?), and now xxx is finally graduating to
real keyword status. Other examples: static_assert, thread_local,
alignas.
Thomas Munro <thomas.munro@gmail.com> writes:
On Mon, Nov 18, 2024 at 7:12 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Having said that, I can reproduce this on gcc 14 using -std=gnu23.
It appears that configure is deciding that <stdbool.h> is not
conformant to C99 because it doesn't declare "bool" as a macro.
Did C23 really remove that !?
Yes, seems to be a general pattern: features introduced as keyword
_Xxx with a library macro xxx -> _Xxx (usually where xxx is already a
keyword in C++ but the C committee was afraid to unleash a new keyword
directly on the world, I guess?), and now xxx is finally graduating to
real keyword status. Other examples: static_assert, thread_local,
alignas.
Fun. Well, now that we insist on C99 support in all branches,
I wonder whether we can just remove all the non-stdbool support.
The one thing that looks tricky is that we insist on sizeof(bool)
being 1, but are there any remaining supported platforms where
it isn't? The buildfarm doesn't have any examples.
regards, tom lane
On Mon, Nov 18, 2024 at 9:26 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fun. Well, now that we insist on C99 support in all branches,
I wonder whether we can just remove all the non-stdbool support.
The one thing that looks tricky is that we insist on sizeof(bool)
being 1, but are there any remaining supported platforms where
it isn't? The buildfarm doesn't have any examples.
So far I have found only Apple/Darwin PPC (RIP), where this was
occasionally an issue. Some projects would apparently compile with
-mone-byte-bool to unbreak local assumptions, but risk breaking ABI
with other libraries (as we do?). GCC filed that switch under Darwin
options rather than somewhere more general... can we call that a clue
that it was highly unusual?
https://gcc.gnu.org/onlinedocs/gcc/Darwin-Options.html
There may be a systematic way to survey ABIs from the LLVM or GCC
source trees... hmm, I am no expert so take this with a grain of salt
but I found where the LLVM project defines BoolWidth and BoolAlign,
starting from the commit where they removed Darwin PPC support
(2fe49ea0), and it looks like it was the only target ABI that overrode
the default of 8 there (it had 32, meaning bits).
BTW animal "alligator" has just shown the failure. Ah, yes, due to
this GCC switch being flipped a couple of days ago:
https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=55e3bd376b2214e200fa76d12b67ff259b06c212
Thomas Munro <thomas.munro@gmail.com> writes:
On Mon, Nov 18, 2024 at 9:26 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fun. Well, now that we insist on C99 support in all branches,
I wonder whether we can just remove all the non-stdbool support.
The one thing that looks tricky is that we insist on sizeof(bool)
being 1, but are there any remaining supported platforms where
it isn't? The buildfarm doesn't have any examples.
So far I have found only Apple/Darwin PPC (RIP), where this was
occasionally an issue.
Yeah. Well, what say we leave the "typedef unsigned char bool"
pathway in place, but set things up to use that only if sizeof
the stdbool type isn't 1 --- and then it's up to any hypothetical
users of that pathway to choose a compiler and compiler options
that won't choke on it.
regards, tom lane
On Mon, Nov 18, 2024 at 10:49 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Yeah. Well, what say we leave the "typedef unsigned char bool"
pathway in place, but set things up to use that only if sizeof
the stdbool type isn't 1 --- and then it's up to any hypothetical
users of that pathway to choose a compiler and compiler options
that won't choke on it.
It sounds like we should stop using the old and broken
AC_CHECK_HEADER_STDBOOL macro. I think it was doing two jobs in old
times: there were some systems that shipped a defective/pre-standard
stdbool.h, and some systems without it. I think both classes of
system are gone from the universe. Later autoconf versions check for
C99 "or later", but we're stuck with the old one and I doubt we are
going to upgrade it. Found in their NEWS:
*** AC_HEADER_STDBOOL, AC_CHECK_HEADER_STDBOOL are obsolescent and less picky.
These macros are now obsolescent, as programs can simply include
stdbool.h unconditionally. If you use these macros, they now accept
a stdbool.h that exists but does nothing, so long as ‘bool’, ‘true’,
and ‘false’ work anyway. This is for compatibility with C 2023 and
with C++.
Thomas Munro <thomas.munro@gmail.com> writes:
It sounds like we should stop using the old and broken
AC_CHECK_HEADER_STDBOOL macro.
Yeah, that's what I was imagining: assume that <stdbool.h> exists
and works, and check only to see if sizeof(bool) is acceptable.
... Later autoconf versions check for
C99 "or later", but we're stuck with the old one and I doubt we are
going to upgrade it.
I'm not sure --- there was some discussion a week or two ago about
upgrading autoconf after all. But whether we do or not, it's hard
to see what AC_HEADER_STDBOOL is buying us.
regards, tom lane
On Mon, Nov 18, 2024 at 11:50 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Thomas Munro <thomas.munro@gmail.com> writes:
It sounds like we should stop using the old and broken
AC_CHECK_HEADER_STDBOOL macro.Yeah, that's what I was imagining: assume that <stdbool.h> exists
and works, and check only to see if sizeof(bool) is acceptable.
I think this is the minimal change, which I'd push back to 13 post-freeze.
I found a few lines we could just delete in master. I wonder if we
should also just require sizeof(bool) == 1 more explicitly going
forward with an error, since we don't have coverage or any expectation
of ever getting any for the alternative code AFAICS, even if it is
small.
Thomas Munro <thomas.munro@gmail.com> writes:
I found a few lines we could just delete in master. I wonder if we
should also just require sizeof(bool) == 1 more explicitly going
forward with an error, since we don't have coverage or any expectation
of ever getting any for the alternative code AFAICS, even if it is
small.
Yeah, that's a fair criticism. I don't think we've tested that code
path since I retired prairiedog, so who's to say that it works even
now? Maybe it's best to just delete that code, and if we ever find a
new platform with wider bool, figure out what to do at that time.
regards, tom lane
On 18.11.24 02:30, Thomas Munro wrote:
On Mon, Nov 18, 2024 at 11:50 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Thomas Munro <thomas.munro@gmail.com> writes:
It sounds like we should stop using the old and broken
AC_CHECK_HEADER_STDBOOL macro.Yeah, that's what I was imagining: assume that <stdbool.h> exists
and works, and check only to see if sizeof(bool) is acceptable.I think this is the minimal change, which I'd push back to 13 post-freeze.
Note that if we backpatch C23 support, we also need to backpatch at
least commits a67a49648d9 and d2b4b4c2259.
Peter Eisentraut <peter@eisentraut.org> writes:
Note that if we backpatch C23 support, we also need to backpatch at
least commits a67a49648d9 and d2b4b4c2259.
Yeah. Our normal theory for this kind of thing is "people are
likely to build our old branches with modern toolchains", so
we are going to have to back-patch C23 compatibility sooner or
later. In fact, we'll have to back-patch to 9.2, or else
decide that those branches are unbuildable on modern platforms
and hence out of scope for compatibility testing.
We have a little bit of grace time before this needs to happen,
but perhaps not very much.
regards, tom lane
On 20.11.24 16:32, Tom Lane wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
Note that if we backpatch C23 support, we also need to backpatch at
least commits a67a49648d9 and d2b4b4c2259.Yeah. Our normal theory for this kind of thing is "people are
likely to build our old branches with modern toolchains", so
we are going to have to back-patch C23 compatibility sooner or
later. In fact, we'll have to back-patch to 9.2, or else
decide that those branches are unbuildable on modern platforms
and hence out of scope for compatibility testing.
I have checked that with this patch and the two above (well, one is just
to remove a warning), you can get PG16 and up building cleanly with
gcc-14 -std=gnu23.
Before that, you get a ton of warnings and errors related to the node
tree walker routines. This is presumably related to commit 1c27d16e6e5.
Going further back, the bool patch proposed here assumes that stdbool.h
exists unconditionally, which is C99, which is not the baseline for
older branches. I think for those it's probably best to leave it alone
and just use gcc-15 -std=gnu89 or whatever.
On Thu, Nov 21, 2024 at 8:23 PM Peter Eisentraut <peter@eisentraut.org> wrote:
On 20.11.24 16:32, Tom Lane wrote:
Yeah. Our normal theory for this kind of thing is "people are
likely to build our old branches with modern toolchains", so
we are going to have to back-patch C23 compatibility sooner or
later. In fact, we'll have to back-patch to 9.2, or else
decide that those branches are unbuildable on modern platforms
and hence out of scope for compatibility testing.I have checked that with this patch and the two above (well, one is just
to remove a warning), you can get PG16 and up building cleanly with
gcc-14 -std=gnu23.
Thanks. I pushed the <stdbool.h> thing, which didn't require going
back very far.
Before that, you get a ton of warnings and errors related to the node
tree walker routines. This is presumably related to commit 1c27d16e6e5.
Alligator is now getting past the bool troubles and reaching that
stuff. I was expecting it to be green in master. It's OK on my
slightly older "gcc version 15.0.0 20241110 (experimental) (FreeBSD
Ports Collection)" with -std=gnu23, but alligator now shows a weird
error with tsearch data types. Something about flexible array members
(casting from non-flex to flex?, without saying where the cast is?),
but IDK, it's an internal error asking for a bug to be filed, not a
user-facing one.
This might be relevant: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113688
Going further back, the bool patch proposed here assumes that stdbool.h
exists unconditionally, which is C99, which is not the baseline for
older branches. I think for those it's probably best to leave it alone
and just use gcc-15 -std=gnu89 or whatever.
There's only one C89 branch that knows about <stdbool.h>:
REL_11_STABLE. That's recent enough that it's still easy to work
with, so I just changed it to use AC_CHECK_HEADER instead. In other
words, we've removed the bogus "conforms" check. Whether you still
need a presence check depends on the C version, and only for 11 is the
answer yes. Obviously nobody is really going to build with an actual
C89 system so the presence check is never going to fail, but it would
be weird on principle to suddenly require a C99 thing...
On 25.11.24 11:01, Thomas Munro wrote:
I have checked that with this patch and the two above (well, one is just
to remove a warning), you can get PG16 and up building cleanly with
gcc-14 -std=gnu23.Thanks. I pushed the <stdbool.h> thing, which didn't require going
back very far.Before that, you get a ton of warnings and errors related to the node
tree walker routines. This is presumably related to commit 1c27d16e6e5.Alligator is now getting past the bool troubles and reaching that
stuff. I was expecting it to be green in master. It's OK on my
slightly older "gcc version 15.0.0 20241110 (experimental) (FreeBSD
Ports Collection)" with -std=gnu23, but alligator now shows a weird
error with tsearch data types. Something about flexible array members
(casting from non-flex to flex?, without saying where the cast is?),
but IDK, it's an internal error asking for a bug to be filed, not a
user-facing one.This might be relevant: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113688
Going further back, the bool patch proposed here assumes that stdbool.h
exists unconditionally, which is C99, which is not the baseline for
older branches. I think for those it's probably best to leave it alone
and just use gcc-15 -std=gnu89 or whatever.There's only one C89 branch that knows about <stdbool.h>:
REL_11_STABLE. That's recent enough that it's still easy to work
with, so I just changed it to use AC_CHECK_HEADER instead. In other
words, we've removed the bogus "conforms" check. Whether you still
need a presence check depends on the C version, and only for 11 is the
answer yes. Obviously nobody is really going to build with an actual
C89 system so the presence check is never going to fail, but it would
be weird on principle to suddenly require a C99 thing...
Where does this leave us regarding backpatching the other two
C23-related patches? The node tree walker issue looks like a very hard
barrier. I don't want to spend too much effort backpatching anything to
ancient version if there's little hope of getting the whole thing working.
On Tue, Nov 26, 2024 at 4:57 AM Peter Eisentraut <peter@eisentraut.org> wrote:
Where does this leave us regarding backpatching the other two
C23-related patches? The node tree walker issue looks like a very hard
barrier. I don't want to spend too much effort backpatching anything to
ancient version if there's little hope of getting the whole thing working.
Oh. Yeah. 1c27d16e6e5 was not back-patchable. And what f9a56e72 did
in 15 and older doesn't seem to have any equivalent in C23, at least
without going way overboard. -Wdeprecated-non-prototype was
recognising a category of function type that no longer exists, so the
code now falls into the more general case of
-Wincompatible-pointer-types in C23, which you certainly wouldn't want
to suppress. So perhaps we actually can't make any branch older than
PostgreSQL 16 into a valid C23 program, and if that's true, I needn't
have back-patched the <stdbool.h> change any further back than 16.
Perhaps we should reconsider that, then. And if it can't be all the
back-branches, we could even decide to focus just on master. Where do
we want our C23 support to begin?
Coincidental observation: We added -Wdeprecated-non-prototype back
when Clang 15 invented it, but I noticed that GCC 15 has just now
added it too[1]https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=701d8e7e60b85809cae348c1e9edb3b0f4924325, so alligator started detecting and using that in
REL_15_STABLE last week. Of course it doesn't help once you're
talking C23.
[1]: https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=701d8e7e60b85809cae348c1e9edb3b0f4924325
[2]: https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=alligator&dt=2024-11-18%2019%3A23%3A30&stg=configure
Thomas Munro <thomas.munro@gmail.com> writes:
Oh. Yeah. 1c27d16e6e5 was not back-patchable. And what f9a56e72 did
in 15 and older doesn't seem to have any equivalent in C23, at least
without going way overboard. -Wdeprecated-non-prototype was
recognising a category of function type that no longer exists, so the
code now falls into the more general case of
-Wincompatible-pointer-types in C23, which you certainly wouldn't want
to suppress. So perhaps we actually can't make any branch older than
PostgreSQL 16 into a valid C23 program, and if that's true, I needn't
have back-patched the <stdbool.h> change any further back than 16.
Perhaps we should reconsider that, then. And if it can't be all the
back-branches, we could even decide to focus just on master. Where do
we want our C23 support to begin?
Unless somebody has a better idea than 1c27d16e6e5, it would seem
reasonable to say that we'll support C23 in v16 and later, but
to build an older branch you have to back off to an older C version.
I don't feel a need to revert those <stdbool.h> changes.
If nothing else, that saved a bit of configure runtime.
If we leave it like this, alligator will need some configuration
adjustments, and so will other BF animals when they migrate to new
gcc, and so will individual hackers when they're trying to build
old branches. A possible compromise to reduce the manual pain
level could be to adjust configure to add "-std=gnu99" or so to
CFLAGS in the pre-v16 branches, if the compiler accepts that.
(OTOH, maybe that'd cause pain for some extensions?)
regards, tom lane
On Tue, Nov 26, 2024 at 9:07 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
If we leave it like this, alligator will need some configuration
adjustments, and so will other BF animals when they migrate to new
gcc, and so will individual hackers when they're trying to build
old branches. A possible compromise to reduce the manual pain
level could be to adjust configure to add "-std=gnu99" or so to
CFLAGS in the pre-v16 branches, if the compiler accepts that.
We already have tests to see if we need to add -std=c99 to go forwards
in time (from a quick look at the build farm, now used only by EOL
distros testing GCC 4). Something tells me we might want to be less
draconian when travelling backwards, but I dunno... our stuff is
working fine with (implicit) -std=c17 all over the place, and we also
have:
# Do we need -std=c99 to compile C99 code? We don't want to add -std=c99
# unnecessarily, because we optionally rely on newer features.
I noticed that later autoconf killed AC_PROG_CC_C99 and its AC_PROG_CC
is figuring out the highest C standard available and requesting that,
though it doesn't know about C23 yet.
(OTOH, maybe that'd cause pain for some extensions?)
So we're talking about -std=XXX suddenly appearing in pg_config
--cflags? Of course we want to try quite hard to keep emitting
nothing for that if we can. We already emit -std=gnu99 from old tests
that keep those GCC 4 build farm zombies at bay (did extensions ever
complain about that back in the GCC 4 days? I'd guess not), but no
one really uses those in real life. If we one day dare to dream about
moving our own baseline to C11/C17, we'll still emit nothing as the
compilers are already there by default. We will start to emit a new
flag to disable C23 if required, but I think it might be unlikely to
upset anyone if it works something like this:
* For 16+ nothing, we're going to be C23 clean (after a couple more
back-patches)
* For 9.2-15 on GCC < 15 it'll stay as nothing too
* For 9.2-15 on early GCC 15 adopter distros like Fedora/Gentoo etc
we'll detect C23, and perhaps start spitting out -std=c17 (if you've
detected C23, I think you can assume that C17 is available so we don't
have to do a C17-C11-C99[-C89] search?)
* When 12-15 fall out of support and all compilers are eventually C23+
compilers, they'll eventually always be getting -std=c17 by the above
rules but no one will mind about that in the ancient branches
I think if someone writes new extension code in C23 and wants to use
it with PostgreSQL 15 that came out in 2022, they can expect a few
time travel problems, but by the time C23 really starts to take off,
15 will be retired, and it's great that we got the hardest part of
this into 16. I don't think the problems would be too hard to deal
with if you do it. One saving grace here is that all this stuff is
converging with C++, and we already ensure our headers are valid C++.
As for what they actually mean, we also know that C++ extensions are
happily using the tree walker stuff in the wild, which I think must be
about the same level of C calling convention abuse whether you do it
from C23 or C++, and apparently doesn't break. Example:
The C++ people aren't using --cflags of course. I guess a
really-written-in-C23 extension using --cflags to compile its own code
would need to append -std=c23 on the end when building against 12-15
(both clang and gcc will take the last of multiple -std flags), if we
decide to start emitting -std=c17 in 12-15 because we've detected a
C23 compiler. In 16+ they could put -std=c23 on the end, or not if
they somehow know it's the default.
We could suppress it in pg_config --cflags even though we need it to
build the backend (by the arguments above, we know the headers are
more acceptable as C23 than the backend .c files), but that seems
bound to confuse matters. I don't know how exactly, I'm no expert in
the gnarly details of extension buildfiles, and that's just some first
thoughts. I might try some ideas out in a few days when my local
gcc15-devel package catches up with the new defaults, and see if
everything I wrote is complete nonsense.
Thomas Munro <thomas.munro@gmail.com> writes:
... I think it might be unlikely to
upset anyone if it works something like this:
* For 16+ nothing, we're going to be C23 clean (after a couple more
back-patches)
* For 9.2-15 on GCC < 15 it'll stay as nothing too
* For 9.2-15 on early GCC 15 adopter distros like Fedora/Gentoo etc
we'll detect C23, and perhaps start spitting out -std=c17 (if you've
detected C23, I think you can assume that C17 is available so we don't
have to do a C17-C11-C99[-C89] search?)
* When 12-15 fall out of support and all compilers are eventually C23+
compilers, they'll eventually always be getting -std=c17 by the above
rules but no one will mind about that in the ancient branches
Sounds plausible to me. Will you work on making that happen?
regards, tom lane
