From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 5AE703858C2F for ; Sat, 2 Jul 2022 11:07:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5AE703858C2F Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 6C01222382; Sat, 2 Jul 2022 11:07:10 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 521D013451; Sat, 2 Jul 2022 11:07:10 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id BnW7Et4mwGIBMAAAMHmgww (envelope-from ); Sat, 02 Jul 2022 11:07:10 +0000 Content-Type: multipart/mixed; boundary="------------c07Z29JdC1lEZ3AWgkGHun01" Message-ID: <2c75174c-8468-ba23-a30d-c8f99dc9589a@suse.de> Date: Sat, 2 Jul 2022 13:07:09 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH 2/5] [gdb/symtab] Fix data race on per_cu->dwarf_version Content-Language: en-US To: gdb-patches@sourceware.org Cc: Tom Tromey References: <20220629152914.13149-1-tdevries@suse.de> <20220629152914.13149-2-tdevries@suse.de> <58c8a00c-a931-8f97-ef05-66f404a6756a@suse.de> From: Tom de Vries In-Reply-To: <58c8a00c-a931-8f97-ef05-66f404a6756a@suse.de> X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 02 Jul 2022 11:07:13 -0000 This is a multi-part message in MIME format. --------------c07Z29JdC1lEZ3AWgkGHun01 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 7/1/22 13:16, Tom de Vries via Gdb-patches wrote: > On 6/29/22 17:29, Tom de Vries via Gdb-patches wrote: >> When building gdb with -fsanitize=thread and gcc 12, and running >> test-case >> gdb.dwarf2/dwz.exp, we run into a data race between thread T2 and the >> main >> thread in the same write: >> ... >> Write of size 1 at 0x7b200000300c:^M >>      #0 cutu_reader::cutu_reader(dwarf2_per_cu_data*, >> dwarf2_per_objfile*, \ >>      abbrev_table*, dwarf2_cu*, bool, abbrev_cache*) >> gdb/dwarf2/read.c:6252 \ >>      (gdb+0x82f3b3)^M >> ... >> which is here: >> ... >>           this_cu->dwarf_version = cu->header.version; >> ... >> >> Both writes are called from the parallel for in >> dwarf2_build_psymtabs_hard, >> this one directly: >> ... >>      #1 process_psymtab_comp_unit gdb/dwarf2/read.c:6774 (gdb+0x8304d7)^M >>      #2 operator() gdb/dwarf2/read.c:7098 (gdb+0x8317be)^M >>      #3 operator() gdbsupport/parallel-for.h:163 (gdb+0x872380)^M >> ... >> and this via the PU import: >> ... >>      #1 cooked_indexer::ensure_cu_exists(cutu_reader*, >> dwarf2_per_objfile*, \ >>      sect_offset, bool,  bool) gdb/dwarf2/read.c:17964 (gdb+0x85c43b)^M >>      #2 cooked_indexer::index_imported_unit(cutu_reader*, unsigned >> char const*, \ >>      abbrev_info const*) gdb/dwarf2/read.c:18248 (gdb+0x85d8ff)^M >>      #3 cooked_indexer::index_dies(cutu_reader*, unsigned char const*, \ >>      cooked_index_entry const*, bool) gdb/dwarf2/read.c:18302 >> (gdb+0x85dcdb)^M >>      #4 cooked_indexer::make_index(cutu_reader*) >> gdb/dwarf2/read.c:18443 \ >>      (gdb+0x85e68a)^M >>      #5 process_psymtab_comp_unit gdb/dwarf2/read.c:6812 (gdb+0x830879)^M >>      #6 operator() gdb/dwarf2/read.c:7098 (gdb+0x8317be)^M >>      #7 operator() gdbsupport/parallel-for.h:171 (gdb+0x8723e2)^M >> ... >> >> Fix this by setting the field earlier, in read_comp_units_from_section. >> >> The write in cutu_reader::cutu_reader() is still needed, in case >> read_comp_units_from_section is not used. >> >> Make the write conditional, such that it doesn't trigger if the field is >> already set by read_comp_units_from_section. >> > > I realized there's a similar write in the this_cu->is_debug_types == > true clause in cutu_reader::cutu_reader(), which I missed.  So I > introduced a member function set_version to make sure it's set in a > consistent way. > This is what I ended up committing. Further changes: - add assert in set_version - rename field to m_dwarf_version. Thanks, - Tom --------------c07Z29JdC1lEZ3AWgkGHun01 Content-Type: text/x-patch; charset=UTF-8; name="0001-gdb-symtab-Fix-data-race-on-per_cu-dwarf_version.patch" Content-Disposition: attachment; filename*0="0001-gdb-symtab-Fix-data-race-on-per_cu-dwarf_version.patch" Content-Transfer-Encoding: base64 W2dkYi9zeW10YWJdIEZpeCBkYXRhIHJhY2Ugb24gcGVyX2N1LT5kd2FyZl92ZXJzaW9uCgpX aGVuIGJ1aWxkaW5nIGdkYiB3aXRoIC1mc2FuaXRpemU9dGhyZWFkIGFuZCBnY2MgMTIsIGFu ZCBydW5uaW5nIHRlc3QtY2FzZQpnZGIuZHdhcmYyL2R3ei5leHAsIHdlIHJ1biBpbnRvIGEg ZGF0YSByYWNlIGJldHdlZW4gdGhyZWFkIFQyIGFuZCB0aGUgbWFpbgp0aHJlYWQgaW4gdGhl IHNhbWUgd3JpdGU6Ci4uLgpXcml0ZSBvZiBzaXplIDEgYXQgMHg3YjIwMDAwMDMwMGM6Xk0K ICAgICMwIGN1dHVfcmVhZGVyOjpjdXR1X3JlYWRlcihkd2FyZjJfcGVyX2N1X2RhdGEqLCBk d2FyZjJfcGVyX29iamZpbGUqLCBcCiAgICBhYmJyZXZfdGFibGUqLCBkd2FyZjJfY3UqLCBi b29sLCBhYmJyZXZfY2FjaGUqKSBnZGIvZHdhcmYyL3JlYWQuYzo2MjUyIFwKICAgIChnZGIr MHg4MmYzYjMpXk0KLi4uCndoaWNoIGlzIGhlcmU6Ci4uLgogICAgICAgICB0aGlzX2N1LT5k d2FyZl92ZXJzaW9uID0gY3UtPmhlYWRlci52ZXJzaW9uOwouLi4KCkJvdGggd3JpdGVzIGFy ZSBjYWxsZWQgZnJvbSB0aGUgcGFyYWxsZWwgZm9yIGluIGR3YXJmMl9idWlsZF9wc3ltdGFi c19oYXJkLAp0aGlzIG9uZSBkaXJlY3RseToKLi4uCiAgICAjMSBwcm9jZXNzX3BzeW10YWJf Y29tcF91bml0IGdkYi9kd2FyZjIvcmVhZC5jOjY3NzQgKGdkYisweDgzMDRkNyleTQogICAg IzIgb3BlcmF0b3IoKSBnZGIvZHdhcmYyL3JlYWQuYzo3MDk4IChnZGIrMHg4MzE3YmUpXk0K ICAgICMzIG9wZXJhdG9yKCkgZ2Ric3VwcG9ydC9wYXJhbGxlbC1mb3IuaDoxNjMgKGdkYisw eDg3MjM4MCleTQouLi4KYW5kIHRoaXMgdmlhIHRoZSBQVSBpbXBvcnQ6Ci4uLgogICAgIzEg Y29va2VkX2luZGV4ZXI6OmVuc3VyZV9jdV9leGlzdHMoY3V0dV9yZWFkZXIqLCBkd2FyZjJf cGVyX29iamZpbGUqLCBcCiAgICBzZWN0X29mZnNldCwgYm9vbCwgIGJvb2wpIGdkYi9kd2Fy ZjIvcmVhZC5jOjE3OTY0IChnZGIrMHg4NWM0M2IpXk0KICAgICMyIGNvb2tlZF9pbmRleGVy OjppbmRleF9pbXBvcnRlZF91bml0KGN1dHVfcmVhZGVyKiwgdW5zaWduZWQgY2hhciBjb25z dCosIFwKICAgIGFiYnJldl9pbmZvIGNvbnN0KikgZ2RiL2R3YXJmMi9yZWFkLmM6MTgyNDgg KGdkYisweDg1ZDhmZileTQogICAgIzMgY29va2VkX2luZGV4ZXI6OmluZGV4X2RpZXMoY3V0 dV9yZWFkZXIqLCB1bnNpZ25lZCBjaGFyIGNvbnN0KiwgXAogICAgY29va2VkX2luZGV4X2Vu dHJ5IGNvbnN0KiwgYm9vbCkgZ2RiL2R3YXJmMi9yZWFkLmM6MTgzMDIgKGdkYisweDg1ZGNk YileTQogICAgIzQgY29va2VkX2luZGV4ZXI6Om1ha2VfaW5kZXgoY3V0dV9yZWFkZXIqKSBn ZGIvZHdhcmYyL3JlYWQuYzoxODQ0MyBcCiAgICAoZ2RiKzB4ODVlNjhhKV5NCiAgICAjNSBw cm9jZXNzX3BzeW10YWJfY29tcF91bml0IGdkYi9kd2FyZjIvcmVhZC5jOjY4MTIgKGdkYisw eDgzMDg3OSleTQogICAgIzYgb3BlcmF0b3IoKSBnZGIvZHdhcmYyL3JlYWQuYzo3MDk4IChn ZGIrMHg4MzE3YmUpXk0KICAgICM3IG9wZXJhdG9yKCkgZ2Ric3VwcG9ydC9wYXJhbGxlbC1m b3IuaDoxNzEgKGdkYisweDg3MjNlMileTQouLi4KCkZpeCB0aGlzIGJ5IHNldHRpbmcgdGhl IGZpZWxkIGVhcmxpZXIsIGluIHJlYWRfY29tcF91bml0c19mcm9tX3NlY3Rpb24uCgpUaGUg d3JpdGUgaW4gY3V0dV9yZWFkZXI6OmN1dHVfcmVhZGVyKCkgaXMgc3RpbGwgbmVlZGVkLCBp biBjYXNlCnJlYWRfY29tcF91bml0c19mcm9tX3NlY3Rpb24gaXMgbm90IHVzZWQgKHJ1biB0 aGUgdGVzdC1jYXNlIHdpdGggc2F5LCB0YXJnZXQKYm9hcmQgY2Mtd2l0aC1nZGItaW5kZXgp LgoKTWFrZSB0aGUgd3JpdGUgY29uZGl0aW9uYWwsIHN1Y2ggdGhhdCBpdCBkb2Vzbid0IHRy aWdnZXIgaWYgdGhlIGZpZWxkIGlzCmFscmVhZHkgc2V0IGJ5IHJlYWRfY29tcF91bml0c19m cm9tX3NlY3Rpb24uICBJbnN0ZWFkLCB2ZXJpZnkgdGhhdCB0aGUKZmllbGQgYWxyZWFkeSBo YXMgdGhlIHZhbHVlIHRoYXQgd2UncmUgdHJ5aW5nIHRvIHNldCBpdCB0by4KCk1vdmUgdGhp cyBsb2dpYyBpbnRvIGludG8gYSBtZW1iZXIgZnVuY3Rpb24gc2V0X3ZlcnNpb24gKGluIGFu YWxvZ3kgdG8gdGhlCmFscmVhZHkgcHJlc2VudCBtZW1iZXIgZnVuY3Rpb24gdmVyc2lvbikg dG8gbWFrZSBzdXJlIGl0J3MgdXNlZCBjb25zaXN0ZW5seSwKYW5kIG1ha2UgdGhlIGZpZWxk IHByaXZhdGUgaW4gb3JkZXIgdG8gZW5mb3JjZSBhY2Nlc3MgdGhyb3VnaCB0aGUgbWVtYmVy CmZ1bmN0aW9ucywgYW5kIHJlbmFtZSBpdCB0byBtX2R3YXJmX3ZlcnNpb24uCgpXaGlsZSB3 ZSdyZSBhdCBpdCwgbWFrZSBzdXJlIHRoYXQgdGhlIHZlcnNpb24gaXMgc2V0IGJlZm9yZSBy ZWFkLCB0byBhdm9pZApzYXkgcmV0dXJuaW5nIHRydWUgZm9yICJwZXJfY3UudmVyc2lvbiAo KSA8IDUiIGlmICJwZXJfY3UudmVyc2lvbiAoKSA9PSAwIi4KClRlc3RlZCBvbiB4ODZfNjQt bGludXguCgotLS0KIGdkYi9kd2FyZjIvcmVhZC5jIHwgMTAgKysrKysrKy0tLQogZ2RiL2R3 YXJmMi9yZWFkLmggfCAxOCArKysrKysrKysrKysrKysrLS0KIDIgZmlsZXMgY2hhbmdlZCwg MjMgaW5zZXJ0aW9ucygrKSwgNSBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9nZGIvZHdh cmYyL3JlYWQuYyBiL2dkYi9kd2FyZjIvcmVhZC5jCmluZGV4IGQ1MDg4Mzk1ZmIxLi40MTBj YzkwOWMyNiAxMDA2NDQKLS0tIGEvZ2RiL2R3YXJmMi9yZWFkLmMKKysrIGIvZ2RiL2R3YXJm Mi9yZWFkLmMKQEAgLTYyMzUsNyArNjIzNSw3IEBAIGN1dHVfcmVhZGVyOjpjdXR1X3JlYWRl ciAoZHdhcmYyX3Blcl9jdV9kYXRhICp0aGlzX2N1LAogCSAgc2lnX3R5cGUtPnR5cGVfb2Zm c2V0X2luX3NlY3Rpb24gPQogCSAgICB0aGlzX2N1LT5zZWN0X29mZiArIHRvX3VuZGVybHlp bmcgKHNpZ190eXBlLT50eXBlX29mZnNldF9pbl90dSk7CiAKLQkgIHRoaXNfY3UtPmR3YXJm X3ZlcnNpb24gPSBjdS0+aGVhZGVyLnZlcnNpb247CisJICB0aGlzX2N1LT5zZXRfdmVyc2lv biAoY3UtPmhlYWRlci52ZXJzaW9uKTsKIAl9CiAgICAgICBlbHNlCiAJewpAQCAtNjI0OSw3 ICs2MjQ5LDcgQEAgY3V0dV9yZWFkZXI6OmN1dHVfcmVhZGVyIChkd2FyZjJfcGVyX2N1X2Rh dGEgKnRoaXNfY3UsCiAJICAgIHRoaXNfY3UtPmxlbmd0aCA9IGN1LT5oZWFkZXIuZ2V0X2xl bmd0aCAoKTsKIAkgIGVsc2UKIAkgICAgZ2RiX2Fzc2VydCAodGhpc19jdS0+bGVuZ3RoID09 IGN1LT5oZWFkZXIuZ2V0X2xlbmd0aCAoKSk7Ci0JICB0aGlzX2N1LT5kd2FyZl92ZXJzaW9u ID0gY3UtPmhlYWRlci52ZXJzaW9uOworCSAgdGhpc19jdS0+c2V0X3ZlcnNpb24gKGN1LT5o ZWFkZXIudmVyc2lvbik7CiAJfQogICAgIH0KIApAQCAtNzIwNiw2ICs3MjA2LDEwIEBAIHJl YWRfY29tcF91bml0c19mcm9tX3NlY3Rpb24gKGR3YXJmMl9wZXJfb2JqZmlsZSAqcGVyX29i amZpbGUsCiAgICAgICB0aGlzX2N1LT5sZW5ndGggPSBjdV9oZWFkZXIubGVuZ3RoICsgY3Vf aGVhZGVyLmluaXRpYWxfbGVuZ3RoX3NpemU7CiAgICAgICB0aGlzX2N1LT5pc19kd3ogPSBp c19kd3o7CiAgICAgICB0aGlzX2N1LT5zZWN0aW9uID0gc2VjdGlvbjsKKyAgICAgIC8qIElu aXQgdGhpcyBhc2FwLCB0byBhdm9pZCBhIGRhdGEgcmFjZSBpbiB0aGUgc2V0X3ZlcnNpb24g aW4KKwkgY3V0dV9yZWFkZXI6OmN1dHVfcmVhZGVyICh3aGljaCBtYXkgYmUgcnVuIGluIHBh cmFsbGVsIGZvciB0aGUgY29va2VkCisJIGluZGV4IGNhc2UpLiAgKi8KKyAgICAgIHRoaXNf Y3UtPnNldF92ZXJzaW9uIChjdV9oZWFkZXIudmVyc2lvbik7CiAKICAgICAgIGluZm9fcHRy ID0gaW5mb19wdHIgKyB0aGlzX2N1LT5sZW5ndGg7CiAgICAgICBwZXJfb2JqZmlsZS0+cGVy X2JmZC0+YWxsX2NvbXBfdW5pdHMucHVzaF9iYWNrIChzdGQ6Om1vdmUgKHRoaXNfY3UpKTsK QEAgLTExMjIyLDcgKzExMjI2LDcgQEAgb3Blbl9hbmRfaW5pdF9kd29fZmlsZSAoZHdhcmYy X2N1ICpjdSwgY29uc3QgY2hhciAqZHdvX25hbWUsCiAgIGNyZWF0ZV9jdXNfaGFzaF90YWJs ZSAocGVyX29iamZpbGUsIGN1LCAqZHdvX2ZpbGUsIGR3b19maWxlLT5zZWN0aW9ucy5pbmZv LAogCQkJIGR3b19maWxlLT5jdXMpOwogCi0gIGlmIChjdS0+cGVyX2N1LT5kd2FyZl92ZXJz aW9uIDwgNSkKKyAgaWYgKGN1LT5wZXJfY3UtPnZlcnNpb24gKCkgPCA1KQogICAgIHsKICAg ICAgIGNyZWF0ZV9kZWJ1Z190eXBlc19oYXNoX3RhYmxlIChwZXJfb2JqZmlsZSwgZHdvX2Zp bGUuZ2V0ICgpLAogCQkJCSAgICAgZHdvX2ZpbGUtPnNlY3Rpb25zLnR5cGVzLCBkd29fZmls ZS0+dHVzKTsKZGlmZiAtLWdpdCBhL2dkYi9kd2FyZjIvcmVhZC5oIGIvZ2RiL2R3YXJmMi9y ZWFkLmgKaW5kZXggNTFlMDJkZmM0NTcuLmI3YTAzOTMzYWE1IDEwMDY0NAotLS0gYS9nZGIv ZHdhcmYyL3JlYWQuaAorKysgYi9nZGIvZHdhcmYyL3JlYWQuaApAQCAtMTIyLDkgKzEyMiwx MSBAQCBzdHJ1Y3QgZHdhcmYyX3Blcl9jdV9kYXRhCiAgIHNlY3Rfb2Zmc2V0IHNlY3Rfb2Zm IHt9OwogICB1bnNpZ25lZCBpbnQgbGVuZ3RoID0gMDsKIAorcHJpdmF0ZToKICAgLyogRFdB UkYgc3RhbmRhcmQgdmVyc2lvbiB0aGlzIGRhdGEgaGFzIGJlZW4gcmVhZCBmcm9tIChzdWNo IGFzIDQgb3IgNSkuICAqLwotICB1bnNpZ25lZCBjaGFyIGR3YXJmX3ZlcnNpb24gPSAwOwor ICB1bnNpZ25lZCBjaGFyIG1fZHdhcmZfdmVyc2lvbiA9IDA7CiAKK3B1YmxpYzoKICAgLyog RmxhZyBpbmRpY2F0aW5nIHRoaXMgY29tcGlsYXRpb24gdW5pdCB3aWxsIGJlIHJlYWQgaW4g YmVmb3JlCiAgICAgIGFueSBvZiB0aGUgY3VycmVudCBjb21waWxhdGlvbiB1bml0cyBhcmUg cHJvY2Vzc2VkLiAgKi8KICAgdW5zaWduZWQgaW50IHF1ZXVlZCA6IDE7CkBAIC0yODcsNyAr Mjg5LDE5IEBAIHN0cnVjdCBkd2FyZjJfcGVyX2N1X2RhdGEKICAgLyogUmV0dXJuIERXQVJG IHZlcnNpb24gbnVtYmVyIG9mIHRoaXMgQ1UuICAqLwogICBzaG9ydCB2ZXJzaW9uICgpIGNv bnN0CiAgIHsKLSAgICByZXR1cm4gZHdhcmZfdmVyc2lvbjsKKyAgICAvKiBNYWtlIHN1cmUg aXQncyBzZXQgYWxyZWFkeS4gICovCisgICAgZ2RiX2Fzc2VydCAobV9kd2FyZl92ZXJzaW9u ICE9IDApOworICAgIHJldHVybiBtX2R3YXJmX3ZlcnNpb247CisgIH0KKworICB2b2lkIHNl dF92ZXJzaW9uIChzaG9ydCB2ZXJzaW9uKQorICB7CisgICAgaWYgKG1fZHdhcmZfdmVyc2lv biA9PSAwKQorICAgICAgLyogU2V0IGlmIG5vdCBzZXQgYWxyZWFkeS4gICovCisgICAgICBt X2R3YXJmX3ZlcnNpb24gPSB2ZXJzaW9uOworICAgIGVsc2UKKyAgICAgIC8qIElmIGFscmVh ZHkgc2V0LCB2ZXJpZnkgdGhhdCBpdCdzIHRoZSBzYW1lIHZhbHVlLiAgKi8KKyAgICAgIGdk Yl9hc3NlcnQgKG1fZHdhcmZfdmVyc2lvbiA9PSB2ZXJzaW9uKTsKICAgfQogCiAgIC8qIEZy ZWUgYW55IGNhY2hlZCBmaWxlIG5hbWVzLiAgKi8K --------------c07Z29JdC1lEZ3AWgkGHun01--