Thread: gcc 12.1.0 warning
Hi, Not sure if these compiler-mutterings are worth reporting but I guess we're trying to get a silent compile. System: Debian 4.9.303-1 (2022-03-07) x86_64 GNU/Linux Compiling with gcc 12.1.0 causes the below 'warning' and 'note'. Compiling with --enable-cassert --enable-debug is silent, no warnings) In function ‘guc_var_compare’, inlined from ‘bsearch’ at /usr/include/x86_64-linux-gnu/bits/stdlib-bsearch.h:33:23, inlined from ‘find_option’ at guc.c:5649:35: guc.c:5736:38: warning: array subscript ‘const struct config_generic[0]’ is partly outside array bounds of ‘const char[8]’ [-Warray-bounds] 5736 | return guc_name_compare(confa->name, confb->name); | ~~~~~^~~~~~ guc.c: In function ‘find_option’: guc.c:5636:25: note: object ‘name’ of size 8 5636 | find_option(const char *name, bool create_placeholders, bool skip_errors, | ~~~~~~~~~~~~^~~~ (Compiling with gcc 6.3.0 does not complain.) Below are the two configure lines, FWIW. Erik Rijkers # cassert-build: no warning/note ./configure \ --prefix=/home/aardvark/pg_stuff/pg_installations/pgsql.HEAD \ --bindir=/home/aardvark/pg_stuff/pg_installations/pgsql.HEAD/bin \ --libdir=/home/aardvark/pg_stuff/pg_installations/pgsql.HEAD/lib \ --with-pgport=6515 --quiet --enable-depend \ --enable-cassert --enable-debug \ --with-openssl --with-perl --with-libxml --with-libxslt --with-zlib \ --enable-tap-tests --with-extra-version=_0506_HEAD_701d --with-lz4 # normal build: causes warning/note: ./configure \ --prefix=/home/aardvark/pg_stuff/pg_installations/pgsql.HEAD \ --bindir=/home/aardvark/pg_stuff/pg_installations/pgsql.HEAD/bin.fast \ --libdir=/home/aardvark/pg_stuff/pg_installations/pgsql.HEAD/lib.fast \ --with-pgport=6515 --quiet --enable-depend \ --with-openssl --with-perl --with-libxml --with-libxslt --with-zlib \ --enable-tap-tests --with-extra-version=_0506_HEAD_701d --with-lz4
Erik Rijkers <er@xs4all.nl> writes: > Not sure if these compiler-mutterings are worth reporting but I guess > we're trying to get a silent compile. > System: Debian 4.9.303-1 (2022-03-07) x86_64 GNU/Linux > Compiling with gcc 12.1.0 causes the below 'warning' and 'note'. > Compiling with --enable-cassert --enable-debug is silent, no warnings) > In function ‘guc_var_compare’, > inlined from ‘bsearch’ at > /usr/include/x86_64-linux-gnu/bits/stdlib-bsearch.h:33:23, > inlined from ‘find_option’ at guc.c:5649:35: > guc.c:5736:38: warning: array subscript ‘const struct config_generic[0]’ > is partly outside array bounds of ‘const char[8]’ [-Warray-bounds] > 5736 | return guc_name_compare(confa->name, confb->name); > | ~~~~~^~~~~~ I'd call that a compiler bug. regards, tom lane
Hi, On Fri, 27 Oct 2023 at 12:34, Erik Rijkers <er@xs4all.nl> wrote: > > Hi, > > Not sure if these compiler-mutterings are worth reporting but I guess > we're trying to get a silent compile. > > System: Debian 4.9.303-1 (2022-03-07) x86_64 GNU/Linux > Compiling with gcc 12.1.0 causes the below 'warning' and 'note'. > Compiling with --enable-cassert --enable-debug is silent, no warnings) > > In function ‘guc_var_compare’, > inlined from ‘bsearch’ at > /usr/include/x86_64-linux-gnu/bits/stdlib-bsearch.h:33:23, > inlined from ‘find_option’ at guc.c:5649:35: > guc.c:5736:38: warning: array subscript ‘const struct config_generic[0]’ > is partly outside array bounds of ‘const char[8]’ [-Warray-bounds] > 5736 | return guc_name_compare(confa->name, confb->name); > | ~~~~~^~~~~~ > > guc.c: In function ‘find_option’: > guc.c:5636:25: note: object ‘name’ of size 8 > 5636 | find_option(const char *name, bool create_placeholders, bool > skip_errors, > | ~~~~~~~~~~~~^~~~ > > (Compiling with gcc 6.3.0 does not complain.) I was testing 'upgrading CI Debian images to bookworm'. I tested the bookworm on REL_15, REL_16 and upstream. REL_16 and upstream finished successfully but the CompilerWarnings task failed on REL_15 with the same error. gcc version: 12.2.0 CI Run: https://cirrus-ci.com/task/6151742664998912 Regards, Nazir Bilal Yavuz Microsoft
Hi, On 2023-10-27 13:09:01 +0300, Nazir Bilal Yavuz wrote: > I was testing 'upgrading CI Debian images to bookworm'. I tested the > bookworm on REL_15, REL_16 and upstream. REL_16 and upstream finished > successfully but the CompilerWarnings task failed on REL_15 with the > same error. Is that still the case? I briefly tried to repro this outside of CI and couldn't reproduce the warning. I'd really like to upgrade the CI images, it doesn't seem great to continue using oldstable. > gcc version: 12.2.0 > > CI Run: https://cirrus-ci.com/task/6151742664998912 Unfortunately the logs aren't accessible anymore, so I can't check the precise patch level of the compiler and/or the precise invocation used. Greetings, Andres Freund
Hi, On Sat, 13 Apr 2024 at 05:25, Andres Freund <andres@anarazel.de> wrote: > > Hi, > > On 2023-10-27 13:09:01 +0300, Nazir Bilal Yavuz wrote: > > I was testing 'upgrading CI Debian images to bookworm'. I tested the > > bookworm on REL_15, REL_16 and upstream. REL_16 and upstream finished > > successfully but the CompilerWarnings task failed on REL_15 with the > > same error. > > Is that still the case? I briefly tried to repro this outside of CI and > couldn't reproduce the warning. > > I'd really like to upgrade the CI images, it doesn't seem great to continue > using oldstable. > > > > gcc version: 12.2.0 > > > > CI Run: https://cirrus-ci.com/task/6151742664998912 > > Unfortunately the logs aren't accessible anymore, so I can't check the precise > patch level of the compiler and/or the precise invocation used. I am able to reproduce this. I regenerated the debian bookworm image and ran CI on REL_15_STABLE with this image. CI Run: https://cirrus-ci.com/task/4978799442395136 -- Regards, Nazir Bilal Yavuz Microsoft
Hi, On 2024-04-15 11:25:05 +0300, Nazir Bilal Yavuz wrote: > I am able to reproduce this. I regenerated the debian bookworm image > and ran CI on REL_15_STABLE with this image. > > CI Run: https://cirrus-ci.com/task/4978799442395136 Hm, not sure why I wasn't able to repro - now I can. It actually seems like a legitimate warning: The caller allocates the key as static struct config_generic * find_option(const char *name, bool create_placeholders, bool skip_errors, int elevel) { const char **key = &name; and then does res = (struct config_generic **) bsearch((void *) &key, (void *) guc_variables, num_guc_variables, sizeof(struct config_generic *), guc_var_compare); while guc_var_compare() assume it's being passed a full config_generic: static int guc_var_compare(const void *a, const void *b) { const struct config_generic *confa = *(struct config_generic *const *) a; const struct config_generic *confb = *(struct config_generic *const *) b; return guc_name_compare(confa->name, confb->name); } which several versions of gcc then complain about: In function ‘guc_var_compare’, inlined from ‘bsearch’ at /usr/include/x86_64-linux-gnu/bits/stdlib-bsearch.h:33:23, inlined from ‘find_option’ at /home/andres/src/postgresql-15/src/backend/utils/misc/guc.c:5640:35: /home/andres/src/postgresql-15/src/backend/utils/misc/guc.c:5727:38: warning: array subscript ‘const struct config_generic[0]’is partly outside array bounds of ‘const char[8]’ [-Warray-bounds=] 5727 | return guc_name_compare(confa->name, confb->name); | ~~~~~^~~~~~ /home/andres/src/postgresql-15/src/backend/utils/misc/guc.c: In function ‘find_option’: /home/andres/src/postgresql-15/src/backend/utils/misc/guc.c:5627:25: note: object ‘name’ of size 8 5627 | find_option(const char *name, bool create_placeholders, bool skip_errors, Which seems entirely legitimate. ISTM that guc_var_compare() ought to only cast the pointers to the key type, i.e. char *. And incidentally that does prevent the warning. The reason it doesn't happen in newer versions of postgres is that we aren't using guc_var_compare() in the relevant places anymore... Greetings, Andres Freund
Hi, On Tue, 23 Apr 2024 at 19:59, Andres Freund <andres@anarazel.de> wrote: > > > Which seems entirely legitimate. ISTM that guc_var_compare() ought to only > cast the pointers to the key type, i.e. char *. And incidentally that does > prevent the warning. > > The reason it doesn't happen in newer versions of postgres is that we aren't > using guc_var_compare() in the relevant places anymore... The fix is attached. It cleanly applies from REL_15_STABLE to REL_12_STABLE, fixes the warnings and the tests pass. -- Regards, Nazir Bilal Yavuz Microsoft
Attachment
Hi, On 2024-05-10 12:13:21 +0300, Nazir Bilal Yavuz wrote: > On Tue, 23 Apr 2024 at 19:59, Andres Freund <andres@anarazel.de> wrote: > > > > > > Which seems entirely legitimate. ISTM that guc_var_compare() ought to only > > cast the pointers to the key type, i.e. char *. And incidentally that does > > prevent the warning. > > > > The reason it doesn't happen in newer versions of postgres is that we aren't > > using guc_var_compare() in the relevant places anymore... > > The fix is attached. It cleanly applies from REL_15_STABLE to > REL_12_STABLE, fixes the warnings and the tests pass. Thanks! I've applied it to all branches - while it's not required to avoid a warning in newer versions, it's still not correct as it was... Greetings, Andres
On Mon, Jul 15, 2024 at 09:41:55AM -0700, Andres Freund wrote: > On 2024-05-10 12:13:21 +0300, Nazir Bilal Yavuz wrote: >> The fix is attached. It cleanly applies from REL_15_STABLE to >> REL_12_STABLE, fixes the warnings and the tests pass. > > Thanks! I've applied it to all branches - while it's not required to avoid a > warning in newer versions, it's still not correct as it was... nitpick: pgindent thinks one of the spaces is unnecessary. diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index a043d529ef..b0947a4cf1 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -1289,8 +1289,8 @@ find_option(const char *name, bool create_placeholders, bool skip_errors, static int guc_var_compare(const void *a, const void *b) { - const char *namea = **(const char ** const *) a; - const char *nameb = **(const char ** const *) b; + const char *namea = **(const char **const *) a; + const char *nameb = **(const char **const *) b; return guc_name_compare(namea, nameb); } -- nathan
On 2024-07-15 12:14:47 -0500, Nathan Bossart wrote: > On Mon, Jul 15, 2024 at 09:41:55AM -0700, Andres Freund wrote: > > On 2024-05-10 12:13:21 +0300, Nazir Bilal Yavuz wrote: > >> The fix is attached. It cleanly applies from REL_15_STABLE to > >> REL_12_STABLE, fixes the warnings and the tests pass. > > > > Thanks! I've applied it to all branches - while it's not required to avoid a > > warning in newer versions, it's still not correct as it was... > > nitpick: pgindent thinks one of the spaces is unnecessary. Ugh. Sorry for that, will take a look at fixing that :(