[applied] Bug 32975 - Filter out replacing a data member with a compatible union

Message ID 87cyc3jlqn.fsf@redhat.com
State New
Headers
Series [applied] Bug 32975 - Filter out replacing a data member with a compatible union |

Commit Message

Dodji Seketeli May 20, 2025, 4:22 p.m. UTC
  Hello,

The RH_KABI_REPLACE macro replaces a data member into another while
ensuring that ABI is preserved.  It does that by replacing a data
member with a anonymous union of the old and new data member.

This patch teaches libabigail to recognize that pattern of change and
filter it out from the change report by default.

	* include/abg-comp-filter.h
	(is_type_to_compatible_anonymous_type_change)
	(is_data_member_to_compatible_anonymous_dm_change): Declare ...
	* src/abg-comp-filter.cc
	(is_type_to_compatible_anonymous_type_change)
	(is_data_member_to_compatible_anonymous_dm_change): ... new
	functions.
	(is_non_compatible_distinct_change): Use the new
	is_type_to_compatible_anonymous_type_change.
	(has_harmless_name_change): Use the new
	is_type_to_compatible_anonymous_type_change and
	is_data_member_to_compatible_anonymous_dm_change.
	(has_anonymous_data_member_change): Tighten this up by making the
	test more precise.
	* tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-report-1.txt:
	Add new reference test output.
	* tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v{0,1}.{c,o}:
	Add new test binary output as well as its source code.
	* tests/data/Makefile.am: Add the new test input material to
	source distribution.
	* tests/test-abidiff-exit.cc (in_out_specs): Add the new test to
	this harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Applied to the mainline
---
 include/abg-comp-filter.h                     |  20 +++
 src/abg-comp-filter.cc                        | 169 +++++++++++++++++-
 tests/data/Makefile.am                        |   5 +
 ...-dm-with-compatible-anon-dm-1-report-1.txt |   3 +
 .../replace-dm-with-compatible-anon-dm-1-v0.c |  11 ++
 .../replace-dm-with-compatible-anon-dm-1-v0.o | Bin 0 -> 3200 bytes
 .../replace-dm-with-compatible-anon-dm-1-v1.c |  17 ++
 .../replace-dm-with-compatible-anon-dm-1-v1.o | Bin 0 -> 3384 bytes
 tests/test-abidiff-exit.cc                    |  15 ++
 9 files changed, 238 insertions(+), 2 deletions(-)
 create mode 100644 tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-report-1.txt
 create mode 100644 tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v0.c
 create mode 100644 tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v0.o
 create mode 100644 tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v1.c
 create mode 100644 tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v1.o

new file mode 100644
index 00000000..f865ad87
index c9f2b0a9..584ced34 100644
  

Patch

diff --git a/include/abg-comp-filter.h b/include/abg-comp-filter.h
index ad7e9695..26751f50 100644
--- a/include/abg-comp-filter.h
+++ b/include/abg-comp-filter.h
@@ -148,6 +148,26 @@  has_incompatible_fn_or_var_change(const diff* d);
 bool
 has_incompatible_fn_or_var_change(const diff_sptr& d);
 
+bool
+is_type_to_compatible_anonymous_type_change(const diff* d);
+
+bool
+is_type_to_compatible_anonymous_type_change(const diff_sptr& d);
+
+bool
+is_type_to_compatible_anonymous_type_change(const type_base_sptr&,
+					    const type_base_sptr&);
+
+bool
+is_data_member_to_compatible_anonymous_dm_change(const diff* d);
+
+bool
+is_data_member_to_compatible_anonymous_dm_change(const diff_sptr& d);
+
+bool
+is_data_member_to_compatible_anonymous_dm_change(const decl_base_sptr&,
+						 const decl_base_sptr&);
+
 struct filter_base;
 /// Convenience typedef for a shared pointer to filter_base
 typedef shared_ptr<filter_base> filter_base_sptr;
diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc
index c4f6317f..985ce260 100644
--- a/src/abg-comp-filter.cc
+++ b/src/abg-comp-filter.cc
@@ -902,6 +902,7 @@  is_non_compatible_distinct_change(const diff *d)
     {
       if (dd->compatible_child_diff()
 	  || is_compatible_type_change(d)
+	  || is_type_to_compatible_anonymous_type_change(d)
 	  || (!dd->first_subject() || !dd->second_subject()))
 	// The distinct diff node carries a compatible or benign
 	// change
@@ -961,7 +962,18 @@  has_harmless_name_change(const decl_base_sptr& f,
 	      || (is_type(f)
 		  && is_type(s)
 		  && types_are_compatible(is_type(f), is_type(s)))
+	      // ... a harmless enum change ...
 	      || has_harmless_enum_change(is_type(f), is_type(s), ctxt)
+	      // ... a type replaced by a compatible anonymous union
+	      // or struct ...
+	      || (is_type(f) && is_type(s)
+		  && is_type_to_compatible_anonymous_type_change(is_type(f),
+								 is_type(s)))
+	      // ... a data member replaced by a compatible anonymous
+	      // data member ...
+	      || (is_data_member(f) && is_data_member(s)
+		  && is_data_member_to_compatible_anonymous_dm_change(is_decl(f),
+								      is_decl(s)))
 	      // ... or a data member name change, without having its
 	      // type changed ...
 	      || (is_data_member(f)
@@ -1853,8 +1865,10 @@  is_mostly_distinct_diff(const diff *d)
 bool
 has_anonymous_data_member_change(const diff *d)
 {
-  if (is_anonymous_data_member(d->first_subject())
-      || is_anonymous_data_member(d->second_subject()))
+  if ((is_anonymous_data_member(d->first_subject())
+       && !is_anonymous_data_member(d->second_subject()))
+      || (is_anonymous_data_member(d->second_subject())
+	  && !is_anonymous_data_member(d->first_subject())))
     return true;
   return false;
 }
@@ -2397,6 +2411,157 @@  bool
 has_incompatible_fn_or_var_change(const diff_sptr& d)
 {return has_incompatible_fn_or_var_change(d.get());}
 
+/// Test if a diff node carries a change where a type T is modified
+/// into an anonymous type T' of the same size which contains a data
+/// member of the same type as T.
+///
+/// T and T' are thus said to be compatible.
+///
+/// @param d the diff node to consider.
+///
+/// @return true iff @p d carriesa change where a type T is modified
+/// into an anonymous type T' of the same size which contains a data
+/// member of the same type as T.
+bool
+is_type_to_compatible_anonymous_type_change(const diff_sptr& d)
+{return is_type_to_compatible_anonymous_type_change(d.get());}
+
+/// Test if a diff node carries a change where a type T is modified
+/// into an anonymous type T' of the same size which contains a data
+/// member of the same type as T.
+///
+/// T and T' are thus said to be compatible.
+///
+/// @param d the diff node to consider.
+///
+/// @return true iff @p d carriesa change where a type T is modified
+/// into an anonymous type T' of the same size which contains a data
+/// member of the same type as T.
+bool
+is_type_to_compatible_anonymous_type_change(const diff* d)
+{
+  type_base_sptr fs = is_type(d->first_subject());
+  type_base_sptr ss = is_type(d->second_subject());
+
+  return is_type_to_compatible_anonymous_type_change(fs, ss);
+}
+
+/// Test if a type 'F' is modified into an anonymous type 'S' of the
+/// same size which contains a data member of the same type as 'F'.
+///
+/// F and S are thus said to be compatible.
+///
+/// @param F the first type to consider.
+///
+/// @param S the second version of type @p S to consider.
+///
+/// @return true iff @p 'F' is modified into an anonymous type 'S' of
+/// the same size which contains a data member of the same type as
+/// 'F'.
+bool
+is_type_to_compatible_anonymous_type_change(const type_base_sptr& f,
+					    const type_base_sptr& s)
+
+{
+  if (!f || !s)
+    return false;
+
+  if (is_anonymous_type(f) || !is_anonymous_type(s))
+    return false;
+
+  class_or_union_sptr second_cou = is_class_or_union_type(s);
+  if (!second_cou)
+    return false;
+
+  if (f->get_size_in_bits() != second_cou->get_size_in_bits()
+      || f->get_alignment_in_bits() != second_cou->get_alignment_in_bits())
+    return false;
+
+  string_decl_base_sptr_map non_anonymous_dms_in_second_class;
+  collect_non_anonymous_data_members(second_cou,
+				       non_anonymous_dms_in_second_class);
+  for (const auto& entry : non_anonymous_dms_in_second_class)
+    if (var_decl_sptr dm = is_data_member(entry.second))
+      if (type_base_sptr t = dm->get_type())
+	if (types_are_compatible(f, t))
+	return true;
+
+  return false;
+}
+
+/// Test if a diff node carries a change where a data member F is
+/// modified into an anonymous data member S that contains F at the
+/// same offset.
+///
+/// F and S are thus said to be compatible.
+///
+/// @param d the diff node to consider.
+///
+/// @return true iff @p d carries a change where a data member F is
+/// modified into an anonymous data member S that contains F at the
+/// same offset.
+bool
+is_data_member_to_compatible_anonymous_dm_change(const diff* d)
+{
+  var_decl_sptr f_dm = is_data_member(d->first_subject());
+  var_decl_sptr s_dm = is_data_member(d->second_subject());
+  return is_data_member_to_compatible_anonymous_dm_change(f_dm, s_dm);
+}
+
+/// Test if a diff node carries a change where a data member F is
+/// modified into an anonymous data member S that contains F at the
+/// same offset.
+///
+/// F and S are thus said to be compatible.
+///
+/// @param d the diff node to consider.
+///
+/// @return true iff @p d carries a change where a data member F is
+/// modified into an anonymous data member S that contains F at the
+/// same offset.
+bool
+is_data_member_to_compatible_anonymous_dm_change(const diff_sptr& d)
+{return is_data_member_to_compatible_anonymous_dm_change(d.get());}
+
+/// Test if a data member F is modified into an anonymous data member
+/// S that contains F at the same offset.
+///
+/// F and S are thus said to be compatible.
+///
+/// @param f the first data member to consider.
+///
+/// @param s the second data member to consider.
+///
+/// @return true iff @p f is modified into @p s that contains F at the
+/// same offset.
+bool
+is_data_member_to_compatible_anonymous_dm_change(const decl_base_sptr& f,
+						 const decl_base_sptr& s)
+{
+  var_decl_sptr f_dm = is_data_member(f);
+  var_decl_sptr s_dm = is_data_member(s);
+
+  if (!f_dm || !s_dm)
+    return false;
+
+  if (is_anonymous_data_member(f_dm)
+      || !is_anonymous_data_member(s_dm)
+      || get_data_member_offset(f_dm) != get_data_member_offset(s_dm))
+    return false;
+
+  string_decl_base_sptr_map non_anonymous_dms_in_second_dm;
+  class_or_union_sptr cou = anonymous_data_member_to_class_or_union(s_dm);
+  ABG_ASSERT(cou);
+  collect_non_anonymous_data_members(cou, non_anonymous_dms_in_second_dm);
+
+  for (const auto& entry : non_anonymous_dms_in_second_dm)
+    if (var_decl_sptr dm = is_data_member(entry.second))
+      if (types_are_compatible(f_dm->get_type(), dm->get_type()))
+	return true;
+
+  return false;
+}
+
 /// Test if a variable diff node carries a CV qualifier change on its type.
 ///
 /// @param dif the diff node to consider.  Note that if it's not of
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 7a1f7989..8655d7b2 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -507,6 +507,11 @@  test-abidiff-exit/test-PR32902/test-PR32902-2.v0.c \
 test-abidiff-exit/test-PR32902/test-PR32902-2.v0.o \
 test-abidiff-exit/test-PR32902/test-PR32902-2.v1.c \
 test-abidiff-exit/test-PR32902/test-PR32902-2.v1.o \
+test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v0.c \
+test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v0.o \
+test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v1.c \
+test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v1.o \
+test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-report-1.txt \
 \
 test-diff-dwarf/test0-v0.cc		\
 test-diff-dwarf/test0-v0.o			\
diff --git a/tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-report-1.txt b/tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-report-1.txt
new file mode 100644
index 00000000..9666a8fd
--- /dev/null
+++ b/tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-report-1.txt
@@ -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
+
diff --git a/tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v0.c b/tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v0.c
new file mode 100644
index 00000000..4511d1d4
--- /dev/null
+++ b/tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v0.c
@@ -0,0 +1,11 @@ 
+/* gcc -g -c replace-dm-with-compatible-anon-dm-1-v0.c */
+
+struct user_type
+{
+  int member;
+};
+
+void
+function(struct user_type * u __attribute__((unused)))
+{
+}
diff --git a/tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v0.o b/tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v0.o
new file mode 100644
index 0000000000000000000000000000000000000000..e7c301ecba6a91206f25099e5034d548eff42cd4
GIT binary patch
literal 3200
zcmcgtOK%%h6h7Az#}3mt4_D9-stgEdT9~mNk|t?MF$!%HQB@&Qq;8NI+t>CKdmP#0
zkQPvpDj`*Dg2a*~8(1On2awpXV#QxzPj{7&fOBT<v9Cu{5rM>&X3lrM^SqC_^Wgfe
z8z~`xhk$D^(wGJKEHlF85SHOA<l$iF(QiACK7DfX{!b5(PJ_sjHB50n^wVCZhEs$m
zgpu7vX^<k)8N$d;q&+{tTsXwkup5pRUw?vz$pPBV6H0yAf=t#4z30R$xx8sCEiTQ8
z`ONHdXw3^FXPQ^ctLCOD@>pvcj9jGd<~K0u#LHls*GvN&PdnnR1-4#Tpd96M_ZKK^
zG7UzS!&2t}GBX&qixzmDnxOI!&OJo*;)aKW0y`+v=z9sJ5aN&AGzpp~J_we=DW}9z
zY9>k>#-#;J4OL@GJWLtreDn5!wOv}bN+oB_DOxMVmDS>UY13NXmvw8$4V1xNyJ*>+
zVBpKGru1dctD)-lYOSr~jm!4sRr_egvYRmI%U&h8cPODT@N0qB^`Rp>RoR0=tJ{%<
zdbfVpD>S{J(Dtfs)oZ$5yRf(a#>(dUS|O1ApkJuFfg3^_?RBrwu;sBAKu;dF-I}!P
z9s46MXxX)H=g<wjYFpZ_-}Q+v*+)gE_CHoSeNS+6d;5yDjEjBo6e~^JbvE2H+|K*^
zAdY9n;+ahL1o!m?<msUio{zV4v!AE7rzdZqLqsRO=}1h#<G+%Ocn*?i{HliMWz23w
z-`f~Rih&i9h5?gFH2Q0og63hsl%|cK@suyZfHNZ}4H=Nrv=KC5W~6AyfN4#O;A<%6
zwe=Z(lc(SO;io_ud5@k!tgi}JCSITYJO$w(o}My3G@gybkXI@^uK?|+zI&H6f9`>(
zg2q$b+e&{k#=A;?FUH?dI?H%no+I@hUdYOSR>`>kON!_D-c>r?ku;e9E>fYSB=f&2
z{i>4j{FdTrZS?g4r+=>#xK)%vFOn@rfjkbtp^FKeYQGOoPqtmBE~|rP#q%3ow1wEM
zR(tYj%<T4DzbVH<+MX{t1WPa^x)SPmw{;}`z#P9DNCz(syC1l<4^?8>sx*3TN2-bZ
zN3n?Uv_`y7oUW|E@VqOXeD(1nD#V3<V9oHZhIJT^zko(MkN-BF_CKDJ>T&&tq-Z1V
zOXrGit+M7ru&55}?~0#8Nq+q8;14_;TlkLWB*QPE72rro7IEJ_G{iabeMgZ+4pcJk
zOY=<X-%=aO@zg)vw(<V!$i;Ia1%V=Ok)n;bZv|~h_4ic$4^%AGr~4&d{}4IK$$cUW
z$B(K|iwPyUKFg1hPl{hwANQaC5dXzf;$Kw<lJ7@O!*yAHrQ+Gn@*sYom4V~f?^^`K
zjon44-%;@!?5HHKAJg9;NLv5z)%x=}rS+%39>+7ERGV_2sQN9{F#U(<ZyJxMJx!YG
c6KV>_Y9xm5`24A_r1+mz|33^PBM^`O8^$9L(*OVf

literal 0
HcmV?d00001

diff --git a/tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v1.c b/tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v1.c
--- /dev/null
+++ b/tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v1.c
@@ -0,0 +1,17 @@ 
+/* gcc -g -c replace-dm-with-compatible-anon-dm-1-v0.c */
+struct user_type
+{
+  union
+  {
+    int replaced_member;
+    struct
+    {
+      int member;
+    } rh_kabi_hidden_5;
+  };
+};
+
+void
+function(struct user_type * u __attribute__((unused)))
+{
+}
diff --git a/tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v1.o b/tests/data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v1.o
new file mode 100644
index 0000000000000000000000000000000000000000..2feba3cd3ba3ea7f17d3d41abecb74e5f465d1f7
GIT binary patch
literal 3384
zcmcgu&2Jl35TEC@V<+1<AFgOZsNy0}+QP2mkTf4vic!=y5fQ2)l^d9~_r>1UUPtRq
zNQw{zBt*oeNE|tG;Twqycm4$ql`14I`~lpMkbs%>Ja_Z55fMl{X?JFR^ZDj|?7nk-
z^F~StP(#2qcq}OtU@7;QuS9GG&O!lhuOI%pe)#t94_^E6TZA(pBEuA!oR92cFEhp)
zX_3ubM<7k07&M8;0O^e9hcJ*9X`@970tW_38L>lIn~*HM(jt|+k2JSQ5)9HyjP$El
z*~F(`p<-wkb?1poecS{y*NUu%!N{*%5LeA*bEAN&G4X7^U>eg?(_>;hJ9-YlC<xPB
zG08%pMLP0`#ErrdrpP=4rg_aYaM}asj+kZ(!pO&cbHmW{8MMWVlbARPGc!r4Diil6
zDgICfj9gE#QAkYym?IdUYJ=ygcB&d;Yk`uQXQYa~){)Pk?Pa7wh(GefM5sOZATu3x
zDkY{<BfYSZZ$`jiUo=L<`zZsB1G&?3>e8*YWxFN=xcSQM;#zsJST5TO_H1!(c5Z%l
zvAkToxFy}<x)ZX(TDVlS+TpG*R~yoofmcV-3F^(&{iQ3`m3eD#u4pwNXjbn!HLu$A
zTvz(l1=#J%pc)?RNZ8)>>!H{2ffJXSowh8w9run`YItF(<<-!$;drgmt*sa4mKPUF
zq3nj;lIw&{#9F9#z3pvF?t3BdL@c*$z3GKbtKMnvIH6Z-Nz3s&KH+6+uWZ-h|I2jx
zn&9Tz+Dh>vcK^~TT2JeFHd<6%?|WMy_D99knQZO>F8ou7(?cWLKriJ-KS-?&58Ob5
z9-R25J(Gst{z_8fMoEIf;BBib4b&tJqjx79QyDOD3?snkT?fZ>M(;{Uz>|-R0cVcc
z5@1071bpNKeE1mt93tbf{`4z2{o?nJ0tHuVG=zM-D=IVLbRmx7Lu0&0fN**$EI+|i
z;Vi<F%+QMVuLI;SGfwuZ27;DiIOTnh<ySSFMQ}9yb(Y&2-eLJ&4L@M{L#BG2swY%u
zhHZ$be(3g3vyJM*9hT|RrJ?XIdLopWD*PvwuQJu+7a6Bs#`_6&_n;j*HKbwCqfNzy
zaz6x{E;6ud-7eUHY&o_oYrBoA=WlmV7x7N57RbFLYA0~~hCJ%h@_eaWFoj|Xu0}H6
zg>8u+U)%45(#A{1>V{7JE(ey)>UQ9?B~Ro(k}3439;tOw;VR7Z&j-t78y^=?AWXkO
zUXB~Zc-?;nb+mu~tv{{59+T3k{NIxh8@es+EBf=P#B7NDfj4WB9mWtNJ9XCZk02VW
z_|{{R;?GD6@HJCKblY2~&@r<8lEGQ!$W*tbc_!8WkQY?>Q~mU_rq}NxrpH7G9x=E{
zLTu=^Rn#Tr&qaTR0B^7_<)`~d&%c8h#Z+}78;u!0oQDcvs`4xP7UD_%-?6`12Ng#7
zDf-a~{@?J1EGb8(%1_Zx*k3b)pY;u``78U++5gC0gxD|aukLeYullFt&yY##f0O&K
z_9^wBzLLsc;WQ6DH$pH$L2)#BqV(OOudnV;Ynn9G`+R=pQ6C?wZwjNj68*XUe>%T^
E1AH|o7ytkO

literal 0
HcmV?d00001

diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -1609,6 +1609,21 @@  InOutSpec in_out_specs[] =
     "data/test-abidiff-exit/test-PR32902/test-PR32902-2-report-1.txt",
     "output/test-abidiff-exit/test-PR32902/test-PR32902-2-report-1.txt"
   },
+  {
+    "data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v0.o",
+    "data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v1.o",
+    "",
+    "",
+    "",
+    "",
+    "",
+    "",
+    "",
+    "--no-default-suppression",
+    abigail::tools_utils::ABIDIFF_OK,
+    "data/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-report-1.txt",
+    "output/test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-report-1.txt"
+  },
 #ifdef WITH_BTF
   {
     "data/test-abidiff-exit/btf/test0-v0.o",