From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net [217.70.183.200]) by sourceware.org (Postfix) with ESMTPS id 1C2CC3858C2C for ; Mon, 20 Jun 2022 16:12:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1C2CC3858C2C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=seketeli.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=seketeli.org Received: (Authenticated sender: dodji@seketeli.org) by mail.gandi.net (Postfix) with ESMTPSA id F0F6720007; Mon, 20 Jun 2022 16:12:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=seketeli.org; s=gm1; t=1655741552; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=RPILPHT4vQ3foDge8999kQ1VSBv6JAxAdKJgavthJME=; b=ePyBf+KyJByfcVINYUWU0j+Lg/pw2StHVVKPgO0k9d7Xi8cDmOOHPRhL6QEge5JHTmkaWF bCGkjpWmnuVv1AmfUGg75S1PanVvHgcuPZL+OpUCDmN3ldnN8y+RKopZJjg9sltbJGlYdx BPzNTC6NxWsB+4eVsupimFH5N34gzFOzucGeclWAX9OgD7QPgVtWeOEzaXsiouvMG2730Z 9SHRMwIgPxlEhRGxzkfD13cjMBL3RRM71Yn2nzxX4vxpE6Blxe2Sg7KMDG85JTmYXVboFV YZRY/QOBAu8KRqbTCpxB787a8VG+TDPDwSKDiYqEDXR6p6s6/OcvNlWHINvjFA== Received: by localhost (Postfix, from userid 1000) id 4A70E5800FC; Mon, 20 Jun 2022 18:12:31 +0200 (CEST) From: Dodji Seketeli To: Thomas Schwinge Cc: , Mark Wielaard Subject: Re: 'src/abg-dwarf-reader.cc:compare_dies_string_attribute_value' optimization Organization: Me, myself and I References: <87wnijv616.fsf@dirichlet.schwinge.homeip.net> X-Operating-System: Fedora 37 X-URL: http://www.seketeli.net/~dodji Date: Mon, 20 Jun 2022 18:12:31 +0200 In-Reply-To: <87wnijv616.fsf@dirichlet.schwinge.homeip.net> (Thomas Schwinge's message of "Fri, 28 Jan 2022 23:39:49 +0100") Message-ID: <87edzjgvps.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, JMQ_SPF_NEUTRAL, RCVD_IN_DNSWL_LOW, 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: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 20 Jun 2022 16:12:39 -0000 Hello Thomas et al., I have finally applied a fix for this issue that you reported. It can be read at https://sourceware.org/git/?p=3Dlibabigail.git;a=3Dcommit= ;h=3D0529e3b9ee09c29137952b3a5d4a7cc10484649a. Thanks for reporting this and sorry for the inconvenience. Cheers, Thomas Schwinge a =C3=A9crit: > Hi! > > It's late on a Friday evening, and very well this may just be me not > understanding libabigail/DWARF ;-) -- in context of > or > , I did a > simple experiment to disable the > 'src/abg-dwarf-reader.cc:compare_dies_string_attribute_value' > optimization: > > --- src/abg-dwarf-reader.cc > +++ src/abg-dwarf-reader.cc > [...] > /// This function is a fast routine (optimization) to compare the > /// values of two string attributes of two DIEs. > /// > /// @param l the first DIE to consider. > /// > /// @param r the second DIE to consider. > /// > /// @param attr_name the name of the attribute to compare, on the two > /// DIEs above. > /// > /// @param result out parameter. This is set to the result of the > /// comparison. If the value of attribute @p attr_name on DIE @p l > /// equals the value of attribute @p attr_name on DIE @p r, then the > /// the argument of this parameter is set to true. Otherwise, it's > /// set to false. Note that the argument of this parameter is set i= ff > /// the function returned true. > /// > /// @return true iff the comparison could be performed. There are > /// cases in which the comparison cannot be performed. For instance, > /// if one of the DIEs does not have the attribute @p attr_name. In > /// any case, if this function returns true, then the parameter @p > /// result is set to the result of the comparison. > static bool > compare_dies_string_attribute_value(const Dwarf_Die *l, const Dwarf_= Die *r, > unsigned attr_name, > bool &result) > { > Dwarf_Attribute l_attr, r_attr; > if (!dwarf_attr_integrate(const_cast(l), attr_name, &l= _attr) > || !dwarf_attr_integrate(const_cast(r), attr_name,= &r_attr)) > return false; > > ABG_ASSERT(l_attr.form =3D=3D DW_FORM_strp > || l_attr.form =3D=3D DW_FORM_string > || l_attr.form =3D=3D DW_FORM_GNU_strp_alt > || form_is_DW_FORM_strx(l_attr.form) > || form_is_DW_FORM_line_strp(l_attr.form)); > > ABG_ASSERT(r_attr.form =3D=3D DW_FORM_strp > || r_attr.form =3D=3D DW_FORM_string > || r_attr.form =3D=3D DW_FORM_GNU_strp_alt > || form_is_DW_FORM_strx(r_attr.form) > || form_is_DW_FORM_line_strp(r_attr.form)); > > +#if 0 > if ((l_attr.form =3D=3D DW_FORM_strp > && r_attr.form =3D=3D DW_FORM_strp) > || (l_attr.form =3D=3D DW_FORM_GNU_strp_alt > && r_attr.form =3D=3D DW_FORM_GNU_strp_alt) > || (form_is_DW_FORM_strx(l_attr.form) > && form_is_DW_FORM_strx(r_attr.form)) > || (form_is_DW_FORM_line_strp(l_attr.form) > && form_is_DW_FORM_line_strp(r_attr.form))) > { > // So these string attributes are actually pointers into a > // string table. The string table is most likely de-duplicated > // so comparing the *values* of the pointers should be enough. > // > // This is the fast path. > if (l_attr.valp =3D=3D r_attr.valp) > result =3D true; > else if (l_attr.valp && r_attr.valp) > result =3D *l_attr.valp =3D=3D *r_attr.valp; > else > result =3D false; > return true; > } > +#endif > > // If we reached this point it means we couldn't use the fast path > // because the string atttributes are strings that are "inline" in > // the debug info section. Let's just compare them the slow and > // obvious way. > string l_str =3D die_string_attribute(l, attr_name), > r_str =3D die_string_attribute(r, attr_name); > result =3D l_str =3D=3D r_str; > > return true; > } > > My assumption was that this shouldn't change anything other than possibly > regress performance. However: > > [-PASS:-]{+FAIL:+} runtestreaddwarf > > --- build-libabigail/tests/runtestreaddwarf.log > +++ build-libabigail/tests/runtestreaddwarf.log > [...] > -PASS runtestreaddwarf (exit status: 0) > +--- [...]/source-libabigail/tests/data/test-read-dwarf/PR25007-sdhci= .ko.abi 2022-01-28 22:48:07.049805043 +0100 > ++++ [...]/build-libabigail/tests/output/test-read-dwarf/PR25007-sdhc= i.ko.abi 2022-01-28 23:15:30.261483063 +0100 > +@@ -10107,10 +10107,19 @@ > + > + > + > ++ > ++ > ++ > ++ > ++ > + > + > + > + > ++ > ++ > ++ > ++ > + > + > + > +@@ -10138,6 +10147,10 @@ > + > + > + > ++ > ++ > ++ > ++ > + > + > + > +@@ -10181,6 +10194,16 @@ > + > + > + > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > + > + > + > +@@ -10370,6 +10393,13 @@ > + > + > + > ++ > ++ > ++ > ++ > ++ > ++ > ++ > + > + > + > +@@ -10519,6 +10549,31 @@ > + > + > + > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > + > + > + > +@@ -10531,6 +10586,17 @@ > + > + > + > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > + > + > + > +@@ -10591,6 +10657,10 @@ > + > + > + > ++ > ++ > ++ > ++ > + > + > + > +@@ -10631,6 +10701,22 @@ > + > + > + > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > + > + > + > +@@ -10746,6 +10832,10 @@ > + > + > + > ++ > ++ > ++ > ++ > + > + > + > +@@ -10770,6 +10860,14 @@ > + > + > + > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > + > + > + > +@@ -10815,6 +10913,9 @@ > + > + > + > ++ > ++ > ++ > + > + > + > +@@ -10859,6 +10960,15 @@ > + > + > + > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > + > + > + > +@@ -10885,11 +10995,44 @@ > + > + > + > ++ > ++ > ++ > ++ > ++ > ++ > ++ > + > + > + > + > + > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > + > + > + > +@@ -10923,6 +11066,18 @@ > + > + > + > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > + > + > + > +@@ -10936,6 +11091,14 @@ > + > + > + > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > + > + > + > +@@ -10949,6 +11112,10 @@ > + > + > + > ++ > ++ > ++ > ++ > + > + > + > +@@ -11075,12 +11242,26 @@ > + > + > + > ++ > ++ > ++ > ++ > + > + > + > + > + > + > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > + > + > + > +@@ -11124,6 +11305,15 @@ > + > + > + > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > + > + > + > +@@ -11192,10 +11382,19 @@ > + > + > + > ++ > ++ > ++ > ++ > ++ > + > + > + > + > ++ > ++ > ++ > ++ > + > + > + > +@@ -11210,6 +11409,10 @@ > + > + > + > ++ > ++ > ++ > ++ > + > + > + > +@@ -11224,6 +11427,9 @@ > + > + > + > ++ > ++ > ++ > + > + > + > +@@ -11241,223 +11447,6 @@ > + > + > + > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > + > + > + > +@@ -11477,48 +11466,19 @@ > + > + > + > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > ++ > + > + > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > +- > + > + > + > +ABIs differ: > +[...]/source-libabigail/tests/data/test-read-dwarf/PR25007-sdhci.ko.= abi > +and: > +[...]/build-libabigail/tests/output/test-read-dwarf/PR25007-sdhci.ko= .abi > + > +FAIL runtestreaddwarf (exit status: 1) > > Only that one test case. > > The reordered 'function-type's -- but I've not yet verified whether > they're really just reordered -- may hint towards a sorting stability > issue, but there are also the disappearing 'class-decl's. > > Unless somebody points out any misunderstanding on my side, I suppose I > shall try to figure out what's going wrong. > > > Gr=C3=BC=C3=9Fe > Thomas > ----------------- > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra=C3=9Fe 2= 01, 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch= =C3=A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellsc= haft: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955 --=20 Dodji