corpus: Allow several variables with same ID to be exported

The libQt6Network.so.6.3.1 exhibits some interesting debug info where
two variables with slightly different qualified name have the same ELF
symbol.

Consider the DIE for this variable:
 [ 1aab3]    variable             abbrev: 118
             name                 (GNU_strp_alt) "backendMutex"
             decl_file            (implicit_const) qsslsocket_p.h (42)
             decl_line            (data1) 196
             decl_column          (data1) 26
             linkage_name         (GNU_strp_alt) "_ZN17QSslSocketPrivate12backendMutexE"
             type                 (ref_udata) [ 1aa35]
             external             (flag_present) yes
             declaration          (flag_present) yes
             inline               (implicit_const) declared_inlined (3)
             location             (exprloc)
              [ 0] addr +0x1e3050 <_ZN17QSslSocketPrivate12backendMutexE>

This DIE designates a variable named backendMutex in the global
namespace.  It's associated to the ELF symbol
_ZN17QSslSocketPrivate12backendMutexE.

Then, later, there is this DIE:

 [d05d3e]    class_type           abbrev: 260
             name                 (GNU_strp_alt) "QSslSocketPrivate"
             declaration          (flag_present) yes
             sibling              (ref_udata) [d05d70]
[...]
 [d05d70]    variable             abbrev: 642
             name                 (GNU_strp_alt) "backendMutex"
             decl_file            (implicit_const) qsslsocket_p.h (54)
             decl_line            (data1) 196
             decl_column          (data1) 26
             linkage_name         (GNU_strp_alt) "_ZN17QSslSocketPrivate12backendMutexE"
             type                 (ref_addr) [  fdc5]
             external             (flag_present) yes
             declaration          (flag_present) yes
             inline               (implicit_const) declared_inlined (3)
             location             (exprloc)
              [ 0] addr +0x1e3050 <_ZN17QSslSocketPrivate12backendMutexE>

Here, this DIE represents a static data member named
QSslSocketPrivate::backendMutex, as it's member of the
QSslSocketPrivate class.  Note how it's also associated with the ELF
symbol _ZN17QSslSocketPrivate12backendMutexE.

So both variables are associated with the same ELF symbol and should
thus be exported from the ABI corpus.

Today, only one of these variables is represented as exported in the
corpus.  Depending on the order in which the variables are analyzed,
the variable that is represented as exported might be either one or
the other, leading to some spurious self-comparison errors.

This patch fixes the issue by allowing the corpus to represent more
than one variable having a given ELF symbol to be exported.  This is
similar to what is already for functions.

This fixes the self comparison of the qt6-qtbase package, which can be
witnessed by issuing the command:

    $ fedabipkgdiff --self-compare -a --from fc36 qt6-qtbase

	* include/abg-corpus.h (corpus::lookup_variables): Replace
	corpus::lookup_variable with this one which returns the set of
	variables associated with a given ID of variable.
	* src/abg-corpus-priv.h (istr_var_ptr_set_map_type): Define new
	typedef.
	(corpus::exported_decls_builder::priv::id_vars_map_): Replace
	id_var_map_ with this new data member of type
	istr_var_ptr_set_map_type.
	(corpus::exported_decls_builder::priv::id_fns_map): Fix comment.
	(corpus::exported_decls_builder::priv::id_vars_map): Replace
	previous id_var_map with this member function.
	(corpus::exported_decls_builder::priv::var_id_is_in_id_vars_map):
	Replace the var_id_is_in_id_var_map member function.
	(corpus::exported_decls_builder::priv::var_is_in_vars): Define new
	static member function.
	(corpus::exported_decls_builder::priv::{var_is_in_id_vars_map, add_var_to_id_vars_map}):
	Define new member functions.
	(corpus::exported_decls_builder::priv::add_var_to_map): Remove.
	(corpus::exported_decls_builder::priv::add_var_to_exported):
	Adjust by using the new var_is_in_id_vars_map and
	add_var_to_id_vars_map.
	* src/abg-corpus.cc
	(corpus::exported_decls_builder::maybe_add_var_to_exported_vars):
	Adjust, use the new
	corpus::exported_decls_builder::priv::var_is_in_id_vars_map.
	(corpus::lookup_variables): Replace corpus::lookup_variable with
	this one which returns the set of variables associated with a
	given ID of variable.
	* tools/abicompat.cc
	(compare_expected_against_provided_variables): Adjust.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>

include/abg-corpus.h

