Content-Type: multipart/mixed; boundary="------------dBPmQ27wVX0560YfjUaB4jkS"
Message-ID: <9e3acda6-76b8-d6b4-c013-f78f84d7c5ab@gmail.com>
Date: Thu, 28 Sep 2023 08:00:00 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: BUG #18135: Incorrect memory access occurs when attaching a
 partition with an index
Content-Language: en-US
To: Michael Paquier <michael@paquier.xyz>, pgsql-bugs@lists.postgresql.org
References: <18135-532f4a755e71e4d2@postgresql.org>
 <ZRTzX0h68fxZ5Dth@paquier.xyz>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: <ZRTzX0h68fxZ5Dth@paquier.xyz>
Archived-At: 
 <https://www.postgresql.org/message-id/9e3acda6-76b8-d6b4-c013-f78f84d7c5ab%40gmail.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------dBPmQ27wVX0560YfjUaB4jkS
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Hi Michael,

28.09.2023 06:30, Michael Paquier wrote:
> On Wed, Sep 27, 2023 at 10:00:01AM +0000, PG Bug reporting form wrote:
>> executed under Valgrind, it leads to an incorrect memory access:
>> ==00:00:00:03.947 396156== Invalid read of size 2
>> ==00:00:00:03.947 396156==    at 0x2E323D: CompareIndexInfo (index.c:2572)
>> ==00:00:00:03.947 396156==    by 0x3D009B: AttachPartitionEnsureIndexes
>> (tablecmds.c:18797)
>> ==00:00:00:03.947 396156==    by 0x3D8B4F: ATExecAttachPartition
>> (tablecmds.c:18578)
>> ==00:00:00:03.947 396156==    by 0x3D9A88: ATExecCmd (tablecmds.c:5379)
>> ==00:00:00:03.947 396156==    by 0x3D9BC7: ATRewriteCatalogs
>> (tablecmds.c:5063)

Thank you for paying attention to it!

> I have just tested that on HEAD and REL_16_STABLE, but fail to see
> this report, which is weird (3.19.0 here).  Are you using any specific
> option of valgrind I should be aware of?  Here is what I used, for
> reference:
> valgrind \
>    --suppressions=$PG_SOURCE/src/tools/valgrind.supp \
>    --trace-children=yes --track-origins=yes --read-var-info=yes \
>    postgres -D REST_OF_ARGS

