From patchwork Wed Apr 26 12:09:05 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 68298 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 778583858412 for ; Wed, 26 Apr 2023 12:09:18 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 778583858412 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1682510958; bh=fE4TcIUcjbszC4HgRZDMihM6AgtnJIPe/KxuRUAyDZo=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Help: List-Subscribe:From:Reply-To:From; b=Houz5eHq4yJD0XWJikiDy+To7fl/Ot7HplkANdBLgb3Cg5ieMb5GBUv+lAKB7CBBF QLInyLCgoLFl9EEwJRj4/ryU8msCBw0AkEV3zDnxDwrV7aZSUAT5PDzmUyutlKvexr FHVqGvhOC8Hyz9P9sGrqBir+rBJG18oGeHnNeFho= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 9EF483858CDA for ; Wed, 26 Apr 2023 12:09:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9EF483858CDA Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-151-0rCWMGaBPkOe1E-QjqCxyg-1; Wed, 26 Apr 2023 08:09:08 -0400 X-MC-Unique: 0rCWMGaBPkOe1E-QjqCxyg-1 Received: by mail-qk1-f198.google.com with SMTP id af79cd13be357-74df1fa3064so550624285a.1 for ; Wed, 26 Apr 2023 05:09:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682510948; x=1685102948; h=mime-version:user-agent:message-id:date:organization:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=fE4TcIUcjbszC4HgRZDMihM6AgtnJIPe/KxuRUAyDZo=; b=LR4tPfFKY+3M43ATX1ApLttyi8CeY0JFOhJlVqIpOu84xV8lvBA1rs5JrH8qua7kOE fV5NfECROtTXcbin2feXRKFagjr5GSL8GzWsXBme5v+5K4IVOItcXf37KrhzGzr4Vrhz rJ+i3ytfhVtCyvE5RxjqvkzBWsopRGwFb9bpkIaM3zb81rcN8OlP9AtZV+NUIRdpWdEO Txj9UqaTCRUO8KJnKOqdOwKEPhc0qRT95dLuopdiWJnj63/iqEYljDDo+mQBDa1x+vWE Th+UGaZwLNJ2jpRMxiOOR1/X/5NH2iuPv3vvghTHE8kGiP2ADAG0J5+GMGuczoi3GzXn glEg== X-Gm-Message-State: AAQBX9fQGHdgz+vPyX0WCj8dMe8S8dCRz2kxWskg+5vbCfVKxGxhodCp l8ZKZSGykdwR1IQqEVpIzPyhflt1jXxEQsNaymhUFKN1KDlcwmsJF89leQHJRLfi6ZE6UzXKzVn +1rWSsWIXynGE0JSmZxU2gZmVtxEN7wIkvOvQS2oK26oWN5DfzTvHeskPlcgAJrh5EolQ3eZ9Qs if X-Received: by 2002:a05:6214:29c6:b0:5e8:5167:e254 with SMTP id gh6-20020a05621429c600b005e85167e254mr30103642qvb.42.1682510947980; Wed, 26 Apr 2023 05:09:07 -0700 (PDT) X-Google-Smtp-Source: AKy350ZQ7dGIpHxTnIPmx0FRmpzhrOo5DIoKHUu1JVk4pZ/Px3Y+mdz3eV5hEv2ItwDDFYaY+Vittg== X-Received: by 2002:a05:6214:29c6:b0:5e8:5167:e254 with SMTP id gh6-20020a05621429c600b005e85167e254mr30103586qvb.42.1682510947340; Wed, 26 Apr 2023 05:09:07 -0700 (PDT) Received: from localhost ([88.120.130.27]) by smtp.gmail.com with ESMTPSA id u18-20020a05622a14d200b003eda962ed24sm5218691qtx.22.2023.04.26.05.09.06 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Apr 2023 05:09:06 -0700 (PDT) Received: by localhost (Postfix, from userid 1000) id 5C9A5B5078; Wed, 26 Apr 2023 14:09:05 +0200 (CEST) To: libabigail@sourceware.org Subject: [PATCH, applied] Improve self-comparison debug mode Organization: Red Hat / France X-Operating-System: CentOS Stream release 9 X-URL: http://www.redhat.com Date: Wed, 26 Apr 2023 14:09:05 +0200 Message-ID: <87edo6976m.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-Patchwork-Original-From: Dodji Seketeli via Libabigail From: Dodji Seketeli Reply-To: Dodji Seketeli Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libabigail" 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 '). 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 --- 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(-) 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