Horton

Thread

Commits

  1. Fix bad indentation introduced in 43cd30bcd1c

  2. Fix type confusion in guc_var_compare()

  1. gcc 12.1.0 warning

    Erik Rijkers <er@xs4all.nl> — 2022-05-06T14:34:41Z

    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

  2. Re: gcc 12.1.0 warning

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-05-06T14:46:20Z

    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

  3. Re: gcc 12.1.0 warning

    Nazir Bilal Yavuz <byavuz81@gmail.com> — 2023-10-27T10:09:01Z

    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

  4. Re: gcc 12.1.0 warning

    Andres Freund <andres@anarazel.de> — 2024-04-13T02:25:47Z

    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

  5. Re: gcc 12.1.0 warning

    Nazir Bilal Yavuz <byavuz81@gmail.com> — 2024-04-15T08:25:05Z

    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

  6. Re: gcc 12.1.0 warning

    Andres Freund <andres@anarazel.de> — 2024-04-23T16:59:39Z

    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

  7. Re: gcc 12.1.0 warning

    Nazir Bilal Yavuz <byavuz81@gmail.com> — 2024-05-10T09:13:21Z

    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

  8. Re: gcc 12.1.0 warning

    Andres Freund <andres@anarazel.de> — 2024-07-15T16:41:55Z

    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

  9. Re: gcc 12.1.0 warning

    Nathan Bossart <nathandbossart@gmail.com> — 2024-07-15T17:14:47Z

    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

  10. Re: gcc 12.1.0 warning

    Andres Freund <andres@anarazel.de> — 2024-07-15T20:36:27Z

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