Thread: gcc 13 warnings

gcc 13 warnings

From
Pavel Stehule
Date:
Hi

see

[504/2287] Compiling C object src/backend/postgres_lib.a.p/access_transam_xlogrecovery.c.o
In function ‘recoveryStopsAfter’,
    inlined from ‘PerformWalRecovery’ at ../src/backend/access/transam/xlogrecovery.c:1749:8:
../src/backend/access/transam/xlogrecovery.c:2737:42: warning: ‘recordXtime’ may be used uninitialized [-Wmaybe-uninitialized]
 2737 |                         recoveryStopTime = recordXtime;
      |                         ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
../src/backend/access/transam/xlogrecovery.c: In function ‘PerformWalRecovery’:
../src/backend/access/transam/xlogrecovery.c:2628:21: note: ‘recordXtime’ was declared here
 2628 |         TimestampTz recordXtime;
      |                     ^~~~~~~~~~~
[1642/2287] Compiling C object src/bin/pgbench/pgbench.p/pgbench.c.o
In function ‘coerceToInt’,
    inlined from ‘evalStandardFunc’ at ../src/bin/pgbench/pgbench.c:2617:11:
../src/bin/pgbench/pgbench.c:2042:17: warning: ‘vargs[0].type’ may be used uninitialized [-Wmaybe-uninitialized]
 2042 |         if (pval->type == PGBT_INT)
      |             ~~~~^~~~~~
../src/bin/pgbench/pgbench.c: In function ‘evalStandardFunc’:
../src/bin/pgbench/pgbench.c:2250:22: note: ‘vargs’ declared here
 2250 |         PgBenchValue vargs[MAX_FARGS];
      |                      ^~~~~
In function ‘coerceToInt’,
    inlined from ‘evalStandardFunc’ at ../src/bin/pgbench/pgbench.c:2617:11:
../src/bin/pgbench/pgbench.c:2044:32: warning: ‘vargs[0].u.ival’ may be used uninitialized [-Wmaybe-uninitialized]
 2044 |                 *ival = pval->u.ival;
      |                         ~~~~~~~^~~~~
../src/bin/pgbench/pgbench.c: In function ‘evalStandardFunc’:
../src/bin/pgbench/pgbench.c:2250:22: note: ‘vargs’ declared here
 2250 |         PgBenchValue vargs[MAX_FARGS];
      |                      ^~~~~
In function ‘coerceToInt’,
    inlined from ‘evalStandardFunc’ at ../src/bin/pgbench/pgbench.c:2617:11:
../src/bin/pgbench/pgbench.c:2049:40: warning: ‘vargs[0].u.dval’ may be used uninitialized [-Wmaybe-uninitialized]
 2049 |                 double          dval = rint(pval->u.dval);
      |                                        ^~~~~~~~~~~~~~~~~~
../src/bin/pgbench/pgbench.c: In function ‘evalStandardFunc’:
../src/bin/pgbench/pgbench.c:2250:22: note: ‘vargs’ declared here
 2250 |         PgBenchValue vargs[MAX_FARGS];
      |                      ^~~~~
[1700/2287] Compiling C object src/pl/plpgsql/src/plpgsql.so.p/pl_exec.c.o
In file included from ../src/include/access/htup_details.h:22,
                 from ../src/pl/plpgsql/src/pl_exec.c:21:
In function ‘assign_simple_var’,
    inlined from ‘exec_set_found’ at ../src/pl/plpgsql/src/pl_exec.c:8307:2:
../src/include/varatt.h:230:36: warning: array subscript 0 is outside array bounds of ‘char[0]’ [-Warray-bounds=]
  230 |         (((varattrib_1b_e *) (PTR))->va_tag)
      |                                    ^
../src/include/varatt.h:94:12: note: in definition of macro ‘VARTAG_IS_EXPANDED’
   94 |         (((tag) & ~1) == VARTAG_EXPANDED_RO)
      |            ^~~