Please try the following procedure (I've simplified my own):
With the attached patch (for HEAD) applied
CPPFLAGS="-DUSE_VALGRIND -Og" ./configure -q --enable-debug --enable-cassert && make -s -j8
sed 's/create index on idxpart1 ((a + b));/create index on idxpart1 ((a));/' -i src/test/regress/sql/indexing.sql
TESTS=indexing make check-tests

I get:
not ok 1     - indexing                                 9844 ms
# (test process exited with exit code 2)

(I use valgrind-3.18.1.)

If you still see no error, please share details of your method for running
valgrind.

>
>> In addition, there is a check whether both indexes are (are not)
>> expression indexes, but it's placed below...
> Sure, but this makes the check a bit cheaper if the indexes to compare
> use expr and non-expr attributes at the same attnums, no?  Except if I
> am missing something, the attached should be sufficient.

I thought about placing that check before the loop, but your fix looks
more clear (and my testing confirms that it works).

Best regards,
Alexander
--------------dBPmQ27wVX0560YfjUaB4jkS
Content-Type: text/x-patch; charset=UTF-8; name="install-vrunner-17.patch"
Content-Disposition: attachment; filename="install-vrunner-17.patch"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL2luc3RhbGwtdnJ1bm5lci5zaCBiL2luc3RhbGwtdnJ1bm5lci5zaApu
ZXcgZmlsZSBtb2RlIDEwMDc1NQppbmRleCAwMDAwMDAwMDAwMC4uMjE4ODFkNGM3MjkKLS0t
IC9kZXYvbnVsbAorKysgYi9pbnN0YWxsLXZydW5uZXIuc2gKQEAgLTAsMCArMSwzMSBAQAor
IyEvYmluL2Jhc2gKK3NldCAtZQorCitTUkNESVI9JDEKK1BHQklOPSQyCisKK2NkICRQR0JJ
TgorY3AgJFNSQ0RJUi9zcmMvdG9vbHMvdmFsZ3JpbmQuc3VwcCAuLworCitlY2hvICIiCitj
YXQgPDwgJ0VPRicgPiB2cnVubmVyCisjIS9iaW4vYmFzaAorc2V0IC1lCitCRD0iJCggY2Qg
IiQoIGRpcm5hbWUgIiR7QkFTSF9TT1VSQ0VbMF19IiApIiAmJiBwd2QgKSIKK1NOPSIkKGJh
c2VuYW1lICIke0JBU0hfU09VUkNFWzBdfSIpIgorT0JJTj0iXyRTTiIKK1tbICRMRF9QUkVM
T0FEID0gKiJ2YWxncmluZCIqIF1dICYmIGV4ZWMgJEJELyRPQklOICIkQCIKKworZXhlYyB2
YWxncmluZCAtLXF1aWV0ICAtLWV4aXQtb24tZmlyc3QtZXJyb3I9eWVzIC0tdHJhY2stb3Jp
Z2lucz1ubyAtLWVycm9yLWV4aXRjb2RlPTEgLS1sZWFrLWNoZWNrPW5vIC0tdGltZS1zdGFt
cD15ZXMgXAorIC0tZ2VuLXN1cHByZXNzaW9ucz1hbGwgLS1zdXBwcmVzc2lvbnM9JEJEL3Zh
bGdyaW5kLnN1cHAgXAorIC0tdHJhY2UtY2hpbGRyZW49eWVzIC0tdHJhY2UtY2hpbGRyZW4t
c2tpcD0iL2Jpbi8qLC91c3IvYmluLyoiIFwKKyAkQkQvJE9CSU4gIiRAIgorRU9GCitjaG1v
ZCBhK3ggdnJ1bm5lcgorCitmb3IgZiBpbiAqOyBkbworICAgIGlmIFsgIiRmIiA9ICJ2cnVu
bmVyIiBdIHx8IFsgISAteCAiJGYiIF07IHRoZW4gY29udGludWU7IGZpCisgICAgbXYgIiRm
IiAiXyRmIgorICAgIGxuIC1zIHZydW5uZXIgIiRmIgorICAgIGNobW9kIGEreCAiJGYiCitk
b25lCmRpZmYgLS1naXQgYS9zcmMvTWFrZWZpbGUuZ2xvYmFsLmluIGIvc3JjL01ha2VmaWxl
Lmdsb2JhbC5pbgppbmRleCA4ODRkNjY5MjBiNi4uODdhZjdmZDlhYjggMTAwNjQ0Ci0tLSBh
L3NyYy9NYWtlZmlsZS5nbG9iYWwuaW4KKysrIGIvc3JjL01ha2VmaWxlLmdsb2JhbC5pbgpA
QCAtNDExLDIgKzQxMSwzIEBAIGlmZXEgKCQoTUFLRUxFVkVMKSwwKQogCSQoTUFLRSkgLWox
ICQoaWYgJChDSEVDS1BSRVBfVE9QKSwtQyAkKENIRUNLUFJFUF9UT1ApLCkgY2hlY2twcmVw
ID4+JyQoYWJzX3RvcF9idWlsZGRpciknL3RtcF9pbnN0YWxsL2xvZy9pbnN0YWxsLmxvZyAy
PiYxCisJJyQoYWJzX3RvcF9idWlsZGRpciknL2luc3RhbGwtdnJ1bm5lci5zaCAnJChhYnNf
dG9wX2J1aWxkZGlyKScgJyQoYWJzX3RvcF9idWlsZGRpcikvdG1wX2luc3RhbGwkKGJpbmRp
ciknCiAKZGlmZiAtLWdpdCBhL3NyYy9iaW4vcGdfY3RsL3BnX2N0bC5jIGIvc3JjL2Jpbi9w
Z19jdGwvcGdfY3RsLmMKaW5kZXggN2Y0MjdlN2Q2MGUuLmU2MTE1ODgxZjZiIDEwMDY0NAot
LS0gYS9zcmMvYmluL3BnX2N0bC9wZ19jdGwuYworKysgYi9zcmMvYmluL3BnX2N0bC9wZ19j
dGwuYwpAQCAtNjIwLDEyICs2MjAsNyBAQCB3YWl0X2Zvcl9wb3N0bWFzdGVyKHBncGlkX3Qg
cG1fcGlkLCBib29sIGRvX2NoZWNrcG9pbnQpCiAJCQlwbXBpZCA9IGF0b2wob3B0bGluZXNb
TE9DS19GSUxFX0xJTkVfUElEIC0gMV0pOwogCQkJcG1zdGFydCA9IGF0b2wob3B0bGluZXNb
TE9DS19GSUxFX0xJTkVfU1RBUlRfVElNRSAtIDFdKTsKIAkJCWlmIChwbXN0YXJ0ID49IHN0
YXJ0X3RpbWUgLSAyICYmCi0jaWZuZGVmIFdJTjMyCi0JCQkJcG1waWQgPT0gcG1fcGlkCi0j
ZWxzZQotCQkJLyogV2luZG93cyBjYW4gb25seSByZWplY3Qgc3RhbmRhbG9uZS1iYWNrZW5k
IFBJRHMgKi8KLQkJCQlwbXBpZCA+IDAKLSNlbmRpZgorCQkJCXBtcGlkID09IHBtX3BpZCAg
JiYga2lsbChwbXBpZCwgMCkgPT0gMAogCQkJCSkKIAkJCXsKIAkJCQkvKgo=

--------------dBPmQ27wVX0560YfjUaB4jkS--