@@ -230,8 +230,11 @@ public:
   const std::unordered_set<function_decl*>*
   lookup_functions(const char* id) const;
 
-  const var_decl_sptr
-  lookup_variable(const interned_string& id) const;
+  const std::unordered_set<var_decl_sptr>*
+  lookup_variables(const interned_string& id) const;
+
+  const std::unordered_set<var_decl_sptr>*
+  lookup_variables(const char* id) const;
 
   void
   sort_functions();

src/abg-corpus-priv.h

@@ -63,6 +63,12 @@ typedef unordered_map<interned_string,
 		      var_decl_sptr,
 		      hash_interned_string> istr_var_ptr_map_type;
 
+/// Convenience typedef for a hash map which key is an interned_string
+/// and which data is a set of abigail::ir::var_decl_sptr
+typedef unordered_map<interned_string,
+		      std::unordered_set<var_decl_sptr>,
+		      hash_interned_string> istr_var_ptr_set_map_type;
+
 /// The type of the private data of @ref
 /// corpus::exported_decls_builder type.
 class corpus::exported_decls_builder::priv
@@ -75,7 +81,7 @@ class corpus::exported_decls_builder::priv
   functions&		fns_;
   variables&		vars_;
   // A map that associates a function ID (function symbol and its
-  // version) to to a vector of functions with that ID.  Normally, one
+  // version) to a vector of functions with that ID.  Normally, one
   // would think that in the corpus, there must only one function for
   // a given ID.  Actually, in c++, there can be two function template
   // instantiations that produce the same function ID because the
@@ -83,7 +89,7 @@ class corpus::exported_decls_builder::priv
   // of the first instantiation, for instance.  So there can be cases
   // where one ID appertains to more than one function.
   istr_fn_ptr_set_map_type	id_fns_map_;
-  istr_var_ptr_map_type	id_var_map_;
+  istr_var_ptr_set_map_type	id_vars_map_;
   strings_type&	fns_suppress_regexps_;
   regex_t_sptrs_type	compiled_fns_suppress_regexp_;
   strings_type&	vars_suppress_regexps_;
@@ -223,12 +229,11 @@ public:
   /// Getter for a map of the IDs of the functions that are present in
   /// the set of exported functions.
   ///
-  /// This map is useful during the construction of the set of
-  /// exported functions, at least to ensure that every function is
-  /// present only once in that set.
+  /// The map associates the ID of a function with the set of
+  /// functions having the symbol that matches the ID.
   ///
-  /// @return a map which key is a string and which data is a pointer
-  /// to a function.
+  /// @return a map which key is a string and which data is a set of
+  /// functions.
   istr_fn_ptr_set_map_type&
   id_fns_map()
   {return id_fns_map_;}
@@ -236,28 +241,26 @@ public:
   /// Getter for a map of the IDs of the variables that are present in
   /// the set of exported variables.
   ///
-  /// This map is useful during the construction of the set of
-  /// exported variables, at least to ensure that every function is
-  /// present only once in that set.
+  /// The map associates the ID of a variable with the set of variables
+  /// having the symbol that matches the ID.
   ///
-  /// @return a map which key is a string and which data is a pointer
-  /// to a function.
-  const istr_var_ptr_map_type&
-  id_var_map() const
-  {return id_var_map_;}
+  /// @return a map which key is a string and which data is a set of
+  /// variables.
+  istr_var_ptr_set_map_type&
+  id_vars_map()
+  {return id_vars_map_;}
 
   /// Getter for a map of the IDs of the variables that are present in
   /// the set of exported variables.
   ///
-  /// This map is useful during the construction of the set of
-  /// exported variables, at least to ensure that every function is
-  /// present only once in that set.
+  /// The map associates the ID of a variable with the set of variables
+  /// having the symbol that matches the ID.
   ///
-  /// @return a map which key is a string and which data is a pointer
-  /// to a function.
-  istr_var_ptr_map_type&
-  id_var_map()
-  {return id_var_map_;}
+  /// @return a map which key is a string and which data is a set of
+  /// variables.
+  const istr_var_ptr_set_map_type&
+  id_vars_map() const
+  {return id_vars_map_;}
 
   /// Returns an ID for a given function.
   ///
@@ -428,34 +431,123 @@ public:
     while (sym && !sym->is_main_symbol());
   }
 
-  /// Test if a given (ID of a) varialble is present in the variable
+  /// Test if a given (ID of a) variable is present in the variable
   /// map.  In other words, it tests if a given variable is present in
   /// the set of exported variables.
   ///
   /// @param fn_id the ID of the variable to consider.
   ///
