Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Dmitry Mityugov <d.mityugov@postgrespro.ru>
Cc: pgsql-hackers@postgresql.org
Date: 2025-07-12T17:42:54Z
Lists: pgsql-hackers
Attachments
- v2-silence-uninitialized-warnings.patch (text/x-diff) patch v2
Dmitry Mityugov <d.mityugov@postgrespro.ru> writes:
> When compiled with Assert() macro disabled, GCC 15 produces warnings
> about possibly uninitialized variables in
> src/backend/utils/adt/jsonb_util.c module. This problem was discussed in
> detail in this thread, in April 2025:
> https://www.postgresql.org/message-id/988bf1bc-3f1f-99f3-bf98-222f1cd9dc5e@xs4all.nl
> .
> Recently introduced pg_assume() macro let fix such problems easily. The
> attached patch fixes them in jsonb_util.c module. I verified that
> PostgreSQL compiles clearly with this patch and GCC 15.1.1 on an x86
> 64-bit machine (with and without --enable-cassert), and with GCC 14.2.1
> on a 64-bit ARM machine. `make check` also passes.
I don't care for this patch: replacing an Assert with pg_assume just
seems like a very bad idea. If the assertion condition ever failed,
which doesn't seem all that impossible in logic as complicated as
this, then instead of an identifiable assertion trap we'd get
unspecified and impossible-to-debug behavior.
We could conceivably do
+#ifdef USE_ASSERT_CHECKING
Assert(ra != WJB_END_ARRAY && ra != WJB_END_OBJECT);
Assert(rb != WJB_END_ARRAY && rb != WJB_END_OBJECT);
+#else
+ pg_assume(ra != WJB_END_ARRAY && ra != WJB_END_OBJECT);
+ pg_assume(rb != WJB_END_ARRAY && rb != WJB_END_OBJECT);
+#endif
but that seems ugly. In any case, it's just doubling down on the
assumption that a compiler capable of detecting the "may be used
uninitialized" issue will conclude that rejecting WJB_END_ARRAY
and WJB_END_OBJECT (but not WJB_DONE) eliminates the possibility of
va.type/vb.type not being set.
What I think we should do about this is what I mentioned in the
other thread: adjust the code to guarantee that va.type/vb.type
are defined in all cases. There are a couple of ways we could
do that, but after reflection I think the best way is to modify
JsonbIteratorNext to make that guarantee. I've checked that
the attached silences the warning on gcc 15.1.1 (current
Fedora 42).
regards, tom lane
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Clarify the ra != rb case in compareJsonbContainers().
- 2a3a3964328a 19 (unreleased) landed
-
Silence uninitialized-value warnings in compareJsonbContainers().
- f32e45641460 15.14 landed
- aad1617b76ae 19 (unreleased) landed
- 8254b7e33be9 14.19 landed
- 6c93bf735ce1 13.22 landed
- 5db55e13f210 16.10 landed
- 5a2139a90690 17.6 landed
- 0b6dfce0ce4d 18.0 landed