From patchwork Mon Mar 4 15:55:52 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 86746 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 0214E3858C50 for ; Mon, 4 Mar 2024 15:56:03 +0000 (GMT) X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::223]) by sourceware.org (Postfix) with ESMTPS id 88ED83858D33; Mon, 4 Mar 2024 15:55:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 88ED83858D33 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=seketeli.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=seketeli.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 88ED83858D33 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:4b98:dc4:8::223 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709567759; cv=none; b=UfQKnbZf5Hx9agneqM0mEnFQkQKnsQxOTKFlHHEjd0tq+dWmSOhlhty9ZizZzi5cDOwK0CmoI911BD6iwK3LqNkXpsuea/SN6O0sP5ef6TQrBl0Vc0r5RUpUZBqT2ucsvE3auJD2YDiwdeRnDnhZfuWx3XgfGtvL6xLxqia/KXU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709567759; c=relaxed/simple; bh=S2qx/H82+PF21tyX0ICVRR7TF5BPXcG8jGOzB3yz/Bs=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=W3NwYRwScDF6I5Uv5f3I8zJC5LC+au6/ri23LDLYSTM3REOghPZ8MwWtuklEdCZg2DV07BxIjN5SjZGZhxKVED+ldHcfP8VQhnTe7XnNzJSD5ivwo3V3zKLSGa9UOy/PHYtBU2ilyogiMFSiAuMuQFc5dS3fUc6yWQfS8b7jiO8= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail.gandi.net (Postfix) with ESMTPSA id 5AAF66000B; Mon, 4 Mar 2024 15:55:53 +0000 (UTC) Received: by localhost (Postfix, from userid 1000) id E89A55071022; Mon, 4 Mar 2024 16:55:52 +0100 (CET) From: Dodji Seketeli To: builder@sourceware.org Cc: libabigail@sourceware.org Subject: [PATCH 2/2] comparison: Better sort function difference report Organization: Me, myself and I References: <20240302014047.E768A3858D39@sourceware.org> X-Operating-System: AlmaLinux 9.3 X-URL: http://www.seketeli.net/~dodji Date: Mon, 04 Mar 2024 16:55:52 +0100 In-Reply-To: <20240302014047.E768A3858D39@sourceware.org> (builder@sourceware.org's message of "Sat, 02 Mar 2024 01:40:47 +0000") Message-ID: <87le6y56uv.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 X-GND-Sasl: dodj@seketeli.org X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, SPF_HELO_PASS, SPF_PASS, 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.30 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org Hello, [This is the second of the two patches intended to fix the build.] Looking at the result of builds on various platforms, it appears that the order of the method change reports in the test29-vtable-changes-report-0.txt of runtestdiffdwarf was not stable across all platforms. debian-armhf was exhibiting this change in output, for instance: --- /var/lib/buildbot/workers/wildebeest/libabigail-try-debian-armhf/build/tests/data/test-diff-dwarf/test29-vtable-changes-report-0.txt 2024-03-04 08:56:42.307366590 +0000 +++ /var/lib/buildbot/workers/wildebeest/libabigail-try-debian-armhf/build/tests/output/test-diff-dwarf/test29-vtable-changes-report-0.txt 2024-03-04 09:20:22.146334907 +0000 @@ -19,13 +19,13 @@ implicit parameter 0 of type 'S* const' has sub-type changes: in unqualified underlying type 'S*': pointed to type 'struct S' changed, as being reported + 'method virtual S::~S(int)' has some sub-type changes: + implicit parameter 0 of type 'S*' has sub-type changes: + pointed to type 'struct S' changed, as being reported 'method virtual S::~S()' has some sub-type changes: implicit parameter 0 of type 'S* const' has sub-type changes: in unqualified underlying type 'S*': pointed to type 'struct S' changed, as being reported - 'method virtual S::~S(int)' has some sub-type changes: - implicit parameter 0 of type 'S*' has sub-type changes: - pointed to type 'struct S' changed, as being reported 'method virtual void S::fn0()' has some sub-type changes: implicit parameter 0 of type 'S*' has sub-type changes: pointed to type 'struct S' changed, as being reported Better handling the sorting of function changes should hopefully fix this issue. * src/abg-comparison-priv.h (is_less_than): Declare new helper function. (function_decl_diff_comp::operator(const function_decl_diff&, const function_decl_diff&)): Use it here. (virtual_member_function_diff_comp::operator(const function_decl_diff&, const function_decl_diff&)): Likewise. * tests/data/test-abidiff/test-PR18791-report0.txt: Adjust. * tests/data/test-diff-dwarf/test29-vtable-changes-report-0.txt: Adjust. * tests/data/test-diff-dwarf/test30-vtable-changes-report-0.txt: Adjust. * tests/data/test-diff-dwarf/test31-vtable-changes-report-0.txt: Adjust. * tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt: Adjust. Signed-off-by: Dodji Seketeli --- src/abg-comparison-priv.h | 36 +++++----------- src/abg-comparison.cc | 41 +++++++++++++++++++ .../test-abidiff/test-PR18791-report0.txt | 24 +++++------ .../test29-vtable-changes-report-0.txt | 6 +-- .../test30-vtable-changes-report-0.txt | 16 ++++---- .../test31-vtable-changes-report-0.txt | 10 ++--- .../test41-PR20476-hidden-report-0.txt | 16 ++++---- 7 files changed, 87 insertions(+), 62 deletions(-) diff --git a/src/abg-comparison-priv.h b/src/abg-comparison-priv.h index 434e6267..adc1a23c 100644 --- a/src/abg-comparison-priv.h +++ b/src/abg-comparison-priv.h @@ -774,6 +774,9 @@ struct data_member_diff_comp } }; // end struct var_diff_comp +bool +is_less_than(const function_decl_diff& first, const function_decl_diff& second); + /// A comparison functor for instances of @ref function_decl_diff that /// represent changes between two virtual member functions. struct virtual_member_function_diff_comp @@ -785,8 +788,12 @@ struct virtual_member_function_diff_comp ABG_ASSERT(get_member_function_is_virtual(l.first_function_decl())); ABG_ASSERT(get_member_function_is_virtual(r.first_function_decl())); - return (get_member_function_vtable_offset(l.first_function_decl()) - < get_member_function_vtable_offset(r.first_function_decl())); + size_t l_offset = get_member_function_vtable_offset(l.first_function_decl()); + size_t r_offset = get_member_function_vtable_offset(r.first_function_decl()); + if (l_offset != r_offset) + return l_offset < r_offset; + + return is_less_than(l, r); } bool @@ -1257,30 +1264,7 @@ struct function_decl_diff_comp operator()(const function_decl_diff& first, const function_decl_diff& second) { - function_decl_sptr f = first.first_function_decl(), - s = second.first_function_decl(); - - string fr = f->get_qualified_name(), - sr = s->get_qualified_name(); - - if (fr == sr) - { - if (f->get_symbol()) - fr = f->get_symbol()->get_id_string(); - else if (!f->get_linkage_name().empty()) - fr = f->get_linkage_name(); - else - fr = f->get_pretty_representation(); - - if (s->get_symbol()) - sr = s->get_symbol()->get_id_string(); - else if (!s->get_linkage_name().empty()) - sr = s->get_linkage_name(); - else - sr = s->get_pretty_representation(); - } - - return (fr.compare(sr) < 0); + return is_less_than(first, second); } /// The actual less than operator. diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc index 1af10529..bd7c224e 100644 --- a/src/abg-comparison.cc +++ b/src/abg-comparison.cc @@ -168,6 +168,47 @@ sort_changed_data_members(changed_var_sptrs_type& to_sort) std::sort(to_sort.begin(), to_sort.end(), comp); } +/// Compare two @ref function_decl_diff for the purpose of sorting. +/// +/// @param first the first @ref function_decl_diff to consider. +/// +/// @param second the second @ref function_decl_diff to consider. +/// +/// @return true iff @p first compares less than @p second. +bool +is_less_than(const function_decl_diff& first, const function_decl_diff& second) +{ + function_decl_sptr f = first.first_function_decl(), + s = second.first_function_decl(); + + string fr = f->get_qualified_name(), sr = s->get_qualified_name(); + + if (fr != sr) + return fr < sr; + + if (!f->get_linkage_name().empty() + && !s->get_linkage_name().empty()) + { + fr = f->get_linkage_name(); + sr = s->get_linkage_name(); + if (fr != sr) + return fr < sr; + } + + if (f->get_symbol() && s->get_symbol()) + { + fr = f->get_symbol()->get_id_string(); + sr = s->get_symbol()->get_id_string(); + if (fr != sr) + return fr < sr; + } + + fr = f->get_pretty_representation(true, true); + sr = s->get_pretty_representation(true, true); + + return fr < sr; +} + /// Sort an instance of @ref string_function_ptr_map map and stuff a /// resulting sorted vector of pointers to function_decl. /// diff --git a/tests/data/test-abidiff/test-PR18791-report0.txt b/tests/data/test-abidiff/test-PR18791-report0.txt index 18cb0cf9..4d2a3c86 100644 --- a/tests/data/test-abidiff/test-PR18791-report0.txt +++ b/tests/data/test-abidiff/test-PR18791-report0.txt @@ -54,16 +54,16 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable in referenced type 'const sigc::connection': unqualified underlying type 'struct sigc::connection' changed, as reported earlier - [C] 'method sigc::connection::connection()' has some indirect sub-type changes: - implicit parameter 0 of type 'sigc::connection*' has sub-type changes: - pointed to type 'struct sigc::connection' changed, as reported earlier - [C] 'method sigc::connection::connection(sigc::slot_base&)' has some indirect sub-type changes: implicit parameter 0 of type 'sigc::connection*' has sub-type changes: pointed to type 'struct sigc::connection' changed, as reported earlier parameter 1 of type 'sigc::slot_base&' has sub-type changes: referenced type 'class sigc::slot_base' changed, as reported earlier + [C] 'method sigc::connection::connection()' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::connection*' has sub-type changes: + pointed to type 'struct sigc::connection' changed, as reported earlier + [C] 'method void sigc::connection::disconnect()' has some indirect sub-type changes: implicit parameter 0 of type 'sigc::connection*' has sub-type changes: pointed to type 'struct sigc::connection' changed, as reported earlier @@ -215,10 +215,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable in referenced type 'const sigc::signal_base': unqualified underlying type 'struct sigc::signal_base' changed, as reported earlier - [C] 'method sigc::signal_base::signal_base()' has some indirect sub-type changes: - implicit parameter 0 of type 'sigc::signal_base*' has sub-type changes: - pointed to type 'struct sigc::signal_base' changed, as reported earlier - [C] 'method sigc::signal_base::signal_base(const sigc::signal_base&)' has some indirect sub-type changes: implicit parameter 0 of type 'sigc::signal_base*' has sub-type changes: pointed to type 'struct sigc::signal_base' changed, as reported earlier @@ -226,6 +222,10 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable in referenced type 'const sigc::signal_base': unqualified underlying type 'struct sigc::signal_base' changed, as reported earlier + [C] 'method sigc::signal_base::signal_base()' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::signal_base*' has sub-type changes: + pointed to type 'struct sigc::signal_base' changed, as reported earlier + [C] 'method sigc::signal_base::size_type sigc::signal_base::size()' has some indirect sub-type changes: implicit parameter 0 of type 'const sigc::signal_base*' has sub-type changes: in pointed to type 'const sigc::signal_base': @@ -245,10 +245,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable parameter 1 of type 'sigc::slot_base::rep_type*' has sub-type changes: pointed to type 'typedef sigc::slot_base::rep_type' changed, as reported earlier - [C] 'method sigc::slot_base::slot_base()' has some indirect sub-type changes: - implicit parameter 0 of type 'sigc::slot_base*' has sub-type changes: - pointed to type 'class sigc::slot_base' changed, as reported earlier - [C] 'method sigc::slot_base::slot_base(const sigc::slot_base&)' has some indirect sub-type changes: implicit parameter 0 of type 'sigc::slot_base*' has sub-type changes: pointed to type 'class sigc::slot_base' changed, as reported earlier @@ -256,6 +252,10 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable in referenced type 'const sigc::slot_base': unqualified underlying type 'class sigc::slot_base' changed, as reported earlier + [C] 'method sigc::slot_base::slot_base()' has some indirect sub-type changes: + implicit parameter 0 of type 'sigc::slot_base*' has sub-type changes: + pointed to type 'class sigc::slot_base' changed, as reported earlier + [C] 'method sigc::slot_base::~slot_base(int)' has some indirect sub-type changes: implicit parameter 0 of type 'sigc::slot_base*' has sub-type changes: pointed to type 'class sigc::slot_base' changed, as reported earlier diff --git a/tests/data/test-diff-dwarf/test29-vtable-changes-report-0.txt b/tests/data/test-diff-dwarf/test29-vtable-changes-report-0.txt index d9873f9b..780d5945 100644 --- a/tests/data/test-diff-dwarf/test29-vtable-changes-report-0.txt +++ b/tests/data/test-diff-dwarf/test29-vtable-changes-report-0.txt @@ -15,6 +15,9 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable 1 member function insertion: 'method virtual void S::fn1()', virtual at voffset 3/3 4 member function changes: + 'method virtual void S::fn0()' has some sub-type changes: + implicit parameter 0 of type 'S*' has sub-type changes: + pointed to type 'struct S' changed, as being reported 'method virtual S::~S()' has some sub-type changes: implicit parameter 0 of type 'S* const' has sub-type changes: in unqualified underlying type 'S*': @@ -26,9 +29,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable 'method virtual S::~S(int)' has some sub-type changes: implicit parameter 0 of type 'S*' has sub-type changes: pointed to type 'struct S' changed, as being reported - 'method virtual void S::fn0()' has some sub-type changes: - implicit parameter 0 of type 'S*' has sub-type changes: - pointed to type 'struct S' changed, as being reported [C] 'method virtual S::~S()' has some indirect sub-type changes: implicit parameter 0 of type 'S* const' has sub-type changes: diff --git a/tests/data/test-diff-dwarf/test30-vtable-changes-report-0.txt b/tests/data/test-diff-dwarf/test30-vtable-changes-report-0.txt index e75be941..d32f99b2 100644 --- a/tests/data/test-diff-dwarf/test30-vtable-changes-report-0.txt +++ b/tests/data/test-diff-dwarf/test30-vtable-changes-report-0.txt @@ -15,6 +15,14 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable 1 member function insertion: 'method virtual void S::fvtable_breaker()', virtual at voffset 3/4 5 member function changes: + 'method virtual void S::fn0()' has some sub-type changes: + implicit parameter 0 of type 'S*' has sub-type changes: + pointed to type 'struct S' changed, as being reported + 'method virtual void S::fn1()' has some sub-type changes: + the vtable offset of method virtual void S::fn1() changed from 3 to 4 + note that this is an ABI incompatible change to the vtable of struct S + implicit parameter 0 of type 'S*' has sub-type changes: + pointed to type 'struct S' changed, as being reported 'method virtual S::~S()' has some sub-type changes: implicit parameter 0 of type 'S* const' has sub-type changes: in unqualified underlying type 'S*': @@ -26,14 +34,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable 'method virtual S::~S(int)' has some sub-type changes: implicit parameter 0 of type 'S*' has sub-type changes: pointed to type 'struct S' changed, as being reported - 'method virtual void S::fn0()' has some sub-type changes: - implicit parameter 0 of type 'S*' has sub-type changes: - pointed to type 'struct S' changed, as being reported - 'method virtual void S::fn1()' has some sub-type changes: - the vtable offset of method virtual void S::fn1() changed from 3 to 4 - note that this is an ABI incompatible change to the vtable of struct S - implicit parameter 0 of type 'S*' has sub-type changes: - pointed to type 'struct S' changed, as being reported [C] 'method virtual void S::fn1()' has some indirect sub-type changes: the vtable offset of method virtual void S::fn1() changed from 3 to 4 diff --git a/tests/data/test-diff-dwarf/test31-vtable-changes-report-0.txt b/tests/data/test-diff-dwarf/test31-vtable-changes-report-0.txt index fa904b7c..dcfde0f5 100644 --- a/tests/data/test-diff-dwarf/test31-vtable-changes-report-0.txt +++ b/tests/data/test-diff-dwarf/test31-vtable-changes-report-0.txt @@ -17,6 +17,11 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable 1 member function deletion: 'method virtual void S::fn0()', virtual at voffset 2/3 4 member function changes: + 'method virtual void S::fn1()' has some sub-type changes: + the vtable offset of method virtual void S::fn1() changed from 3 to 2 + note that this is an ABI incompatible change to the vtable of struct S + implicit parameter 0 of type 'S*' has sub-type changes: + pointed to type 'struct S' changed, as being reported 'method virtual S::~S()' has some sub-type changes: implicit parameter 0 of type 'S* const' has sub-type changes: in unqualified underlying type 'S*': @@ -28,11 +33,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable 'method virtual S::~S(int)' has some sub-type changes: implicit parameter 0 of type 'S*' has sub-type changes: pointed to type 'struct S' changed, as being reported - 'method virtual void S::fn1()' has some sub-type changes: - the vtable offset of method virtual void S::fn1() changed from 3 to 2 - note that this is an ABI incompatible change to the vtable of struct S - implicit parameter 0 of type 'S*' has sub-type changes: - pointed to type 'struct S' changed, as being reported [C] 'method virtual S::~S()' has some indirect sub-type changes: implicit parameter 0 of type 'S* const' has sub-type changes: diff --git a/tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt b/tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt index 17a1662e..0a695856 100644 --- a/tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt +++ b/tests/data/test-diff-dwarf/test41-PR20476-hidden-report-0.txt @@ -11,6 +11,14 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable 1 member function insertion: 'method virtual void Interface::method2()', virtual at voffset 3/4 5 member function changes: + 'method virtual void Interface::method1()' has some sub-type changes: + implicit parameter 0 of type 'Interface*' has sub-type changes: + pointed to type 'class Interface' changed, as being reported + 'method virtual void Interface::method3()' has some sub-type changes: + the vtable offset of method virtual void Interface::method3() changed from 3 to 4 + note that this is an ABI incompatible change to the vtable of class Interface + implicit parameter 0 of type 'Interface*' has sub-type changes: + pointed to type 'class Interface' changed, as being reported 'method virtual Interface::~Interface()' has some sub-type changes: implicit parameter 0 of type 'Interface* const' has sub-type changes: in unqualified underlying type 'Interface*': @@ -22,14 +30,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable 'method virtual Interface::~Interface(int)' has some sub-type changes: implicit parameter 0 of type 'Interface*' has sub-type changes: pointed to type 'class Interface' changed, as being reported - 'method virtual void Interface::method1()' has some sub-type changes: - implicit parameter 0 of type 'Interface*' has sub-type changes: - pointed to type 'class Interface' changed, as being reported - 'method virtual void Interface::method3()' has some sub-type changes: - the vtable offset of method virtual void Interface::method3() changed from 3 to 4 - note that this is an ABI incompatible change to the vtable of class Interface - implicit parameter 0 of type 'Interface*' has sub-type changes: - pointed to type 'class Interface' changed, as being reported [C] 'method virtual Interface::~Interface()' has some indirect sub-type changes: implicit parameter 0 of type 'Interface* const' has sub-type changes: