Horton

Thread

Commits

  1. Use pg_assume() to avoid compiler warning below exec_set_found()

  2. Add pg_assume(expr) macro

  3. meson: Change default buildtype to debugoptimized

  1. gcc 13 warnings

    Pavel Stehule <pavel.stehule@gmail.com> — 2023-03-16T13:40:04Z

    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

  2. Re: gcc 13 warnings

    Melanie Plageman <melanieplageman@gmail.com> — 2023-03-16T14:21:23Z

    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.

  3. Re: gcc 13 warnings

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-03-16T15:43:58Z

    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

  4. Re: gcc 13 warnings

    Pavel Stehule <pavel.stehule@gmail.com> — 2023-03-16T16:00:47Z

    č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
>

  5. Re: gcc 13 warnings

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-03-16T16:10:27Z

    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

  6. Re: gcc 13 warnings

    Andres Freund <andres@anarazel.de> — 2023-03-16T17:05:06Z

    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

  7. Re: gcc 13 warnings

    Andres Freund <andres@anarazel.de> — 2023-03-16T17:28:18Z

    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

  8. Re: gcc 13 warnings

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-03-16T17:54:29Z

    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

  9. Re: gcc 13 warnings

    Andres Freund <andres@anarazel.de> — 2023-03-16T18:11:00Z

    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

  10. Re: gcc 13 warnings

    John Naylor <john.naylor@enterprisedb.com> — 2023-03-17T03:14:56Z

    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

  11. Re: gcc 13 warnings

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2023-03-17T08:06:05Z

    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.

  12. Re: gcc 13 warnings

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-03-17T14:26:20Z

    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

  13. Re: gcc 13 warnings

    Andres Freund <andres@anarazel.de> — 2023-03-17T23:54:27Z

    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

  14. Re: gcc 13 warnings

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2023-03-22T09:45:59Z

    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.

  15. Re: gcc 13 warnings

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2023-03-29T07:51:19Z

    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 to me, at least for development builds.

I have committed the change of buildtype to debugoptimized.

  16. Re: gcc 13 warnings

    Aleksander Alekseev <aleksander@timescale.com> — 2024-07-05T11:19:12Z

    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

  17. Re: gcc 13 warnings

    Andres Freund <andres@anarazel.de> — 2024-07-12T17:45:32Z

    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.

  18. Re: gcc 13 warnings

    Aleksander Alekseev <aleksander@timescale.com> — 2024-07-15T08:38:42Z

    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