[PATCH 11/14] ir: Fix adding a canonical type to its homonym type group

Dodji Seketeli dodji@seketeli.org
Fri Sep 25 21:06:05 GMT 2026


When looking at some performance issues, I realized that
homonym_type_group::add_canonical_type could in theory duplicate
canonical types in its homonym_type_group::canonical_types_ data
members because it fails to update the
homonym_type_group::canonical_types_set_ data member.  This patch
fixes that.

The patch also uses the fixed homonym_type_group::add_canonical_type
in homonym_type_group::get_canonical_type_for, instead of poorly
open-coding the updating of homonym_type_group::canonical_types_.
Incidentally, this ensures that type_base::priv::group is updated by
homonym_type_group::get_canonical_type_for.

	* src/abg-ir-priv.h (homonym_type_group::get_canonical_type_for):
	Add a new homonym_type_group_sptr parameter.
	(homonym_type_group::canonicalize): Likewise.  Also, fix the
	comments and adjust the calls to get_canonical_type_for.
	* src/abg-ir.cc (add_canonical_type): Update the
	homonym_type_group::canonical_types_set_ data member.
	(homonym_type_group::get_canonical_type_for): Add assert.  Use
	add_canonical_type.  Fix comment.
	(canonicalize): Adjust call to
	homonym_type_group::get_canonical_type_for.

Signed-off-by: Dodji Seketeli <dodji@seketeli.org>
---
 src/abg-ir-priv.h | 25 ++++++++++++-------------
 src/abg-ir.cc     | 18 ++++++++++++++----
 2 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/src/abg-ir-priv.h b/src/abg-ir-priv.h
index f04fdfe9..ed453c58 100644
--- a/src/abg-ir-priv.h
+++ b/src/abg-ir-priv.h
@@ -676,27 +676,26 @@ public:
   get_canonical_types() const
   {return canonical_types_;}
 
-  /// Get the canonical type for a given canonical type of the current
-  /// group.
-  ///
-  /// @param t the type to consider.
-  ///
-  /// @return the canonical type for t.
   type_base_sptr
-  get_canonical_type_for(type_base_sptr t);
+  get_canonical_type_for(type_base_sptr t, homonym_type_group_sptr thiz);
 
-  /// @brief Canonicalize all types in this homonym type group.
+  /// @brief Canonicalize a given type that belongs to the current
+  /// homonym type group.
+  ///
+  /// @param t the type to canonicalize.  The type must belong to the
+  /// current homonym type group.
   ///
-  /// Iterate over all types in the homonym type group and compute
-  /// their canonical type.  Types that are structurally equivalent
-  /// will share the same canonical type.
+  /// @param this_group a shared pointer to the current @ref
+  /// homonym_type_group.  This is useful because the type will point
+  /// back to that group.
   ///
   /// @param do_log if true, log canonicalization activity to stderr.
   ///
   /// @param show_stats if true, show timing statistics for each type
   /// canonicalization.
   type_base_sptr
-  canonicalize(type_base_sptr t, bool do_log, bool show_stats)
+  canonicalize(type_base_sptr t, homonym_type_group_sptr this_group,
+	       bool do_log, bool show_stats)
   {
     if (!t)
       return t;
@@ -717,7 +716,7 @@ public:
 
     if (do_log && show_stats)
       tmr.start();
-    type_base_sptr canonical = get_canonical_type_for(t);
+    type_base_sptr canonical = get_canonical_type_for(t, this_group);
 
     if (do_log && show_stats)
       tmr.stop();
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index a97e77cd..2d300ca1 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -4019,6 +4019,7 @@ add_canonical_type(type_base_sptr t, homonym_type_group_sptr group)
 {
   if (group->canonical_types_set_.find(t) != group->types_set_.end())
     return;
+  group->canonical_types_set_.insert(t);
   group->canonical_types_.push_back(t);
   t->priv_->group = group;
 }
@@ -4043,10 +4044,17 @@ add_canonical_type(type_base_sptr t, homonym_type_group_sptr group)
 /// @param t a smart pointer to instance of @ref type_base we want to
 /// compute a canonical type for.
 ///
+/// @param this_group this a shared_ptr to the current instance of
+/// @ref homonym_type_group.  This is used as an argument for
+/// homonym_type_group::add_canonical_type.
+///
 /// @return the canonical type for @p t.
 type_base_sptr
-homonym_type_group::get_canonical_type_for(type_base_sptr t)
+homonym_type_group::get_canonical_type_for(type_base_sptr t,
+					   homonym_type_group_sptr this_group)
 {
+  ABG_ASSERT(this_group.get() == this);
+
   if (!t)
     return t;
 
@@ -4221,7 +4229,9 @@ homonym_type_group::get_canonical_type_for(type_base_sptr t)
 				       canonical_type_index))
 	t->priv_->canonical_type_index = canonical_type_index;
     }
-  canonical_types_.push_back(result);
+
+  add_canonical_type(result, this_group);
+
   return result;
 }
 
@@ -16886,7 +16896,7 @@ maybe_adjust_canonical_type(const type_base_sptr& canonical,
 
 /// Compute the canonical type of a given type.
 ///
-/// It means that after invoking this function, comparing the intance
+/// It means that after invoking this function, comparing the instance
 /// instance @ref type_base and another one (on which
 /// type_base::enable_canonical_equality() would have been invoked as
 /// well) is performed by just comparing the pointer values of the
@@ -16931,7 +16941,7 @@ canonicalize(type_base_sptr t,
   if (do_log && show_stats)
     tmr.start();
 
-  type_base_sptr canonical = group->get_canonical_type_for(t);
+  type_base_sptr canonical = group->get_canonical_type_for(t, group);
 
   if (do_log && show_stats)
     tmr.stop();
-- 
2.55.0



More information about the Libabigail mailing list