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 068B63861899 for ; Tue, 29 Sep 2020 17:59:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 068B63861899 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 0A76E3000A21; Tue, 29 Sep 2020 19:59:28 +0200 (CEST) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id B40BE470869E; Tue, 29 Sep 2020 19:59:28 +0200 (CEST) Message-ID: <0a54a00b4b663a61dda849c0aaeee2976579875f.camel@klomp.org> Subject: Re: [PATCH] Support updating DWARF5 .debug_loclists. From: Mark Wielaard To: Jakub Jelinek Cc: dwz@sourceware.org Date: Tue, 29 Sep 2020 19:59:28 +0200 In-Reply-To: <20200929084223.GX2176@tucnak> References: <20200928080759.8619-1-mark@klomp.org> <20200929084223.GX2176@tucnak> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Evolution 3.28.5 (3.28.5-8.el7) Mime-Version: 1.0 X-Spam-Status: No, score=-5.9 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no 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: Tue, 29 Sep 2020 17:59:31 -0000 Hi Jakub, On Tue, 2020-09-29 at 10:42 +0200, Jakub Jelinek wrote: > On Mon, Sep 28, 2020 at 10:07:59AM +0200, Mark Wielaard wrote: > > ptr +=3D offset; > > while (ptr < endsec) > > { > > - low =3D read_size (ptr, ptr_size); > > - high =3D read_size (ptr + ptr_size, ptr_size); > > - ptr +=3D 2 * ptr_size; > > - if (low =3D=3D 0 && high =3D=3D 0) > > - break; > > + if (cu->cu_version < 5) >=20 > I wonder if it wouldn't be clearer to use sec =3D=3D DEBUG_LOC > in this spot (i.e. leave the decision based on cu_version just to the > start of the function). Yes, it would be clearer. Changed. > > + case DW_LLE_start_length: > > + ptr +=3D ptr_size; > > + skip_leb128 (ptr); > > + len =3D read_uleb128 (ptr); > > + break; >=20 > Wonder if we shouldn't handle case DW_LLE_GNU_view_part here, but perhaps > only for cu->cu_version =3D=3D 5 because it isn't in a separate vendor co= de > space and thus in DWARF 6 it could mean something different. Yes, it would. gcc doesn't emit it by default at the moment. But handling it is easy: case DW_LLE_GNU_view_pair: = =20 if (cu->cu_version !=3D 5) = =20 error (0, 0, = =20 "%s: DW_LLE_GNU_view_pair used with DWARF version %u= \n",=20 dso->filename, cu->cu_version); = =20 skip_leb128 (ptr); = =20 skip_leb128 (ptr); = =20 continue; It will warn, but still try to handle it as is. We probably never hit this because dwz doesn't actually accept DWARFv6 CUs, because they don't exist yet :) > Otherwise LGTM. Thanks. Pushed with those changes. Tested against a gcc -gvariable-location-views=3Dincompat5 build. BTW. I fixed binutils readelf so that it now also correctly shows .debug_loclists: https://sourceware.org/pipermail/binutils/2020-September/113510.html Cheers, Mark