From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd2c.google.com (mail-io1-xd2c.google.com [IPv6:2607:f8b0:4864:20::d2c]) by sourceware.org (Postfix) with ESMTPS id 4F8AB3858D28 for ; Tue, 12 Apr 2022 15:23:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4F8AB3858D28 Received: by mail-io1-xd2c.google.com with SMTP id e22so22619763ioe.11 for ; Tue, 12 Apr 2022 08:23:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=91vRPSlf9AaRSypZEQEmKohovlYNnVr4wGkY8EUdayg=; b=ARJ609y5JRjHN3OmtF0dNc7laz+MxUqLdKFYL26XHOe6BNggvrQ0RijrCPpMpYSjo4 /J1fpGnEoxyAiR11E8m4lwmqINdfYCKULomKzYtvw7dH66jVGZxAxOnISp+jmdUuphPj fuprJqiFay8pxiCvVPRDM0igQulr6HTADmpuklwlmi5j9hqDj4bk+qlo6ZfRmuwwCmk3 /GceFiyaT1UVTRpFARkbon+CaYgUVia/ZqPb1v8X+to8ou4F58RlqEZWxiDd114OyAn+ 87zR01TCCQKAVsA+nnlwSYFtfYjwTLvswUCUjDKmArjv+xD4EHruCvGQNEgCp0PGRjpv yu3w== X-Gm-Message-State: AOAM531DSqGIUqBVwRqI0Gpd3gnLhpenrOV4TloshzmBSZwtciAe9jd7 Lzg5nMbqZdTCeJVocMvjcE7qkWqLBzAjYN77RmIYbQDIh0k= X-Google-Smtp-Source: ABdhPJzyWw0f5eAf7yuO9XStdEk1gC6KfYNegTJKHsQZ42ORsdo5FpQyQgWyLDvQZqmdIwMN7utpDvbkHXEr8Ao4ibs= X-Received: by 2002:a05:6602:2f0a:b0:64f:99ed:d732 with SMTP id q10-20020a0566022f0a00b0064f99edd732mr4600220iow.150.1649776984072; Tue, 12 Apr 2022 08:23:04 -0700 (PDT) MIME-Version: 1.0 References: <87wnijv616.fsf@dirichlet.schwinge.homeip.net> <87r18oynpn.fsf@euler.schwinge.homeip.net> <877d7vhcmv.fsf@seketeli.org> In-Reply-To: <877d7vhcmv.fsf@seketeli.org> From: Giuliano Procida Date: Tue, 12 Apr 2022 16:20:51 +0100 Message-ID: Subject: Re: 'src/abg-dwarf-reader.cc:compare_dies_string_attribute_value' optimization To: Dodji Seketeli Cc: Thomas Schwinge , Giuliano Procida via Libabigail , Mark Wielaard X-Spam-Status: No, score=-21.1 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, HTML_MESSAGE, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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: Tue, 12 Apr 2022 15:23:07 -0000 I started looking at this today but I've run out of time. I'll be able to take another look in about a week. Checks fail (various types are renumbered in XML output) starting at commit 57b1c714. I haven't tried this on kernel or framework libraries yet, but I know there are plenty of typedefs in both. Regards, Giuliano On Mon, 11 Apr 2022 at 16:18, Dodji Seketeli wrote: > Hello Thomas and Giuliano, > > [...] > > > Thomas Schwinge a =C3=A9crit: > > [...] > > >> OK. Then: > >> > >> (gdb) print l_attr.valp && r_attr.valp > >> $8 =3D true > >> (gdb) print *l_attr.valp =3D=3D *r_attr.valp > >> $9 =3D true > >> > >> This now does 'return true;'! > >> > >> Given 'unsigned char *valp', what this only checks is that one byte at > >> 'l_attr.valp' equals one byte at 'r_attr.valp' -- which evidently isn'= t > >> sufficient to demonstrate that 'l_attr' and 'r_attr' do match. > Therefore > >> it seems that this optimization is not correct? > >> > > Giuliano Procida a =C3=A9crit: > > > Looks like you've found one bug. It has compared addresses and moved on > to > > test data. > > > > It could do strcmp at this point, assuming these things are > null-terminated > > strings, which is the case for DW_FORM_strp but is not for other things= . > > > > But there is another bug. I've instrumented the code and found cases (i= n > > the test suite) where the fast path returns false, but the slow path > > returns true. Both sides are DW_FORM_strx1. The data are not strings bu= t > > indexes and that the same string appears at different indexes. > > I agree. This is a bug. > > I tried to fix and indeed, hell did break lose as I realized there were > several other issues across the pipeline (including several issues that > I have long wanted to address) that needed so that this one fits in > correctly. > > I came up with a patch set that lies in the branch 'fix-dwarf-str-cmp', > at > > https://sourceware.org/git/?p=3Dlibabigail.git;a=3Dshortlog;h=3Drefs/head= s/fix-dwarf-str-cmp > . > > The tip of that patch set addresses this issue specifically: > > https://sourceware.org/git/?p=3Dlibabigail.git;a=3Dcommit;h=3D3d277a9cc05= 873cf4aeb97273d585e0b07af917d > . > > But for that fix to be applied, we need the whole patch set that fixes > several other issues left and right. > > Giuliano & Thomas, could you please test that branch on your specific > environments? If it works OK, then I can consider posting the patch-set > properly to the list. If it doesn't, at least I'd wouldn't have broken > the world without notice ;-) > > Thanks. > > -- > Dodji >