Hello,
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>
---
include/abg-ir.h | 14 ++-
src/abg-ir-priv.h | 96 ++++++++++++++-
src/abg-ir.cc | 112 ++++++++++++------
src/abg-reader.cc | 6 +-
src/abg-writer.cc | 24 ++--
.../test31-pr18535-libstdc++-report-1.txt | 2 +-
6 files changed, 199 insertions(+), 55 deletions(-)
@@ -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;
@@ -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
@@ -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)
{
@@ -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
@@ -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
@@ -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