../src/include/varatt.h:284:57: note: in expansion of macro ‘VARTAG_1B_E’
  284 | #define VARTAG_EXTERNAL(PTR)                            VARTAG_1B_E(PTR)
      |                                                         ^~~~~~~~~~~
../src/include/varatt.h:301:57: note: in expansion of macro ‘VARTAG_EXTERNAL’
  301 |         (VARATT_IS_EXTERNAL(PTR) && !VARTAG_IS_EXPANDED(VARTAG_EXTERNAL(PTR)))
      |                                                         ^~~~~~~~~~~~~~~
../src/pl/plpgsql/src/pl_exec.c:8495:17: note: in expansion of macro ‘VARATT_IS_EXTERNAL_NON_EXPANDED’
 8495 |                 VARATT_IS_EXTERNAL_NON_EXPANDED(DatumGetPointer(newvalue)))
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘exec_set_found’:
cc1: note: source object is likely at address zero

Regards

Pavel

Re: gcc 13 warnings

From
Melanie Plageman
Date:
On Thu, Mar 16, 2023 at 9:40 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
> [1700/2287] Compiling C object src/pl/plpgsql/src/plpgsql.so.p/pl_exec.c.o
> In file included from ../src/include/access/htup_details.h:22,
>                  from ../src/pl/plpgsql/src/pl_exec.c:21:
> In function ‘assign_simple_var’,
>     inlined from ‘exec_set_found’ at ../src/pl/plpgsql/src/pl_exec.c:8307:2:
> ../src/include/varatt.h:230:36: warning: array subscript 0 is outside array bounds of ‘char[0]’ [-Warray-bounds=]
>   230 |         (((varattrib_1b_e *) (PTR))->va_tag)
>       |                                    ^
> ../src/include/varatt.h:94:12: note: in definition of macro ‘VARTAG_IS_EXPANDED’
>    94 |         (((tag) & ~1) == VARTAG_EXPANDED_RO)
>       |            ^~~
> ../src/include/varatt.h:284:57: note: in expansion of macro ‘VARTAG_1B_E’
>   284 | #define VARTAG_EXTERNAL(PTR)                            VARTAG_1B_E(PTR)
>       |                                                         ^~~~~~~~~~~
> ../src/include/varatt.h:301:57: note: in expansion of macro ‘VARTAG_EXTERNAL’
>   301 |         (VARATT_IS_EXTERNAL(PTR) && !VARTAG_IS_EXPANDED(VARTAG_EXTERNAL(PTR)))
>       |                                                         ^~~~~~~~~~~~~~~
> ../src/pl/plpgsql/src/pl_exec.c:8495:17: note: in expansion of macro ‘VARATT_IS_EXTERNAL_NON_EXPANDED’
>  8495 |                 VARATT_IS_EXTERNAL_NON_EXPANDED(DatumGetPointer(newvalue)))
>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In function ‘exec_set_found’:
> cc1: note: source object is likely at address zero

I see these with gcc 12.2.0 also.



Re: gcc 13 warnings

From
Tom Lane
Date:
Melanie Plageman <melanieplageman@gmail.com> writes:
> On Thu, Mar 16, 2023 at 9:40 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> ../src/include/varatt.h:230:36: warning: array subscript 0 is outside array bounds of ‘char[0]’ [-Warray-bounds=]

> I see these with gcc 12.2.0 also.

Hmm, I do not see any warnings on HEAD with Fedora 37's gcc 12.2.1.
What non-default configure switches, CFLAGS, etc are you using?

            regards, tom lane



Re: gcc 13 warnings

From
Pavel Stehule
Date:


čt 16. 3. 2023 v 16:43 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
Melanie Plageman <melanieplageman@gmail.com> writes:
> On Thu, Mar 16, 2023 at 9:40 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> ../src/include/varatt.h:230:36: warning: array subscript 0 is outside array bounds of ‘char[0]’ [-Warray-bounds=]

> I see these with gcc 12.2.0 also.

