[06/17] ctf-reader: During re-initialization, only clear canonicalize-able types

Message ID 20240716145541.473065-7-dodji@redhat.com
State New
Headers
Series Support self comparison of vmlinux & modules using BTF/CTF |

Commit Message

Dodji Seketeli July 16, 2024, 2:55 p.m. UTC
  From: Dodji Seketeli <dodji@redhat.com>

The patch
https://sourceware.org/git/?p=libabigail.git;a=commit;h=666555665cc4fcfc8ae19661c489822e0df00ae3
introduced a mistake: It frees the CTF archive object during the
re-initialization of the reader between each binary (kernel or
module).  The archive contains CTF type information for a kernel and
all its modules.  So it should be kept around until all those binaries
are analyzed.  Instead, that patch frees the CTF archive object after
handling each binary.  Oops.

This patch fixes that problem by free-ing the CTF archive only when
the reader itself is freed, presumably, after analyzing all binaries.

Similarly, the type map maintained by the reader contains types for
the kernel and all its modules.  So it should not be freed at
re-initialization time.  Rather, what should be freed is only the
types that are to be canonicalized at the end of processing of a given
binary.  So the patch makes the reader maintain types to be
canonicalized in a vector and clears that vector at each
re-initialization.

At re-initialization time, the patch also avoids resetting the corpus
group that the current reader feeds.

	* src/abg-ctf-reader.cc (reader::types_to_canonicalize): Add data
	member.
	(reader::add_type): Add the new type to the vector of types to be
	canonicalized.  Update comment.
	(reader::canonicalize_all_types): Now that the reader maintains
	the vector of types to be canonicalized, just pass that vector to
	canonicalize_types.  reader::types_map contains all the types that
	have been created in all the binaries processed by this reader so
	far.  Many of these types have already been canonicalized at the
	end of the analysis of binaries that have already been processed.
	Only the types created during the processing of the current binary
	have not yet be canonicalized and reader::types_to_canonicalize is
	where they are maintained.  So reader::types_map should be left
	alone by this function.
	(reader::initialize): Do not close the CTF archive here.  Do not
	clear reader::{types_map, corpus_group} either.
	(reader::read_corpus): Use nullptr, not NULL.  Make sure to not
	re-open an archive that has already been opened.
	(reader::~reader): Mark a closed archive.  This is more cosmetic
	than anything else.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 src/abg-ctf-reader.cc | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)
  

Patch

diff --git a/src/abg-ctf-reader.cc b/src/abg-ctf-reader.cc
index 3dc09b29..c93608ca 100644
--- a/src/abg-ctf-reader.cc
+++ b/src/abg-ctf-reader.cc
@@ -145,6 +145,7 @@  class reader : public elf_based_reader
   /// A map associating CTF type ids with libabigail IR types.  This
   /// is used to reuse already generated types.
   string_type_base_sptr_map_type types_map;
+  vector<type_base_sptr> types_to_canonicalize;
 
   /// A set associating unknown CTF type ids
   std::set<ctf_id_t> unknown_types_set;
@@ -167,6 +168,10 @@  public:
 
   /// Associate a given CTF type ID with a given libabigail IR type.
   ///
+  /// The IR type is a newly created type that needs to be
+  /// canonicalized at the end of the processing of the current
+  /// corpus.
+  ///
   /// @param dic the dictionnary the type belongs to.
   ///
   /// @param ctf_type the type ID.
@@ -176,7 +181,8 @@  public:
   add_type(ctf_dict_t *dic, ctf_id_t ctf_type, type_base_sptr type)
   {
     string key = dic_type_key(dic, ctf_type);
-    types_map.insert(std::make_pair(key, type));
+    if (types_map.insert(std::make_pair(key, type)).second)
+      types_to_canonicalize.push_back(type);
   }
 
   /// Insert a given CTF unknown type ID.
@@ -216,12 +222,8 @@  public:
   void
   canonicalize_all_types(void)
   {
-    vector<type_base_sptr> types;
-    for (const auto& entry : types_map)
-      types.push_back(entry.second);
-
     canonicalize_types
-      (types.begin(), types.end(),
+      (types_to_canonicalize.begin(), types_to_canonicalize.end(),
        [](vector<type_base_sptr>::iterator& i)
        {return *i;});
   }
@@ -262,14 +264,8 @@  public:
   void
   initialize()
   {
-    if (ctfa)
-      {
-	ctf_close(ctfa);
-	ctfa = nullptr;
-      }
-    types_map.clear();
+    types_to_canonicalize.clear();
     cur_tu_.reset();
-    corpus_group().reset();
   }
 
   /// Initializer of the reader.
@@ -697,7 +693,7 @@  public:
     if ((corp->get_origin() & corpus::LINUX_KERNEL_BINARY_ORIGIN)
 	&& corpus_group())
       {
-	if (ctfa == NULL)
+	if (ctfa == nullptr)
 	  {
 	    std::string ctfa_filename;
 	    if (find_ctfa_file(ctfa_filename))
@@ -709,8 +705,9 @@  public:
 	 and process the CTF archive in the read context, if any.
 	 Information about the types, variables, functions, etc contained
 	 in the archive are added to the given corpus.  */
-      ctfa = ctf_arc_bufopen(&ctf_sect, &symtab_sect,
-			     &strtab_sect, &errp);
+      if (ctfa == nullptr)
+	ctfa = ctf_arc_bufopen(&ctf_sect, &symtab_sect,
+			       &strtab_sect, &errp);
 
     if (do_log())
       {
@@ -749,6 +746,7 @@  public:
   ~reader()
   {
     ctf_close(ctfa);
+    ctfa = nullptr;
   }
 }; // end class reader.