Improve self-comparison debug mode

When looking at debugging some self-comparison difference errors, I
felt the need to improve the debugging support for self-comparison
(triggered by 'abidw --debug-abidiff <binary>').

This patch fixes some typos in the existing diagnostics emitted by the
self-comparison debugging mode and improves the detection of a change
in a canonical type value between a type that is serialized to abixml,
and the same type that is de-serialized from abixml.  This later check
is now performed during the process of handling canonical type
propagation when confirming/canceling (speculatively) propagated
canonical types.  It's useful to find problems in that process of
confirming/canceling.

	* include/abg-ir.h
	(environment::{get_type_id_canonical_type_map}): Const-ify the
	existing member function and add non-const overloads.
	(environment::{get_type_id_from_pointer,
	get_canonical_type_from_type_id}): Const-ify.
	(environment::get_pointer_type_id_map): Add new member function.
	* src/abg-ir-priv.h
	(environment::priv::{confirm_ct_propagation_for_types_dependant_on,
	confirm_ct_propagation}): Call
	check_abixml_canonical_type_propagation_during_self_comp() here.
	(environment::priv::{get_type_id_canonical_type_map,
	get_pointer_type_id_map, get_type_id_from_pointer,
	get_type_id_from_type, get_canonical_type_from_type_id,
	check_abixml_canonical_type_propagation_during_self_comp}): Add
	new member functions.
	* src/abg-ir.cc (return_comparison_result): Call
	check_abixml_canonical_type_propagation_during_self_comp on every
	single type with non confirmed propagated canonical type.
	(environment::{get_type_id_canonical_type_map,
	get_pointer_type_id_map, get_type_id_from_pointer,
	get_type_id_from_type, get_canonical_type_from_type_id}): Delegate
	to the new implementations that are now member functions of
	environment::priv.
	(type_base::get_canonical_type_for): Fix typo in diagnostics when
	debugging self-comparison. Add more context.
	* src/abg-reader.cc
	(abixml::reader::maybe_check_abixml_canonical_type_stability):
	Likewise.
	* src/abg-writer.cc (write_type_record): Do not try to get abixml
	type-id for a type (originating from the DWARF) that has no
	canonical type, otherwise, that /can/ introduce a gap in the
	type-ids as those can turn out not being emitted in the abixml
	after all.

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

diff --git a/include/abg-ir.h b/include/abg-ir.h
index 263bff5eb3b679d0dde2a59258ee114274d80922..8269e8857460ec43457d3c0da1471988d8b615aa 100644 (file)
--- a/include/abg-ir.h
+++ b/include/abg-ir.h
@@ -244,20 +244,26 @@ public:
   type_base* get_canonical_type(const char* name, unsigned index);
 
 #ifdef WITH_DEBUG_SELF_COMPARISON
-  unordered_map<string, uintptr_t>&
+  const unordered_map<string, uintptr_t>&
   get_type_id_canonical_type_map() const;
 
+  unordered_map<string, uintptr_t>&
+  get_type_id_canonical_type_map();
+
+  const unordered_map<uintptr_t, string>&
+  get_pointer_type_id_map() const;
+
   unordered_map<uintptr_t, string>&
   get_pointer_type_id_map();
 
   string
-  get_type_id_from_pointer(uintptr_t ptr);
+  get_type_id_from_pointer(uintptr_t ptr) const;
 
   string
-  get_type_id_from_type(const type_base *ptr);
+  get_type_id_from_type(const type_base *ptr) const;
 
   uintptr_t
-  get_canonical_type_from_type_id(const char*);
+  get_canonical_type_from_type_id(const char*) const;
 #endif
 
   friend class class_or_union;

diff --git a/src/abg-ir-priv.h b/src/abg-ir-priv.h
index 04021c3b44a09ee1fb7eea940cd9856166a367be..bea7e2ec98cb333ba822c1e015d890ab43b3bedc 100644 (file)
--- a/src/abg-ir-priv.h
+++ b/src/abg-ir-priv.h
@@ -834,6 +834,9 @@ struct environment::priv
          {
            to_remove.insert(i);
            t->priv_->set_propagated_canonical_type_confirmed(true);
+#ifdef WITH_DEBUG_SELF_COMPARISON
+           check_abixml_canonical_type_propagation_during_self_comp(t);
+#endif
          }
       }
 
@@ -865,6 +868,9 @@ struct environment::priv
     env.priv_->remove_from_types_with_non_confirmed_propagated_ct(t);
     env.priv_->set_is_not_recursive(t);
     t->priv_->set_propagated_canonical_type_confirmed(true);
+#ifdef WITH_DEBUG_SELF_COMPARISON
+    check_abixml_canonical_type_propagation_during_self_comp(t);
+#endif
   }
 
   /// Mark all the types that have been the target of canonical type