Hmm, I do not see any warnings on HEAD with Fedora 37's gcc 12.2.1.
What non-default configure switches, CFLAGS, etc are you using?

meson build without any settings

I think so it is related to meson build, I didn't see these warnings with autoconf

regards

Pavel

 

                        regards, tom lane

Re: gcc 13 warnings

From
Tom Lane
Date:
Pavel Stehule <pavel.stehule@gmail.com> writes:
> čt 16. 3. 2023 v 16:43 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
>> Hmm, I do not see any warnings on HEAD with Fedora 37's gcc 12.2.1.
>> What non-default configure switches, CFLAGS, etc are you using?

> meson build without any settings
> I think so it is related to meson build, I didn't see these warnings with
> autoconf

It wouldn't be entirely surprising if meson is selecting some -W
switches that the configure script doesn't ... but I don't know
where to check or change that.

If that is the case, do we want to beat meson over the head till
it stops doing that, or try to silence the warnings?  The ones
you show here don't look terribly helpful ...

            regards, tom lane



Re: gcc 13 warnings

From
Andres Freund
Date:
Hi,

On 2023-03-16 12:10:27 -0400, Tom Lane wrote:
> Pavel Stehule <pavel.stehule@gmail.com> writes:
> > čt 16. 3. 2023 v 16:43 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
> >> Hmm, I do not see any warnings on HEAD with Fedora 37's gcc 12.2.1.
> >> What non-default configure switches, CFLAGS, etc are you using?
> 
> > meson build without any settings
> > I think so it is related to meson build, I didn't see these warnings with
> > autoconf
> 
> It wouldn't be entirely surprising if meson is selecting some -W
> switches that the configure script doesn't ... but I don't know
> where to check or change that.

I think it's just that meson defaults to -O3 (fwiw, I see substantial gains of
that over -O2).  I see such warnings with autoconf as well if I make it use
-O3.

I think some of these are stemming from
https://postgr.es/m/20230204130708.pta7pjc4dvu225ey%40alap3.anarazel.de

Greetings,

Andres Freund



Re: gcc 13 warnings

From
Andres Freund
Date:
Hi,

On 2023-03-16 10:05:06 -0700, Andres Freund wrote:
> I think it's just that meson defaults to -O3 (fwiw, I see substantial gains of
> that over -O2).  I see such warnings with autoconf as well if I make it use
> -O3.

WRT:

In file included from /home/andres/src/postgresql/src/include/access/htup_details.h:22,
                 from /home/andres/src/postgresql/src/pl/plpgsql/src/pl_exec.c:21:
In function ‘assign_simple_var’,
    inlined from ‘exec_set_found’ at /home/andres/src/postgresql/src/pl/plpgsql/src/pl_exec.c:8307:2:
/home/andres/src/postgresql/src/include/varatt.h:230:36: warning: array subscript 0 is outside array bounds of
‘char[0]’[-Warray-bounds]
 
  230 |         (((varattrib_1b_e *) (PTR))->va_tag)
      |                                    ^
/home/andres/src/postgresql/src/include/varatt.h:94:12: note: in definition of macro ‘VARTAG_IS_EXPANDED’
   94 |         (((tag) & ~1) == VARTAG_EXPANDED_RO)
      |            ^~~
/home/andres/src/postgresql/src/include/varatt.h:284:57: note: in expansion of macro ‘VARTAG_1B_E’
  284 | #define VARTAG_EXTERNAL(PTR)                            VARTAG_1B_E(PTR)
      |                                                         ^~~~~~~~~~~
/home/andres/src/postgresql/src/include/varatt.h:301:57: note: in expansion of macro ‘VARTAG_EXTERNAL’
  301 |         (VARATT_IS_EXTERNAL(PTR) && !VARTAG_IS_EXPANDED(VARTAG_EXTERNAL(PTR)))
      |                                                         ^~~~~~~~~~~~~~~
