From: Dodji Seketeli <dodji@seketeli.org>
To: builder@sourceware.org
Cc: libabigail@sourceware.org
Subject: [PATCH 2/2] comparison: Better sort function difference report
Date: Mon, 04 Mar 2024 16:55:52 +0100 [thread overview]
Message-ID: <87le6y56uv.fsf@seketeli.org> (raw)
In-Reply-To: <20240302014047.E768A3858D39@sourceware.org> (builder@sourceware.org's message of "Sat, 02 Mar 2024 01:40:47 +0000")
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 <dodji@redhat.com>
---
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:
--
2.39.3
--
Dodji
prev parent reply other threads:[~2024-03-04 15:55 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-02 1:40 ☠ Buildbot (Sourceware): libabigail - failed test (failure) (master) builder
2024-03-04 15:55 ` [PATCH 1/2] ir,dwarf-reader: Better handle inline-ness setting or detection Dodji Seketeli
2024-03-04 15:55 ` Dodji Seketeli [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87le6y56uv.fsf@seketeli.org \
--to=dodji@seketeli.org \
--cc=builder@sourceware.org \
--cc=libabigail@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).