From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua1-x931.google.com (mail-ua1-x931.google.com [IPv6:2607:f8b0:4864:20::931]) by sourceware.org (Postfix) with ESMTPS id 1D18F3858C27 for ; Wed, 24 Nov 2021 11:44:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1D18F3858C27 Received: by mail-ua1-x931.google.com with SMTP id a14so4547374uak.0 for ; Wed, 24 Nov 2021 03:44:44 -0800 (PST) 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:content-transfer-encoding; bh=K4a+wGB3WZi8ruS9tqx0pg47AB3a19S4xLZkb9d4a3k=; b=bTuEqYhpH9RvNhWKCljU1xW5Q5r52q21H6nm3PbBYLhZHqXa+J6fuRJuNM1st8hv9J PUL6Z+Vw9fgJTegpZcnOV42KPbyRY0ssmeh77r3w5qOM5RljMeIGJaWZba4is7lEd66I jLuw/4r4UZc0lMOATg1zf+M2VF4FJqPUFXaODbtCqDdcjmOUiRW/+qs9WBEgzQdvuqsR vBHLGCwVg0B0iRMQLoluiV78O0vP1G6ik6Mtp/ofXE0kczgh1rXjmfGt9nZRGcT4soE9 Wx+5w3+ly8FB0v/Yqxf7Umr7JHOlXxtTJVYaOfN/LFU6rd1FormqAqyLSf99kgWEFdKj MXtQ== X-Gm-Message-State: AOAM530OTw0OSsyqmyscE9iwolCUie7B2oM6YSpNgeavNXaDDDnx/Sqb iWrcks86RYGmplXpkSAARKhTnoP1TE5ZfFTSk8+BUKY6+kAvWw== X-Google-Smtp-Source: ABdhPJw41kSzKpLrANxPisikLbYBb8AVRwP3zBHWV4MtLj4Bl8gUSNexKa0l/02E4YhrWmpHgSItOYGNp4LUhspVIrg= X-Received: by 2002:ab0:3c9f:: with SMTP id a31mr9760251uax.134.1637754283042; Wed, 24 Nov 2021 03:44:43 -0800 (PST) MIME-Version: 1.0 References: <20210827185755.953243-1-gprocida@google.com> <20211119150138.2930800-1-gprocida@google.com> <87ilwiu72o.fsf@seketeli.org> In-Reply-To: From: Giuliano Procida Date: Wed, 24 Nov 2021 11:44:06 +0000 Message-ID: Subject: Re: [PATCH v3] Bug 28191 - Interpret DWARF 5 addrx locations To: Dodji Seketeli Cc: libabigail@sourceware.org, kernel-team@android.com, maennich@google.com, mjw@redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-21.8 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, 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 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: Wed, 24 Nov 2021 11:44:45 -0000 Last one I think. On Wed, 24 Nov 2021 at 11:33, Giuliano Procida wrote: > > Hi. > > On Wed, 24 Nov 2021 at 11:11, Giuliano Procida wrot= e: > > > > Hi Dodji. > > > > On Tue, 23 Nov 2021 at 17:15, Dodji Seketeli wrote= : > > > > > > [I am adding Mark to the list recipients as I am going to ask him a > > > question below] > > > > > > > Indeed, I forgot to CC him on the patch. Hi Mark. > > > > > > > > Hello, > > > > > > Giuliano Procida a =C3=A9crit: > > > > > > > This change uses libdw facilities to interpret location expressions > > > > instead of using libabigail's own mini-interpreter. With the fix fo= r > > > > elfutils https://sourceware.org/bugzilla/show_bug.cgi?id=3D28220 in > > > > elfutils-0.186, abidw will correctly interpret Clang DWARF 5 symbol > > > > addresses. Without that fix many declarations will not be linked to > > > > their corresponding symbols due to the incorrect interpretation of > > > > location attribute data. > > > > > > > > * src/abg-dwarf-reader.cc (die_location_address): Use > > > > dwarf_attr_integrate, dwarf_getlocation and > > > > dwarf_getlocation_attr to decode addreses, instead of > > > > die_location_expr and eval_last_constant_dwarf_sub_expr. > > > > * tests/data/test-read-dwarf/PR25042-libgdbm-clang-dwarf5.so.= 6.0.0.abi: > > > > Refresh test reference output; two more symbols have types. > > > > > > Thanks for looking into that. > > > > > > It seems to me that we can also do away with the use of eval_last_con= stant_dwarf_sub_expr from > > > die_member_offset. Would that work on your testing binaries? > > > > > > > I think I originally wrote the code just before a trip away and didn't > > have the time to look into the other callers then. > > > > > > > > From what I am seeing, dwarf_getlocation_attr and dwarf_getlocation a= re > > > present in elfutils 0.176 which is the oldest version that we need to > > > support, so we should be able to ditch eval_last_constant_dwarf_sub_e= xpr > > > and its dependencies altogether. > > > > > > Or what am I missing? > > > > > > > I think it would be good if we could rely on elfutils for this function= ality. > > > > I've had a go at die_member_offset and I think I have it working (at > > least with the test suite). I'll clean up what I have and post it. > > There is considerable overlap with the address function. > > > > I haven't been able to get die_virtual_function_index to work as > > dwarf_getlocation_attr fails. There may be a simpler recipe to follow > > here. > > > > My mistake, the problem is with base-class offsets, not vtable > indexes, so I don't have die_member_offset working after all. It's > possible an extra specialisation of the function for the right DWARF > "form" will fix this. > I concur with Mark: < 1><0x0000009e> DW_TAG_structure_type DW_AT_name s0 DW_AT_byte_size 0x00000030 DW_AT_decl_file 0x00000001 /home/dodji/git/libabigail/dwarf/tests/data/test-read-dwarf/test1.cc DW_AT_decl_line 0x0000000d DW_AT_containing_type <0x0000009e> DW_AT_sibling <0x00000176> < 2><0x000000ad> DW_TAG_inheritance DW_AT_type <0x00000029> DW_AT_data_member_location len 0x0006: 0x1206481c0= 622: DW_OP_dup DW_OP_deref DW_OP_lit24 DW_OP_minus DW_OP_deref DW_OP_plus DW_AT_virtuality DW_VIRTUALITY_virtual < 2><0x000000ba> DW_TAG_inheritance DW_AT_type <0x00000067> DW_AT_data_member_location len 0x0007: 0x120608201c0622: DW_OP_dup DW_OP_deref DW_OP_const1u 32 DW_OP_minus DW_OP_deref DW_OP_plus DW_AT_virtuality DW_VIRTUALITY_virtual Unless there is some exposed elfutils function that will interpret these member locations for us, that code will still be needed in libabigail. > Giuliano. > > > If/when the 3 existing callers are working, there may be some code in > > common to factor out as a new die_location_expr function. > > > > Regards, > > Giuliano. > > > > > > > > [...] > > > > > > Cheers, > > > > > > -- > > > Dodji