/home/andres/src/postgresql/src/pl/plpgsql/src/pl_exec.c:8495:17: note: in expansion of macro
‘VARATT_IS_EXTERNAL_NON_EXPANDED’
 8495 |                 VARATT_IS_EXTERNAL_NON_EXPANDED(DatumGetPointer(newvalue)))
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I think that's basically because gcc does realize that the datum is just an 8
byte by-value datum:
    assign_simple_var(estate, var, BoolGetDatum(state), false, false);

but doesn't (and probably can't, with the available information) grok that
that means we don't even get to the VARATT_IS_EXTERNAL_NON_EXPANDED() in
assign_simple_var().

If I add
    if (var->datatype->typlen == -1)
        pg_unreachable();
to exec_set_found(), the warning indeed goes away.


I've wondered before if we should make at least some Asserts() into something
like the above (if we have something better backing it than abort()), so the
compiler can understand unreachable code paths even when building without
cassert.

Greetings,

Andres Freund



Re: gcc 13 warnings

From
Tom Lane
Date:
Andres Freund <andres@anarazel.de> writes:
> On 2023-03-16 12:10:27 -0400, Tom Lane wrote:
>> It wouldn't be entirely surprising if meson is selecting some -W
>> switches that the configure script doesn't ... but I don't know
>> where to check or change that.

> I think it's just that meson defaults to -O3 (fwiw, I see substantial gains of
> that over -O2).  I see such warnings with autoconf as well if I make it use
> -O3.

Oh, interesting.  Should we try to standardize the two build systems
on the same -O level, and if so which one?

To my mind, you should ideally get the identical built bits out of
either system, so defaulting to a different -O level seems bad.
I'm not sure if we're prepared to go to -O3 by default though,
especially for some of the older buildfarm critters where that
might be buggy.  (I'd imagine you take a hit in gdb-ability too.)

            regards, tom lane



Re: gcc 13 warnings

From
Andres Freund
Date:
Hi,

On 2023-03-16 13:54:29 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2023-03-16 12:10:27 -0400, Tom Lane wrote:
> >> It wouldn't be entirely surprising if meson is selecting some -W
> >> switches that the configure script doesn't ... but I don't know
> >> where to check or change that.
>
> > I think it's just that meson defaults to -O3 (fwiw, I see substantial gains of
> > that over -O2).  I see such warnings with autoconf as well if I make it use
> > -O3.
>
> Oh, interesting.  Should we try to standardize the two build systems
> on the same -O level, and if so which one?

I'm on the fence on this one (and posed it as a question before). O3 does
result in higher performance for me, but it also does take longer to build,
and increases the numbers of warnings.

So I just elected to leave it at the default for meson.


> To my mind, you should ideally get the identical built bits out of
> either system, so defaulting to a different -O level seems bad.

I doubt that is attainable, unfortunately. My experience is that even trivial
changes can lead to substantial changes in output. Even just being in a
different directory (the root build directory for meson vs the subdirectory in
make builds) apparently sometimes leads to different compiler output.


