Re: [PATCH] XML reader: improve XML node traversal

[Matthias Maennich <maennich@google.com>]

On Wed, Mar 31, 2021 at 10:23:51AM +0100, Giuliano Procida wrote:
>Bug 27616: libabigail treats empty and nearly empty XML elements inconsistently
>
>The XML reader uses libxml's Reader API for parsing XML and traverses
>high-level XML nodes using reader cursor advancement. Low-level nodes
>are read into complete node trees and are traversed following
>children, next and (in one instance) parent pointers. Some
>intermediate-level nodes can be parsed both ways.
>
>A lot of code cares about node types, typically skipping over
>non-element nodes. Unfortunately, in a few places, the tracked "corpus
>node" needs to land on text (whitespace) within elements for the XML
>reader to work; stripping text nodes out causes some elements to be
>skipped. Almost all node tree traversals are very straightforward and
>can be expressed using simpler primitves than at present.
>
>This commit changes all the node tree traversals to use two node
>iteration primitives consistently. These are "first_child" and
>"next_child", both of which automatically skip text nodes. They
>replace the existing "advance_to_next_sibling_element" and *all*
>direct references to "children" and "next" nodes. The unwanted
>dependency on text within elements has been fixed.
>
>For-loops constructed with the new primitives present only element
>nodes to the loop body. This allows a lot of redundant checking for
>null / non-element nodes to be removed.
>
>These changes have been tested by using abidiff to comparing all
>current test XML files with copies where all text has been removed.
>One such test case is included in this commit.
>
>	* include/abg-libxml-utils.h
>	(advance_to_next_sibling_element): Remove this function.
>	(first_child): Add new function. (next_child): Likewise. This
>	is simpler than but functionally identical to
>	advance_to_next_sibling_element.
>	* src/abg-libxml-utils.cc (skip_non_elements): New function to
>	skip non-element nodes in a sibling node list. (first_child):
>	New function to find the first child element of a node.
>	(next_child): New function to find the next sibling element of
>	a node.
>	* src/abg-reader.cc (walk_xml_node_to_map_type_ids): Remove
>	redundant node checks. Iterate over children with first_child
>	and next_child helpers. (read_translation_unit): Likewise.
>	(read_translation_unit_from_input): Remove a no-longer-needed
>	->next and redundant node checks. Advance "corpus node" after
>	reading the translation unit. (read_symbol_db_from_input):
>	Remove a no-longer-needed ->next and advance "corpus node" as
>	part of for-loop.  Remove redundant node checks.
>	(build_needed): Remove redundant node checks. Iterate over
>	children with first_child and next_child.
>	(read_elf_needed_from_input): Remove redundant node checks and
>	simplify logic. Move to next element node after reading
>	elf-needed node. (read_corpus_from_input): Remove no-children
>	early return (which was inhibited by text nodes anyway). Set
>	initial "corpus node" using first_child instead of children
>	pointer. Remove redundant "corpus node" updates at end of
>	function as the caller takes care of this and the node can
>	become null with the new primitives.
>	(read_corpus_group_from_input): Iterate over children using
>	first_child and next child. Set "corpus node" for each corpus,
>	not just the first. Incidentally keep going even if one corpus
>	parse fails. (build_namespace_decl): Remove redundant node
>	checks. Iterate over children using first_child and
>	next_child. (build_elf_symbol): Remove redundant node checks.
>	(build_elf_symbol_db): Iterate over children using first_child
>	and next_child. (build_function_decl): Remove redundant node
>	checks. Iterate over children using first_child and
>	next_child. (build_function_type): Likewise.
>	(build_array_type_def): Likewise. (build_enum_type_decl):
>	Likewise. (build_class_decl): Likewise. (build_union_decl):
>	Likewise. (build_function_tdecl): Likewise.
>	(build_type_composition): Likewise.
>	(build_template_tparameter): Likewise.
>	(tests/data/Makefile.am): Add new test files.
>	(tests/data/test-abidiff-exit/test-squished-report.txt): Add
>	empty diff report.
>	(tests/data/test-abidiff-exit/test-squished-v0.abi): Add test
>	ABI file, containing all ELF elements.
>	(tests/data/test-abidiff-exit/test-squished-v1.abi): Likewise
>	and identical, except that all text has been stripped.
>	(tests/test-abidiff-exit.cc): Add new test case.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>

