[PATCH] ctf-reader: Add support for arbitrary CTF archive member as the CTF parent

[PATCH] ctf-reader: Add support for arbitrary CTF archive member as the CTF parent

[claudiu.zissulescu-ianculescu]

From: Claudiu Zissulescu <claudiu.zissulescu-ianculescu@oracle.com>

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    | 25 +++++++++++++++++++++++++
 tools/abidiff.cc         | 27 +++++++++++++++++++++++++++
 tools/abidw.cc           | 19 +++++++++++++++++++
 tools/abipkgdiff.cc      | 38 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 113 insertions(+), 1 deletion(-)

diff --git a/include/abg-ctf-reader.h b/include/abg-ctf-reader.h
index c1f2a3f9..4e7f5a12 100644
--- a/include/abg-ctf-reader.h
+++ b/include/abg-ctf-reader.h
@@ -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__
diff --git a/src/abg-ctf-reader.cc b/src/abg-ctf-reader.cc
index 4b6c6cd8..6ee96265 100644
--- a/src/abg-ctf-reader.cc
+++ b/src/abg-ctf-reader.cc
@@ -156,6 +156,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.
@@ -464,6 +467,10 @@ 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(),
@@ -745,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()
   {
@@ -1748,6 +1762,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.
 ///
diff --git a/tools/abidiff.cc b/tools/abidiff.cc
index cadb742b..515b5128 100644
--- a/tools/abidiff.cc
+++ b/tools/abidiff.cc
@@ -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,15 @@ 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 +1543,14 @@ 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
diff --git a/tools/abidw.cc b/tools/abidw.cc
index 14f2078c..41909512 100644
--- a/tools/abidw.cc
+++ b/tools/abidw.cc
@@ -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"))
@@ -683,6 +693,15 @@ 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
   // ...
diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc
index d1ddd341..0d6a1fb2 100644
--- a/tools/abipkgdiff.cc
+++ b/tools/abipkgdiff.cc
@@ -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,15 @@ 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 +1586,16 @@ 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 +1762,15 @@ 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 +3661,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"))
-- 
2.45.2

Re: [PATCH] ctf-reader: Add support for arbitrary CTF archive member as the CTF parent

[Dodji Seketeli]

Hello Claudiu,

claudiu.zissulescu-ianculescu@oracle.com a écrit:

> From: Claudiu Zissulescu <claudiu.zissulescu-ianculescu@oracle.com>
>
> If the CTF section contains ambiguously-defined types,

Just to be sure I understand it correctly.  What do you mean by an
"ambiguously-defined type"?

For instance, suppose that in a translation unit a.c, we define a
(struct) type named T.

And then in another translation unit b.c, (in the same binary) we define
another type named T as well, but with a definition different from the
one coming from a.c.

Is T an ambiguously defined type?  If yes, would that be what the CTF
doc calls "types with identical names but conflicting definitions" at
https://sourceware.org/binutils/docs/ctf-spec.html#CTF-archive?


> it will consist of an archive of many CTF dictionaries, all inheriting
> from one dictionary containing unambiguous types.

Right. (Assuming my understanding above is correct).

> This member is by default named .ctf, like the section containing it,

By "member", do you mean the parent dictionary that contains the
unambiguous types?

> but it is possible to change this name using the
> "ctf_link_set_memb_name_changer" function at link time.

OK.  In that case, when looking at the child dictionary referencing the
parent dictionary, is it possible to discover the name of the parent?
Is it possible to do that using libctf?

> 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.

OK.  If the name can be discovered by looking at the binary itself
(maybe using libctf) can we do that instead, rather than relying the
--ctf-parent option?  I am not asking for this patch to be dropped.
It's just by curiosity.  If it's possible to discover the parent name,
maybe that could be a subsequent patch, who knows.

[...]

> diff --git a/src/abg-ctf-reader.cc b/src/abg-ctf-reader.cc
> index 4b6c6cd8..6ee96265 100644
> --- a/src/abg-ctf-reader.cc
> +++ b/src/abg-ctf-reader.cc
> @@ -156,6 +156,9 @@ class reader : public elf_based_reader
>    ctf_sect_t strtab_sect;
>    translation_unit_sptr cur_tu_;
>  
> +  ///CTF parent name.

Please insert a space before the CTF word, for the sake of consistency
with the rest of the comments of the front-end.

> +  std::string ctf_parent_name;
> +
>  public:
>  
>    /// Getter of the exported decls builder object.
> @@ -464,6 +467,10 @@ public:
>  
>  	std::replace(dict_name.begin(), dict_name.end(), '-', '_');
>        }
> +    else
> +      {
> +	dict_name = ctf_parent_name;
> +      }