> I'm not sure if we're prepared to go to -O3 by default though,
> especially for some of the older buildfarm critters where that
> might be buggy.  (I'd imagine you take a hit in gdb-ability too.)

My experience is that debuggability is already bad enough at O2 that the
difference to O3 is pretty marginal. But it certainly depends a bit on the
compiler version and what level of debug information one enables.

Greetings,

Andres Freund



Re: gcc 13 warnings

From
John Naylor
Date:
On Fri, Mar 17, 2023 at 1:11 AM Andres Freund <andres@anarazel.de> wrote:

> On 2023-03-16 13:54:29 -0400, Tom Lane wrote:

> So I just elected to leave it at the default for meson.

In my build scripts I've been setting it to -O2, because that seemed the obvious thing to do, and assumed some later commit would get rid of the need to do it manually. (if it was discussed before, I missed that)

> > I'm not sure if we're prepared to go to -O3 by default though,
> > especially for some of the older buildfarm critters where that
> > might be buggy.  (I'd imagine you take a hit in gdb-ability too.)

Newer platforms could be buggy enough. A while back, IIUC gcc moved an optimization pass from O3 to O2, which resulted in obviously bad code generation, which I know because of a bug report filed by one Andres Freund:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101481

...which was never properly addressed as far as I know.

I'm a bit surprised we would even consider changing optimization level based on a build tool default.

--
John Naylor
EDB: http://www.enterprisedb.com

Re: gcc 13 warnings

From
Peter Eisentraut
Date:
On 16.03.23 19:11, Andres Freund wrote:
> On 2023-03-16 13:54:29 -0400, Tom Lane wrote:
>> Andres Freund <andres@anarazel.de> writes:
>>> On 2023-03-16 12:10:27 -0400, Tom Lane wrote:
>>>> It wouldn't be entirely surprising if meson is selecting some -W
>>>> switches that the configure script doesn't ... but I don't know
>>>> where to check or change that.
>>
>>> I think it's just that meson defaults to -O3 (fwiw, I see substantial gains of
>>> that over -O2).  I see such warnings with autoconf as well if I make it use
>>> -O3.
>>
>> Oh, interesting.  Should we try to standardize the two build systems
>> on the same -O level, and if so which one?
> 
> I'm on the fence on this one (and posed it as a question before). O3 does
> result in higher performance for me, but it also does take longer to build,
> and increases the numbers of warnings.
> 
> So I just elected to leave it at the default for meson.

AFAICT, the default for meson is buildtype=debug, which is -O0.  The -O3 
comes from meson.build setting buildtype=release.

I think a good compromise would be buildtype=debugoptimized, which is 
-O2 with debug symbols, which also sort of matches the default in the 
autoconf world.

(https://mesonbuild.com/Builtin-options.html#details-for-buildtype)

At least during the transition phase I would prefer having the same 
default optimization level in both build systems, mainly because of how 
this affects warnings.




Re: gcc 13 warnings

From
Tom Lane
Date:
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> On 16.03.23 19:11, Andres Freund wrote:
>> So I just elected to leave it at the default for meson.

> AFAICT, the default for meson is buildtype=debug, which is -O0.  The -O3 
> comes from meson.build setting buildtype=release.

> I think a good compromise would be buildtype=debugoptimized, which is 
> -O2 with debug symbols, which also sort of matches the default in the 
> autoconf world.

That sounds promising.

> At least during the transition phase I would prefer having the same 
> default optimization level in both build systems, mainly because of how 
> this affects warnings.

I'd prefer sticking to -O2 mainly because of the risk of new bugs.
The meson conversion is a big enough job without adding "harden
Postgres against -O3" to the list of tasks that must be accomplished.
We can take that on in due time, but let's keep it separate.

            regards, tom lane



Re: gcc 13 warnings

From
Andres Freund
Date:
Hi,

On 2023-03-17 09:06:05 +0100, Peter Eisentraut wrote:
> AFAICT, the default for meson is buildtype=debug, which is -O0.  The -O3
> comes from meson.build setting buildtype=release.

Right - my point about -O3 was just that buildtype=release defaults to it.


> I think a good compromise would be buildtype=debugoptimized, which is -O2
> with debug symbols, which also sort of matches the default in the autoconf
> world.

Looks like that'd result in a slightly worse build with msvc, as afaict we
wouldn't end up with /OPT:REF doesn't get specified, which automatically gets
disabled if /DEBUG is specified. I guess we can live with that.

Greetings,

Andres Freund



Re: gcc 13 warnings

From
Peter Eisentraut
Date:
On 18.03.23 00:54, Andres Freund wrote:
>> I think a good compromise would be buildtype=debugoptimized, which is -O2
>> with debug symbols, which also sort of matches the default in the autoconf
>> world.
> 
> Looks like that'd result in a slightly worse build with msvc, as afaict we
> wouldn't end up with /OPT:REF doesn't get specified, which automatically gets
> disabled if /DEBUG is specified. I guess we can live with that.

I looked up what /OPT:REF does 
(https://learn.microsoft.com/en-us/cpp/build/reference/opt-optimizations?view=msvc-170), 
and it seems pretty obscure to me, at least for development builds.



Re: gcc 13 warnings

From
Peter Eisentraut
Date:
On 22.03.23 10:45, Peter Eisentraut wrote:
> On 18.03.23 00:54, Andres Freund wrote:
>>> I think a good compromise would be buildtype=debugoptimized, which is 
>>> -O2
>>> with debug symbols, which also sort of matches the default in the 
>>> autoconf
>>> world.
>>
>> Looks like that'd result in a slightly worse build with msvc, as 
>> afaict we
>> wouldn't end up with /OPT:REF doesn't get specified, which 
>> automatically gets
>> disabled if /DEBUG is specified. I guess we can live with that.
> 
> I looked up what /OPT:REF does 
> (https://learn.microsoft.com/en-us/cpp/build/reference/opt-optimizations?view=msvc-170), and it seems pretty obscure
tome, at least for development builds.
 

I have committed the change of buildtype to debugoptimized.



Re: gcc 13 warnings

From
Aleksander Alekseev
Date:
Hi,

> I have committed the change of buildtype to debugoptimized.

There is still a warning previously reported by Melanie:

```
[1391/1944] Compiling C object src/pl/plpgsql/src/plpgsql.so.p/pl_exec.c.o
In file included from ../src/include/access/htup_details.h:22,
                 from ../src/pl/plpgsql/src/pl_exec.c:21:
In function ‘assign_simple_var’,
    inlined from ‘exec_set_found’ at ../src/pl/plpgsql/src/pl_exec.c:8382:2:
../src/include/varatt.h:230:36: warning: array subscript 0 is outside
array bounds of ‘char[0]’ [-Warray-bounds]
  230 |         (((varattrib_1b_e *) (PTR))->va_tag)
      |                                    ^
../src/include/varatt.h:94:12: note: in definition of macro ‘VARTAG_IS_EXPANDED’
   94 |         (((tag) & ~1) == VARTAG_EXPANDED_RO)
      |            ^~~
../src/include/varatt.h:284:57: note: in expansion of macro ‘VARTAG_1B_E’
  284 | #define VARTAG_EXTERNAL(PTR)                            VARTAG_1B_E(PTR)
      |                                                         ^~~~~~~~~~~
../src/include/varatt.h:301:57: note: in expansion of macro ‘VARTAG_EXTERNAL’
  301 |         (VARATT_IS_EXTERNAL(PTR) &&
!VARTAG_IS_EXPANDED(VARTAG_EXTERNAL(PTR)))
      |                                                         ^~~~~~~~~~~~~~~
../src/pl/plpgsql/src/pl_exec.c:8570:17: note: in expansion of macro
‘VARATT_IS_EXTERNAL_NON_EXPANDED’
 8570 |
VARATT_IS_EXTERNAL_NON_EXPANDED(DatumGetPointer(newvalue)))
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[1687/1944] Compiling C object
src/test/modules/test_dsa/test_dsa.so.p/test_dsa.c.o^C
ninja: build stopped: interrupted by user.
``

Displayed only for the release builds, e.g.:

```
git clean -dfx
meson setup --buildtype release -DPG_TEST_EXTRA="kerberos ldap ssl"
-Dldap=disabled -Dssl=openssl -Dcassert=true -Dtap_tests=enabled
-Dprefix=/home/eax/pginstall build
ninja -C build
```

Compiler version is:

```
gcc (Debian 12.2.0-14) 12.2.0
```

The overall environment is Raspberry Pi 5 with pretty much default
configuration - Raspbian etc.

How to fix it? Absolutely no idea :)

--
Best regards,
Aleksander Alekseev



Re: gcc 13 warnings

From
Andres Freund
Date:
Hi,

On 2024-07-05 14:19:12 +0300, Aleksander Alekseev wrote:
> There is still a warning previously reported by Melanie:
>
> ```
> [1391/1944] Compiling C object src/pl/plpgsql/src/plpgsql.so.p/pl_exec.c.o
> In file included from ../src/include/access/htup_details.h:22,
>                  from ../src/pl/plpgsql/src/pl_exec.c:21:
> In function ‘assign_simple_var’,
>     inlined from ‘exec_set_found’ at ../src/pl/plpgsql/src/pl_exec.c:8382:2:
> ../src/include/varatt.h:230:36: warning: array subscript 0 is outside
> array bounds of ‘char[0]’ [-Warray-bounds]
>   230 |         (((varattrib_1b_e *) (PTR))->va_tag)
>       |                                    ^
> ../src/include/varatt.h:94:12: note: in definition of macro ‘VARTAG_IS_EXPANDED’
>    94 |         (((tag) & ~1) == VARTAG_EXPANDED_RO)
>       |            ^~~
> ../src/include/varatt.h:284:57: note: in expansion of macro ‘VARTAG_1B_E’
>   284 | #define VARTAG_EXTERNAL(PTR)                            VARTAG_1B_E(PTR)
>       |                                                         ^~~~~~~~~~~
> ../src/include/varatt.h:301:57: note: in expansion of macro ‘VARTAG_EXTERNAL’
>   301 |         (VARATT_IS_EXTERNAL(PTR) &&
> !VARTAG_IS_EXPANDED(VARTAG_EXTERNAL(PTR)))
>       |                                                         ^~~~~~~~~~~~~~~
> ../src/pl/plpgsql/src/pl_exec.c:8570:17: note: in expansion of macro
> ‘VARATT_IS_EXTERNAL_NON_EXPANDED’
>  8570 |
> VARATT_IS_EXTERNAL_NON_EXPANDED(DatumGetPointer(newvalue)))
>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> [1687/1944] Compiling C object
> src/test/modules/test_dsa/test_dsa.so.p/test_dsa.c.o^C
> ninja: build stopped: interrupted by user.
> ``

> The overall environment is Raspberry Pi 5 with pretty much default
> configuration - Raspbian etc.
>
> How to fix it? Absolutely no idea :)

I think it's actually a somewhat reasonable warning - the compiler can't know
that in exec_set_found() we'll always deal with typlen == 1 and thus can't
ever reach the inside of the branch it warns about.

Once the compiler knows about that "restriction", the warning vanishes. Try
adding the following to exec_set_found():

    /*
     * Prevent spurious warning due to compiler not realizing
     * VARATT_IS_EXTERNAL_NON_EXPANDED() branch in assign_simple_var() isn't
     * reachable due to "found" being byvalue.
     */
    if (var->datatype->typlen != 1)
        pg_unreachable();

I'm somewhat inclined to think it'd be worth adding something along those
lines to avoid this warning ([1]).

Greetings,

Andres Freund


[1]

In general we're actually hiding a lot of useful information from the compiler
in release builds, due to asserts not being enabled. I've been wondering about
a version of Assert() that isn't completely removed in release builds but
instead transform into the pg_unreachable() form for compilers with an
"efficient" pg_unreachable() (i.e. not using abort()).

We can't just do that for all asserts though, it only makes sense for ones
that are "trivial" in some form (i.e. the compiler can realize it doesn't
have side effects and doesn't need be generated).

That's about generating more optimized code though.



Re: gcc 13 warnings

From
Aleksander Alekseev
Date:
Hi,

>         /*
>          * Prevent spurious warning due to compiler not realizing
>          * VARATT_IS_EXTERNAL_NON_EXPANDED() branch in assign_simple_var() isn't
>          * reachable due to "found" being byvalue.
>          */
>         if (var->datatype->typlen != 1)
>                 pg_unreachable();
>
> I'm somewhat inclined to think it'd be worth adding something along those
> lines to avoid this warning ([1]).

IMO we shouldn't allow warnings to appear in release builds, even
harmless ones. Otherwise we start ignoring them and will skip
something important one day. So I think we should do this.

-- 
Best regards,
Aleksander Alekseev