That is a very nice simplification that comes with this fix! Thanks!

Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>---
> include/abg-libxml-utils.h                    |   5 +-
> src/abg-libxml-utils.cc                       |  47 ++--
> src/abg-reader.cc                             | 208 +++++-------------
> tests/data/Makefile.am                        |   3 +
> .../test-squished-report.txt                  |   0
> .../test-abidiff-exit/test-squished-v0.abi    |  43 ++++
> .../test-abidiff-exit/test-squished-v1.abi    |   1 +
> tests/test-abidiff-exit.cc                    |  11 +
> 8 files changed, 136 insertions(+), 182 deletions(-)
> create mode 100644 tests/data/test-abidiff-exit/test-squished-report.txt
> create mode 100644 tests/data/test-abidiff-exit/test-squished-v0.abi
> create mode 100644 tests/data/test-abidiff-exit/test-squished-v1.abi
>
>diff --git a/include/abg-libxml-utils.h b/include/abg-libxml-utils.h
>index 69e940f6..f87ee5a0 100644
>--- a/include/abg-libxml-utils.h
>+++ b/include/abg-libxml-utils.h
>@@ -82,7 +82,10 @@ int get_xml_node_depth(xmlNodePtr);
>   reinterpret_cast<char*>(xml_char_str.get())
>
> xmlNodePtr
>-advance_to_next_sibling_element(xmlNodePtr node);
>+first_child(xmlNodePtr node);
>+
>+xmlNodePtr
>+next_child(xmlNodePtr node);
>
> void
> escape_xml_string(const std::string& str,
>diff --git a/src/abg-libxml-utils.cc b/src/abg-libxml-utils.cc
>index a1acff1f..baea25ac 100644
>--- a/src/abg-libxml-utils.cc
>+++ b/src/abg-libxml-utils.cc
>@@ -413,40 +413,39 @@ unescape_xml_comment(const std::string& str)
>   return result;
> }
>
>-/// Maybe get the next sibling element node of an XML node, or stay to the sam
>+/// Skip until we reach an XML element node or run out of (child) nodes.
> ///
>-/// If there is no next sibling xml element node, the function returns
>-/// the initial node.
>+/// @param a pointer to a node.
> ///
>-/// @param node the initial node to consider.
>-///
>-/// @return the next sibling node or the initial node @p node.
>-static xmlNodePtr
>-go_to_next_sibling_element_or_stay(xmlNodePtr node)
>+/// @return a pointer to an XML element node or nil.
>+xmlNodePtr
>+skip_non_elements(xmlNodePtr node)
> {
>-  xmlNodePtr n;
>-  for (n = node; n; n = n->next)
>-    {
>-      if (n->type == XML_ELEMENT_NODE)
>-	break;
>-    }
>-  return n ? n : node;
>+  while (node && node->type != XML_ELEMENT_NODE)
>+    node = node->next;
>+  return node;
> }
>
>-/// Get the next sibling element node of an XML node.
>+/// Get the first child element of an XML node.
> ///
>-/// If there is no next sibling xml element node, the function returns nil.
>+/// @param a pointer to the node whose children are to be perused.
> ///
>-/// @param node the XML node to consider.
>+/// @return a pointer to the first XML element child of @p node or nil.
>+xmlNodePtr
>+first_child(xmlNodePtr node)
>+{
>+  return skip_non_elements(node->children);
>+}
>+
>+/// Get the next sibling element of an XML node.
> ///
>-/// @return the next sibling element node or nil.
>+/// @param a pointer to the XML node whose following siblings are to be perused.
>+///
>+/// @return a pointer to the next sibling element of @p node or nil.
> xmlNodePtr
>-advance_to_next_sibling_element(xmlNodePtr node)
>+next_child(xmlNodePtr node)
> {
>-  xmlNodePtr n = go_to_next_sibling_element_or_stay(node->next);
>-  if (n == 0 || n->type != XML_ELEMENT_NODE)
>-    return 0;
>-  return n;
>+  return skip_non_elements(node->next);
> }
>
> }//end namespace xml
>diff --git a/src/abg-reader.cc b/src/abg-reader.cc
>index 3e552864..a143eb58 100644
>--- a/src/abg-reader.cc
>+++ b/src/abg-reader.cc
>@@ -1343,18 +1343,13 @@ static void
> walk_xml_node_to_map_type_ids(read_context& ctxt,
> 			      xmlNodePtr node)
> {
>-  xmlNodePtr n = node;
>-
>-  if (!n || n->type != XML_ELEMENT_NODE)
>-    return;
>-
>-  if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(n, "id"))
>+  if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "id"))
>     {
>       string id = CHAR_STR(s);
>-      ctxt.map_id_and_node(id, n);
>+      ctxt.map_id_and_node(id, node);
>     }
>
>-  for (n = n->children; n; n = n->next)
>+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
>     walk_xml_node_to_map_type_ids(ctxt, n);
> }
>
>@@ -1396,12 +1391,8 @@ read_translation_unit(read_context& ctxt, translation_unit& tu, xmlNodePtr node)
>       || !ctxt.get_corpus())
>     walk_xml_node_to_map_type_ids(ctxt, node);
>
>-  for (xmlNodePtr n = node->children; n; n = n->next)
>-    {
>-      if (n->type != XML_ELEMENT_NODE)
>-	continue;
>-      handle_element_node(ctxt, n, /*add_decl_to_scope=*/true);
>-    }
>+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
>+    handle_element_node(ctxt, n, /*add_decl_to_scope=*/true);
>
>   ctxt.pop_scope_or_abort(tu.get_global_scope());
>
>@@ -1495,16 +1486,9 @@ read_translation_unit_from_input(read_context&	ctxt)
>   else
>     {
>       node = 0;
>-      for (xmlNodePtr n = ctxt.get_corpus_node()->next; n; n = n->next)
>-	{
>-	  if (!n
>-	      || n->type != XML_ELEMENT_NODE)
>-	    continue;
>-	  if (!xmlStrEqual(n->name, BAD_CAST("abi-instr")))
>-	    return nil;
>-	  node = n;
>-	  break;
>-	}
>+      xmlNodePtr n = ctxt.get_corpus_node();
>+      if (n && xmlStrEqual(n->name, BAD_CAST("abi-instr")))
>+	node = n;
>     }
>
>   if (node == 0)
>@@ -1517,9 +1501,9 @@ read_translation_unit_from_input(read_context&	ctxt)
>   // case, after that unexpected call to read_translation_unit(), the
>   // current corpus node of the context is going to point to that
>   // translation unit that has been read under the hood.  Let's set
>-  // the corpus node to the one we initially called
>+  // the corpus node to just after the one we initially called
>   // read_translation_unit() on here.
>-  ctxt.set_corpus_node(node);
>+  ctxt.set_corpus_node(xml::next_child(node));
>   return tu;
> }
>
>@@ -1593,11 +1577,8 @@ read_symbol_db_from_input(read_context&		 ctxt,
> 	xmlTextReaderNext(reader.get());
>       }
>   else
>-    for (xmlNodePtr n = ctxt.get_corpus_node()->next; n; n = n->next)
>+    for (xmlNodePtr n = ctxt.get_corpus_node(); n; n = xml::next_child(n), ctxt.set_corpus_node(n))

