From: Dodji Seketeli <dodji@seketeli.org>
To: "Guillermo E. Martinez" <guillermo.e.martinez@oracle.com>
Cc: libabigail@sourceware.org
Subject: Re: [PATCH] abipkgdiff: Add support to compare packages with CTF debug format
Date: Tue, 17 May 2022 00:09:51 +0200 [thread overview]
Message-ID: <875ym5uo28.fsf@seketeli.org> (raw)
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")
Hello Guillermo,
"Guillermo E. Martinez via Libabigail" <libabigail@sourceware.org> a
écrit:
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 <guillermo.e.martinez@oracle.com>
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 = 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 = abigail::ctf_reader::create_read_context
> + (elf2.path, di_dirs2, env.get());
Likewise.
> +#ifdef WITH_CTF
> + if (opts.use_ctf)
> + {
> + ctxt_ctf = abigail::ctf_reader::create_read_context(elf.path,
> + di_dirs,
> + env.get());
Likewise.
[...]
> 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.
Do you plan on adding some regression tests for this patch? That
would be neat and useful.
>
> Signed-off-by: Guillermo E. Martinez <guillermo.e.martinez@oracle.com>
Applied the patch below to master, thanks!
From 28a629347f598e3b5a35152538c4a4638ca3995a Mon Sep 17 00:00:00 2001
From: "Guillermo E. Martinez" <guillermo.e.martinez@oracle.com>
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 <guillermo.e.martinez@oracle.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
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
==== SELF CHECK SUCCEEDED for 'libGLU.so.1.3.1' ====
$
+ * ``--ctf``
+
+ This is used to compare packages with CTF debug information, if present.
+
.. _abipkgdiff_return_value_label:
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
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<string> kabi_whitelist_packages;
vector<string> suppression_paths;
vector<string> 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 comparing "
"binaries inside the input package against their ABIXML representation\n"
+#ifdef WITH_CTF
+ << " --ctf use CTF instead of DWARF in ELF files\n"
+#endif
<< " --help|-h display this help message\n"
<< " --version|-v display program version information"
" and exit\n";
@@ -1302,15 +1316,31 @@ compare(const elf_file& elf1,
<< " ...\n";
corpus_sptr corpus1;
+#ifdef WITH_CTF
+ abigail::ctf_reader::read_context_sptr ctxt_ctf;
+#endif
+ read_context_sptr ctxt_dwarf;
{
- read_context_sptr c =
- create_read_context(elf1.path, di_dirs1, env.get(),
- /*load_all_types=*/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 = abigail::ctf_reader::create_read_context(elf1.path,
+ env.get());
+ ABG_ASSERT(ctxt_ctf);
+ corpus1 = abigail::ctf_reader::read_corpus(ctxt_ctf.get(),
+ c1_status);
+ }
+ else
+#endif
+ {
+ ctxt_dwarf = create_read_context(elf1.path, di_dirs1, env.get(),
+ /*load_all_types=*/opts.show_all_types);
+ add_read_context_suppressions(*ctxt_dwarf, priv_types_supprs1);
+ if (!opts.kabi_suppressions.empty())
+ add_read_context_suppressions(*ctxt_dwarf, opts.kabi_suppressions);
- corpus1 = read_corpus_from_elf(*c, c1_status);
+ corpus1 = read_corpus_from_elf(*ctxt_dwarf, c1_status);
+ }
bool bail_out = 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,
corpus_sptr corpus2;
{
- read_context_sptr c =
- create_read_context(elf2.path, di_dirs2, env.get(),
- /*load_all_types=*/opts.show_all_types);
- add_read_context_suppressions(*c, priv_types_supprs2);
+#ifdef WITH_CTF
+ if (opts.use_ctf)
+ {
+ ctxt_ctf = abigail::ctf_reader::create_read_context(elf2.path,
+ env.get());
+ corpus2 = abigail::ctf_reader::read_corpus(ctxt_ctf.get(),
+ c2_status);
+ }
+ else
+#endif
+ {
+ ctxt_dwarf = create_read_context(elf2.path, di_dirs2, env.get(),
+ /*load_all_types=*/opts.show_all_types);
+ add_read_context_suppressions(*ctxt_dwarf, priv_types_supprs2);
- 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_suppressions);
- corpus2 = read_corpus_from_elf(*c, c2_status);
+ corpus2 = read_corpus_from_elf(*ctxt_dwarf, c2_status);
+ }
bool bail_out = 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";
corpus_sptr corp;
+#ifdef WITH_CTF
+ abigail::ctf_reader::read_context_sptr ctxt_ctf;
+#endif
+ read_context_sptr ctxt_dwarf;
{
- read_context_sptr c =
- create_read_context(elf.path, di_dirs, env.get(),
- /*read_all_types=*/opts.show_all_types);
+#ifdef WITH_CTF
+ if (opts.use_ctf)
+ {
+ ctxt_ctf = abigail::ctf_reader::create_read_context(elf.path,
+ env.get());
+ ABG_ASSERT(ctxt_ctf);
+ corp = abigail::ctf_reader::read_corpus(ctxt_ctf.get(),
+ c_status);
+ }
+ else
+#endif
+ {
+ ctxt_dwarf =
+ create_read_context(elf.path, di_dirs, env.get(),
+ /*read_all_types=*/opts.show_all_types);
- corp = read_corpus_from_elf(*c, c_status);
+ corp = read_corpus_from_elf(*ctxt_dwarf, c_status);
+ }
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 = true;
+#endif
else if (!strcmp(argv[i], "--help")
|| !strcmp(argv[i], "-h"))
{
--
2.35.0.rc2
Cheers,
--
Dodji
next prev parent reply other threads:[~2022-05-16 22:09 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-07 2:03 Guillermo E. Martinez
2022-05-16 22:09 ` Dodji Seketeli [this message]
2022-05-16 22:21 ` Guillermo E. Martinez
2022-05-17 7:11 ` Dodji Seketeli
2022-05-17 13:13 ` Guillermo E. Martinez
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=875ym5uo28.fsf@seketeli.org \
--to=dodji@seketeli.org \
--cc=guillermo.e.martinez@oracle.com \
--cc=libabigail@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).