False positive warning in verify_heapam.c with GCC 03
Hi!
While playing with some unrelated to the topic stuff, I've noticed a
strange warning from verify_heapam.c:730:25:
warning: ‘xmax_status’ may be used uninitialized in this function.
This happens only when get_xid_status is inlined, and only in GCC with O3.
I use a GCC version 11.3.0.
For the purpose of investigation, I've created a PFA patch to force
get_xid_status
inline.
$ CFLAGS="-O3" ./configure -q && make -s -j12 >/dev/null && make -s -j12 -C
contrib
verify_heapam.c: In function ‘check_tuple_visibility’:
verify_heapam.c:730:25: warning: ‘xmax_status’ may be used uninitialized in
this function [-Wmaybe-uninitialized]
730 | XidCommitStatus xmax_status;
| ^~~~~~~~~~~
verify_heapam.c:909:25: warning: ‘xmin_status’ may be used uninitialized in
this function [-Wmaybe-uninitialized]
909 | else if (xmin_status != XID_COMMITTED)
|
I believe, this warning is false positive, since mentioned row is
unreachable. If xid is invalid, we return from get_xid_status
XID_INVALID and could not pass
770 ········if (HeapTupleHeaderXminInvalid(tuphdr))
771 ············return false;·······/* inserter aborted, don't check */
So, I think this warning is false positive. On the other hand, we could
simply init status variable in get_xid_status and
make this code more errors/warnings safe. Thoughts?
--
Best regards,
Maxim Orlov.
Attachments:
0002-Init-status-var-in-get_xid_status.patchapplication/octet-stream; name=0002-Init-status-var-in-get_xid_status.patchDownload
From 9059ab2ba2903aebae73448a5a8445e66e908d3b Mon Sep 17 00:00:00 2001
From: Maxim Orlov <m.orlov@postgrespro.ru>
Date: Tue, 27 Dec 2022 18:31:15 +0300
Subject: [PATCH 2/2] Init status var in get_xid_status
---
contrib/amcheck/verify_heapam.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index d51f50f8c1d..a3547c51bbb 100644
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -1688,7 +1688,11 @@ get_xid_status(TransactionId xid, HeapCheckContext *ctx,
/* Quick check for special xids */
if (!TransactionIdIsValid(xid))
+ {
+ if (status != NULL)
+ *status = XID_COMMITTED;
return XID_INVALID;
+ }
else if (xid == BootstrapTransactionId || xid == FrozenTransactionId)
{
if (status != NULL)
--
2.34.1
0001-Force-inline-of-get_xid_status.patchapplication/octet-stream; name=0001-Force-inline-of-get_xid_status.patchDownload
From 1eff7f88d283838373286c70bb009b42d4791e65 Mon Sep 17 00:00:00 2001
From: Maxim Orlov <m.orlov@postgrespro.ru>
Date: Tue, 27 Dec 2022 17:45:48 +0300
Subject: [PATCH 1/2] Force inline of get_xid_status
---
contrib/amcheck/verify_heapam.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index b72a5c96d12..d51f50f8c1d 100644
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -173,7 +173,7 @@ static XidBoundsViolation check_mxid_in_range(MultiXactId mxid,
HeapCheckContext *ctx);
static XidBoundsViolation check_mxid_valid_in_rel(MultiXactId mxid,
HeapCheckContext *ctx);
-static XidBoundsViolation get_xid_status(TransactionId xid,
+static inline XidBoundsViolation get_xid_status(TransactionId xid,
HeapCheckContext *ctx,
XidCommitStatus *status);
@@ -1679,7 +1679,7 @@ check_mxid_valid_in_rel(MultiXactId mxid, HeapCheckContext *ctx)
* appears to be valid in this relation, the status argument will be set with
* the commit status of the transaction ID.
*/
-static XidBoundsViolation
+static inline XidBoundsViolation
get_xid_status(TransactionId xid, HeapCheckContext *ctx,
XidCommitStatus *status)
{
--
2.34.1