From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id BE9B538582BA for ; Thu, 14 Jul 2022 16:01:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BE9B538582BA Received: from mail-oi1-f199.google.com (mail-oi1-f199.google.com [209.85.167.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-81-D69A41qbPve6mM3fezIEWA-1; Thu, 14 Jul 2022 12:01:49 -0400 X-MC-Unique: D69A41qbPve6mM3fezIEWA-1 Received: by mail-oi1-f199.google.com with SMTP id bc7-20020a056808170700b003351d0ad859so1225410oib.12 for ; Thu, 14 Jul 2022 09:01:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:message-id:mime-version:subject:date :in-reply-to:cc:to:references; bh=mDBqZQx9WrN52Z/qro51dcF7vugOa4GJF5sLxY8kNy8=; b=e5ZTllH0bKMPZLFHQUQZES31dTfG258XBNSOE4UEpeOr0ngCrMa5WAdj1nrx6kKpNv j1DO80io5unysEhyW2fFaQAArZU8bYVRu00b2pqiloG+mijC5frSp+opWGv4HOcyBZzs 4jlOK9eLwj0kzG1XMukjcte+Yv+rrSnxhlxqESaT6O7e/MloD48nW21H30ISkhSgVocx 7bhGEFfsoLbIhhmMyxubeTliZXjg5nd/c3BDAjI7rWeYd5jy7JVvyN73hvbB8ry622qe DDVIhseRr0JYqNoT8KbFJkRtpl9O4lDcR43Q5FU25cD/iJVFpoJSopJgVqMKwJi2x7LG ZRzw== X-Gm-Message-State: AJIora+7683OhKiNC4g6X/2rEIrAV9GP5jqE6KyC4T2x0B8oNNgat4yS YSCWzIR2+IpZelltxUz1wYcRLZk0se0FW3mpOSNeAfZSLvMAZT6ByAYbdhAgv4O149qSmlaTctj rTQsOtTJDXqdAI5yLDYSY X-Received: by 2002:a9d:6296:0:b0:61c:7f89:7364 with SMTP id x22-20020a9d6296000000b0061c7f897364mr480346otk.191.1657814507263; Thu, 14 Jul 2022 09:01:47 -0700 (PDT) X-Google-Smtp-Source: AGRyM1vR3dItgszsCb5CYHohdv6VJ6/szAjbClJPru8GGkrjwAsZm2NdDlCnXoXQtDvO3mio+kyQgQ== X-Received: by 2002:a9d:6296:0:b0:61c:7f89:7364 with SMTP id x22-20020a9d6296000000b0061c7f897364mr480263otk.191.1657814505438; Thu, 14 Jul 2022 09:01:45 -0700 (PDT) Received: from smtpclient.apple ([47.208.199.57]) by smtp.gmail.com with ESMTPSA id be35-20020a05687058a300b0010be134ac60sm998117oab.19.2022.07.14.09.01.44 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 14 Jul 2022 09:01:44 -0700 (PDT) From: Ben Woodard Message-Id: <2D2DA8C8-6793-4FA9-A923-A7D4C82748F4@redhat.com> Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.100.31\)) Subject: Re: Use CTF as a fallback when no DWARFs debug info is present Date: Thu, 14 Jul 2022 09:01:41 -0700 In-Reply-To: <3494804.5fSG56mABF@sali> Cc: Ben Woodard via Libabigail To: "Guillermo E. Martinez" References: <21824871.EfDdHjke4D@sali> <952C9B5A-C267-4BC2-9DF1-4045C73C704C@redhat.com> <3494804.5fSG56mABF@sali> X-Mailer: Apple Mail (2.3696.100.31) X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-6.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, HTML_MESSAGE, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, 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 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: Thu, 14 Jul 2022 16:01:54 -0000 > On Jul 14, 2022, at 8:06 AM, Guillermo E. Martinez wrote: >=20 > On Wednesday, July 13, 2022 8:37:13 PM CDT Ben Woodard wrote: > Hi Ben, >=20 >> So let me begin by saying, I think we should push changes to the API lik= e this until after 2.1 is released. Dodji isn=E2=80=99t calling it a beta o= r RC yet but we have been treating it that way. I=E2=80=99ve been trying to= get all my testing done and he has been trying to get those same bugs fixe= d. >>=20 >> After that, I want to really revisit the API and make it more generally = useful and less closely tied to the tools. What you are proposing below is = a baby step along the lines of what I I had in mind. >>=20 >>> On Jul 13, 2022, at 4:25 PM, Guillermo E. Martinez via Libabigail wrote: >>>=20 >>> Hello libabigail team, >>>=20 >>> Talking with my colleges Jose E. Marchesi and Nick Alcock about of the = user >>> interface provided by abidw, abidiff and some other tools in libabigail= , we >>> think such tools should be looks for DWARF debug information, and if it= is not >>> present fall back for CTF even without --ctf option, this behaviour is = used by >>> GDB (looking for .ctf and .debug_* section, >>=20 >> First of all, I think rather than having a: >>=20 >> =E2=80=94enable-ctf compile time option >>=20 >> Why don=E2=80=99t we simply make =E2=80=9CWITH_CTF=E2=80=9D an autoconf = option that is disabled if AC_CHECK_HEADER and AC_CHECK_LIB doesn=E2=80=99t= find the things that you need. > Ok, in that case we need to decide whether CTF support always will ship i= n libabigail, > i.e, if the only dependency: libctf is meet, then CTF support will be the= re by default. If the header is there and the library is there, then let=E2=80=99s build i= t in without the config time option. I think we should do this but after 2.= 1 releases though. I can=E2=80=99t see why it shouldn=E2=80=99t be there as= long as autotools disables it if the required headers and library are not = there at config time. My general philosophy is: Use it more, test it more =E2=80=94 find more bug= s. I personally haven=E2=80=99t been building or testing CTF. It might be w= orth it for you to do what I just did with Fedora 36 which was in essence: foreach package fedabipkgdiff =E2=80=94self-compare -a --from fc36 If you can do something equivalent with Oracle Linux 9 which I guess is bui= lt with CTF, then I think that would be a really good test. I found that fl= ux-core https://github.com/flux-framework/flux-core is a really good schedu= ler for this kind of thing. I basically spin up flux and then do: $ cat list-of-packages | flux mini bulksubmit --wait --progress --job-name= =3D{} --output=3D{}.out fedabipkgdiff --self-compare -a --from fc36 {} You can either send me the PR and I will merge it into my develop branch on= github (right now practically empty) to be passed on to the list after 2.1= releases or you can send it to the list and we=E2=80=99ll just sit on it u= ntil 2.1 releases. The problem with the latter approach is you might have t= o send an updated patch if it falls out of sync with the master. >>=20 >>=20 >> I don=E2=80=99t think that it should matter where we get the information= about the types from. > Yes. >> I would also be tempted to get rid the =E2=80=94use-ctf options except I= think that we should have the ability to do something like >> abidw =E2=80=94abidiff =E2=80=94use-ctf somelib.so=20 >> And it would read the DWARF, then read the CTF and then compare the corp= us that we get from them. > Ok, IMHO, it just be needed in the testing harness, because at the end of= the day both corpus > with the same input, will be generated no differences, just in some prope= rties e.g: filepath, line, > column, etc. Heh in theory that should be the case, just like in theory=20 $ abidw =E2=80=94abidiff =20 should always return no changes. However, I have found literally hundreds o= f bugs doing that. I expect that if we were to take a substantial portion o= f a distribution built with CTF we would find plenty of problems. I=E2=80=99m not enthusiastic about the syntax for the command line that I s= uggested maybe we should nix the =E2=80=94with-ctf options too and just hav= e something like: $ abidw =E2=80=94abidiff-dwarf # does DWARF to DWARF comparison -= fails if no DWARF is found $ abidw =E2=80=94abidiff-ctf # does CTF to CTF - fails if no CTF = is found Then the current syntax: $ abidw =E2=80=94abidiff =20 # looks for DWARF and CTF if both are present then it compares DWARF to CTF # if only DWARF is present then it compares DWARF to DWARF # if only CTF is present then it compares CTF to CTF What do other people think? Is there a better syntax to do this?=20 I think that both Dodji and I kind of thought that wedging it into abidw wa= s a short term expedient and optimization and we never thought that we woul= d still be battling problems with this literally years later. It is simpler= than what I was doing at the beginning which was: abidw > /tmp/.abixml abidiff /tmp/.abixml but as a coworker pointed out that is a non-intuitive way to approach the p= roblerm. I argued that doing A=3DA is counter-intuitive test and yet discov= ering that A!=3DA is mind-spinning until you realize that it represents a b= ug. She thought a better command line syntax would be: abidiff =E2=80=94self-compare >> Or maybe it would be=20 >>> but in libabigail is done by the >>> symtab class), so I did some minor changes in abidw and abidiff=20 >>=20 >> don=E2=80=99t forget abicompat > Ok. >>> setting the >>> corpus origin depending of `opts.use_ctf', trying to build the copus wi= th DWARF >>> debug information, but if it is not possible (`STATUS_DEBUG_INFO_NOT_FO= UND') >>> looks for CTF info. >>=20 >> I think that we should do a deeper rework here and work the way that GDB= works. In essence doing: >>=20 >> In essence: >> read ELF >> if( elf-file has DWARF) then use that >> else if (look for .gnu_reflinks DWARF) then use that >> else if( look for DWARF5 split DWARF) then use that >> else if( look for CTF) then use that >> else if (fail-on-no-debug-info) then exit=20 >> else warn on limited functionality. >>=20 >> I think that all this logic should be encapsulated in an exported functi= on read_corpus and then all the programs use that interface rather than the= functions which are specific to what format the information is gathered fr= om. > Agree, because now days every reader implements the logic to get ELF info= : > `read_debug_info_into_corpus' and `slurp_elf_info'. >>> corpus::origin origin =3D opts.use_ctf ? CTF_ORIGIN : DWARF_ORIGIN; >>> ... >>> if (origin =3D=3D DWARF_ORIGIN) >>> { >>> // Build corpus with DWARF >>> } >>> if ((status & STATUS_DEBUG_INFO_NOT_FOUND) || >>> origin =3D=3D CTF_ORIGIN) >>> { >>> // Build corpus with CTF >>> } >>>=20 >>> if (status & STATUS_DEBUG_INFO_NOT_FOUND) >>> { >>> // lets to know the user that no debug info >>> // was found in DWARF neither CTF >>> } >>>=20 >>> Doing in this way, seem to work, however there are functions to notifyi= ng the >>> user about of missing debug information (e.g `handle_error') that use a= n >>> specific DWARF `read_context' reference, so if we want to reuse it, the= n we >>> could split common functionality/interface for both readers in a base c= lass: >>>=20 >>=20 >> Dodji can weigh in here but this whole contex thing as an API is one of = the things that I wanted get rid of in the general purpose API, IMHO it is = too tightly tied into how the tools work. I kind of know what he was trying= to do, he didn=E2=80=99t want 50 parameters for the functions and so he ma= de this class to pass them all at once. >> I think we need to think about making a good general purpose API and I t= hink we can do better than sticking a data structure with parsed command li= ne options in it. >>> class base_read_context >>> { >>> ... >>> virtual const string& >>> alt_debug_info_path() const; >>>=20 >>> void >>> exported_decls_builder(corpus::exported_decls_builder* b); >>> ... >>> }; >>>=20 >>> class dwarf_reader::read_context : public base_read_context >>> { >>> ... >>> }; >>>=20 >>> class ctf_reader::read_context : public base_read_context >>> { >>> ... >>> }; >>>=20 >>>=20 >>> // Now reusing refers_to_alt_debug_info for both readers >>> bool >>> refers_to_alt_debug_info(const read_context&=09ctxt, >>> string& alt_di_path) >>> { >>> ... >>> } >>>=20 >>> Please let met know your feedback, I'll really appreciate!=20 >>=20 >> If you want to collaborate on this, send me a PR and we can iterate on i= t on my develop branch on github and then I will push it to Dodji right aft= er we release 2.1 > Ok, of course I will do so following the advice of people that have more = experience working in libabigail.=20 > Thanks for your comments! >> https://github.com/woodard/libabigail >=20 > Regards, > guillermo