comparison: Fix erroneous detection of deleted anonymous data members

It has been reported over IRC by Robin Jarry that in some cases,
abidiff would wrongly consider anonymous data members as being
deleted.  These are cases where the anonymous data members were moved
into other anonymous data members without incurring any size or offset
change.

This patch reduces (and hopefully eliminates) false positives in
detecting anonymous data members deletion by considering the leaf data
members of the anonymous data members.

It considers that if all leaf data members contained in the previous
anonymous data members are still present in the new version of the
type, then no data member deletion occurred, basically.

The non-regression test added to this patch contains two binaries that
Robin Jarry sent me.  They were built for the source code before and
after the patch he submitted at
http://patches.dpdk.org/project/dpdk/patch/20240327091440.1166119-2-rjarry@redhat.com/.

Note that I have also added a synthetic reproducer example I came up
with after understanding the issue at hand.

	* src/abg-comparison.cc
	(class_or_union_diff::ensure_lookup_tables_populated): If leaf
	data members of an anonymous data member are still present in the
	new version of the class, then the anonymous data member cannot be
	considered as being removed from the second version of the class.
	* tests/data/test-abidiff-exit/non-del-anon-dm/non-regr/report0.txt:
	Reference test output.
	* tests/data/test-abidiff-exit/non-del-anon-dm/non-regr/test0-v{0,1}.c:
	Source code of the binary input below.
	* tests/data/test-abidiff-exit/non-del-anon-dm/non-regr/test0-v{0,1}.o:
	New binary test input.
	* tests/data/test-abidiff-exit/non-del-anon-dm/reported/binaries-origin.txt:
	File mentioning the origin of the reported binary.
	* tests/data/test-abidiff-exit/non-del-anon-dm/reported/librte_graph.so.24.{0,1}:
	Binary test input.
	* tests/data/test-abidiff-exit/non-del-anon-dm/reported/report0.txt:
	Reference test output.
	* tests/data/Makefile.am: Add the test material above to source
	distribution.
	* tests/test-abidiff-exit.cc (in_out_specs): Add test material
	above to this harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This commit is contained in:
Dodji Seketeli 2024-03-29 12:22:06 +01:00
parent 338394f545
commit f821c2be3f
12 changed files with 136 additions and 0 deletions

View File

@ -5370,6 +5370,59 @@ class_or_union_diff::ensure_lookup_tables_populated(void) const
priv_->inserted_data_members_.erase
(i->second->second_var()->get_anon_dm_reliable_name());
}
// Now detect anonymous data members that might appear as deleted
// even though all their data members are still present. Consider
// these as being non-deleted.
string_decl_base_sptr_map non_anonymous_dms_in_second_class;
collect_non_anonymous_data_members(second_class_or_union(),
non_anonymous_dms_in_second_class);
vector<string> deleted_data_members_to_delete;
// Walk data members considered deleted ...
for (auto& entry : priv_->deleted_data_members_)
{
var_decl_sptr data_member = is_var_decl(entry.second);
ABG_ASSERT(data_member);
if (is_anonymous_data_member(data_member))
{
// Let's look at this anonymous data member that is
// considered deleted because it's moved from where it was
// initially, at very least. If its leaf data members are
// still present in the second class then, we won't
// consider it as deleted.
class_or_union_sptr cou = anonymous_data_member_to_class_or_union(data_member);
ABG_ASSERT(cou);
string_decl_base_sptr_map non_anonymous_data_members;
// Lets collect the leaf data members of the anonymous
// data member.
collect_non_anonymous_data_members(cou, non_anonymous_data_members);
bool anonymous_dm_members_present = true;
// Let's see if at least one of the leaf members of the
// anonymous data member is NOT present in the second
// version of the class.
for (auto& e : non_anonymous_data_members)
{
if (non_anonymous_dms_in_second_class.find(e.first)
== non_anonymous_dms_in_second_class.end())
// Grrr, OK, it looks like at least one leaf data
// member of the original anonymous data member was
// removed from the class, so yeah, the anonymous
// data member might have been removed after all.
anonymous_dm_members_present = false;
}
if (anonymous_dm_members_present)
// All leaf data members of the anonymous data member
// are still present in the second version of the class.
// So let's mark that anonymous data member as NOT being
// deleted.
deleted_data_members_to_delete.push_back(data_member->get_anon_dm_reliable_name());
}
}
// All anonymous data members that were recognized as being NOT
// deleted should be removed from the set of deleted data members
// now.
for (string& name_of_dm_to_delete: deleted_data_members_to_delete)
priv_->deleted_data_members_.erase(name_of_dm_to_delete);
}
sort_string_data_member_diff_sptr_map(priv_->subtype_changed_dm_,
priv_->sorted_subtype_changed_dm_);

