Re: gcc 13 warnings

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