gcc 12.1.0 warning

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

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 :(