MIME-Version: 1.0
From: Daniil Davydov <3danissimo@gmail.com>
Date: Wed, 25 Dec 2024 19:44:03 +0700
Message-ID: <CAJDiXgh4CX7u95TvJH7LXShtuJ3Fu0-gYdLHfGwt8RZ5k27Hmg@mail.gmail.com>
Subject: Improve consistency checks in tbm_prepare_shared_iterate
To: pgsql-hackers@postgresql.org
Content-Type: multipart/mixed; boundary="00000000000051a7c8062a179654"
Archived-At: <https://www.postgresql.org/message-id/CAJDiXgh4CX7u95TvJH7LXShtuJ3Fu0-gYdLHfGwt8RZ5k27Hmg%40mail.gmail.com>
Precedence: bulk

--00000000000051a7c8062a179654
Content-Type: text/plain; charset="UTF-8"

Hi,
Currently, iterating through TIDBitmap contains this code (REL_16_STABLE) :
***
while ((page = pagetable_iterate(tbm->pagetable, &i)) != NULL)
{
    idx = page - ptbase->ptentry;
    if (page->ischunk)
        ptchunks->index[nchunks++] = idx;
    else
        ptpages->index[npages++] = idx;
}

Assert(npages == tbm->npages);
Assert(nchunks == tbm->nchunks);
***

Two asserts in the end seem to be overdue to me, because if (for
example) nchunks > tbm->nchunks (due to another error),
we have already accessed to invalid memory chunk and overwritten it.
If we want to monitor the correctness of these variables, it might be
better to add a few checks, as in the attached patch.

I'm not sure if the comment in the error message is written correctly,
but first I would like to hear your opinion.

--
Best regards,
Daniil Davydov

--00000000000051a7c8062a179654
Content-Type: text/x-patch; charset="US-ASCII"; 
	name="0001-Add-new-checks-for-tidbitmap-scan.patch"
Content-Disposition: attachment; 
	filename="0001-Add-new-checks-for-tidbitmap-scan.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_m53vlx9h0>
X-Attachment-Id: f_m53vlx9h0

RnJvbSAwNGU0MTk2OWVmOWVhMmM5NmFmMjE4YTU1MTliNWVmYjE5YmFlZWQ1IE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBEYW5paWwgRGF2aWRvdiA8ZGF2eWRvdmRhbmlpbDUwMUBnbWFp
bC5jb20+CkRhdGU6IFdlZCwgMjUgRGVjIDIwMjQgMTk6MzI6NTUgKzA3MDAKU3ViamVjdDogW1BB
VENIXSBBZGQgbmV3IGNoZWNrcyBmb3IgdGlkYml0bWFwIHNjYW4KCi0tLQogc3JjL2JhY2tlbmQv
bm9kZXMvdGlkYml0bWFwLmMgfCAxMSArKysrKysrKystLQogMSBmaWxlIGNoYW5nZWQsIDkgaW5z
ZXJ0aW9ucygrKSwgMiBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9zcmMvYmFja2VuZC9ub2Rl
cy90aWRiaXRtYXAuYyBiL3NyYy9iYWNrZW5kL25vZGVzL3RpZGJpdG1hcC5jCmluZGV4IDI5YTE4
NTg0NDEwLi4zNGEwMTVlNWVhNyAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvbm9kZXMvdGlkYml0
bWFwLmMKKysrIGIvc3JjL2JhY2tlbmQvbm9kZXMvdGlkYml0bWFwLmMKQEAgLTgyNywxMCArODI3
LDE3IEBAIHRibV9wcmVwYXJlX3NoYXJlZF9pdGVyYXRlKFRJREJpdG1hcCAqdGJtKQogCQkJd2hp
bGUgKChwYWdlID0gcGFnZXRhYmxlX2l0ZXJhdGUodGJtLT5wYWdldGFibGUsICZpKSkgIT0gTlVM
TCkKIAkJCXsKIAkJCQlpZHggPSBwYWdlIC0gcHRiYXNlLT5wdGVudHJ5OwotCQkJCWlmIChwYWdl
LT5pc2NodW5rKQorCQkJCWlmIChwYWdlLT5pc2NodW5rICYmIChuY2h1bmtzICsgMSA8PSB0Ym0t
Pm5jaHVua3MpKQogCQkJCQlwdGNodW5rcy0+aW5kZXhbbmNodW5rcysrXSA9IGlkeDsKLQkJCQll
bHNlCisJCQkJZWxzZSBpZiAobnBhZ2VzICsgMSA8PSB0Ym0tPm5wYWdlcykKIAkJCQkJcHRwYWdl
cy0+aW5kZXhbbnBhZ2VzKytdID0gaWR4OworCQkJCWVsc2UKKwkJCQkJZXJlcG9ydChFUlJPUiwK
KwkJCQkJCQkoZXJyY29kZShFUlJDT0RFX0lOVEVSTkFMX0VSUk9SKSwKKwkJCQkJCQkgZXJybXNn
KCJpbnZhbGlkIGNodW5rcy9wYWdlcyBudW0gaW4gdGlkYml0bWFwIDogJWQiCisJCQkJCQkJIAkJ
IiBjaHVua3MgbWV0IG9mICVkIGF2YWlsYWJsZSwgJWQgcGFnZXMgbWV0ICIKKwkJCQkJCQkJCSJv
ZiAlZCBhdmFpbGFibGUiLCBuY2h1bmtzLCB0Ym0tPm5jaHVua3MsCisJCQkJCQkJCQlucGFnZXMs
IHRibS0+bnBhZ2VzKSkpOwogCQkJfQogCiAJCQlBc3NlcnQobnBhZ2VzID09IHRibS0+bnBhZ2Vz
KTsKLS0gCjIuNDMuMAoK
--00000000000051a7c8062a179654--