[applied] ir: Better handle int[5][2] changed into int[2][5]

Message ID 875xjw5e1z.fsf@redhat.com
State New
Headers
Series [applied] ir: Better handle int[5][2] changed into int[2][5] |

Commit Message

Dodji Seketeli March 26, 2025, 9:36 a.m. UTC
  Hello,

Changing int[5][2] into int[2][5] should be considered as being a
local (i.e, not an indirect) type change because fundamentally the
structure of the type does change, as per the C rules in 6.2.5/20 and
6.5.2.1/3 of https://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf.

Fixed thus.

	* src/abg-ir.cc (types_have_similar_structure): Arrays of
	dimensions of different sizes should not be considered as having a
	similar structure.
	* tests/data/test-diff-filter/test-multi-array1-v{0,1}.cc: Add new
	source files for binary test input.
	* tests/data/test-diff-filter/test-multi-array1-v{0,1}.o: Add new
	binaries.
	* tests/data/test-diff-filter/test-multi-array1.txt: Add new
	reference output.
	* tests/data/Makefile.am: Add the new test material above to
	source distribution.
	* tests/test-diff-filter.cc (in_out_specs): Add the new test input
	above to the test harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Applied to the mainline.
---
 src/abg-ir.cc                                 |  27 +++++++++++++++++-
 tests/data/Makefile.am                        |   5 ++++
 .../test-diff-filter/test-multi-array1-v0.cc  |   9 ++++++
 .../test-diff-filter/test-multi-array1-v0.o   | Bin 0 -> 3264 bytes
 .../test-diff-filter/test-multi-array1-v1.cc  |   9 ++++++
 .../test-diff-filter/test-multi-array1-v1.o   | Bin 0 -> 3264 bytes
 .../test-diff-filter/test-multi-array1.txt    |  14 +++++++++
 tests/test-diff-filter.cc                     |   7 +++++
 8 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 tests/data/test-diff-filter/test-multi-array1-v0.cc
 create mode 100644 tests/data/test-diff-filter/test-multi-array1-v0.o
 create mode 100644 tests/data/test-diff-filter/test-multi-array1-v1.cc
 create mode 100644 tests/data/test-diff-filter/test-multi-array1-v1.o
 create mode 100644 tests/data/test-diff-filter/test-multi-array1.txt

new file mode 100644
index 00000000..c884d19e
new file mode 100644
index 00000000..3e5ee555
  

Patch

diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index b189f47e..51d2847d 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -28615,14 +28615,39 @@  types_have_similar_structure(const type_base* first,
   if (const array_type_def* ty1 = is_array_type(first))
     {
       const array_type_def* ty2 = is_array_type(second);
-      // TODO: Handle int[5][2] vs int[2][5] better.
       if (!indirect_type)
 	{
 	  if (ty1->get_size_in_bits() != ty2->get_size_in_bits()
 	      || ty1->get_dimension_count() != ty2->get_dimension_count())
 	    return false;
+
+	  // Handle int[5][2] vs int[2][5] ...
+	  //
+	  // 6.2.5/20 of
+	  //  https://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf
+	  //  says:
+	  //
+	  //  "Array types are characterized by their element
+	  //  type and by the number of elements in the array"
+	  //
+	  // and 6.5.2.1/3 says:
+	  //
+	  //  "arrays are stored in row-major order (last subscript
+	  //   varies fastest)."
+	  //
+	  // So, let's ensure that all dimensions (sub-ranges) have
+	  // the same length.
+
+	  for (auto r1 = ty1->get_subranges().begin(),
+		 r2 = ty1->get_subranges().begin();
+	       (r1 != ty1->get_subranges().end()
+		&& r2 != ty2->get_subranges().end());
+	       ++r1, ++r2)
+	    if ((*r1)->get_length() != (*r2)->get_length())
+	      return false;
 	}
 
+      // ... then compare the elements of the arrays.
       if (!types_have_similar_structure(ty1->get_element_type(),
 					ty2->get_element_type(),
 					/*indirect_type=*/true))
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index eee5e2b2..9f05f169 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -1369,6 +1369,11 @@  test-diff-filter/test-PR29811-0-v0.o \
 test-diff-filter/test-PR29811-0-v1.o \
 test-diff-filter/test-PR29811-0-v0.c \
 test-diff-filter/test-PR29811-0-v1.c \
+test-diff-filter/test-multi-array1-v0.cc \
+test-diff-filter/test-multi-array1-v0.o \
+test-diff-filter/test-multi-array1-v1.cc \
+test-diff-filter/test-multi-array1-v1.o \
+test-diff-filter/test-multi-array1.txt \
 \
 test-diff-suppr/test0-type-suppr-v0.cc	\
 test-diff-suppr/test0-type-suppr-v1.cc	\
diff --git a/tests/data/test-diff-filter/test-multi-array1-v0.cc b/tests/data/test-diff-filter/test-multi-array1-v0.cc
new file mode 100644
index 00000000..cdec3d7d
--- /dev/null
+++ b/tests/data/test-diff-filter/test-multi-array1-v0.cc
@@ -0,0 +1,9 @@ 
+// g++ -g -c -Wall test-multi-array1-v0.cc
+
+int tab[4][3] = {};
+
+int
+foo()
+{
+  return tab[3][2];
+}
diff --git a/tests/data/test-diff-filter/test-multi-array1-v0.o b/tests/data/test-diff-filter/test-multi-array1-v0.o
new file mode 100644
index 0000000000000000000000000000000000000000..bdc5869cfae48c86bd48b6e5180e893cb76ce428
GIT binary patch
literal 3264
zcmbtWPj4Gl5TECb?F2Wj6Z0n_plnf#(w1G@Nt&juni>M6Q7ecdMIa=UtnIb4#a>tT
zCQ5rD7eJ_&o{*6E0$dPB?nsCaz*peZOF46ZncbPq`e`jyFwwr5`MsHW^XAQa&X*tD
z|1hHz2r1ZrGfhx{)BKrS55+pX0x!ef-EaSVk|p5tUqe_MzCk?;YLeg#4Hsxg(z%3U
z+(YGk{1gpFIA&mCGF&`6k@frla)HwTWn^u%m@;q(Dc|n*LFK~KDTt6nS3iKd0NdqC
z0rQwqmx|?rvAD1}qh|A^*JvrVT~=j)iHlGuY!r+lrp+Seo?8WD@@C;y;YMM*d}ls0
z2avbn&b)wral1^R>T{3hapiaL3pWelz9kosWMY5?N;r$QSCA{E{w_`vqrKpV!eY3s
zj9Sc`kHUt2Sp`$EI+vZjmsOoi2JQDhTfu4^!`4T8=H}I_wd-cBW?!?b=5lp;wR*jF
z!(7^RTjpIS;0f#6Wz*^fN4|Tz<NEHvYoh85nuoVf*4M1HRqJ@!v^vo3`yKPhA9@`h
z-RUd>cNkc`qi*0?&S2m?s#(WXyV-=wVZY~ATK(1ouhQ{?O4n;R4X@*P-OA4HN^Na*
zxk9ePO3MkHP_$ZJyKS|-Zr~37ulu;H#^JEFxp~uE!g5`Hfl7t7$Ig#PJ>CU%Qc?@&
z@{>>TnBG8`KBDlr-z}ED&TLNSw$LDgQ%5@C3Ha-uv`HiNMq<<AUm6)0DUA$lvNSRv
zak&!)hSt6^Wxz`!j&Ng4gBvhCVke3L=S3Xh29!p0Q4GLasS#bbnZ$mpJpU&0*+kvq
zt2eF*=v9%6gT6VALl=xzU1D9%kSZWHG$lJEd<s9Yo25x4!jn~dHAK&Fd{IJrojLVG
ze7RA+!Z^i{*aU4Qa9a0M)(@CdoMhD=GEQ;GUg(4ceO=ij*(bY4PDr<Zi{p?zeV?#S
zPa=`v-$g3am<#?B>$jNe@lD35Hp%*cJ$%%oGqi`nAmWE24cx9{2kuD#HoZ4sH-<y7
zL$u{Kjyn6E-|pjFC_9bDz&#$Bb_R~$aYtRcp6`kat|KDB`=Q?K_j)cqk+$CtTpQmO
zYZy4qFF3G!xZfT)J(qX$-<gT7R3kYjiBxAH?t9kBHc1Pp&}&gof+)}NVM||9ACs^2
zp5SjYuD?Y05l<t1ed-H}srw(Wv)pG^!4!e{8ge}*O&!pH^dw^OsM}KirR~4U`+v@!
zw10y1{lABp9+O<2(SY<IV)3Zk?%|A%k?|gb`u=s>9kfl$-{gi6f6AY7*Yo!f(_<n8
zUorSGDGAbTTR4}t{tmByz`nFT-En>WBg81C<cVTPfAB)%>L+=eqIH`820sqJMd9L6
z{DeOp<3GU-S&2O(LHvY&&;FVjtmqBR2I437^xhFR@|K~#&i?CS$Xx0t@Q)~@)&FO%
zzuZ%*e+fVF7d&lkiv5t+Kjnn+U*qt1ME9rPkhHBnr=*ZBMJyim{m<Y`n*Z;d|1Ys;
IB<TMC0u^QWnE(I)

literal 0
HcmV?d00001

diff --git a/tests/data/test-diff-filter/test-multi-array1-v1.cc b/tests/data/test-diff-filter/test-multi-array1-v1.cc
--- /dev/null
+++ b/tests/data/test-diff-filter/test-multi-array1-v1.cc
@@ -0,0 +1,9 @@ 
+// g++ -g -c -Wall test-multi-array1-v1.cc
+
+int tab[3][4] = {};
+
+int
+foo()
+{
+  return tab[3][2];
+}
diff --git a/tests/data/test-diff-filter/test-multi-array1-v1.o b/tests/data/test-diff-filter/test-multi-array1-v1.o
new file mode 100644
index 0000000000000000000000000000000000000000..22ea0ed5b06cffecc2b9349165bde85c7d301e7d
GIT binary patch
literal 3264
zcmcIm&2Jl35TECb^MM;TiTOwfDBFsnez0phNz=4d6GNbB)C!_V5eNw-YkTc%vDcNo
ziPB!k1rX|`CnO~P0WOFmcO=9gz`ww$mvZI+GjC_M>(`BhkeF!S%>3Sby?Gzb7a!ca
zol^>g6s*B>O;CW7vFCD4i#2!!UWVN}-~917PrzrtXxJFOMm-N|oZuV{=V(aNnUrGP
zMdf|`6b(5&S}-!MljleBK^Vbk<kdl0c^54vEF3hAwR$~Jqq=koA|%n(_n^+fR;gUT
zxYO!Fu~e`Y<`<^b%-Gax0G3i)B~=0#c?$}KwSrZ|vRTC1N0-4GzfrhZxL(*Qt<U9V
z0mfWdpA#@FZj~rhZT8U|W_|}hy;#uemRdlHNdOk8;5^!1My{0lt2jxF)`A}j3wl{O
zwU9d-ht+<01rv!no1eLxSM6L5?e{)i!fqVF=7+oX#+578Yj(BjUUe(>Vr6l;a;<vZ
zzPRHz?K@t?6VBC3w$qIcL;qIW5B+}7K-KFv4sIQ<t~e{p&e5Xnw4u`r+xB5N2-+dK
z(_WMhdR@QV>^1KP<#rI2J3-y62W>Ctl(%=5sw>Nj<;WjIgL2c0JS{rSpw)6(K_~M2
zKyyy_uoDH2*YA4|tIko?Z8ZL`u&1Si&5ey4_C;*hrGL>W-FrOwF{wv8ppK{1{F$-w
zC%8>-AWR>T-tKpcQ(xsaCPz2XAciwX+TjuS^Y5%lEAvL;p~b%}!W)wXqqV-MFU(nx
zy4<N*%Q(I=Wx-1#4sq+018%|Okew(NoE341TQD`Gi(&!ZN`vUSO{dmd;rTa_&!qa6
zT)oo{0lg}6anN^^<In}8tjny+9#RLyhNfhNgiqinb~7}IL^yrxq#(T-vjk@qUT1;M
zLvp!MzRWnqkk|xmrf|yp3G4gJDNed;4;ZI7WG{5gf|*y=NcQQ|BRgc;pXWGaPv6I^
z)00Rf__whNRpx^K$ofs@W_+7*>P@;o;0_*kX%F2&)Q|arNPWNKxsiVyflKcYxb?vR
zT#YvU`eA!72wOdz)3R5u_x+=xX|L~vZGYIM6NJ9FU>*?(-qU)c*X{cFM7m)w@?Cse
zoI&I@KIg#x!CtHHb$wpVe`hwjQjcVxBvOrq<lM7PwrQG2g<gwV8bo=PHyeNHG|@RG
zUpae%zs<P$65Z2YFAipYIu{hv^xtP!xzDVEEduiu<Yr8o+NS~Oam<p@w59Wxwf+jP
z{~3GI`Ux`E{~lsyOmcZj1JeDNC8KG(i!&xh#=8uf>o;w;(Kf4on-7HeQ~gxCS$}|-
z851G+lEIHiNs(#W#JQ~e+nj%&eJMZPaWnrRViZ&AL{Zc4oanUvNgXF>o#nsAkHfEV
zxMUPR;ZIKSAK?R8PCR2l{DgnU{)QPWnFpE;#82$$y(4VsEkk{k{a3}1x%5xqA5h5Z
z|4-b1xu?|sDg4A=@T}Yv`vK=a;ezpBWBof~`qOVn)>5BQQAihJmW<~5r*S6B|2MAx
L=fpD>O#goXxd`~s

literal 0
HcmV?d00001

diff --git a/tests/data/test-diff-filter/test-multi-array1.txt b/tests/data/test-diff-filter/test-multi-array1.txt
--- /dev/null
+++ b/tests/data/test-diff-filter/test-multi-array1.txt
@@ -0,0 +1,14 @@ 
+Functions changes summary: 0 Removed, 0 Changed, 0 Added function
+Variables changes summary: 0 Removed, 1 Changed, 0 Added variable
+
+1 Changed variable:
+
+  [C] 'int tab[4][3]' was changed to 'int tab[3][4]' at test-multi-array1-v1.cc:3:1:
+    type of variable changed:
+      type name changed from 'int[4][3]' to 'int[3][4]'
+      type size hasn't changed
+      array subrange 1 changed: 
+        upper bound of '<anonymous range>[4]' change from '3' to '2'
+      array subrange 2 changed: 
+        upper bound of '<anonymous range>[3]' change from '2' to '3'
+
diff --git a/tests/test-diff-filter.cc b/tests/test-diff-filter.cc
index 4b0da48f..94abc906 100644
--- a/tests/test-diff-filter.cc
+++ b/tests/test-diff-filter.cc
@@ -836,6 +836,13 @@  InOutSpec in_out_specs[] =
     "data/test-diff-filter/test-PR29811-0-report-1.txt",
     "output/test-diff-filter/test-PR29811-0-report-1.txt",
   },
+  {
+    "data/test-diff-filter/test-multi-array1-v0.o",
+    "data/test-diff-filter/test-multi-array1-v1.o",
+    "--harmless --no-default-suppression",
+    "data/test-diff-filter/test-multi-array1.txt",
+    "output/test-diff-filter/test-multi-array1.txt",
+  },
 #ifdef WITH_CTF
   {
    "data/test-diff-filter/test-PR29811-unknown-size-array-dwarf-ctf-DWARF.o",