[PATCH] Bug 27233 - fedabipkgdiff fails on package gnupg2 from Fedora 33

[PATCH] Bug 27233 - fedabipkgdiff fails on package gnupg2 from Fedora 33

[Dodji Seketeli]

Hello,

At the core of this issue, Libabigail is failing to canonicalize some
types in some cases.  And that is triggering the assertion at the end
of hash_as_canonical_type_or_constant when emitting ABIXML.

It turns out read_context::canonicalize_types_scheduled in the dwarf
reader sometimes fails to canonicalize the types contained in
read_context::extra_types_to_canonicalize().

This patch fixes that.

Incidentally, this patch also fixes a previous issue where
hash_as_canonical_type_or_constant would hit that assert at its end
because of non-canonicalized function types.  I am now removing the
band-aid I put in place at the time by loosening the assertion there.

	* src/abg-dwarf-reader.cc
	(read_context::canonicalize_types_scheduled): Don't forget to
	canonicalize types stored in extra_types_to_canonicalize_.
	* src/abg-ir.cc (type_base::get_canonical_type_for): Add better
	comment.
	(hash_as_canonical_type_or_constant): Remove crutch that is
	useless now that we canonicalize almost all types in the system.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 src/abg-dwarf-reader.cc |  3 ++-
 src/abg-ir.cc           | 12 +++++++++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index f95dbb98..099bff75 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -4895,7 +4895,8 @@ public:
 	cn_timer.start();
       }
 
-    if (!types_to_canonicalize(source).empty())
+    if (!types_to_canonicalize(source).empty()
+	|| !extra_types_to_canonicalize().empty())
       {
 	tools_utils::timer single_type_cn_timer;
 	size_t total = types_to_canonicalize(source).size();
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 6d7a60a6..d1d02f3a 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -12367,7 +12367,14 @@ type_base::get_canonical_type_for(type_base_sptr t)
 
   class_or_union_sptr class_or_union = is_class_or_union_type(t);
 
-  // Look through declaration-only classes
+  // Look through declaration-only classes when we are dealing with
+  // C++ or languages where we assume the "One Definition Rule".  In
+  // that context, we assume that a declaration-only non-anonymous
+  // class equals all fully defined classes of the same name.
+  //
+  // Otherwise, all classes, including declaration-only classes are
+  // canonicalized and only canonical comparison is going to be used
+  // in the system.
   if (decl_only_class_equals_definition)
     if (class_or_union)
       {
@@ -23419,8 +23426,7 @@ hash_as_canonical_type_or_constant(const type_base *t)
   // non-canonicalized type.  It must be a decl-only class or a
   // function type, otherwise it means that for some weird reason, the
   // type hasn't been canonicalized.  It should be!
-  ABG_ASSERT(is_declaration_only_class_or_union_type(t)
-	     || is_function_type(t));
+  ABG_ASSERT(is_declaration_only_class_or_union_type(t));
 
   return 0xDEADBABE;
 }
-- 
2.30.0


-- 
		Dodji