From: Claudiu Zissulescu <claudiu.zissulescu-ianculescu@oracle.com>
Hi Dodji,
Please find my updated patch with your feedback in. As for the test,
I'll try to come with one against big-tests. AFAIK, the ctf-parent
option is used only with non-standard linkers like in the case of
Oracle Linux CTFA archive.
Thank you,
Claudiu
If the CTF section contains ambiguously-defined types, it will consist
of an archive of many CTF dictionaries, all inheriting from one
dictionary containing unambiguous types. This member is by default
named .ctf, like the section containing it, but it is possible to
change this name using the "ctf_link_set_memb_name_changer" function
at link time. When looking at CTF archives that have been created by
a linker that uses the name changer to rename the parent archive
member, --ctf-parent can be used to specify the name used for the
parent. This new option can be used with abidw, abidiff and
abipkgdiff tools.
* include/abg-ctf-reader.h (parent_name): New function.
* src/abg-ctf-reader.cc (ctf_parent_name): New variable.
(ctf_parent_name): Sets ctf parent name.
(parent_name): Gets the elf base reader and sets the parent name.
* tools/abidiff.cc (ctf_parent): New option.
(parse_command_line): Parse the new option.
(main): Handle the new option.
* tools/abidw.cc (ctf_parent): New option.
(parse_command_line): Parse the new option.
(load_corpus_and_write_abixml): Handle the new option.
* tools/abipkgdiff.cc (ctf_parent): New option.
(compare): Handle the new option.
(compare_to_self): Likewise.
(parse_command_line): Parse the new option.
Signed-off-by: Claudiu Zissulescu <claudiu.zissulescu-ianculescu@oracle.com>
---
include/abg-ctf-reader.h | 5 ++++-
src/abg-ctf-reader.cc | 23 +++++++++++++++++++++++
tools/abidiff.cc | 23 +++++++++++++++++++++++
tools/abidw.cc | 17 +++++++++++++++++
tools/abipkgdiff.cc | 32 ++++++++++++++++++++++++++++++++
5 files changed, 99 insertions(+), 1 deletion(-)
Hello Claudiu,
claudiu.zissulescu-ianculescu@oracle.com a écrit:
> From: Claudiu Zissulescu <claudiu.zissulescu-ianculescu@oracle.com>
>
> Hi Dodji,
>
> Please find my updated patch with your feedback in. As for the test,
> I'll try to come with one against big-tests. AFAIK, the ctf-parent
> option is used only with non-standard linkers like in the case of
> Oracle Linux CTFA archive.
>
> Thank you,
> Claudiu
>
>
> If the CTF section contains ambiguously-defined types, it will consist
> of an archive of many CTF dictionaries, all inheriting from one
> dictionary containing unambiguous types. This member is by default
> named .ctf, like the section containing it, but it is possible to
> change this name using the "ctf_link_set_memb_name_changer" function
> at link time. When looking at CTF archives that have been created by
> a linker that uses the name changer to rename the parent archive
> member, --ctf-parent can be used to specify the name used for the
> parent. This new option can be used with abidw, abidiff and
> abipkgdiff tools.
>
> * include/abg-ctf-reader.h (parent_name): New function.
> * src/abg-ctf-reader.cc (ctf_parent_name): New variable.
> (ctf_parent_name): Sets ctf parent name.
> (parent_name): Gets the elf base reader and sets the parent name.
> * tools/abidiff.cc (ctf_parent): New option.
> (parse_command_line): Parse the new option.
> (main): Handle the new option.
> * tools/abidw.cc (ctf_parent): New option.
> (parse_command_line): Parse the new option.
> (load_corpus_and_write_abixml): Handle the new option.
> * tools/abipkgdiff.cc (ctf_parent): New option.
> (compare): Handle the new option.
> (compare_to_self): Likewise.
> (parse_command_line): Parse the new option.
Thanks for the updated patch!
As the goal of this patch is to add a --ctf-parent <parent-dict-name>
option to abi{diff,pkgdiff,dw} tools, its current form looks good to me.
The only missing things is the documentation update, namely,
doc/manuals/abi{diff,pkgdiff,dw}.rst files.
However, after more discussions with yourself and Nick (in copy of this
email), I think a more concise and user-friendly approach would be to
avoid having to pass --ctf-parent <parent-dict-name> to the tools
altogether.
We could do that by enumerating all the dictionaries of a given archive
instead of having to know the name of a given dictionary to open it.
If I understand what Nick said, we can do that using the
ctf_archive_next function, a bit like what lookup_symbol_in_ctf_archive
does already in the CTF reader.
I'll thus follow this message up with a patch where I try this idea. I
won't be able to test that patch on a binary having a CTF dictionary
section with an unexpected name as I don't have access to such a
specimen. I'd would instead rely on you for that test and would be glad
to have such a binary be added to, e.g,
https://sourceware.org/git/gitweb.cgi?p=libabigail-tests.git.
[...]
Cheers,
@@ -37,7 +37,10 @@ void
reset_reader(elf_based_reader& ctxt,
const std::string& elf_path,
const vector<char**>& debug_info_root_path);
+
+void
+parent_name(elf_based_reader& rdr,
+ const std::string& parent_name);
} // end namespace ctf_reader
} // end namespace abigail
-
#endif // ! __ABG_CTF_READER_H__
@@ -157,6 +157,9 @@ class reader : public elf_based_reader
ctf_sect_t strtab_sect;
translation_unit_sptr cur_tu_;
+ /// CTF parent name.
+ std::string ctf_parent_name;
+
public:
/// Getter of the exported decls builder object.
@@ -463,6 +466,8 @@ public:
std::replace(dict_name.begin(), dict_name.end(), '-', '_');
}
+ else
+ dict_name = ctf_parent_name;
if ((ctf_dict = ctf_dict_open(ctfa,
dict_name.empty() ? NULL : dict_name.c_str(),
@@ -747,6 +752,13 @@ public:
return corp;
}
+ /// Sets ctf_parent_name.
+ void
+ set_parent_name(const std::string& parent_name)
+ {
+ ctf_parent_name = parent_name;
+ }
+
/// Destructor of the CTF reader.
~reader()
{
@@ -1757,6 +1769,17 @@ reset_reader(elf_based_reader& rdr,
r.initialize(elf_path, debug_info_root_path);
}
+/// Set a given parent name.
+///
+/// @param ctf_parent_name the custom ctf parent name.
+void
+parent_name(elf_based_reader& rdr,
+ const std::string& parent_name)
+{
+ ctf::reader& r = dynamic_cast<reader&>(rdr);
+ r.set_parent_name(parent_name);
+}
+
/// Returns a key to be use in types_map dict conformed by
/// dictionary id and the CTF type id for a given type.
///
@@ -134,6 +134,7 @@ struct options
#endif
#ifdef WITH_CTF
bool use_ctf;
+ string ctf_parent;
#endif
#ifdef WITH_BTF
bool use_btf;
@@ -310,6 +311,7 @@ display_usage(const string& prog_name, ostream& out)
<< " --stats show statistics about various internal stuff\n"
#ifdef WITH_CTF
<< " --ctf use CTF instead of DWARF in ELF files\n"
+ << " --ctf-parent <name> set the CTF archive parent name\n"
#endif
#ifdef WITH_BTF
<< " --btf use BTF instead of DWARF in ELF files\n"
@@ -756,6 +758,14 @@ parse_command_line(int argc, char* argv[], options& opts)
#ifdef WITH_CTF
else if (!strcmp(argv[i], "--ctf"))
opts.use_ctf = true;
+ else if (!strcmp(argv[i], "--ctf-parent"))
+ {
+ int j = i + 1;
+ if (j >= argc)
+ return false;
+ opts.ctf_parent = argv[j];
+ ++i;
+ }
#endif
#ifdef WITH_BTF
else if (!strcmp(argv[i], "--btf"))
@@ -1432,6 +1442,13 @@ main(int argc, char* argv[])
ABG_ASSERT(rdr);
set_generic_options(*rdr, opts);
set_suppressions(*rdr, opts);
+
+#ifdef WITH_CTF
+ // If the user sets the ctf parent make sure we propagate it.
+ if (!opts.ctf_parent.empty()
+ && opts.use_ctf)
+ abigail::ctf::parent_name(*rdr, opts.ctf_parent);
+#endif
c1 = rdr->read_corpus(c1_status);
if (!c1
@@ -1524,6 +1541,12 @@ main(int argc, char* argv[])
set_generic_options(*rdr, opts);
set_suppressions(*rdr, opts);
+#ifdef WITH_CTF
+ // If the user sets the ctf parent make sure we propagate it.
+ if (!opts.ctf_parent.empty()
+ && opts.use_ctf)
+ abigail::ctf::parent_name(*rdr, opts.ctf_parent);
+#endif
c2 = rdr->read_corpus(c2_status);
if (!c2
@@ -118,6 +118,7 @@ struct options
bool list_dependencies;
#ifdef WITH_CTF
bool use_ctf;
+ string ctf_parent;
#endif
#ifdef WITH_BTF
bool use_btf;
@@ -261,6 +262,7 @@ display_usage(const string& prog_name, ostream& out)
#endif
#ifdef WITH_CTF
<< " --ctf use CTF instead of DWARF in ELF files\n"
+ << " --ctf-parent <name> set the CTF archive parent name\n"
#endif
<< " --no-leverage-dwarf-factorization do not use DWZ optimisations to "
"speed-up the analysis of the binary\n"
@@ -402,6 +404,14 @@ parse_command_line(int argc, char* argv[], options& opts)
#ifdef WITH_CTF
else if (!strcmp(argv[i], "--ctf"))
opts.use_ctf = true;
+ else if (!strcmp(argv[i], "--ctf-parent"))
+ {
+ int j = i + 1;
+ if (j >= argc)
+ return false;
+ opts.ctf_parent = argv[j];
+ ++i;
+ }
#endif
#ifdef WITH_BTF
else if (!strcmp(argv[i], "--btf"))
@@ -798,6 +808,13 @@ load_corpus_and_write_abixml(char* argv[],
set_generic_options(*reader, opts);
set_suppressions(*reader, opts);
+#ifdef WITH_CTF
+ // If the user sets the ctf parent make sure we propagate it.
+ if (!opts.ctf_parent.empty()
+ && opts.use_ctf)
+ abigail::ctf::parent_name(*reader, opts.ctf_parent);
+#endif
+
// If the user asked us to check if we found the "alternate debug
// info file" associated to the input binary, then proceed to do so
// ...
@@ -214,6 +214,7 @@ public:
optional<bool> exported_interfaces_only;
#ifdef WITH_CTF
bool use_ctf;
+ string ctf_parent;
#endif
#ifdef WITH_BTF
bool use_btf;
@@ -922,6 +923,7 @@ display_usage(const string& prog_name, ostream& out)
"binaries inside the input package against their ABIXML representation\n"
#ifdef WITH_CTF
<< " --ctf use CTF instead of DWARF in ELF files\n"
+ << " --ctf-parent <name> set the CTF archive parent name\n"
#endif
#ifdef WITH_BTF
<< " --btf use BTF instead of DWARF in ELF files\n"
@@ -1486,6 +1488,13 @@ compare(const elf_file& elf1,
reader->add_suppressions(supprs);
set_generic_options(*reader, opts);
+#ifdef WITH_CTF
+ // If the user sets the ctf parent make sure we propagate it.
+ if (!opts.ctf_parent.empty()
+ && opts.use_ctf)
+ abigail::ctf::parent_name(*reader, opts.ctf_parent);
+#endif
+
corpus1 = reader->read_corpus(c1_status);
bool bail_out = false;
@@ -1575,6 +1584,14 @@ compare(const elf_file& elf1,
reader->add_suppressions(priv_types_supprs2);
set_generic_options(*reader, opts);
+#ifdef WITH_CTF
+ // If the user sets the ctf parent make sure we propagate it.
+ // Note: Do we need to have sparate parents per package?
+ if (!opts.ctf_parent.empty()
+ && opts.use_ctf)
+ abigail::ctf::parent_name(*reader, opts.ctf_parent);
+#endif
+
corpus2 = reader->read_corpus(c2_status);
bool bail_out = false;
@@ -1741,6 +1758,13 @@ compare_to_self(const elf_file& elf,
reader->add_suppressions(supprs);
corp = reader->read_corpus(c_status);
+#ifdef WITH_CTF
+ // If the user sets the ctf parent make sure we propagate it.
+ if (!opts.ctf_parent.empty()
+ && opts.use_ctf)
+ abigail::ctf::parent_name(*reader, opts.ctf_parent);
+#endif
+
if (!(c_status & abigail::fe_iface::STATUS_OK))
{
if (opts.verbose)
@@ -3631,6 +3655,14 @@ parse_command_line(int argc, char* argv[], options& opts)
#ifdef WITH_CTF
else if (!strcmp(argv[i], "--ctf"))
opts.use_ctf = true;
+ else if (!strcmp(argv[i], "--ctf-parent"))
+ {
+ int j = i + 1;
+ if (j >= argc)
+ return false;
+ opts.ctf_parent = argv[j];
+ ++i;
+ }
#endif
#ifdef WITH_BTF
else if (!strcmp(argv[i], "--btf"))