From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (wildebeest.demon.nl [212.238.236.112]) by sourceware.org (Postfix) with ESMTPS id A3C9938708F5 for ; Thu, 18 Feb 2021 20:01:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A3C9938708F5 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mark@klomp.org Received: from tarox.wildebeest.org (tarox.wildebeest.org [172.31.17.39]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 134CF300BF39; Thu, 18 Feb 2021 21:01:43 +0100 (CET) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id BB7B641294BC; Thu, 18 Feb 2021 21:01:43 +0100 (CET) Message-ID: <990efcdbd7e57467ce989b30b6aea7ae6edf722b.camel@klomp.org> Subject: Re: [PATCH] Don't handle blocks as exprlocs for DWARF version 4 or higher. From: Mark Wielaard To: Jakub Jelinek Cc: dwz@sourceware.org Date: Thu, 18 Feb 2021 21:01:43 +0100 In-Reply-To: <20210218165940.GJ4020736@tucnak> References: <20210213224622.16521-1-mark@klomp.org> <3fd1ebde0c9e1b8cbe09ea858a3e0f0a84af44b4.camel@klomp.org> <20210218140947.GG4020736@tucnak> <20210218165940.GJ4020736@tucnak> Content-Type: multipart/mixed; boundary="=-5/Y2iikDob6+FOrsk48H" X-Mailer: Evolution 3.28.5 (3.28.5-10.el7) Mime-Version: 1.0 X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: dwz@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Dwz mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Feb 2021 20:01:47 -0000 --=-5/Y2iikDob6+FOrsk48H Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, On Thu, 2021-02-18 at 17:59 +0100, Jakub Jelinek wrote: > On Thu, Feb 18, 2021 at 05:18:10PM +0100, Mark Wielaard wrote: > > > > > if (form =3D=3D DW_FORM_block1) > > >=20 > > > And likewise here: > > > - if (form =3D=3D DW_FORM_block1) > > > + if (form =3D=3D DW_FORM_block1 && cu->cu_version < 4) > >=20 > > But here we do need to handle the DW_FORM_block && cu->cu_version > > >=3D4 > > version separately. But that can be done by not indention the large > > block and adding an small else if block. >=20 > Ah, I got confused by DW_FORM_block{2,4,} cases changing form to > DW_FORM_block1, indeed, for all of DW_FORM_{block{1,2,4,},exprloc} we > need > to do ptr +=3D len; > But perhaps we could do instead do: > - if (form =3D=3D DW_FORM_block1) > + if (form =3D=3D DW_FORM_block1 && cu->cu_version < 4) > ... > - ptr +=3D len; > ... > - ptr +=3D len; > } > + ptr +=3D len; > ? > len is only set to non-0 for: > case DW_FORM_block1: > len =3D *ptr++; > break; > case DW_FORM_block2: > len =3D read_16 (ptr); > form =3D DW_FORM_block1; > break; > case DW_FORM_block4: > len =3D read_32 (ptr); > form =3D DW_FORM_block1; > break; > case DW_FORM_block: > len =3D read_uleb128 (ptr); > form =3D DW_FORM_block1; > break; > case DW_FORM_exprloc: > len =3D read_uleb128 (ptr); > break; > i.e. exactly the cases we want to move. OK. That would make the code even simpler. You are right, we set len to zero at the start and only set it for the block/exprlocs, so we can unconditionally do a ptr +=3D len at the end. > Anyway, looking around some more, > if (unlikely (low_mem_phase1) > && add_locexpr_dummy_dies (dso, cu, die, ptr, form, > t->attr[i].attr, len)) > goto fail; > looks incorrect to me, form in that case will be DW_FORM_block{2,4,} > and won't be canonicalized to DW_FORM_block1. And furthermore > len will be always 0. It is preceded only by > size_t len =3D 0; > and a loop handling DW_FORM_indirect. So, ptr will always be > the pointer to the block count too. I had missed that. Also fixed in the attached patch. Cheers, Mark --=-5/Y2iikDob6+FOrsk48H Content-Disposition: inline; filename*0=0001-Don-t-handle-blocks-as-exprlocs-for-DWARF-version-4-.pat; filename*1=ch Content-Transfer-Encoding: base64 Content-Type: text/x-patch; name="0001-Don-t-handle-blocks-as-exprlocs-for-DWARF-version-4-.patch"; charset="UTF-8" RnJvbSBmNTgwM2Q0MWRiZGRkNTllMmUwMjY3ODA2ODI2ZGQzNTAzZGQyYWJkIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBNYXJrIFdpZWxhYXJkIDxtYXJrQGtsb21wLm9yZz4KRGF0ZTog U2F0LCAxMyBGZWIgMjAyMSAyMzozNDo1NSArMDEwMApTdWJqZWN0OiBbUEFUQ0hdIERvbid0IGhh bmRsZSBibG9ja3MgYXMgZXhwcmxvY3MgZm9yIERXQVJGIHZlcnNpb24gNCBvcgogaGlnaGVyLgoK U2luY2UgRFdBUkYgdmVyc2lvbiA0IGJsb2NrcyBqdXN0IGNvbnRhaW4gYnl0ZXMsIHRyeWluZyB0 byBpbnRlcnByZXQKdGhlbSBhcyBleHBybG9jcyB3aWxsIG1vc3QgbGlrZWx5IGZhaWwuIEFsc28g bWFrZSBzdXJlIGJsb2NrMSBmb3JtCmFuZCBsZW4gYXJlIGNvcnJlY3RseSBwYXNzZWQgdG8gYWRk X2xvY2V4cHJfZHVtbXlfZGllcy4KCiAgICAgKiBkd3ouYyAoYWRkX2xvY2V4cHJfZHVtbXlfZGll cyk6IE9ubHkgaGFuZGxlIGJsb2NrIGFzIGV4cHJsb2MKICAgICBmb3IgY3VfdmVyc2lvbiA8IDQu CiAgICAgKGNoZWNrc3VtX2RpZSk6IExpa2V3aXNlLgogICAgICh3cml0ZV9kaWUpOiBMaWtld2lz ZS4KICAgICAocmVhZF9kZWJ1Z19pbmZvKTogR2V0IGJsb2NrIGxlbmd0aCBiZWZvcmUgY2FsbGlu ZwogICAgIGFkZF9sb2NleHByX2R1bW15X2RpZXMuCgpodHRwczovL3NvdXJjZXdhcmUub3JnL2J1 Z3ppbGxhL3Nob3dfYnVnLmNnaT9pZD0yNjk4NwotLS0KIGR3ei5jIHwgNTAgKysrKysrKysrKysr KysrKysrKysrKysrKysrKysrKysrKy0tLS0tLS0tLS0tLS0tLS0KIDEgZmlsZSBjaGFuZ2VkLCAz NCBpbnNlcnRpb25zKCspLCAxNiBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9kd3ouYyBiL2R3 ei5jCmluZGV4IGQ2YjlkZjAuLjVlNzAwM2EgMTAwNjQ0Ci0tLSBhL2R3ei5jCisrKyBiL2R3ei5j CkBAIC0yOTEwLDcgKzI5MTAsNyBAQCBhZGRfbG9jZXhwcl9kdW1teV9kaWVzIChEU08gKmRzbywg ZHdfY3VfcmVmIGN1LCBkd19kaWVfcmVmIGRpZSwKIAkJCXVuc2lnbmVkIGNoYXIgKnB0ciwgdWlu dDMyX3QgZm9ybSwgdW5zaWduZWQgaW50IGF0dHIsCiAJCQlzaXplX3QgbGVuKQogewotICBpZiAo Zm9ybSA9PSBEV19GT1JNX2Jsb2NrMSkKKyAgaWYgKGZvcm0gPT0gRFdfRk9STV9ibG9jazEgJiYg Y3UtPmN1X3ZlcnNpb24gPCA0KQogICAgIHsKICAgICAgIC8qIE9sZCBEV0FSRiB1c2VzIGJsb2Nr cyBpbnN0ZWFkIG9mIGV4cHJsb2NzLiAgKi8KICAgICAgIHN3aXRjaCAoYXR0cikKQEAgLTM3MzMs NyArMzczMyw3IEBAIGNoZWNrc3VtX2RpZSAoRFNPICpkc28sIGR3X2N1X3JlZiBjdSwgZHdfZGll X3JlZiB0b3BfZGllLCBkd19kaWVfcmVmIGRpZSkKIAkgIGFib3J0ICgpOwogCX0KIAotICAgICAg aWYgKGZvcm0gPT0gRFdfRk9STV9ibG9jazEpCisgICAgICBpZiAoZm9ybSA9PSBEV19GT1JNX2Js b2NrMSAmJiBjdS0+Y3VfdmVyc2lvbiA8IDQpCiAJewogCSAgLyogT2xkIERXQVJGIHVzZXMgYmxv Y2tzIGluc3RlYWQgb2YgZXhwcmxvY3MuICAqLwogCSAgc3dpdGNoICh0LT5hdHRyW2ldLmF0dHIp CkBAIC0zNzgwLDcgKzM3ODAsNiBAQCBjaGVja3N1bV9kaWUgKERTTyAqZHNvLCBkd19jdV9yZWYg Y3UsIGR3X2RpZV9yZWYgdG9wX2RpZSwgZHdfZGllX3JlZiBkaWUpCiAJICAgIGRlZmF1bHQ6CiAJ ICAgICAgYnJlYWs7CiAJICAgIH0KLQkgIHB0ciArPSBsZW47CiAJfQogICAgICAgZWxzZSBpZiAo Zm9ybSA9PSBEV19GT1JNX2V4cHJsb2MpCiAJewpAQCAtMzc5Myw4ICszNzkyLDggQEAgY2hlY2tz dW1fZGllIChEU08gKmRzbywgZHdfY3VfcmVmIGN1LCBkd19kaWVfcmVmIHRvcF9kaWUsIGR3X2Rp ZV9yZWYgZGllKQogCSAgaWYgKHJlYWRfZXhwcmxvYyAoZHNvLCBkaWUsIHB0ciwgbGVuLCBOVUxM KSkKIAkgICAgcmV0dXJuIDE7CiAJICBoYW5kbGVkID0gdHJ1ZTsKLQkgIHB0ciArPSBsZW47CiAJ fQorICAgICAgcHRyICs9IGxlbjsgLyogU2tpcCBleHByL2Jsb2Nrcy4gICovCiAgICAgICBpZiAo IWhhbmRsZWQgJiYgZGllLT5kaWVfY2tfc3RhdGUgIT0gQ0tfQkFEKQogCXsKIAkgIHMgPSB0LT5h dHRyW2ldLmF0dHI7CkBAIC02ODc1LDYgKzY4NzQsMzAgQEAgcmVhZF9kZWJ1Z19pbmZvIChEU08g KmRzbywgaW50IGtpbmQsIHVuc2lnbmVkIGludCAqZGllX2NvdW50KQogCQkgICAgfQogCQl9CiAK KwkgICAgICAvKiBHZXQgbGVuZ3RoIG9mIGV4cHIvYmxvY2tzIGZpcnN0LiAgQ2Fub25pY2FsaXpl IGFsbCwKKwkJIGV4Y2VwdCBleHBybG9jLCB0byBEV19GT1JNX2Jsb2NrMS4gICovCisJICAgICAg c3dpdGNoIChmb3JtKQorCQl7CisJCWNhc2UgRFdfRk9STV9ibG9jazE6CisJCSAgbGVuID0gKnB0 cisrOworCQkgIGJyZWFrOworCQljYXNlIERXX0ZPUk1fYmxvY2syOgorCQkgIGxlbiA9IHJlYWRf MTYgKHB0cik7CisJCSAgZm9ybSA9IERXX0ZPUk1fYmxvY2sxOworCQkgIGJyZWFrOworCQljYXNl IERXX0ZPUk1fYmxvY2s0OgorCQkgIGxlbiA9IHJlYWRfMzIgKHB0cik7CisJCSAgZm9ybSA9IERX X0ZPUk1fYmxvY2sxOworCQkgIGJyZWFrOworCQljYXNlIERXX0ZPUk1fYmxvY2s6CisJCSAgbGVu ID0gcmVhZF91bGViMTI4IChwdHIpOworCQkgIGZvcm0gPSBEV19GT1JNX2Jsb2NrMTsKKwkJICBi cmVhazsKKwkJY2FzZSBEV19GT1JNX2V4cHJsb2M6CisJCSAgbGVuID0gcmVhZF91bGViMTI4IChw dHIpOworCQkgIGJyZWFrOworCQl9CisKIAkgICAgICBpZiAodW5saWtlbHkgKGxvd19tZW1fcGhh c2UxKQogCQkgICYmIGFkZF9sb2NleHByX2R1bW15X2RpZXMgKGRzbywgY3UsIGRpZSwgcHRyLCBm b3JtLAogCQkJCQkgICAgIHQtPmF0dHJbaV0uYXR0ciwgbGVuKSkKQEAgLTY5OTksMjIgKzcwMjIs MTcgQEAgcmVhZF9kZWJ1Z19pbmZvIChEU08gKmRzbywgaW50IGtpbmQsIHVuc2lnbmVkIGludCAq ZGllX2NvdW50KQogCQkgIGJyZWFrOwogCQljYXNlIERXX0ZPUk1faW5kaXJlY3Q6CiAJCSAgYWJv cnQgKCk7CisJCS8qIEFsbCBleHByL2Jsb2NrcyBsZW5ndGhzIGFscmVhZHkgaGFuZGxlZCBhYm92 ZS4KKwkJICAgSnVzdCBjYW5vbmljYWxpemUgZXhwcmxvYyB0byBibG9jazEgdG9vLiAgKi8KKwkJ Y2FzZSBEV19GT1JNX2V4cHJsb2M6CisJCSAgZm9ybSA9IERXX0ZPUk1fYmxvY2sxOworCQkgIGJy ZWFrOwogCQljYXNlIERXX0ZPUk1fYmxvY2sxOgotCQkgIGxlbiA9ICpwdHIrKzsKIAkJICBicmVh azsKIAkJY2FzZSBEV19GT1JNX2Jsb2NrMjoKLQkJICBsZW4gPSByZWFkXzE2IChwdHIpOwotCQkg IGZvcm0gPSBEV19GT1JNX2Jsb2NrMTsKLQkJICBicmVhazsKIAkJY2FzZSBEV19GT1JNX2Jsb2Nr NDoKLQkJICBsZW4gPSByZWFkXzMyIChwdHIpOwotCQkgIGZvcm0gPSBEV19GT1JNX2Jsb2NrMTsK LQkJICBicmVhazsKIAkJY2FzZSBEV19GT1JNX2Jsb2NrOgotCQljYXNlIERXX0ZPUk1fZXhwcmxv YzoKLQkJICBsZW4gPSByZWFkX3VsZWIxMjggKHB0cik7Ci0JCSAgZm9ybSA9IERXX0ZPUk1fYmxv Y2sxOwotCQkgIGJyZWFrOworCQkgIGFib3J0ICgpOwogCQlkZWZhdWx0OgogCQkgIGVycm9yICgw LCAwLCAiJXM6IFVua25vd24gRFdBUkYgJXMiLAogCQkJIGRzby0+ZmlsZW5hbWUsIGdldF9EV19G T1JNX3N0ciAoZm9ybSkpOwpAQCAtMTIzOTIsNyArMTI0MTAsNyBAQCB3cml0ZV9kaWUgKHVuc2ln bmVkIGNoYXIgKnB0ciwgZHdfY3VfcmVmIGN1LCBkd19kaWVfcmVmIGRpZSwKIAkgIHB0ciArPSBp bnB0ciAtIG9yaWdfcHRyOwogCiAJICAvKiBPbGQgRFdBUkYgdXNlcyBibG9ja3MgaW5zdGVhZCBv ZiBleHBybG9jcy4gICovCi0JICBpZiAoZm9ybSA9PSBEV19GT1JNX2Jsb2NrMSkKKwkgIGlmIChm b3JtID09IERXX0ZPUk1fYmxvY2sxICYmIGN1LT5jdV92ZXJzaW9uIDwgNCkKIAkgICAgc3dpdGNo IChyZWZ0LT5hdHRyW2ldLmF0dHIpCiAJICAgICAgewogCSAgICAgIGNhc2UgRFdfQVRfZnJhbWVf YmFzZToKLS0gCjIuMTguNAoK --=-5/Y2iikDob6+FOrsk48H--