From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by sourceware.org (Postfix) with ESMTPS id 1812E3857407 for ; Mon, 16 May 2022 22:09:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1812E3857407 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 5144F60004; Mon, 16 May 2022 22:09:52 +0000 (UTC) Received: by localhost (Postfix, from userid 1000) id 66336581C3D; Tue, 17 May 2022 00:09:51 +0200 (CEST) From: Dodji Seketeli To: "Guillermo E. Martinez" Cc: libabigail@sourceware.org Subject: Re: [PATCH] abipkgdiff: Add support to compare packages with CTF debug format Organization: Me, myself and I References: <20220507020326.1417379-1-guillermo.e.martinez@oracle.com> X-Operating-System: Fedora 36 X-URL: http://www.seketeli.net/~dodji Date: Tue, 17 May 2022 00:09:51 +0200 In-Reply-To: <20220507020326.1417379-1-guillermo.e.martinez@oracle.com> (Guillermo E. Martinez via Libabigail's message of "Fri, 6 May 2022 21:03:26 -0500") Message-ID: <875ym5uo28.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=-9.5 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, 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, 16 May 2022 22:09:56 -0000 Hello Guillermo, "Guillermo E. Martinez via Libabigail" a =C3=A9crit: I have applied the patch to master, thank you very much for it! I have however made some little adjustments. Please read about them below. commit a433820d809bfa134210f5647ee2d1e303d591e7 Author: Guillermo E. Martinez Date: Fri May 6 21:03:26 2022 -0500 [...] > --- a/tools/abipkgdiff.cc > +++ b/tools/abipkgdiff.cc [...] > +#ifdef WITH_CTF > + if (opts.use_ctf) > + { > + ctxt_ctf =3D abigail::ctf_reader::create_read_context(elf1.path, > + di_dirs1, > + env.get()); abigail::ctf_reader::create_read_context only takes two parameters: elf1.path and env.get(). So I removed the di_dirs1 one. [...] > +#ifdef WITH_CTF > + if (opts.use_ctf) > + { > + ctxt_ctf =3D abigail::ctf_reader::create_read_context > + (elf2.path, di_dirs2, env.get()); Likewise. > +#ifdef WITH_CTF > + if (opts.use_ctf) > + { > + ctxt_ctf =3D abigail::ctf_reader::create_read_context(elf.path, > + di_dirs, > + env.get()); Likewise. [...] =20=20=20=20 > This patch add support in `abipkgdiff' to compare binaries with CTF > debug information inside of packages, when `--ctf' option is provided. >=20=20=20=20=20 > * tools/abipkgdiff.cc: Include `abg-ctf-reader.h'. > (options::use_ctf): Add new data member. > (display_usage): Add `--ctf' usage. > (compare): Add condition to use ctf-reader to extract > (parse_command_line): Set `options::use_ctf' when --ctf > option is provided. > and build CTF corpora when `options::use_ctf' is set. > (compare_to_self): Likewise. Do you plan on adding some regression tests for this patch? That would be neat and useful. >=20=20=20=20=20 > Signed-off-by: Guillermo E. Martinez Applied the patch below to master, thanks! >From 28a629347f598e3b5a35152538c4a4638ca3995a Mon Sep 17 00:00:00 2001 From: "Guillermo E. Martinez" Date: Fri, 6 May 2022 21:03:26 -0500 Subject: [PATCH] abipkgdiff: Add support to compare packages with CTF debug= format This patch add support in `abipkgdiff' to compare binaries with CTF debug information inside of packages, when `--ctf' option is provided. * tools/abipkgdiff.cc: Include `abg-ctf-reader.h'. (options::use_ctf): Add new data member. (display_usage): Add `--ctf' usage. (compare): Add condition to use ctf-reader to extract (parse_command_line): Set `options::use_ctf' when --ctf option is provided. and build CTF corpora when `options::use_ctf' is set. (compare_to_self): Likewise. Signed-off-by: Guillermo E. Martinez Signed-off-by: Dodji Seketeli --- doc/manuals/abipkgdiff.rst | 4 ++ tools/abipkgdiff.cc | 114 ++++++++++++++++++++++++++++++------- 2 files changed, 98 insertions(+), 20 deletions(-) diff --git a/doc/manuals/abipkgdiff.rst b/doc/manuals/abipkgdiff.rst index c15dc51f..f297c9a8 100644 --- a/doc/manuals/abipkgdiff.rst +++ b/doc/manuals/abipkgdiff.rst @@ -468,6 +468,10 @@ Options =3D=3D=3D=3D SELF CHECK SUCCEEDED for 'libGLU.so.1.3.1' =3D=3D=3D=3D $ =20 + * ``--ctf`` + + This is used to compare packages with CTF debug information, if prese= nt. + .. _abipkgdiff_return_value_label: =20 Return value diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc index ef9fabf2..eabd19ae 100644 --- a/tools/abipkgdiff.cc +++ b/tools/abipkgdiff.cc @@ -90,6 +90,9 @@ #include "abg-dwarf-reader.h" #include "abg-reader.h" #include "abg-writer.h" +#ifdef WITH_CTF +#include "abg-ctf-reader.h" +#endif =20 using std::cout; using std::cerr; @@ -202,6 +205,10 @@ public: bool fail_if_no_debug_info; bool show_identical_binaries; bool self_check; +#ifdef WITH_CTF + bool use_ctf; +#endif + vector kabi_whitelist_packages; vector suppression_paths; vector kabi_whitelist_paths; @@ -240,6 +247,10 @@ public: fail_if_no_debug_info(), show_identical_binaries(), self_check() +#ifdef WITH_CTF + , + use_ctf() +#endif { // set num_workers to the default number of threads of the // underlying maching. This is the default value for the number @@ -879,6 +890,9 @@ display_usage(const string& prog_name, ostream& out) << " --verbose emit verbose progress messages\n" << " --self-check perform a sanity check by comparin= g " "binaries inside the input package against their ABIXML representation= \n" +#ifdef WITH_CTF + << " --ctf use CTF instead of DWARF in ELF fi= les\n" +#endif << " --help|-h display this help message\n" << " --version|-v display program version informatio= n" " and exit\n"; @@ -1302,15 +1316,31 @@ compare(const elf_file& elf1, << " ...\n"; =20 corpus_sptr corpus1; +#ifdef WITH_CTF + abigail::ctf_reader::read_context_sptr ctxt_ctf; +#endif + read_context_sptr ctxt_dwarf; { - read_context_sptr c =3D - create_read_context(elf1.path, di_dirs1, env.get(), - /*load_all_types=3D*/opts.show_all_types); - add_read_context_suppressions(*c, priv_types_supprs1); - if (!opts.kabi_suppressions.empty()) - add_read_context_suppressions(*c, opts.kabi_suppressions); +#ifdef WITH_CTF + if (opts.use_ctf) + { + ctxt_ctf =3D abigail::ctf_reader::create_read_context(elf1.path, + env.get()); + ABG_ASSERT(ctxt_ctf); + corpus1 =3D abigail::ctf_reader::read_corpus(ctxt_ctf.get(), + c1_status); + } + else +#endif + { + ctxt_dwarf =3D create_read_context(elf1.path, di_dirs1, env.get(), + /*load_all_types=3D*/opts.show_al= l_types); + add_read_context_suppressions(*ctxt_dwarf, priv_types_supprs1); + if (!opts.kabi_suppressions.empty()) + add_read_context_suppressions(*ctxt_dwarf, opts.kabi_suppression= s); =20 - corpus1 =3D read_corpus_from_elf(*c, c1_status); + corpus1 =3D read_corpus_from_elf(*ctxt_dwarf, c1_status); + } =20 bool bail_out =3D false; if (!(c1_status & abigail::elf_reader::STATUS_OK)) @@ -1356,7 +1386,13 @@ compare(const elf_file& elf1, emit_prefix("abipkgdiff", cerr) << "Could not find alternate debug info file"; string alt_di_path; - abigail::dwarf_reader::refers_to_alt_debug_info(*c, alt_di_path); +#ifdef WITH_CTF + if (opts.use_ctf) + ; + else +#endif + abigail::dwarf_reader::refers_to_alt_debug_info(*ctxt_dwarf, + alt_di_path); if (!alt_di_path.empty()) cerr << ": " << alt_di_path << "\n"; else @@ -1389,15 +1425,26 @@ compare(const elf_file& elf1, =20 corpus_sptr corpus2; { - read_context_sptr c =3D - create_read_context(elf2.path, di_dirs2, env.get(), - /*load_all_types=3D*/opts.show_all_types); - add_read_context_suppressions(*c, priv_types_supprs2); +#ifdef WITH_CTF + if (opts.use_ctf) + { + ctxt_ctf =3D abigail::ctf_reader::create_read_context(elf2.path, + env.get()); + corpus2 =3D abigail::ctf_reader::read_corpus(ctxt_ctf.get(), + c2_status); + } + else +#endif + { + ctxt_dwarf =3D create_read_context(elf2.path, di_dirs2, env.get(), + /*load_all_types=3D*/opts.show_al= l_types); + add_read_context_suppressions(*ctxt_dwarf, priv_types_supprs2); =20 - if (!opts.kabi_suppressions.empty()) - add_read_context_suppressions(*c, opts.kabi_suppressions); + if (!opts.kabi_suppressions.empty()) + add_read_context_suppressions(*ctxt_dwarf, opts.kabi_suppression= s); =20 - corpus2 =3D read_corpus_from_elf(*c, c2_status); + corpus2 =3D read_corpus_from_elf(*ctxt_dwarf, c2_status); + } =20 bool bail_out =3D false; if (!(c2_status & abigail::elf_reader::STATUS_OK)) @@ -1443,7 +1490,13 @@ compare(const elf_file& elf1, emit_prefix("abipkgdiff", cerr) << "Could not find alternate debug info file"; string alt_di_path; - abigail::dwarf_reader::refers_to_alt_debug_info(*c, alt_di_path); +#ifdef WITH_CTF + if (opts.use_ctf) + ; + else +#endif + abigail::dwarf_reader::refers_to_alt_debug_info(*ctxt_dwarf, + alt_di_path); if (!alt_di_path.empty()) cerr << ": " << alt_di_path << "\n"; else @@ -1540,12 +1593,29 @@ compare_to_self(const elf_file& elf, << " ...\n"; =20 corpus_sptr corp; +#ifdef WITH_CTF + abigail::ctf_reader::read_context_sptr ctxt_ctf; +#endif + read_context_sptr ctxt_dwarf; { - read_context_sptr c =3D - create_read_context(elf.path, di_dirs, env.get(), - /*read_all_types=3D*/opts.show_all_types); +#ifdef WITH_CTF + if (opts.use_ctf) + { + ctxt_ctf =3D abigail::ctf_reader::create_read_context(elf.path, + env.get()); + ABG_ASSERT(ctxt_ctf); + corp =3D abigail::ctf_reader::read_corpus(ctxt_ctf.get(), + c_status); + } + else +#endif + { + ctxt_dwarf =3D + create_read_context(elf.path, di_dirs, env.get(), + /*read_all_types=3D*/opts.show_all_types); =20 - corp =3D read_corpus_from_elf(*c, c_status); + corp =3D read_corpus_from_elf(*ctxt_dwarf, c_status); + } =20 if (!(c_status & abigail::elf_reader::STATUS_OK)) { @@ -3332,6 +3402,10 @@ parse_command_line(int argc, char* argv[], options& = opts) (make_path_absolute(argv[j]).get()); ++i; } +#ifdef WITH_CTF + else if (!strcmp(argv[i], "--ctf")) + opts.use_ctf =3D true; +#endif else if (!strcmp(argv[i], "--help") || !strcmp(argv[i], "-h")) { --=20 2.35.0.rc2 Cheers, --=20 Dodji