That line looks a little long.

>       {
>-	if (!n || n->type != XML_ELEMENT_NODE)
>-	  continue;
>-
> 	bool has_fn_syms = false, has_var_syms = false;
> 	if (xmlStrEqual(n->name, BAD_CAST("elf-function-symbols")))
> 	  has_fn_syms = true;
>@@ -1605,7 +1586,6 @@ read_symbol_db_from_input(read_context&		 ctxt,
> 	  has_var_syms = true;
> 	else
> 	  break;
>-	ctxt.set_corpus_node(n);
> 	if (has_fn_syms)
> 	  {
> 	    fn_symdb = build_elf_symbol_db(ctxt, n, true);
>@@ -1636,16 +1616,12 @@ read_symbol_db_from_input(read_context&		 ctxt,
> static bool
> build_needed(xmlNode* node, vector<string>& needed)
> {
>-  if (!node)
>-    return false;
>-
>-  if (!node || !xmlStrEqual(node->name,BAD_CAST("elf-needed")))
>+  if (!xmlStrEqual(node->name,BAD_CAST("elf-needed")))
>     return false;
>
>-  for (xmlNodePtr n = node->children; n; n = n->next)
>+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
>     {
>-      if (n->type != XML_ELEMENT_NODE
>-	  || !xmlStrEqual(n->name, BAD_CAST("dependency")))
>+      if (!xmlStrEqual(n->name, BAD_CAST("dependency")))
> 	continue;
>
>       string name;
>@@ -1693,27 +1669,19 @@ read_elf_needed_from_input(read_context&	ctxt,
> 	return false;
>
>       node = xmlTextReaderExpand(reader.get());
>-      if (!node)
>-	return false;
>     }
>   else
>     {
>-      for (xmlNodePtr n = ctxt.get_corpus_node()->next; n; n = n->next)
>-	{
>-	  if (!n || n->type != XML_ELEMENT_NODE)
>-	    continue;
>-	  if (!xmlStrEqual(n->name, BAD_CAST("elf-needed")))
>-	    return false;
>-	  node = n;
>-	  break;
>-	}
>+      xmlNodePtr n = ctxt.get_corpus_node();
>+      if (n && xmlStrEqual(n->name, BAD_CAST("elf-needed")))
>+	node = n;
>     }
>
>   bool result = false;
>   if (node)
>     {
>       result = build_needed(node, needed);
>-      ctxt.set_corpus_node(node);
>+      ctxt.set_corpus_node(xml::next_child(node));
>     }
>
>   return result;
>@@ -1917,10 +1885,7 @@ read_corpus_from_input(read_context& ctxt)
> 	corp.set_soname(reinterpret_cast<char*>(soname_str.get()));
>     }
>
>-  if (!node->children)
>-    return nil;
>-
>-  ctxt.set_corpus_node(node->children);
>+  ctxt.set_corpus_node(xml::first_child(node));
>
>   corpus& corp = *ctxt.get_corpus();
>
>@@ -1986,17 +1951,6 @@ read_corpus_from_input(read_context& ctxt)
>       // call at the beginning of the function.
>       xmlTextReaderNext(reader.get());
>     }
>-  else
>-    {
>-      node = ctxt.get_corpus_node();
>-      node = xml::advance_to_next_sibling_element(node);
>-      if (!node)
>-	{
>-	  node = ctxt.get_corpus_node();
>-	  node = xml::advance_to_next_sibling_element(node->parent);
>-	}
>-      ctxt.set_corpus_node(node);
>-    }
>
>   return ctxt.get_corpus();
> }
>@@ -2046,13 +2000,13 @@ read_corpus_group_from_input(read_context& ctxt)
>   if (!node)
>     return nil;
>
>-  //node = xml::get_first_element_sibling_if_text(node->children);
>-  node = xml::advance_to_next_sibling_element(node->children);
>-  ctxt.set_corpus_node(node);
>-
>-  corpus_sptr corp;
>-  while ((corp = read_corpus_from_input(ctxt)))
>-    ctxt.get_corpus_group()->add_corpus(corp);
>+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
>+    {
>+      ctxt.set_corpus_node(n);
>+      corpus_sptr corp = read_corpus_from_input(ctxt);
>+      if (corp)
>+	ctxt.get_corpus_group()->add_corpus(corp);
>+    }
>
>   xmlTextReaderNext(reader.get());
>
>@@ -2738,12 +2692,8 @@ build_namespace_decl(read_context&	ctxt,
>   ctxt.push_decl_to_current_scope(decl, add_to_current_scope);
>   ctxt.map_xml_node_to_decl(node, decl);
>
>-  for (xmlNodePtr n = node->children; n; n = n->next)
>-    {
>-      if (n->type != XML_ELEMENT_NODE)
>-	continue;
>-      handle_element_node(ctxt, n, /*add_to_current_scope=*/true);
>-    }
>+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
>+    handle_element_node(ctxt, n, /*add_to_current_scope=*/true);
>
>   ctxt.pop_scope_or_abort(decl);
>
>@@ -2767,9 +2717,7 @@ build_elf_symbol(read_context& ctxt, const xmlNodePtr node,
> {
>   elf_symbol_sptr nil;
>
>-  if (!node
>-      || node->type != XML_ELEMENT_NODE
>-      || !xmlStrEqual(node->name, BAD_CAST("elf-symbol")))
>+  if (!xmlStrEqual(node->name, BAD_CAST("elf-symbol")))
>     return nil;
>
>   string name;
>@@ -2928,7 +2876,7 @@ build_elf_symbol_db(read_context& ctxt,
>   xml_node_ptr_elf_symbol_sptr_map_type xml_node_ptr_elf_symbol_map;
>
>   elf_symbol_sptr sym;
>-  for (xmlNodePtr n = node->children; n; n = n->next)
>+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
>     {
>       if ((sym = build_elf_symbol(ctxt, n, /*drop_if_suppress=*/true)))
> 	{
>@@ -3094,12 +3042,9 @@ build_function_decl(read_context&	ctxt,
>   std::vector<function_decl::parameter_sptr> parms;
>   type_base_sptr return_type = env->get_void_type();
>
>-  for (xmlNodePtr n = node->children; n ; n = n->next)
>+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
>     {
>-      if (n->type != XML_ELEMENT_NODE)
>-	continue;
>-
>-      else if (xmlStrEqual(n->name, BAD_CAST("parameter")))
>+      if (xmlStrEqual(n->name, BAD_CAST("parameter")))
> 	{
> 	  if (function_decl::parameter_sptr p =
> 	      build_function_parameter(ctxt, n))
>@@ -3789,12 +3734,9 @@ build_function_type(read_context&	ctxt,
>   ctxt.get_translation_unit()->bind_function_type_life_time(fn_type);
>   ctxt.key_type_decl(fn_type, id);
>
>-  for (xmlNodePtr n = node->children; n ; n = n->next)
>+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
>     {
>-      if (n->type != XML_ELEMENT_NODE)
>-	continue;
>-
>-      else if (xmlStrEqual(n->name, BAD_CAST("parameter")))
>+      if (xmlStrEqual(n->name, BAD_CAST("parameter")))
> 	{
> 	  if (function_decl::parameter_sptr p =
> 	      build_function_parameter(ctxt, n))
>@@ -4025,12 +3967,9 @@ build_array_type_def(read_context&	ctxt,
>   read_location(ctxt, node, loc);
>   array_type_def::subranges_type subranges;
>
>-  for (xmlNodePtr n = node->children; n ; n = n->next)
>+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
>     {
>-      if (n->type != XML_ELEMENT_NODE)
>-	continue;
>-
>-      else if (xmlStrEqual(n->name, BAD_CAST("subrange")))
>+      if (xmlStrEqual(n->name, BAD_CAST("subrange")))
> 	{
> 	  if (array_type_def::subrange_sptr s =
> 	      build_subrange_type(ctxt, n))
>@@ -4183,11 +4122,8 @@ build_enum_type_decl(read_context&	ctxt,
>
>   string base_type_id;
>   enum_type_decl::enumerators enums;
>-  for (xmlNodePtr n = node->children; n; n = n->next)
>+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
>     {
>-      if (n->type != XML_ELEMENT_NODE)
>-	continue;
>-
>       if (xmlStrEqual(n->name, BAD_CAST("underlying-type")))
> 	{
> 	  xml_char_sptr a = xml::build_sptr(xmlGetProp(n, BAD_CAST("type-id")));
>@@ -4554,11 +4490,8 @@ build_class_decl(read_context&		ctxt,
>       decl->set_naming_typedef(naming_typedef);
>     }
>
>-  for (xmlNodePtr n = node->children; !is_decl_only && n; n = n->next)
>+  for (xmlNodePtr n = xml::first_child(node); !is_decl_only && n; n = xml::next_child(n))

Likewise.

>     {
>-      if (n->type != XML_ELEMENT_NODE)
>-	continue;
>-
>       if (xmlStrEqual(n->name, BAD_CAST("base-class")))
> 	{
> 	  access_specifier access =
>@@ -4605,11 +4538,8 @@ build_class_decl(read_context&		ctxt,
>
> 	  ctxt.map_xml_node_to_decl(n, decl);
>
>-	  for (xmlNodePtr p = n->children; p; p = p->next)
>+	  for (xmlNodePtr p = xml::first_child(n); p; p = xml::next_child(p))
> 	    {
>-	      if (p->type != XML_ELEMENT_NODE)
>-		continue;
>-
> 	      if (type_base_sptr t =
> 		  build_type(ctxt, p, /*add_to_current_scope=*/true))
> 		{
>@@ -4643,11 +4573,8 @@ build_class_decl(read_context&		ctxt,
> 	  bool is_static = false;
> 	  read_static(n, is_static);
>
>-	  for (xmlNodePtr p = n->children; p; p = p->next)
>+	  for (xmlNodePtr p = xml::first_child(n); p; p = xml::next_child(p))
> 	    {
>-	      if (p->type != XML_ELEMENT_NODE)
>-		continue;
>-
> 	      if (var_decl_sptr v =
> 		  build_var_decl(ctxt, p, /*add_to_cur_scope=*/false))
> 		{
>@@ -4701,11 +4628,8 @@ build_class_decl(read_context&		ctxt,
> 	  bool is_ctor = false, is_dtor = false, is_const = false;
> 	  read_cdtor_const(n, is_ctor, is_dtor, is_const);
>
>-	  for (xmlNodePtr p = n->children; p; p = p->next)
>+	  for (xmlNodePtr p = xml::first_child(n); p; p = xml::next_child(p))
> 	    {
>-	      if (p->type != XML_ELEMENT_NODE)
>-		continue;
>-
> 	      if (function_decl_sptr f =
> 		  build_function_decl_if_not_suppressed(ctxt, p, decl,
> 							/*add_to_cur_sc=*/true))
>@@ -4740,11 +4664,8 @@ build_class_decl(read_context&		ctxt,
> 	  bool is_ctor = false, is_dtor = false, is_const = false;
> 	  read_cdtor_const(n, is_ctor, is_dtor, is_const);
>
>-	  for (xmlNodePtr p = n->children; p; p = p->next)
>+	  for (xmlNodePtr p = xml::first_child(n); p; p = xml::next_child(p))
> 	    {
>-	      if (p->type != XML_ELEMENT_NODE)
>-		continue;
>-
> 	      if (shared_ptr<function_tdecl> f =
> 		  build_function_tdecl(ctxt, p,
> 				       /*add_to_current_scope=*/true))
>@@ -4954,11 +4875,8 @@ build_union_decl(read_context& ctxt,
>   ctxt.map_xml_node_to_decl(node, decl);
>   ctxt.key_type_decl(decl, id);
>
>-  for (xmlNodePtr n = node->children; !is_decl_only && n; n = n->next)
>+  for (xmlNodePtr n = xml::first_child(node); !is_decl_only && n; n = xml::next_child(n))

Likewise.

>     {
>-      if (n->type != XML_ELEMENT_NODE)
>-	continue;
>-
>       if (xmlStrEqual(n->name, BAD_CAST("member-type")))
> 	{
> 	  access_specifier access = private_access;
>@@ -4966,11 +4884,8 @@ build_union_decl(read_context& ctxt,
>
> 	  ctxt.map_xml_node_to_decl(n, decl);
>
>-	  for (xmlNodePtr p = n->children; p; p = p->next)
>+	  for (xmlNodePtr p = xml::first_child(n); p; p = xml::next_child(p))
> 	    {
>-	      if (p->type != XML_ELEMENT_NODE)
>-		continue;
>-
> 	      if (type_base_sptr t =
> 		  build_type(ctxt, p, /*add_to_current_scope=*/true))
> 		{
>@@ -4998,11 +4913,8 @@ build_union_decl(read_context& ctxt,
> 	  bool is_static = false;
> 	  read_static(n, is_static);
>
>-	  for (xmlNodePtr p = n->children; p; p = p->next)
>+	  for (xmlNodePtr p = xml::first_child(n); p; p = xml::next_child(p))
> 	    {
>-	      if (p->type != XML_ELEMENT_NODE)
>-		continue;
>-
> 	      if (var_decl_sptr v =
> 		  build_var_decl(ctxt, p, /*add_to_cur_scope=*/false))
> 		{
>@@ -5040,11 +4952,8 @@ build_union_decl(read_context& ctxt,
> 	  bool is_ctor = false, is_dtor = false, is_const = false;
> 	  read_cdtor_const(n, is_ctor, is_dtor, is_const);
>
>-	  for (xmlNodePtr p = n->children; p; p = p->next)
>+	  for (xmlNodePtr p = xml::first_child(n); p; p = xml::next_child(p))
> 	    {
>-	      if (p->type != XML_ELEMENT_NODE)
>-		continue;
>-
> 	      if (function_decl_sptr f =
> 		  build_function_decl_if_not_suppressed(ctxt, p, decl,
> 							/*add_to_cur_sc=*/true))
>@@ -5073,11 +4982,8 @@ build_union_decl(read_context& ctxt,
> 	  bool is_ctor = false, is_dtor = false, is_const = false;
> 	  read_cdtor_const(n, is_ctor, is_dtor, is_const);
>
>-	  for (xmlNodePtr p = n->children; p; p = p->next)
>+	  for (xmlNodePtr p = xml::first_child(n); p; p = xml::next_child(p))
> 	    {
>-	      if (p->type != XML_ELEMENT_NODE)
>-		continue;
>-
> 	      if (function_tdecl_sptr f =
> 		  build_function_tdecl(ctxt, p,
> 				       /*add_to_current_scope=*/true))
>@@ -5152,11 +5058,8 @@ build_function_tdecl(read_context& ctxt,
>   ctxt.push_decl_to_current_scope(fn_tmpl_decl, add_to_current_scope);
>
>   unsigned parm_index = 0;
>-  for (xmlNodePtr n = node->children; n; n = n->next)
>+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
>     {
>-      if (n->type != XML_ELEMENT_NODE)
>-	continue;
>-
>       if (template_parameter_sptr parm =
> 	  build_template_parameter(ctxt, n, parm_index, fn_tmpl_decl))
> 	{
>@@ -5216,11 +5119,8 @@ build_class_tdecl(read_context&	ctxt,
>   ctxt.push_decl_to_current_scope(class_tmpl, add_to_current_scope);
>
>   unsigned parm_index = 0;
>-  for (xmlNodePtr n = node->children; n; n = n->next)
>+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
>     {
>-      if (n->type != XML_ELEMENT_NODE)
>-	continue;
>-
>       if (template_parameter_sptr parm=
> 	  build_template_parameter(ctxt, n, parm_index, class_tmpl))
> 	{
>@@ -5335,11 +5235,8 @@ build_type_composition(read_context&		ctxt,
>   ctxt.push_decl_to_current_scope(dynamic_pointer_cast<decl_base>(result),
> 				  /*add_to_current_scope=*/true);
>
>-  for (xmlNodePtr n = node->children; n; n = n->next)
>+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
>     {
>-      if (n->type != XML_ELEMENT_NODE)
>-	continue;
>-
>       if ((composed_type =
> 	   build_pointer_type_def(ctxt, n,
> 				  /*add_to_current_scope=*/true))
>@@ -5465,11 +5362,8 @@ build_template_tparameter(read_context&	ctxt,
>
>   // Go parse template parameters that are children nodes
>   int parm_index = 0;
>-  for (xmlNodePtr n = node->children; n; n = n->next)
>+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
>     {
>-      if (n->type != XML_ELEMENT_NODE)
>-	continue;
>-
>       if (shared_ptr<template_parameter> p =
> 	  build_template_parameter(ctxt, n, parm_index, result))
> 	{
>diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
>index 4fb2d6a4..7bdae7bf 100644
>--- a/tests/data/Makefile.am
>+++ b/tests/data/Makefile.am
>@@ -197,6 +197,9 @@ test-abidiff-exit/test-non-leaf-array-v0.o \
> test-abidiff-exit/test-non-leaf-array-v1.c \
> test-abidiff-exit/test-non-leaf-array-v1.o \
> test-abidiff-exit/test-non-leaf-array-report.txt \
>+test-abidiff-exit/test-squished-v0.abi \
>+test-abidiff-exit/test-squished-v1.abi \
>+test-abidiff-exit/test-squished-report.txt \
> \
> test-diff-dwarf/test0-v0.cc		\
> test-diff-dwarf/test0-v0.o			\
>diff --git a/tests/data/test-abidiff-exit/test-squished-report.txt b/tests/data/test-abidiff-exit/test-squished-report.txt
>new file mode 100644
>index 00000000..e69de29b
>diff --git a/tests/data/test-abidiff-exit/test-squished-v0.abi b/tests/data/test-abidiff-exit/test-squished-v0.abi
>new file mode 100644
>index 00000000..6b3d0460
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-squished-v0.abi
>@@ -0,0 +1,43 @@
>+<abi-corpus version='2.0' path='data/test-read-dwarf/test6.so'>
>+  <elf-needed>
>+    <dependency name='libstdc++.so.6'/>
>+    <dependency name='libm.so.6'/>
>+    <dependency name='libgcc_s.so.1'/>
>+    <dependency name='libc.so.6'/>
>+  </elf-needed>
>+  <elf-function-symbols>
>+    <elf-symbol name='_Z3barv' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
>+    <elf-symbol name='_Z4blehv' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
>+    <elf-symbol name='_ZN1B3fooEv' type='func-type' binding='weak-binding' visibility='default-visibility' is-defined='yes'/>
>+    <elf-symbol name='_fini' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
>+    <elf-symbol name='_init' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
>+  </elf-function-symbols>
>+  <elf-variable-symbols>
>+    <elf-symbol name='_ZN1CIiE3barE' size='4' type='object-type' binding='gnu-unique-binding' visibility='default-visibility' is-defined='yes'/>
>+    <elf-symbol name='_ZZN1B3fooEvE1a' size='4' type='object-type' binding='gnu-unique-binding' visibility='default-visibility' is-defined='yes'/>
>+  </elf-variable-symbols>
>+  <abi-instr address-size='64' path='test6.cc' comp-dir-path='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf' language='LANG_C_plus_plus'>
>+    <type-decl name='int' size-in-bits='32' id='type-id-1'/>
>+    <class-decl name='B' size-in-bits='8' is-struct='yes' visibility='default' filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' line='9' column='1' id='type-id-2'>
>+      <member-function access='public'>
>+        <function-decl name='foo' mangled-name='_ZN1B3fooEv' filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' line='11' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='_ZN1B3fooEv'>
>+          <parameter type-id='type-id-3' name='this' is-artificial='yes'/>
>+          <return type-id='type-id-1'/>
>+        </function-decl>
>+      </member-function>
>+    </class-decl>
>+    <class-decl name='C&lt;int&gt;' size-in-bits='8' is-struct='yes' visibility='default' filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' line='26' column='1' id='type-id-4'>
>+      <data-member access='public' static='yes'>
>+        <var-decl name='bar' type-id='type-id-1' mangled-name='_ZN1CIiE3barE' visibility='default' filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' line='31' column='1' elf-symbol-id='_ZN1CIiE3barE'/>
>+      </data-member>
>+    </class-decl>
>+    <pointer-type-def type-id='type-id-2' size-in-bits='64' id='type-id-5'/>
>+    <qualified-type-def type-id='type-id-5' const='yes' id='type-id-3'/>
>+    <function-decl name='bar' mangled-name='_Z3barv' filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' line='19' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='_Z3barv'>
>+      <return type-id='type-id-1'/>
>+    </function-decl>
>+    <function-decl name='bleh' mangled-name='_Z4blehv' filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' line='34' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='_Z4blehv'>
>+      <return type-id='type-id-1'/>
>+    </function-decl>
>+  </abi-instr>
>+</abi-corpus>
>diff --git a/tests/data/test-abidiff-exit/test-squished-v1.abi b/tests/data/test-abidiff-exit/test-squished-v1.abi
>new file mode 100644
>index 00000000..3ffa272b
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-squished-v1.abi
>@@ -0,0 +1 @@
>+<abi-corpus version='2.0' path='data/test-read-dwarf/test6.so'><elf-needed><dependency name='libstdc++.so.6'/><dependency name='libm.so.6'/><dependency name='libgcc_s.so.1'/><dependency name='libc.so.6'/></elf-needed><elf-function-symbols><elf-symbol name='_Z3barv' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/><elf-symbol name='_Z4blehv' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/><elf-symbol name='_ZN1B3fooEv' type='func-type' binding='weak-binding' visibility='default-visibility' is-defined='yes'/><elf-symbol name='_fini' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/><elf-symbol name='_init' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/></elf-function-symbols><elf-variable-symbols><elf-symbol name='_ZN1CIiE3barE' size='4' type='object-type' binding='gnu-unique-binding' visibility='default-visibility' is-defined='yes'/><elf-symbol name='_ZZN1B3fooEvE1a' size='4' type='object-type' binding='gnu-unique-binding' visibility='default-visibility' is-defined='yes'/></elf-variable-symbols><abi-instr address-size='64' path='test6.cc' comp-dir-path='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf' language='LANG_C_plus_plus'><type-decl name='int' size-in-bits='32' id='type-id-1'/><class-decl name='B' size-in-bits='8' is-struct='yes' visibility='default' filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' line='9' column='1' id='type-id-2'><member-function access='public'><function-decl name='foo' mangled-name='_ZN1B3fooEv' filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' line='11' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='_ZN1B3fooEv'><parameter type-id='type-id-3' name='this' is-artificial='yes'/><return type-id='type-id-1'/></function-decl></member-function></class-decl><class-decl name='C&lt;int&gt;' size-in-bits='8' is-struct='yes' visibility='default' filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' line='26' column='1' id='type-id-4'><data-member access='public' static='yes'><var-decl name='bar' type-id='type-id-1' mangled-name='_ZN1CIiE3barE' visibility='default' filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' line='31' column='1' elf-symbol-id='_ZN1CIiE3barE'/></data-member></class-decl><pointer-type-def type-id='type-id-2' size-in-bits='64' id='type-id-5'/><qualified-type-def type-id='type-id-5' const='yes' id='type-id-3'/><function-decl name='bar' mangled-name='_Z3barv' filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' line='19' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='_Z3barv'><return type-id='type-id-1'/></function-decl><function-decl name='bleh' mangled-name='_Z4blehv' filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' line='34' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='_Z4blehv'><return type-id='type-id-1'/></function-decl></abi-instr></abi-corpus>
>\ No newline at end of file
>diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
>index 4283e145..f1526b9e 100644
>--- a/tests/test-abidiff-exit.cc
>+++ b/tests/test-abidiff-exit.cc
>@@ -392,6 +392,17 @@ InOutSpec in_out_specs[] =
>     "data/test-abidiff-exit/test-non-leaf-array-report.txt",
>     "output/test-abidiff-exit/test-non-leaf-array-report.txt"
>   },
>+  {
>+    "data/test-abidiff-exit/test-squished-v0.abi",
>+    "data/test-abidiff-exit/test-squished-v1.abi",
>+    "",
>+    "",
>+    "",
>+    "--harmless",
>+    abigail::tools_utils::ABIDIFF_OK,
>+    "data/test-abidiff-exit/test-squished-report.txt",
>+    "output/test-abidiff-exit/test-squished-report.txt"
>+  },
>   {0, 0, 0 ,0, 0, 0, abigail::tools_utils::ABIDIFF_OK, 0, 0}
> };
>
>-- 
>2.31.0.291.g576ba9dcdaf-goog
>