-  /// @return true iff the variable designated by @p fn_id is present
-  /// in the set of exported variables.
-  bool
-  var_id_is_in_id_var_map(const interned_string& var_id) const
+  /// @return a pointer to the set of variables that have the same ID
+  /// as @p var_id.
+  std::unordered_set<var_decl_sptr>*
+  var_id_is_in_id_vars_map(const interned_string& var_id)
   {
-    const istr_var_ptr_map_type& m = id_var_map();
+    istr_var_ptr_set_map_type& m = id_vars_map();
     auto i = m.find(var_id);
-    return i != m.end();
+    if (i != m.end())
+      return &i->second;
+    return nullptr;
   }
 
-  /// Add a given variable to the map of functions that are present in
-  /// the set of exported functions.
+  /// Test if a given (ID of a) variable is present in the variable
+  /// map.  In other words, it tests if a given variable is present in
+  /// the set of exported variables.
   ///
-  /// @param id the variable to add to the map.
-  void
-  add_var_to_map(const var_decl_sptr& var)
+  /// @param fn_id the ID of the variable to consider.
+  ///
+  /// @return a pointer to the set of variables that have the same ID
+  /// as @p var_id.
+  const std::unordered_set<var_decl_sptr>*
+  var_id_is_in_id_vars_map(const interned_string& var_id) const
   {
-    if (var)
+    return const_cast<corpus::exported_decls_builder::priv*>(this)->
+      var_id_is_in_id_vars_map(var_id);
+  }
+
+  /// Test if a given variable is present in a set of variables.
+  ///
+  /// The variable compares the ID and the qualified name of
+  /// variables.
+  ///
+  /// @param fn the variable to consider.
+  ///
+  /// @parm fns the set of variables to consider.
+  static bool
+  var_is_in_vars(const var_decl_sptr& var,
+		 const std::unordered_set<var_decl_sptr>& vars)
+  {
+    if (vars.empty())
+      return false;
+
+    if (vars.find(var) != vars.end())
+      return true;
+
+    const string var_id = var->get_id();
+    for (const auto& v: vars)
+      if (v->get_id() == var_id
+	  && v->get_qualified_name() == var->get_qualified_name())
+	return true;
+
+    return false;
+  }
+
+  /// Test if a given variable is present in a set of variables.
+  ///
+  /// The variable compares the ID and the qualified name of
+  /// variables.
+  ///
+  /// @param fn the variable to consider.
+  ///
+  /// @parm fns the set of variables to consider.
+  bool
+  var_is_in_id_vars_map(const var_decl_sptr& var)
+  {
+    if (!var)
+      return false;
+
+    interned_string var_id = var->get_id();
+    const std::unordered_set<var_decl_sptr>* vars =
+      var_id_is_in_id_vars_map(var_id);
+    if (vars && var_is_in_vars(var, *vars))
+      return true;
+    return false;
+  }
+
+  /// Add a given variable to the map of variables that are present in
+  /// the set of exported variables.
+  ///
+  /// @param fn the variable to add to the map.
+  void
+  add_var_to_id_vars_map(const var_decl_sptr& var)
+  {
+    if (!var)
+      return;
+
+    // First associate the var id to the variable.
+    interned_string var_id = var->get_id();
+    std::unordered_set<var_decl_sptr>* vars = var_id_is_in_id_vars_map(var_id);
+    if (!vars)
+      vars = &(id_vars_map()[var_id] = std::unordered_set<var_decl_sptr>());
+    vars->insert(var);
+
+    // Now associate all aliases of th underlying symbol to the
+    // variable too.
+    elf_symbol_sptr sym = var->get_symbol();
+    ABG_ASSERT(sym);
+    string sym_id;
+    do
       {
-	const interned_string& var_id = get_id(*var);
-	id_var_map()[var_id] = var;
+	sym_id = sym->get_id_string();
+	if (sym_id == var_id)
+	  goto loop;
+	vars = var_id_is_in_id_vars_map(var_id);
+	if (!vars)
+	  vars = &(id_vars_map()[var_id] = std::unordered_set<var_decl_sptr>());
+	vars->insert(var);
+      loop:
+	sym = sym->get_next_alias();
       }
+    while (sym && !sym->is_main_symbol());
   }
 
   /// Add a function to the set of exported functions.
