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.133.124]) by sourceware.org (Postfix) with ESMTPS id D714F3858C54 for ; Thu, 14 Jul 2022 18:09:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D714F3858C54 Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-392-BDazUlZHMBCuTb2E8D6Fmg-1; Thu, 14 Jul 2022 14:09:14 -0400 X-MC-Unique: BDazUlZHMBCuTb2E8D6Fmg-1 Received: by mail-qv1-f70.google.com with SMTP id li2-20020a0562145e0200b0047350bbed70so1696928qvb.19 for ; Thu, 14 Jul 2022 11:09:14 -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=Up/fjCANUbV4rswjeYcB9Rl2WptHzYJeeKfCx3qJgPw=; b=j57RX4n7BZlPNVGf9IT/ZVc+U2zEgG528f3m7j97YKl1b03yb3BomYWvXZ9jz+gpHn jv4xw32WABBvlzA+TQaPV6+GeYliaAEpI8ev7/nxDnQNovY1DBNcOmr2DCHR0OBRGZzy AUN7IS3EBiLf3rVt9nXGiDBbei19cXyTTgfBQHU5ABGueS6vZhjGRDoNXB7W6Xj5E1Ud t9/zsvqOFrlnBcsdFSAcOhOHRPhIZaGsXDrB1D7U/r64FjwngO7IHaG2T5FKAFi2NTUP mgrOdL5NpW/hz8Kv6MZMQa8JorZwPBopmNxRFNVXLNrzj3CPzJND29ImT317s6DAcHTM K/9A== X-Gm-Message-State: AJIora9SbPbJI/UHqqivVO4TnObPBuWuADlOJfBBOU+3YpFEYHjYQPwf sHYjIOese2YglA5xQIibSqGSr9ykwObmTlnkbqpwA/XQgXuNThFcxdzCdzGN1CrLd3b8uwZXCKp uiTXeirowmPF1FOuu7sVC X-Received: by 2002:ae9:e204:0:b0:6b5:91b7:16d2 with SMTP id c4-20020ae9e204000000b006b591b716d2mr6810699qkc.507.1657822153351; Thu, 14 Jul 2022 11:09:13 -0700 (PDT) X-Google-Smtp-Source: AGRyM1tPjfDbHgCq9LJGXhoO441lfhro/6RuwZLHms8dFSZ3d3dnzyTYSw7Kb1N49+tsSwr/hltMVA== X-Received: by 2002:ae9:e204:0:b0:6b5:91b7:16d2 with SMTP id c4-20020ae9e204000000b006b591b716d2mr6810665qkc.507.1657822152792; Thu, 14 Jul 2022 11:09:12 -0700 (PDT) Received: from smtpclient.apple ([47.208.199.57]) by smtp.gmail.com with ESMTPSA id d22-20020a05620a159600b006b5517da3casm1770546qkk.22.2022.07.14.11.09.11 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 14 Jul 2022 11:09:12 -0700 (PDT) From: Ben Woodard Message-Id: <4148AF95-8356-482C-A674-518A3AC0F488@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 11:09:08 -0700 In-Reply-To: <5203709.0VBMTVartN@sali> Cc: Ben Woodard via Libabigail To: "Guillermo E. Martinez" References: <21824871.EfDdHjke4D@sali> <3494804.5fSG56mABF@sali> <2D2DA8C8-6793-4FA9-A923-A7D4C82748F4@redhat.com> <5203709.0VBMTVartN@sali> X-Mailer: Apple Mail (2.3696.100.31) X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, HTML_MESSAGE, RCVD_IN_DNSWL_NONE, SCC_5_SHORT_WORD_LINES, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no 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 18:09:20 -0000 > On Jul 14, 2022, at 10:35 AM, Guillermo E. Martinez wrote: >=20 > On Thursday, July 14, 2022 11:01:41 AM CDT Ben Woodard wrote: >>=20 >>> 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 l= ike this until after 2.1 is released. Dodji isn=E2=80=99t calling it a beta= or 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 fi= xed. >>>>=20 >>>> After that, I want to really revisit the API and make it more generall= y useful and less closely tied to the tools. What you are proposing below i= s 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 th= e user >>>>> interface provided by abidw, abidiff and some other tools in libabiga= il, 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 i= s 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 autocon= f 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= in libabigail, >>> i.e, if the only dependency: libctf is meet, then CTF support will be t= here by default. >>=20 >> If the header is there and the library is there, then let=E2=80=99s buil= d it 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 n= ot there at config time. >=20 > Yeah, although configure statements and conditional compiling make me thi= nk > that was a reason to not incorporate CTF support in libabigail by default= like > to ENABLE_RPM in abipkgdiff. But for sure, these motivations could have > changed, just we need to agreed. I think at the time CTF seemed more experimental. Now that you added a lot = more testing, it feels better tested and thus more supportable. However, we= =E2=80=99ll probably hand all the CTF issues to you and Jose. >=20 >> My general philosophy is: Use it more, test it more =E2=80=94 find more = bugs. I personally haven=E2=80=99t been building or testing CTF. It might b= e worth it for you to do what I just did with Fedora 36 which was in essenc= e: >>=20 >> foreach package >> fedabipkgdiff =E2=80=94self-compare -a --from fc36 >=20 > Actually I did the same experiment just with *some* packages in OL8, usin= g > abiddw and abidiff instead, so I will follow your advice :-). Note that this relies on Koji if you guys don=E2=80=99t use that you can fa= ll back to abipkgdiff wit a bit more scripting. >=20 >> If you can do something equivalent with Oracle Linux 9 which I guess is = built with CTF, then I think that would be a really good test. I found that= flux-core https://github.com/flux-framework/flux-core is a really good scheduler for this kind of thing= . I basically spin up flux and then do: >=20 > It seems good!, will do the same in Oracle Linux. >=20 >> $ cat list-of-packages | flux mini bulksubmit --wait --progress --job-na= me=3D{} --output=3D{}.out fedabipkgdiff --self-compare -a --from fc36 {} >>=20 >> 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 i= t until 2.1 releases. The problem with the latter approach is you might hav= e to send an updated patch if it falls out of sync with the master. >=20 > Ok. agreed. >=20 >>>>=20 >>>>=20 >>>> I don=E2=80=99t think that it should matter where we get the informati= on 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 co= rpus 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 pro= perties e.g: filepath, line, >>> column, etc. >>=20 >> 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 hundred= s of bugs doing that. I expect that if we were to take a substantial portio= n of a distribution built with CTF we would find plenty of problems. >=20 > Oh, I see, I presume such of errors will be raised running `fedabipkgdiff= =E2=80=94self-compare ...' > as you show above. Same errors just using fedabipkgdiff is a bigger hammer. As I=E2=80=99m tracking down issues and try to boil them down to see if the= y are duplicates the process that I tend to do is something like: automated testing as I stated above with fedabipkgdiff reproduce the error with fedabipkgdiff manually try fedabipkgdiff =E2=80=94dry-run which prints out the underlying abipkgdi= ff commands run the abipkgdiff commands manually then if I want a closer look then I use abidw =E2=80=94abidiff=20 if that doesn=E2=80=99t tell me what I want then abidw > /tmp/.abixml; abidiff =E2=80=94harmless /tmp/.abixml >=20 >> I=E2=80=99m not enthusiastic about the syntax for the command line that = I suggested maybe we should nix the =E2=80=94with-ctf options too and just = have something like: >>=20 >> $ abidw =E2=80=94abidiff-dwarf # does DWARF to DWARF compariso= n - fails if no DWARF is found >> $ abidw =E2=80=94abidiff-ctf # does CTF to CTF - fails if no C= TF is found >>=20 >> 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 >>=20 >> What do other people think? Is there a better syntax to do this?=20 >>=20 >> I think that both Dodji and I kind of thought that wedging it into abidw= was a short term expedient and optimization and we never thought that we w= ould still be battling problems with this literally years later. It is simp= ler than what I was doing at the beginning which was: >>=20 >> abidw > /tmp/.abixml >> abidiff /tmp/.abixml >>=20 >> but as a coworker pointed out that is a non-intuitive way to approach th= e problerm. I argued that doing A=3DA is counter-intuitive test and yet dis= covering that A!=3DA is mind-spinning until you realize that it represents = a bug. She thought a better command line syntax would be: >>=20 >> abidiff =E2=80=94self-compare >>=20 >>>> 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 = with DWARF >>>>> debug information, but if it is not possible (`STATUS_DEBUG_INFO_NOT_= FOUND') >>>>> looks for CTF info. >>>>=20 >>>> I think that we should do a deeper rework here and work the way that G= DB 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 func= tion read_corpus and then all the programs use that interface rather than t= he functions which are specific to what format the information is gathered = from. >>> Agree, because now days every reader implements the logic to get ELF in= fo: >>> `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 notif= ying the >>>>> user about of missing debug information (e.g `handle_error') that use= an >>>>> specific DWARF `read_context' reference, so if we want to reuse it, t= hen we >>>>> could split common functionality/interface for both readers in a base= class: >>>>>=20 >>>>=20 >>>> Dodji can weigh in here but this whole contex thing as an API is one o= f the things that I wanted get rid of in the general purpose API, IMHO it i= s too tightly tied into how the tools work. I kind of know what he was tryi= ng to do, he didn=E2=80=99t want 50 parameters for the functions and so he = made this class to pass them all at once. >>>> I think we need to think about making a good general purpose API and I= think we can do better than sticking a data structure with parsed command = line 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= it on my develop branch on github and then I will push it to Dodji right a= fter we release 2.1 >>> Ok, of course I will do so following the advice of people that have mor= e experience working in libabigail.=20 >>> Thanks for your comments! >>>> https://github.com/woodard/libabigail > >>>=20 >=20 > Regards, > guillermo