I don't understand the condition that you intend for setting dict_name
to the argument of the --ctf-parent option here.

My understanding is that the dictionary name is going to be set to the
argument of the --ctf-parent option (provided to the tool) if we are
looking at a user-space binary or if we are looking at a single kernel
binary (i.e, there is no corpus_group being currently built).  Is that
what you want to do?  If yes, why?

Also, the curly brackets are not necessary.  Again, this is for the sake
of consistency with the rest of the project.

[...]


> diff --git a/tools/abidiff.cc b/tools/abidiff.cc
> index cadb742b..515b5128 100644
> --- a/tools/abidiff.cc
> +++ b/tools/abidiff.cc
> @@ -134,6 +134,7 @@ struct options
>  #endif
>  #ifdef WITH_CTF
>    bool			use_ctf;
> +  string                ctf_parent;

Ah I think the rest of the data member are aligned using tabs and this
new one is aligned using spaces, hence the inconsistency.

[...]


> +
> +#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

The curly brackets are unnecessary.

[...]

> +#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

Likewise.

>  	    c2 = rdr->read_corpus(c2_status);
>  
>  	    if (!c2
> diff --git a/tools/abidw.cc b/tools/abidw.cc
> index 14f2078c..41909512 100644
> --- a/tools/abidw.cc
> +++ b/tools/abidw.cc
> @@ -118,6 +118,7 @@ struct options
>    bool			list_dependencies;
>  #ifdef WITH_CTF
>    bool			use_ctf;
> +  string                ctf_parent;

Space vs tabs here as well.

[...]


> +#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
> +

The curly brackets are unnecessary here as well.

[...]

> diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc
> index d1ddd341..0d6a1fb2 100644
> --- a/tools/abipkgdiff.cc
> +++ b/tools/abipkgdiff.cc
> @@ -214,6 +214,7 @@ public:
>    optional<bool> exported_interfaces_only;
>  #ifdef WITH_CTF
>    bool		use_ctf;
> +  string        ctf_parent;

Spaces vs tabs here as well.

[...]

> +#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
> +

The curly brackets are unnecessary here as well.

[...]

> @@ -1575,6 +1586,16 @@ 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
> +

Likewise.

[...]

> @@ -1741,6 +1762,15 @@ 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
> +

Likewise.

>      if (!(c_status & abigail::fe_iface::STATUS_OK))
>        {
>  	if (opts.verbose)
> @@ -3631,6 +3661,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"))

I think the patch would be OK to go with my above comments addressed.  I
have one more question.  Would it be possible to have some test in place
for it?  If it uses a small binary then it could go into tests/
Otherwise, it could go in the libabigail-tests.git repository as a
subsequent patch.

Anyway, many thanks for your work!

Cheers,

-- 
		Dodji

[PATCH] ctf-reader: Add support for arbitrary CTF archive member as the CTF parent

[claudiu.zissulescu-ianculescu]

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(-)

diff --git a/include/abg-ctf-reader.h b/include/abg-ctf-reader.h
index c1f2a3f9..4e7f5a12 100644
--- a/include/abg-ctf-reader.h
+++ b/include/abg-ctf-reader.h
@@ -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__
diff --git a/src/abg-ctf-reader.cc b/src/abg-ctf-reader.cc
index 6879c6e0..7382019d 100644
--- a/src/abg-ctf-reader.cc
+++ b/src/abg-ctf-reader.cc
@@ -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.
 ///
diff --git a/tools/abidiff.cc b/tools/abidiff.cc
index cadb742b..7f2223a4 100644
--- a/tools/abidiff.cc
+++ b/tools/abidiff.cc
@@ -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
diff --git a/tools/abidw.cc b/tools/abidw.cc
index 6bb1ef55..e7fd397b 100644
--- a/tools/abidw.cc
+++ b/tools/abidw.cc
@@ -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
   // ...
diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc
index d1ddd341..ea6d2a28 100644
--- a/tools/abipkgdiff.cc
+++ b/tools/abipkgdiff.cc
@@ -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"))
-- 
2.45.2

Re: [PATCH] ctf-reader: Add support for arbitrary CTF archive member as the CTF parent

[Dodji Seketeli]

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,

-- 
		Dodji