Re: new heapcheck contrib module

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Geoghegan <pg(at)bowt(dot)ie>, "Andrey M(dot) Borodin" <x4mmm(at)yandex-team(dot)ru>, Stephen Frost <sfrost(at)snowman(dot)net>, Michael Paquier <michael(at)paquier(dot)xyz>, Amul Sul <sulamul(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: new heapcheck contrib module
Date: 2020-10-22 20:23:36
Message-ID: 41306.1603398216@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

... btw, having now looked more closely at get_xid_status(), I wonder
how come there aren't more compilers bitching about it, because it
is very very obviously broken. In particular, the case of
requesting status for an xid that is BootstrapTransactionId or
FrozenTransactionId *will* fall through to perform
FullTransactionIdPrecedesOrEquals with an uninitialized fxid.

The fact that most compilers seem to fail to notice that is quite scary.
I suppose it has something to do with FullTransactionId being a struct,
which makes me wonder if that choice was quite as wise as we thought.

Meanwhile, so far as this code goes, I wonder why you don't just change it
to always set that value, ie

XidBoundsViolation result;
FullTransactionId fxid;
FullTransactionId clog_horizon;

+ fxid = FullTransactionIdFromXidAndCtx(xid, ctx);
+
/* Quick check for special xids */
if (!TransactionIdIsValid(xid))
result = XID_INVALID;
else if (xid == BootstrapTransactionId || xid == FrozenTransactionId)
result = XID_BOUNDS_OK;
else
{
/* Check if the xid is within bounds */
- fxid = FullTransactionIdFromXidAndCtx(xid, ctx);
if (!fxid_in_cached_range(fxid, ctx))
{

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Mark Dilger 2020-10-22 20:26:41 Re: new heapcheck contrib module
Previous Message Tom Lane 2020-10-22 20:09:16 Re: new heapcheck contrib module