@@ -885,6 +891,9 @@ struct environment::priv
                   || t->priv_->depends_on_recursive_type());
        t->priv_->set_does_not_depend_on_recursive_type();
        t->priv_->set_propagated_canonical_type_confirmed(true);
+#ifdef WITH_DEBUG_SELF_COMPARISON
+           check_abixml_canonical_type_propagation_during_self_comp(t);
+#endif
       }
     types_with_non_confirmed_propagated_ct_.clear();
   }
@@ -1102,6 +1111,47 @@ struct environment::priv
   }
 
 #ifdef WITH_DEBUG_SELF_COMPARISON
+
+  const unordered_map<string, uintptr_t>&
+  get_type_id_canonical_type_map() const
+  {return type_id_canonical_type_map_;}
+
+  unordered_map<string, uintptr_t>&
+  get_type_id_canonical_type_map()
+  {return type_id_canonical_type_map_;}
+
+  const unordered_map<uintptr_t, string>&
+  get_pointer_type_id_map() const
+  {return pointer_type_id_map_;}
+
+  unordered_map<uintptr_t, string>&
+  get_pointer_type_id_map()
+  {return pointer_type_id_map_;}
+
+  string
+  get_type_id_from_pointer(uintptr_t ptr) const
+  {
+    auto it = get_pointer_type_id_map().find(ptr);
+    if (it != get_pointer_type_id_map().end())
+      return it->second;
+    return "";
+  }
+
+  string
+  get_type_id_from_type(const type_base *t) const
+  {return get_type_id_from_pointer(reinterpret_cast<uintptr_t>(t));}
+
+  uintptr_t
+  get_canonical_type_from_type_id(const char* type_id) const
+  {
+    if (!type_id)
+      return 0;
+    auto it = get_type_id_canonical_type_map().find(type_id);
+    if (it != get_type_id_canonical_type_map().end())
+      return it->second;
+    return 0;
+  }
+
   /// When debugging self comparison, verify that a type T
   /// de-serialized from abixml has the same canonical type as the
   /// initial type built from DWARF that was serialized into T in the
@@ -1109,10 +1159,11 @@ struct environment::priv
   ///
   /// @param t deserialized type (from abixml) to consider.
   ///
-  /// @param c the canonical type @p t should have.
+  /// @param c the canonical type that @p t has, as computed freshly
+  /// from the abixml file.
   ///
-  /// @return true iff @p c is the canonical type that @p t should
-  /// have.
+  /// @return true iff @p c has the same value as the canonical type
+  /// that @p t had before being serialized into abixml.
   bool
   check_canonical_type_from_abixml_during_self_comp(const type_base* t,
                                                    const type_base* c)
@@ -1159,6 +1210,45 @@ struct environment::priv
     return false;
   }
 
+  /// When debugging self comparison, verify that a type T
+  /// de-serialized from abixml has the same canonical type as the
+  /// initial type built from DWARF that was serialized into T in the
+  /// first place.
+  ///
+  /// @param t deserialized type (from abixml) to consider.
+  ///
+  /// @return true iff @p c is the canonical type that @p t should
+  /// have.
+  bool
+  check_abixml_canonical_type_propagation_during_self_comp(const type_base* t)
+  {
+    if (t->get_corpus()
+       && t->get_corpus()->get_origin() == ir::corpus::NATIVE_XML_ORIGIN)
+      {
+       type_base* c = t->get_naked_canonical_type();
+       if (c && !check_canonical_type_from_abixml_during_self_comp(t, c))
+         {
+           string repr = t->get_pretty_representation(true, true);
+           string type_id = get_type_id_from_type(t);
+           std::cerr << "error: canonical type propagation error for '"
+                     << repr
+                     << "' of type-id: '"
+                     << type_id
+                     << "' / type: @"
+                     << std::hex
+                     << t
+                     << "/ canon: @"
+                     << c
+                     << ", should have had canonical type: "
+                     << std::hex
+                     << get_canonical_type_from_type_id(type_id.c_str())
+                     << "\n";
+           return false;
+         }
+      }
+    return true;
+  }
+
   /// When debugging self comparison, verify that a type T
   /// de-serialized from abixml has the same canonical type as the
   /// initial type built from DWARF that was serialized into T in the

diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 7913d31a55960c5378a86b1f524d704eace0ff7f..087baacbbeae46d5d837fe1bd413dd606b3d2bd0 100644 (file)
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -1094,6 +1094,17 @@ return_comparison_result(T& l, T& r, bool value,
     // shall now confirm the propagation for all those types.
     env.priv_->confirm_ct_propagation();
 
+#ifdef WITH_DEBUG_SELF_COMPARISON
+  if (value == false && env.priv_->right_type_comp_operands_.empty())
+    {
+      for (const auto i : env.priv_->types_with_non_confirmed_propagated_ct_)
+       {
+         type_base *t = reinterpret_cast<type_base*>(i);
+         env.priv_->check_abixml_canonical_type_propagation_during_self_comp(t);
+       }
+    }
+#endif
+
   ABG_RETURN(value);
 }
 
@@ -3899,9 +3910,39 @@ environment::get_canonical_type(const char* name, unsigned index)
 ///
 /// @return the set of abixml type-id and the pointer value of the
 /// (canonical) type it's associated to.
-unordered_map<string, uintptr_t>&
+const unordered_map<string, uintptr_t>&
 environment::get_type_id_canonical_type_map() const
-{return priv_->type_id_canonical_type_map_;}
+{return priv_->get_type_id_canonical_type_map();}
+
+/// Get the set of abixml type-id and the pointer value of the
+/// (canonical) type it's associated to.
+///
+/// This is useful for debugging purposes, especially in the context
+/// of the use of the command:
+///   'abidw --debug-abidiff <binary>'.
+///
+/// @return the set of abixml type-id and the pointer value of the
+/// (canonical) type it's associated to.
+unordered_map<string, uintptr_t>&
+environment::get_type_id_canonical_type_map()
+{return priv_->get_type_id_canonical_type_map();}
+
+/// Getter of the map that associates the values of type pointers to
+/// their type-id strings.
+///
+/// Note that this map is populated at abixml reading time, (by
+/// build_type()) when a given XML element representing a type is
+/// read into a corresponding abigail::ir::type_base.
+///
+/// This is used only for the purpose of debugging the
+/// self-comparison process.  That is, when invoking "abidw
+/// --debug-abidiff".
+///
+/// @return the map that associates the values of type pointers to
+/// their type-id strings.
+const unordered_map<uintptr_t, string>&
+environment::get_pointer_type_id_map() const
+{return priv_->get_pointer_type_id_map();}
 
 /// Getter of the map that associates the values of type pointers to
 /// their type-id strings.
@@ -3918,7 +3959,7 @@ environment::get_type_id_canonical_type_map() const
 /// their type-id strings.
 unordered_map<uintptr_t, string>&
 environment::get_pointer_type_id_map()
-{return priv_->pointer_type_id_map_;}
+{return priv_->get_pointer_type_id_map();}
 
 /// Getter of the type-id that corresponds to the value of a pointer
 /// to abigail::ir::type_base that was created from the abixml reader.
@@ -3936,13 +3977,8 @@ environment::get_pointer_type_id_map()
 ///
 /// @return the type-id strings that corresponds
 string
-environment::get_type_id_from_pointer(uintptr_t ptr)
-{
-  auto it = get_pointer_type_id_map().find(ptr);
-  if (it != get_pointer_type_id_map().end())
-    return it->second;
-  return "";
-}
+environment::get_type_id_from_pointer(uintptr_t ptr) const
+{return priv_->get_type_id_from_pointer(ptr);}
 
 /// Getter of the type-id that corresponds to the value of an
 /// abigail::ir::type_base that was created from the abixml reader.
@@ -3960,8 +3996,8 @@ environment::get_type_id_from_pointer(uintptr_t ptr)
 ///
 /// @return the type-id strings that corresponds
 string
-environment::get_type_id_from_type(const type_base *t)
-{return get_type_id_from_pointer(reinterpret_cast<uintptr_t>(t));}
+environment::get_type_id_from_type(const type_base *t) const
+{return priv_->get_type_id_from_type(t);}
 
 /// Getter of the canonical type of the artifact designated by a
 /// type-id.
@@ -3978,16 +4014,10 @@ environment::get_type_id_from_type(const type_base *t)
 /// @return the set of abixml type-id and the pointer value of the
 /// (canonical) type it's associated to.
 uintptr_t
-environment::get_canonical_type_from_type_id(const char* type_id)
-{
-  if (!type_id)
-    return 0;
-  auto it = get_type_id_canonical_type_map().find(type_id);
-  if (it != get_type_id_canonical_type_map().end())
-    return it->second;
-  return 0;
-}
+environment::get_canonical_type_from_type_id(const char* type_id) const
+{return priv_->get_canonical_type_from_type_id(type_id);}
 #endif
+
 // </environment stuff>
 
 // <type_or_decl_base stuff>
@@ -14558,17 +14588,31 @@ type_base::get_canonical_type_for(type_base_sptr t)
                  if (!env.priv_->
                      check_canonical_type_from_abixml_during_self_comp(t,
                                                                        result))