View File

@ -444,6 +444,15 @@ test-abidiff-exit/PR31513/non-regr/test3-v0.cc \
test-abidiff-exit/PR31513/non-regr/test3-v1.cc \
test-abidiff-exit/PR31513/non-regr/test4-v0.cc \
test-abidiff-exit/PR31513/non-regr/test4-v1.cc \
test-abidiff-exit/non-del-anon-dm/non-regr/report0.txt \
test-abidiff-exit/non-del-anon-dm/non-regr/test0-v0.c \
test-abidiff-exit/non-del-anon-dm/non-regr/test0-v0.o \
test-abidiff-exit/non-del-anon-dm/non-regr/test0-v1.c \
test-abidiff-exit/non-del-anon-dm/non-regr/test0-v1.o \
test-abidiff-exit/non-del-anon-dm/reported/binaries-origin.txt \
test-abidiff-exit/non-del-anon-dm/reported/librte_graph.so.24.0 \
test-abidiff-exit/non-del-anon-dm/reported/librte_graph.so.24.1 \
test-abidiff-exit/non-del-anon-dm/reported/report0.txt \
\
test-diff-dwarf/test0-v0.cc \
test-diff-dwarf/test0-v0.o \

View File

@ -0,0 +1,3 @@
Functions changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

View File

@ -0,0 +1,16 @@
struct type
{
int m0;
struct
{
union {int um00; char* um01;};
int* m1;
union {int* um10; char um11;};
};
float m2;
};
void
foo(struct type* __attribute__((unused)))
{
}

View File

@ -0,0 +1,20 @@
struct type
{
struct /* wrapping up all data members into an anonymous data
member */
{
int m0;
struct
{
union {int um00; char* um01;};
int* m1;
union {int* um10; char um11;};
};
float m2;
};
};
void
foo(struct type* __attribute__((unused)))
{
}

View File

@ -0,0 +1,2 @@
The binaries have been generated from before & after this patch:
http://patches.dpdk.org/project/dpdk/patch/20240327091440.1166119-2-rjarry@redhat.com/

View File

@ -0,0 +1,3 @@
Functions changes summary: 0 Removed, 0 Changed (10 filtered out), 0 Added functions
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

View File

@ -1380,6 +1380,36 @@ InOutSpec in_out_specs[] =
"data/test-abidiff-exit/PR31513/non-regr/report4.txt",
"output/test-abidiff-exit/PR31513/non-regr/report4.txt"
},
{
"data/test-abidiff-exit/non-del-anon-dm/reported/librte_graph.so.24.0",
"data/test-abidiff-exit/non-del-anon-dm/reported/librte_graph.so.24.1",
"",
"",
"",
"",
"",
"",
"",
"--no-default-suppression",
abigail::tools_utils::ABIDIFF_OK,
"data/test-abidiff-exit/non-del-anon-dm/reported/report0.txt",
"output/test-abidiff-exit/non-del-anon-dm/reported/report0.txt"
},
{
"data/test-abidiff-exit/non-del-anon-dm/non-regr/test0-v0.o",
"data/test-abidiff-exit/non-del-anon-dm/non-regr/test0-v1.o",
"",
"",
"",
"",
"",
"",
"",
"--no-default-suppression",
abigail::tools_utils::ABIDIFF_OK,
"data/test-abidiff-exit/non-del-anon-dm/non-regr/report0.txt",
"output/test-abidiff-exit/non-del-anon-dm/non-regr/report0.txt"
},
#ifdef WITH_BTF
{
"data/test-abidiff-exit/btf/test0-v0.o",