gcc 15 "array subscript 0" warning at level -O3
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
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. 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.
Anybody have an idea about how to silence it more effectively?
There are going to be more people seeing this as gcc 15 propagates.
regards, tom lane
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.deI'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
Attachments:
v1-0002-Use-pg_assume-to-avoid-compiler-warning-below-exe.patchtext/x-diff; charset=us-asciiDownload
From 3fe01158335d43ed133755aaffd41270c5e7af1a Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Wed, 4 Jun 2025 14:33:53 -0400
Subject: [PATCH v1 2/2] Use pg_assume() to avoid compiler warning below
exec_set_found()
The warning, visible when building with -O3 and a recent-ish gcc, is due to
gcc not realizing that found is a byvalue type and therefore will never be
interpreted as a varlena type.
Discussion: https://postgr.es/m/3prdb6hkep3duglhsujrn52bkvnlkvhc54fzvph2emrsm4vodl@77yy6j4hkemb
Discussion: https://postgr.es/m/20230316172818.x6375uvheom3ibt2%40awork3.anarazel.de
Discussion: https://postgr.es/m/20240207203138.sknifhlppdtgtxnk%40awork3.anarazel.de
---
src/pl/plpgsql/src/pl_exec.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index bb99781c56e..ecac29e830b 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -8606,6 +8606,15 @@ exec_set_found(PLpgSQL_execstate *estate, bool state)
PLpgSQL_var *var;
var = (PLpgSQL_var *) (estate->datums[estate->found_varno]);
+
+ /*
+ * The pg_assume() avoids a spurious warning with some compilers due to
+ * compiler not realizing the VARATT_IS_EXTERNAL_NON_EXPANDED() branch in
+ * assign_simple_var() will never be reached, due to "found" being a
+ * boolean, a byvalue type.
+ */
+ pg_assume(var->datatype->typlen != -1);
+
assign_simple_var(estate, var, BoolGetDatum(state), false, false);
}
--
2.48.1.76.g4e746b1a31.dirty
v1-0001-Add-pg_assume-expr-macro.patchtext/x-diff; charset=us-asciiDownload
From 147e2058e94338802ebc03bcfa7639d3c6bde75a Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Wed, 4 Jun 2025 14:28:19 -0400
Subject: [PATCH v1 1/2] Add pg_assume(expr) macro
This macro can be used to avoid compiler warnings, particularly when using -O3
and not using assertions, and to get the compiler to generate better code.
A subsequent commit introduces a first user.
Discussion: https://postgr.es/m/3prdb6hkep3duglhsujrn52bkvnlkvhc54fzvph2emrsm4vodl@77yy6j4hkemb
Discussion: https://postgr.es/m/20230316172818.x6375uvheom3ibt2%40awork3.anarazel.de
Discussion: https://postgr.es/m/20240207203138.sknifhlppdtgtxnk%40awork3.anarazel.de
---
src/include/c.h | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/src/include/c.h b/src/include/c.h
index 8cdc16a0f4a..318c8ba978f 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -332,6 +332,36 @@
#define pg_unreachable() abort()
#endif
+/*
+ * pg_assume(expr) stats that we assume `expr` to evaluate to true. In assert
+ * enabled builds pg_assume() is turned into an assertion, in optimized builds
+ * we try to clue the compiler into the fact that `expr` is true.
+ *
+ * This is useful for two purposes:
+ *
+ * 1) Avoid compiler warnings by telling the compiler about assumptions the
+ * code makes. This is particularly useful when building with optimizations
+ * and w/o assertions.
+ *
+ * 2) Help the compiler to generate more efficient code
+ *
+ * It is unspecified whether `expr` is evaluated, therefore it better be
+ * side-effect free.
+ */
+#if defined(USE_ASSERT_CHECKING)
+#define pg_assume(expr) Assert(expr)
+#elif defined(HAVE__BUILTIN_UNREACHABLE)
+#define pg_assume(expr) \
+ do { \
+ if (!(expr)) \
+ __builtin_unreachable(); \
+ } while (0)
+#elif defined(_MSC_VER)
+#define pg_assume(expr) __assume(expr)
+#else
+#define pg_assume(expr) ((void) 0)
+#endif
+
/*
* Hints to the compiler about the likelihood of a branch. Both likely() and
* unlikely() return the boolean value of the contained expression.
--
2.48.1.76.g4e746b1a31.dirty
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.deI'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...
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