From: Dodji Seketeli Date: Wed, 19 Apr 2023 12:29:13 +0000 (+0200) Subject: Improve self-comparison debug mode X-Git-Tag: upstream/2.3~12 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=95abd39b5d23cac486518e9cae46b36207ea6e7b;p=platform%2Fupstream%2Flibabigail.git 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 '). 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 --- diff --git a/include/abg-ir.h b/include/abg-ir.h index 263bff5e..8269e885 100644 --- 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& + const unordered_map& get_type_id_canonical_type_map() const; + unordered_map& + get_type_id_canonical_type_map(); + + const unordered_map& + get_pointer_type_id_map() const; + unordered_map& 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 04021c3b..bea7e2ec 100644 --- 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& + get_type_id_canonical_type_map() const + {return type_id_canonical_type_map_;} + + unordered_map& + get_type_id_canonical_type_map() + {return type_id_canonical_type_map_;} + + const unordered_map& + get_pointer_type_id_map() const + {return pointer_type_id_map_;} + + unordered_map& + 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(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 7913d31a..087baacb 100644 --- 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(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& +const unordered_map& 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 '. +/// +/// @return the set of abixml type-id and the pointer value of the +/// (canonical) type it's associated to. +unordered_map& +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& +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& 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(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 + // // @@ -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-"; + 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 fe3d466b..1a7ccc7d 100644 --- a/src/abg-reader.cc +++ b/src/abg-reader.cc @@ -881,13 +881,13 @@ public: } else if (j->second != reinterpret_cast(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 dbe43e22..d6cd78d1 100644 --- a/src/abg-writer.cc +++ b/src/abg-writer.cc @@ -4906,16 +4906,20 @@ write_type_record(xml_writer::write_context& ctxt, // 0x25f9ba8 // - string id = ctxt.get_id_for_type (const_cast(type)); - o << " \n" - << " " << id << "\n" - << " " - << std::hex - << (type->get_canonical_type() - ? reinterpret_cast(type->get_canonical_type().get()) - : 0xdeadbabe) - << "\n" - << " \n"; + type_base* canonical = type->get_naked_canonical_type(); + string id ; + if (canonical) + { + id = ctxt.get_id_for_type (const_cast(type)); + + o << " \n" + << " " << id << "\n" + << " " + << std::hex + << reinterpret_cast(canonical) + << "\n" + << " \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 73f0426a..b58f58d1 100644 --- 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