-                   // The canonical type of the type re-read from abixml
-                   // type doesn't match the canonical type that was
-                   // initially serialized down.
-                   std::cerr << "error: wrong canonical type for '"
-                             << repr
-                             << "' / type: @"
-                             << std::hex
-                             << t.get()
-                             << "/ canon: @"
-                             << result.get()
-                             << std::endl;
+                   {
+                     // The canonical type of the type re-read from abixml
+                     // type doesn't match the canonical type that was
+                     // initially serialized down.
+                     uintptr_t should_have_canonical_type = 0;
+                     string type_id = env.get_type_id_from_type(t.get());
+                     if (type_id.empty())
+                       type_id = "type-id-<not-found>";
+                     else
+                       should_have_canonical_type =
+                         env.get_canonical_type_from_type_id(type_id.c_str());
+                     std::cerr << "error: wrong canonical type for '"
+                               << repr
+                               << "' / type: @"
+                               << std::hex
+                               << t.get()
+                               << "/ canon: @"
+                               << result.get()
+                               << ", type-id: '"
+                               << type_id
+                               << "'.  Should have had canonical type: "
+                               << std::hex
+                               << should_have_canonical_type
+                               << std::endl;
+                   }
                }
              else //!result
                {
@@ -14596,12 +14640,12 @@ type_base::get_canonical_type_for(type_base_sptr t)
                            << repr
                            << "' from second corpus"
                            << ", ptr: " << std::hex << t.get()
-                           << "type-id: " << type_id
+                           << " type-id: " << type_id
                            << std::endl;
                }
            }
        }
-#endif
+#endif //WITH_DEBUG_SELF_COMPARISON
 
       if (!result)
        {

diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index fe3d466b4684c406a4df10276d49f890dc6ba416..1a7ccc7dded6c93179b0f67181c119764e5cc104 100644 (file)
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -881,13 +881,13 @@ public:
          }
        else if (j->second
                 != reinterpret_cast<uintptr_t>(t->get_canonical_type().get()))
-         // So thecanonical type of 't' (at abixml de-serialization
+         // So the canonical type of 't' (at abixml de-serialization
          // time) is different from the canonical type that led to
          // the serialization of 't' at abixml serialization time.
          // Report this because it needs further debugging.
          std::cerr << "error: canonical type for type '"
-                   << t->get_pretty_representation(/*internal=*/false,
-                                                   /*qualified=*/false)
+                   << t->get_pretty_representation(/*internal=*/true,
+                                                   /*qualified=*/true)
                    << "' of type-id '" << type_id
                    << "' changed from '" << std::hex
                    << j->second << "' to '" << std::hex

diff --git a/src/abg-writer.cc b/src/abg-writer.cc
index dbe43e22651712fe622ab288c200db61e4b87959..d6cd78d17851a3dab5e5793e02324aa5ee397458 100644 (file)
--- a/src/abg-writer.cc
+++ b/src/abg-writer.cc
@@ -4906,16 +4906,20 @@ write_type_record(xml_writer::write_context&    ctxt,
   //       <c>0x25f9ba8</c>
   //     </type>
 
-  string id = ctxt.get_id_for_type (const_cast<type_base*>(type));
-  o << "  <type>\n"
-    << "    <id>" << id << "</id>\n"
-    << "    <c>"
-    << std::hex
-    << (type->get_canonical_type()
-       ? reinterpret_cast<uintptr_t>(type->get_canonical_type().get())
-       : 0xdeadbabe)
-    << "</c>\n"
-    << "  </type>\n";
+    type_base* canonical = type->get_naked_canonical_type();
+    string id ;
+  if (canonical)
+    {
+      id = ctxt.get_id_for_type (const_cast<type_base*>(type));
+
+      o << "  <type>\n"
+       << "    <id>" << id << "</id>\n"
+       << "    <c>"
+       << std::hex
+       << reinterpret_cast<uintptr_t>(canonical)
+       << "</c>\n"
+       << "  </type>\n";
+    }
 }
 
 /// Serialize the map that is stored at

diff --git a/tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt b/tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt
index 73f0426a572faaebaa50c5a70ecdbaf5d6d0d6cb..b58f58d144d1c8d97f37a8e872749e328092f2b2 100644 (file)
--- a/tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt
+++ b/tests/data/test-diff-filter/test31-pr18535-libstdc++-report-1.txt
@@ -1,4 +1,4 @@
-Functions changes summary: 0 Removed, 3 Changed (7 filtered out), 13 Added functions
+Functions changes summary: 0 Removed, 3 Changed (12 filtered out), 13 Added functions
 Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
 Function symbols changes summary: 0 Removed, 0 Added function symbol not referenced by debug info
 Variable symbols changes summary: 0 Removed, 6 Added variable symbols not referenced by debug info