[PATCH, applied] PR28365 - Assert on empty typedef on webkit2gtk3-jsc-2.32.3-1.fc34.x86_64

[PATCH, applied] PR28365 - Assert on empty typedef on webkit2gtk3-jsc-2.32.3-1.fc34.x86_64

[Dodji Seketeli]

Hello,

When doing self-comparison check of
/usr/lib64/libjavascriptcoregtk-4.0.so.18.18.7 from
webkit2gtk3-jsc-2.32.3-1.fc34.x86_64, reading back the abixml file
fails because an empty typedef is used as the element type of an
array.

The empty typedef is there (in a transient manner) because the typedef
is being built.  First an empty typedef is built and then its
underlying type is built.  During the construction of the underlying
type however (an enum), the empty typedef itself is used (as the
naming typedef of the enum).  But because its empty, an assert is
violated during the construction of an array which element type is the
(empty) typedef.  A snake eating its own (half-baked) tail, so to
speak.

The patch fixes the issue by constructing the underlying (enum) type
first.  Once its constructed, then it's used to construct the typedef
which is thus never empty, even in a transient manner.

The patch adjusts the building of enums so that the naming typedef is
built only once the enum itself is fully constructed.  This breaks the
vicious cycle exposed above.

The offending RPM is too big to be added to the test suite.  Which
argues (yet again) for the implementation of a separate test suite
that runs libabigail tests on a huge pile of RPMs without having to
embed them in the tarball.  We really ought to start that project.

	* src/abg-reader.cc (build_enum_type_decl): Set the naming typedef
	only after the enum is created and keyed.
	(build_typedef_decl): Build the underlying type of the typedef
	first.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Applied to master
---
 src/abg-reader.cc | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 712b0c1a..bf08b2e7 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -4470,9 +4470,9 @@ build_enum_type_decl(read_context&	ctxt,
   t->set_is_anonymous(is_anonymous);
   t->set_is_artificial(is_artificial);
   t->set_is_declaration_only(is_decl_only);
-  maybe_set_naming_typedef(ctxt, node, t);
   if (ctxt.push_and_key_type_decl(t, id, add_to_current_scope))
     {
+      maybe_set_naming_typedef(ctxt, node, t);
       ctxt.map_xml_node_to_decl(node, t);
       return t;
     }
@@ -4529,19 +4529,14 @@ build_typedef_decl(read_context&	ctxt,
     type_id = CHAR_STR(s);
   ABG_ASSERT(!type_id.empty());
 
-  // Create the typedef type /before/ the underlying type.  After the
-  // creation, the type is 'keyed' using ctxt.push_and_key_type_decl.
-  // This means that the type can be retrieved from its type ID.  This
-  // is so that if the underlying type indirectly (needs to) use(s)
-  // this very same typedef type (via recursion) then that is made
-  // possible.
-  typedef_decl_sptr t(new typedef_decl(name, ctxt.get_environment(), loc));
+  type_base_sptr underlying_type(ctxt.build_or_get_type_decl(type_id, true));
+  ABG_ASSERT(underlying_type);
+
+  typedef_decl_sptr t(new typedef_decl(name, underlying_type, loc));
   maybe_set_artificial_location(ctxt, node, t);
   ctxt.push_and_key_type_decl(t, id, add_to_current_scope);
   ctxt.map_xml_node_to_decl(node, t);
-  type_base_sptr underlying_type(ctxt.build_or_get_type_decl(type_id, true));
-  ABG_ASSERT(underlying_type);
-  t->set_underlying_type(underlying_type);
+
   return t;
 }
 
-- 
2.32.0


-- 
		Dodji