From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Dmitry Mityugov <d(dot)mityugov(at)postgrespro(dot)ru> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings |
Date: | 2025-07-12 17:42:54 |
Message-ID: | 82868.1752342174@sss.pgh.pa.us |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Dmitry Mityugov <d(dot)mityugov(at)postgrespro(dot)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
Attachment | Content-Type | Size |
---|---|---|
v2-silence-uninitialized-warnings.patch | text/x-diff | 2.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2025-07-12 17:55:32 | Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings |
Previous Message | Greg Sabino Mullane | 2025-07-12 15:29:43 | Re: PATCH: warn about, and deprecate, clear text passwords |