@@ -477,11 +569,10 @@ public:
   void
   add_var_to_exported(const var_decl_sptr& var)
   {
-    const interned_string& id = get_id(*var);
-    if (!var_id_is_in_id_var_map(id))
+    if (!var_is_in_id_vars_map(var))
       {
 	vars_.push_back(var);
-	add_var_to_map(var);
+	add_var_to_id_vars_map(var);
       }
   }
 

src/abg-corpus.cc

@@ -198,7 +198,7 @@ corpus::exported_decls_builder::maybe_add_var_to_exported_vars(const var_decl_sp
   const interned_string& var_id = priv_->get_id(*var);
   ABG_ASSERT(!var_id.empty());
 
-  if (priv_->var_id_is_in_id_var_map(var_id))
+  if (priv_->var_is_in_id_vars_map(var))
     return false;
 
   if (priv_->keep_wrt_id_of_vars_to_keep(var)
@@ -1409,20 +1409,36 @@ corpus::lookup_functions(const char* id) const
   return lookup_functions(string_id);
 }
 
-/// Lookup the exported variable which has a given variable ID.
+/// Lookup the exported variables which all have a given variable ID.
 ///
 /// @param id the ID of the variable to look up.
 ///
-/// @return the variable with ID @p id that was found or nil if none
-/// was found.
-const var_decl_sptr
-corpus::lookup_variable(const interned_string& id) const
+/// @return a pointer to the set of variables with ID @p id, or
+/// nullptr if no variable was found with that ID.
+const std::unordered_set<var_decl_sptr>*
+corpus::lookup_variables(const interned_string& id) const
 {
   exported_decls_builder_sptr b = get_exported_decls_builder();
-  auto i = b->priv_->id_var_map_.find(id);
-  if (i == b->priv_->id_var_map_.end())
+  auto i = b->priv_->id_vars_map_.find(id);
+  if (i == b->priv_->id_vars_map_.end())
     return nullptr;
-  return i->second;
+  return &i->second;
+}
+
+/// Lookup the exported variables which all have a given variable ID.
+///
+/// @param id the ID of the variable to look up.
+///
+/// @return a pointer to the set of variables with ID @p id, or
+/// nullptr if no variable was found with that ID.
+const std::unordered_set<var_decl_sptr>*
+corpus::lookup_variables(const char* id) const
+{
+  if (!id)
+    return nullptr;
+
+  interned_string string_id = priv_->env.intern(id);
+  return lookup_variables(string_id);
 }
 
 /// Sort the set of functions exported by this corpus.

tools/abicompat.cc

@@ -542,27 +542,30 @@ compare_expected_against_provided_variables(diff_context_sptr&		ctxt,
     {
       interned_string var_id = expected_var->get_id();
       // ... against the variables exported by the library!
-      const var_decl_sptr exported_var =
+      const std::unordered_set<var_decl_sptr>* exported_vars =
 	reverse_direction
-	? app_corpus->lookup_variable(var_id)
-	: lib_corpus->lookup_variable(var_id);
-      if (exported_var)
+	? app_corpus->lookup_variables(var_id)
+	: lib_corpus->lookup_variables(var_id);
+      if (exported_vars)
 	{
-	  // OK here is where we compare the variable expected by
-	  // the application against the variable exported by the
-	  // library.
-	  diff_sptr type_diff =
-	    compute_diff(expected_var->get_type(),
-			 exported_var->get_type(),
-			 ctxt);
-	  if (type_diff && type_diff->to_be_reported())
+	  for (auto exported_var : *exported_vars)
 	    {
-	      // So there is a type change between the variable
-	      // expected by the application and the variable
-	      // exported by the library.  Let's record that
-	      // change so that we can report about it later.
-	      var_changes.push_back(var_change(expected_var, type_diff, reverse_direction));
-	      status |= abigail::tools_utils::ABIDIFF_ABI_CHANGE;
+	      // OK here is where we compare the variable expected by
+	      // the application against the variable exported by the
+	      // library.
+	      diff_sptr type_diff =
+		compute_diff(expected_var->get_type(),
+			     exported_var->get_type(),
+			     ctxt);
+	      if (type_diff && type_diff->to_be_reported())
+		{
+		  // So there is a type change between the variable
+		  // expected by the application and the variable
+		  // exported by the library.  Let's record that
+		  // change so that we can report about it later.
+		  var_changes.push_back(var_change(expected_var, type_diff, reverse_direction));
+		  status |= abigail::tools_utils::ABIDIFF_ABI_CHANGE;
+		}
 	    }
 	}
     }