gcc 15 "array subscript 0" warning at level -O3

I wrote:

Anybody have an idea about how to silence it more effectively?
There are going to be more people seeing this as gcc 15 propagates.

Meh. I tried this:

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index bb99781c56e..ea489db89c9 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -8794,6 +8794,7 @@ assign_simple_var(PLpgSQL_execstate *estate, PLpgSQL_var *var,
         * not a problem since all array entries are always detoasted.)
         */
        if (!estate->atomic && !isnull && var->datatype->typlen == -1 &&
+           DatumGetPointer(newvalue) != NULL &&
            VARATT_IS_EXTERNAL_NON_EXPANDED(DatumGetPointer(newvalue)))
        {
            MemoryContext oldcxt;

and was rewarded with *two* copies of the warning. So that makes it
smell more like a compiler bug than anything else. (I now vaguely
recall Andres opining that that's what it was on serinus, though
I can't find any such email.)

regards, tom lane

Hi,

On 2025-04-25 13:37:15 -0400, Tom Lane wrote:

Whilst poking at Erik Rijkers' nearby report, I found that
Fedora 42's gcc 15.0.1 will produce this complaint if you
select optimization level -O3:

In file included from ../../../../src/include/access/htup_details.h:22,
from pl_exec.c:21:
In function 'assign_simple_var',
inlined from 'exec_set_found' at pl_exec.c:8609: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)))
| ^~~~~~~~~~~~~~~
pl_exec.c:8797:17: note: in expansion of macro 'VARATT_IS_EXTERNAL_NON_EXPANDED'
8797 | VARATT_IS_EXTERNAL_NON_EXPANDED(DatumGetPointer(newvalue)))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function 'exec_set_found':
cc1: note: source object is likely at address zero

FWIW, I've seen this even before GCC 15.

Buildfarm member serinus has been producing the identical warning for
some time. I'd been ignoring that because it runs "experimental gcc",
but I guess the experiment has leaked out to production distros.

What seems to be happening here is that after inlining
assign_simple_var into exec_set_found, the compiler decides that
"newvalue" might be zero (since it's a BoolGetDatum result),
and then it warns -- in a rather strange way -- about the
potential null dereference.

I don't think it actually is complaining about a null dereference - it thinks
we're interpreting a boolean as a pointer (for which it obviously is not wide
enough)

The dereference is not reachable
because of the preceding "var->datatype->typlen == -1" check,
but that's not stopping the optimizer from bitching.

I experimented with modifying exec_set_found thus:

var = (PLpgSQL_var *) (estate->datums[estate->found_varno]);
+ Assert(var->datatype->typlen == 1);
assign_simple_var(estate, var, BoolGetDatum(state), false, false);

which should be OK since we're expecting the "found" variable to
be boolean. That does silence the warning, but of course only
in --enable-cassert builds.

One way to address this is outlined here:

/messages/by-id/20230316172818.x6375uvheom3ibt2@awork3.anarazel.de
/messages/by-id/20240207203138.sknifhlppdtgtxnk@awork3.anarazel.de

I've been wondering about adding wrapping something like that in a
pg_assume(expr) or such.

Greetings,

Andres Freund

Hi,

On 2025-04-25 15:58:29 -0400, Andres Freund wrote:

On 2025-04-25 13:37:15 -0400, Tom Lane wrote:

Buildfarm member serinus has been producing the identical warning for
some time. I'd been ignoring that because it runs "experimental gcc",
but I guess the experiment has leaked out to production distros.

What seems to be happening here is that after inlining
assign_simple_var into exec_set_found, the compiler decides that
"newvalue" might be zero (since it's a BoolGetDatum result),
and then it warns -- in a rather strange way -- about the
potential null dereference.

I don't think it actually is complaining about a null dereference - it thinks
we're interpreting a boolean as a pointer (for which it obviously is not wide
enough)

The dereference is not reachable
because of the preceding "var->datatype->typlen == -1" check,
but that's not stopping the optimizer from bitching.

I experimented with modifying exec_set_found thus:

var = (PLpgSQL_var *) (estate->datums[estate->found_varno]);
+ Assert(var->datatype->typlen == 1);
assign_simple_var(estate, var, BoolGetDatum(state), false, false);

which should be OK since we're expecting the "found" variable to
be boolean. That does silence the warning, but of course only
in --enable-cassert builds.

One way to address this is outlined here:

/messages/by-id/20230316172818.x6375uvheom3ibt2@awork3.anarazel.de
/messages/by-id/20240207203138.sknifhlppdtgtxnk@awork3.anarazel.de

I've been wondering about adding wrapping something like that in a
pg_assume(expr) or such.

I've been once more annoyed by this warning. Here's a prototype for the
approach outlined above.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

I've been wondering about adding wrapping something like that in a
pg_assume(expr) or such.

I've been once more annoyed by this warning. Here's a prototype for the
approach outlined above.

Looks plausible by eyeball. I did notice a typo in the comment:

+ * pg_assume(expr) stats that we assume `expr` to evaluate to true. In assert

s/stats/states/, I think you meant.

regards, tom lane

On Thu, Jun 5, 2025 at 3:00 AM Andres Freund <andres@anarazel.de> wrote:

The dereference is not reachable
because of the preceding "var->datatype->typlen == -1" check,
but that's not stopping the optimizer from bitching.

I experimented with modifying exec_set_found thus:

var = (PLpgSQL_var *) (estate->datums[estate->found_varno]);
+ Assert(var->datatype->typlen == 1);
assign_simple_var(estate, var, BoolGetDatum(state), false, false);

which should be OK since we're expecting the "found" variable to
be boolean. That does silence the warning, but of course only
in --enable-cassert builds.

One way to address this is outlined here:

/messages/by-id/20230316172818.x6375uvheom3ibt2@awork3.anarazel.de
/messages/by-id/20240207203138.sknifhlppdtgtxnk@awork3.anarazel.de

I've been wondering about adding wrapping something like that in a
pg_assume(expr) or such.

I've been once more annoyed by this warning. Here's a prototype for the
approach outlined above.

I can confirm the warning disappears when using gcc-14.0 compile
source code with the attached patch.
I didn't review it though.
I didn’t find this in the CommitFest, so I added an entry [1]https://commitfest.postgresql.org/patch/5888/ to make sure it
doesn’t get forgotten...

[1]: https://commitfest.postgresql.org/patch/5888/

Hi,

On 2025-06-05 15:50:48 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

I've been wondering about adding wrapping something like that in a
pg_assume(expr) or such.

I've been once more annoyed by this warning. Here's a prototype for the
approach outlined above.

Looks plausible by eyeball. I did notice a typo in the comment:

+ * pg_assume(expr) stats that we assume `expr` to evaluate to true. In assert

s/stats/states/, I think you meant.

Thanks. I pushed it with that tweak and a bit of minor comment burnishing.
Glad to see the last of that warning.

Greetings,

Andres Freund

Hi,

On 2025-07-02 22:13:17 +0800, jian he wrote:

On Thu, Jun 5, 2025 at 3:00 AM Andres Freund <andres@anarazel.de> wrote:

I've been once more annoyed by this warning. Here's a prototype for the
approach outlined above.

I can confirm the warning disappears when using gcc-14.0 compile
source code with the attached patch.
I didn't review it though.
I didn’t find this in the CommitFest, so I added an entry [1] to make sure it
doesn’t get forgotten...

Thanks. Pushed it now and closed the CF entry.

Greetings,

Andres Freund