abg-ir.cc: Add types_have_similar_structure tests.

Message ID 20200326175350.144389-1-gprocida@google.com
State Committed
Headers
Series abg-ir.cc: Add types_have_similar_structure tests. |

Commit Message

Giuliano Procida March 26, 2020, 5:53 p.m. UTC
  This is a follow-up to a recent commit. This patch adds more tests, as
suggested by a reviewer.

	* src/abg-ir.cc (types_have_similar_structure): Update TODO
	regarding structure of arrays - multidimensional arrays are
	the issue.
	* tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
	Updated following changes.
	* tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc: Add
	more cases (see below).
	* tests/data/test-abidiff-exit/test-leaf-peeling-v0.o:
	Updated.
	* tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc: Add
	comment about a potential change to local-change semantics;
	add test cases to demonstrate that * and & and * and *** are
	structurally different; add a TODO regarding multidimensional
	arrays where changes are sometimes missed in leaf mode.
	* tests/data/test-abidiff-exit/test-leaf-peeling-v1.o

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-ir.cc                                 |   2 +-
 .../test-leaf-peeling-report.txt              |  29 ++++++++++++++----
 .../test-abidiff-exit/test-leaf-peeling-v0.cc |  19 ++++++++----
 .../test-abidiff-exit/test-leaf-peeling-v0.o  | Bin 3704 -> 4528 bytes
 .../test-abidiff-exit/test-leaf-peeling-v1.cc |  24 +++++++++++----
 .../test-abidiff-exit/test-leaf-peeling-v1.o  | Bin 3752 -> 4600 bytes
 6 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v1.o b/tests/data/test-abidiff-exit/test-leaf-peeling-v1.o
index e71ade4431f80a78ee73292d60808a2ee5404640..81fcfe0a5a5abecf2d104e71859a04ab529d0b04 100644
GIT binary patch
literal 4600
zcmbtXUuaup6hB{X)8001)--mmR_Cp5xG{6rq-ob}cB`v)E4op|z4(XjCcR06?M+G2
zt(`i@5OHr)L=pPnUr=9s5LDQUpn|C25PT4QP*E88qTqwL2R-Mz=S#nQO%(A!zH`p+
z{Q1uP{^!ZN@12ea0g411f(=cf0OLIy@}v|eAqjnO^4P_nk6nD?k4uky^k2k!vV9SN
zHmTXN+fc5X0rqKfq);w{ErdOdpmi2RL`s;!j*^>fY!S5!WM2i*-bE5XyS*se3bq|>
z(Na(geP>0d{TyP}8i;rYNdR^Vn^*DIDI3h)K``e|*dL?PMTqF@`kFYzzQm@R8tAd9
zi+1V65&ib>2vId#J4phFOD8rD*h&0wGT#s*iK3}!>9ZFBSw@>|1#BS)u{}CK28$G;
zUTVaSc4CTREQs}G^i5C`d3FJl|A~e$;MNuauTTIrVZgWbfLmGwd_@7&gaL2W1BO}z
zSUBqmYQliEdVtd+AWH$%gaOAf07OFUj3;AOd@!C6eI32mqctfkI~F?-I~dy^6G<G`
z9<c0Cok@=4n6&MHSnLo6;f$hlTz78Yg0>lq>R=}ur$LCBBxULxf|=xA%+Q4?<9f<i
zzaWS1C_3N@IKoNdy)10WY5|7_BHM8MU2%9oWeH^YliOZAGC+9}l!u()cKqQdBVi#L
z>03gBVICC=Tx^B77Vp7UZfu+v0X4r`OD+0dA+_xLi>08LI-Pd&c^MuZgwC)ZN^rA?
z-gF^4IMK9?SeUiETunp8FP5q`zcMGa#d1(|R)T7&82IxJ?XT)GLNNDWdPMgena!gA
z$R<1^@eG{yDr0c;#7Spr&z^MF+3#lDw38W5?@y<PGfrx$wi5UgML+N>r97%$CBHDS
zIzE;f%Q~r|lPY-CwIIJxDF@}1YHB4oQwru&H7~d5R{{IYP%}bJmYUH`Wn^U;S&*MY
zU$M8gw7khHExpn$$JG4jQRzS0Fl3nqZKOfVHfW;&N2jI^IJ@rhb0seb$0U{Bx7!#U
zIhAl-$UvOEAH-^}80_w_FX0Nf5pntv$)n-Uc<<AZsh;*}bU=)3caRaOmHpw++|vH|
zzu4FR!Oo^l3`@uR;kUmw84$p=*bGLzzM32C7FwE-#W#>?v!LNLs+%p4H$q)kf(4yQ
zY~WTnVhtE67HF5MX0-rMrzT`U_a<}zk%abLyVzqp@;+^R<eVZl#<<cIKf(A`<U|TD
zpiFs&B)Wm0W1P+?BIVz5Q^=id?hLh)Ok5oYCf@4!Q2W;OS33_c)v!!?b1Ru3HKKH*
z)9jtXA5kJ?(x33~bpp_D;J>g=^FTx^QeS<$B^tgM^izCB!zurdtkXOYsd_>U;4BLY
ze~<My3>+^INpBnYW!B#}@SUuGXW$>Qe$Bu?VSSjlo~q{|#wQpjrmya4!#~IRaRYZ)
zzmGXF8c(&(&KUj=uzt?KKjV5XFiwo}3^D$u;jij@$H4cn|GSJ6qdaP#d}H{Jv;Mt-
zPjQ}~7$-(~)V}E=1xbykd>x{mFz{cc7p_CbRUU>L*15_-Hu>U1Zu(Dfpou$-Q#ag|
zL?#Vf)ql#s)%ct@@FCV;HgGk+;8xd`@Ww`7tAu<(Ni~192JXC9^T5qjtKe4rMbDl0
zb1TKUQcx(PP0C&_SMg6bn7xV@6!8(Dhu|y7S0UI1xcTzZl8=|Z8<cCli?5JWwdUm?
z<sg4yu2Auod@lTdvO$ceG1t!`N;ydllKI9XU!7<i2{UiBGrE%|oAI}yjm{%8{yfL1
zdxTYRl)yZV+{{U)^VA?6)E5ovrZ3Gm{nwkspH5f#ki5$Av~I~yot^Y9CZcs~<|O<w
z$NfNxMyBs9+Dwex*LgsOwOA5O-+K^iR{s^=5GtPPr`Lj6|9q322*DTJ{{kr*nf=e9
zty%x-LQ~^J{mc7{b18fob;_ygWHV$bDH8o()Jds|KZeF;@vrltL+eW?6{q-9XjHQ9
z_ql#(X$I5xC5|_i8C>D`542bk)%Yv;c@&zB-!^^_tg<7GAHBv@yc$2M$LyOzp5YDs
pDHlxd2)YN&c$yP)?;!-U?DzsH8kyfud7H&w;`(3HVo5aP{|3Dhq4EF#

delta 989
zcmZ9LO=uHA6vyZ7=3^(D-ER7k%~~so#<mn2+q5+GD{TtZdeI_CwLPSvu1RQ2ViQ3x
zB6#tb@DRb19~Bg;2t^dn_9*yqsW(Bv9=v$#!GklqlOh?|H}Cy_@6EhnW_J%g?`sT7
zJG42JI=BjicnBW~^IAUl#E3PY6d-^q9M&}D0}}%rfR3l&fM0if6C0Ab&&TRJxRII#
z;!|EQ<l=4zr#R-u#kclwh+}_&;{xC->E-?eOj)viTs@&4Q_Fe?I{nj;2QwCQu@po(
z1~JTNrhkDgA(LVJD);9SqluLj-VE{f5-K?!GqM3*xa;jr?^#JE0HsPfAH7ko74v2L
zO0-z66G!3B(fYM2sa075S+n!STHRjDR;sm8T=n+p+#$>)xfw(BEDU8AN5}q8OwNxx
z33a<xA1>LstHV{>E)~o91fKWy;B6&>6_0}J-u2CUzDckmtjkWq4ljl(<`MkkH|ZRU
zI1OV)i9u_LN!(EeA&VhuS|3GzkC^%GQ1<m=hMuuHg?g@y&f{Zh(u+cUhD~bXHZ`n8
zp}cL&k0J!j;*3y#w9&sJ{v~F98h!;~5o_Xjt=`6k-rc6Z5qbo#t7bCYM*9VwZPR}W
z{Vd>n^)#GB2xd1MK?T?|Y5KZwLyJQWKWkHt9l%LF63B^~3gDtX5qRfw<6Xdq`UL$X
zcGz<2f9r92U%aRhM>*0FmtHG)p(9PV1XyynV&J=iUl@qW?vmYT87VXj1qPgyk`b4d
zJna33@sZDi8%CTiyZ;}#iQkMA-BsMeIkZA4x-Hlpyv(9YqR(Y_w@{o;^Sfr_sjFzg
Ef0VYQb^rhX
  

Comments

Matthias Männich March 29, 2020, 3:13 p.m. UTC | #1
On Thu, Mar 26, 2020 at 05:53:50PM +0000, Giuliano Procida wrote:
>This is a follow-up to a recent commit. This patch adds more tests, as
>suggested by a reviewer.
>
>	* src/abg-ir.cc (types_have_similar_structure): Update TODO
>	regarding structure of arrays - multidimensional arrays are
>	the issue.
>	* tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
>	Updated following changes.
>	* tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc: Add
>	more cases (see below).
>	* tests/data/test-abidiff-exit/test-leaf-peeling-v0.o:
>	Updated.
>	* tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc: Add
>	comment about a potential change to local-change semantics;
>	add test cases to demonstrate that * and & and * and *** are
>	structurally different; add a TODO regarding multidimensional
>	arrays where changes are sometimes missed in leaf mode.
>	* tests/data/test-abidiff-exit/test-leaf-peeling-v1.o
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>

Thanks for adding those additional test cases!
Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>---
> src/abg-ir.cc                                 |   2 +-
> .../test-leaf-peeling-report.txt              |  29 ++++++++++++++----
> .../test-abidiff-exit/test-leaf-peeling-v0.cc |  19 ++++++++----
> .../test-abidiff-exit/test-leaf-peeling-v0.o  | Bin 3704 -> 4528 bytes
> .../test-abidiff-exit/test-leaf-peeling-v1.cc |  24 +++++++++++----
> .../test-abidiff-exit/test-leaf-peeling-v1.o  | Bin 3752 -> 4600 bytes
> 6 files changed, 55 insertions(+), 19 deletions(-)
>
>diff --git a/src/abg-ir.cc b/src/abg-ir.cc
>index 5576c137..e1e757e9 100644
>--- a/src/abg-ir.cc
>+++ b/src/abg-ir.cc
>@@ -22725,7 +22725,7 @@ 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 uint32_t[10] vs uint64_t[5] better.
>+      // TODO: Handle int[5][2] vs int[2][5] better.
>       if (ty1->get_size_in_bits() != ty2->get_size_in_bits()
> 	  || ty1->get_dimension_count() != ty2->get_dimension_count()
> 	  || !types_have_similar_structure(ty1->get_element_type(),
>diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
>index eeffc7b5..119d01b4 100644
>--- a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
>+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
>@@ -1,9 +1,9 @@
>-Leaf changes summary: 4 artifacts changed
>-Changed leaf types summary: 4 leaf types changed
>+Leaf changes summary: 6 artifacts changed
>+Changed leaf types summary: 6 leaf types changed
> Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
> Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
>
>-'struct foo at test-leaf-peeling.0.cc:1:1' changed:
>+'struct foo at test-leaf-peeling-v0.cc:1:1' changed:
>   type size changed from 32 to 64 (in bits)
>   there are data member changes:
>     type 'int' of 'foo::z' changed:
>@@ -12,7 +12,7 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
>     and size changed from 32 to 64 (in bits) (by +32 bits)
>
>
>-'struct ops1 at test-leaf-peeling.0.cc:5:1' changed:
>+'struct ops1 at test-leaf-peeling-v0.cc:5:1' changed:
>   type size hasn't changed
>   there are data member changes:
>     type 'int*' of 'ops1::x' changed:
>@@ -20,15 +20,32 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
>
>
>
>-'struct ops2 at test-leaf-peeling.0.cc:9:1' changed:
>+'struct ops2 at test-leaf-peeling-v0.cc:9:1' changed:
>   type size changed from 320 to 640 (in bits)
>   there are data member changes:
>     'foo ops2::y[10]' size changed from 320 to 640 (in bits) (by +320 bits)
>
>
>-'struct ops3 at test-leaf-peeling.0.cc:13:1' changed:
>+'struct ops3 at test-leaf-peeling-v0.cc:13:1' changed:
>   type size hasn't changed
>   there are data member changes:
>     type 'void (int&)*' of 'ops3::spong' changed:
>       pointer type changed from: 'void (int&)*' to: 'void (int&&)*'
>
>+
>+
>+'struct ops4 at test-leaf-peeling-v0.cc:17:1' changed:
>+  type size hasn't changed
>+  there are data member changes:
>+    type 'int*' of 'ops4::x' changed:
>+      entity changed from 'int*' to 'int&'
>+      type size hasn't changed
>+
>+
>+
>+'struct ops5 at test-leaf-peeling-v0.cc:21:1' changed:
>+  type size hasn't changed
>+  there are data member changes:
>+    type 'int*' of 'ops5::x' changed:
>+      pointer type changed from: 'int*' to: 'int***'
>+
>diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc b/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc
>index b927d15e..745d44f5 100644
>--- a/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc
>+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc
>@@ -14,11 +14,18 @@ struct ops3 {
>   void (*spong)(int & wibble);
> };
>
>-void register_ops1(ops1*) {
>-}
>+struct ops4 {
>+  int * x;
>+};
>+
>+struct ops5 {
>+  int * x;
>+};
>
>-void register_ops2(ops2*) {
>-}
>+int var6[2][5];
>
>-void register_ops3(ops3*) {
>-}
>+void register_ops1(ops1*) { }
>+void register_ops2(ops2*) { }
>+void register_ops3(ops3*) { }
>+void register_ops4(ops4*) { }
>+void register_ops5(ops5*) { }
>diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v0.o b/tests/data/test-abidiff-exit/test-leaf-peeling-v0.o
>index a556a63560122a5e926ab708bb13f0fd40714638..e6ed6218704e957cbac929f82d0a571fd181103f 100644
>GIT binary patch
>literal 4528
>zcmbtXTW?!M5T5mk<KsAXotR#N+v1>Bv?_LE$4M`cHV|kV6{-rU;!Zemjvb5mII>UL
>zq}(b5sC}YHrCd~`3f}q$C~pXmC_*Zgc;OEq5Krv`QYDaJX3wm<UfZCGk-RhW&3v<a
>z_Uz^O&BvcO6cPfI2si+DJcR<>9K2&sOLH1hFbpS-ocra-xz}%>fAO2UF<Zs9h5-7c
>zXUlE_yA=l5quHTyy$*ds*oP3bPJ;+V$ifPJ;i~6Dm~2~w?J{a>6-0lOEP#H=kl`I7
>zYQIC_H4p<4vH&7kY(BuBO-&S{b_BI57q`DaXMhlq7`R3nVqar3NDahg?S7fW0~oS@
>z#Q+Upm<Pat^)^i|1vYR%KEMV9W!FNseUPBSVkAu2QHvasb<~La`bjB?u^}!!jk!r`
>zLhmo)1Jeis!qy&yvqYdKAY5+|w)7x;Nd#&F!lf2ra}UDrM4%=hEVT&f9)vWmh9ors
>z;YlQb7!unPshE{WCx*muB)JizDPh^M*#6i<v3)U-!WoT&We4tTYA?=7-!_QF4#X@Q
>z-y$X_^yL0^7@I{_M2OiG(d|)~P3=ZnlxaBs16VSlmyE3!lsg#4Tzu<EE|Mr}c?U;A
>zn{e85;^2rPdves%n?8JWgy>14QxZIgKYUUY9Hb`tg4bYJK*xd{wnF@wh+`|aEzYh0
>z-);KYCD$oumtA+M>Q%BQ$8x2Tq=(ZG4f>%1_loeAx5DYEu5HM|s`ce&zGuOPTd6jE
>zw=pNPmg-(5v*I<Y70+GB&<<=aBLs8L<;V5R@#96z9|s+qfRj#R5)L0bk(t@GD__j)
>z%N26@Okpg)FP|SPWU@7X#dD`BuIDzYC3KxeX>n?G?__qen8{W$*|O7I^Gb`2x>sLm
>zW>>sZRc|5dJM&9!6R@`eHRIG2shQ{)R8bD92)q5-az`*PgZU1;An`&6J|XdmwkIsK
>zdF@YM1UNi1vp=)rF?YV|c)`PG^Lut0(*XC#cZl!!^iv>KlOjDBvCrc>-;6l@NaPvt
>zNFw=eXeQo&2on&a*uxY==@ow=2=|P?_8;*}{}Sg=hr*%bd4KEgjsOC<`nti0x7Kf4
>z+`>pVvG_(YV-~a>JFTz<^0;sL%CPV%=q6S$V;vz2;vE<PR&bto#4OM*RgR9J9@3s`
>z2jg2&4{P%y-zj2~j4NO1V~lS=O``BJ+Eiy$qId8&7^hQ-MCJE93FPiIH~QUf8+9<4
>zb=2N8akb6xMh}K5uVrNuq-K<^YkGSgfha?PlKF(o3mNr#1HZ{Wy$2Fnk^1V}Dbe6_
>zKtH!>zDZQSfd^1yL*d`D|B-?3V*gVE|DOFT20q39j|P67{of4yC-yh;7E|M?GQN#*
>zQu->+8u`z2e$l|Uv%i-$DVjI6o?bHYpJm@Q@SnMUm2py3XO!{xjQpeQUo`L;&i{mQ
>zQdCFnhp&wMDfYiJ@HqQFuqH)y)c&|-<Zr<k$!!DwBY@#AqYlGu>shT$iX}Ng^{evL
>z`#13nC(sz_r(~K9g{$$uY~X5s&KUS8`<D$|y<f;R*J^nCqV^ksUR0LvuKJK$aC`@H
>z^UWsY8t#&lTX5%BDsxq@T*sKSo%#8Od$KL;G#syj7rIWt?;BT9unWkQ>b07SA0Iid
>z?z=hsqR2LVr}PSw+{L+a!>PGE@c*gO$fr5i&m1Z_O%00q^+vh6&^!`m-dv~jB+WMY
>z4`7VW9g}~CIqJS(7c$CVokwlfq|g~^Q0~_k4f<v-y>I%DHjO`BuJU1cgZZ>>DUV^G
>zFJ2N_w`NVkKVzQS|Adj4xyQR;ly{mZWQ-h5O#V@fbsPU#-VlmU<EO6zbNma4nKcoD
>zE8PDgIhvTca~SK^|5ff^%@g%c_mtWHGGbIyjT22N!&DTdVsxlnrC-IIZu~VqbZC9)
>zqIgQbjX`DWd6)G=OADB}A2Z)rW}x_2H7=Rz`4xNtjc)UIgXeFR6KVbu_)~l}uQVRB
>vZ=yWI8~Pd#n7$Y29yIy%PRzZB5M(&<A~~9v&rfx`@sIF_%jX)GOq2f)#t@r1
>
>delta 986
>zcmZ`&&ubGw6rS15HZ$2~lTB<kORH@VH3Sk;1-ELO7C{<C3h`1gU=!Ol24b2t5$weT
>zyoe{mL%c+65f2`ON-sGoB8V65#e*mP2So9r;K7;fY!8bL?ECS(`QE<w_U+E$UlR|K
>z(t!3-R^%#1$cOkL%;GkbgVElz=N*l^*cjo;J*qHp|2MNjTu8keXoU2c<q!`1hmd0F
>zLtHq7+CD-5H-YqvoKeK^P<k<u!Skl9omS7Nr_{Qp<A{GD{^+cUqwESa<pfT^k5R+F
>z%(BWT4KL-R^O<aBb(Q%DDdUJrLyHMHj1_!W8PDxYNk$mg*6Wq2TlH48Qn%Nps`WO^
>z5i{CqFc@vvm1?VPZ>+30TGOzk96Z7e#KdtfPN25kYNu;<`Fgrx+qG)FGJH35e9JJY
>z#6eQV;0__MsT^Zvz!PP2_wvXLHsh?E2pW!kSe-lFEWGv`G%unt&`CmZGR~3nFd#|X
>zg|Eagt3wt-V?qa*rtJ9!I$boc2=t~Wx&lpVm;ywPJ<&DTrbZ_v&<~#IMd5$);6H``
>zS!mu9pLk68Zj)|>If45{^DZv(UO=h{xQX1+OnV4fk!Z$)M+80#O?45^!nSI_M>W5j
>zA0?RWN+D?BGT#{33MPeB3Wni(Fiy*YdJXnM2K)&U$ZGMxme|rS7%pii=^OFBx6sop
>z>7E4dwVc!?5D4YyK<ru(3L#6{A#k68QQ1kWvaFVL$OnONPCDmfHN>D4PSU*d6QbMD
>k3tRLZac<{fk9n^|tP8qs(FIXS2@1>$^AFC>Q`XbuU(0!qTL1t6
>
>diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc b/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc
>index 7c88df9f..439f7c0b 100644
>--- a/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc
>+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc
>@@ -7,6 +7,10 @@ struct ops1 {
> };
>
> struct ops2 {
>+  // A change to foo's size is currently considered local here.  Arguably this
>+  // should be considered non-local as the change to foo is being reported
>+  // independently.  If this happens, the test case will need to be updated (to
>+  // remove the reporting of an ops5 diff).
>   foo y[10];
> };
>
>@@ -14,11 +18,19 @@ struct ops3 {
>   void (*spong)(int && wibble);
> };
>
>-void register_ops1(ops1*) {
>-}
>+struct ops4 {
>+  int & x;
>+};
>+
>+struct ops5 {
>+  int *** x;
>+};
>
>-void register_ops2(ops2*) {
>-}
>+// TODO: This *should* be considered a local change, but currently is not.
>+int var6[5][2];
>
>-void register_ops3(ops3*) {
>-}
>+void register_ops1(ops1*) { }
>+void register_ops2(ops2*) { }
>+void register_ops3(ops3*) { }
>+void register_ops4(ops4*) { }
>+void register_ops5(ops5*) { }
>diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v1.o b/tests/data/test-abidiff-exit/test-leaf-peeling-v1.o
>index e71ade4431f80a78ee73292d60808a2ee5404640..81fcfe0a5a5abecf2d104e71859a04ab529d0b04 100644
>GIT binary patch
>literal 4600
>zcmbtXUuaup6hB{X)8001)--mmR_Cp5xG{6rq-ob}cB`v)E4op|z4(XjCcR06?M+G2
>zt(`i@5OHr)L=pPnUr=9s5LDQUpn|C25PT4QP*E88qTqwL2R-Mz=S#nQO%(A!zH`p+
>z{Q1uP{^!ZN@12ea0g411f(=cf0OLIy@}v|eAqjnO^4P_nk6nD?k4uky^k2k!vV9SN
>zHmTXN+fc5X0rqKfq);w{ErdOdpmi2RL`s;!j*^>fY!S5!WM2i*-bE5XyS*se3bq|>
>z(Na(geP>0d{TyP}8i;rYNdR^Vn^*DIDI3h)K``e|*dL?PMTqF@`kFYzzQm@R8tAd9
>zi+1V65&ib>2vId#J4phFOD8rD*h&0wGT#s*iK3}!>9ZFBSw@>|1#BS)u{}CK28$G;
>zUTVaSc4CTREQs}G^i5C`d3FJl|A~e$;MNuauTTIrVZgWbfLmGwd_@7&gaL2W1BO}z
>zSUBqmYQliEdVtd+AWH$%gaOAf07OFUj3;AOd@!C6eI32mqctfkI~F?-I~dy^6G<G`
>z9<c0Cok@=4n6&MHSnLo6;f$hlTz78Yg0>lq>R=}ur$LCBBxULxf|=xA%+Q4?<9f<i
>zzaWS1C_3N@IKoNdy)10WY5|7_BHM8MU2%9oWeH^YliOZAGC+9}l!u()cKqQdBVi#L
>z>03gBVICC=Tx^B77Vp7UZfu+v0X4r`OD+0dA+_xLi>08LI-Pd&c^MuZgwC)ZN^rA?
>z-gF^4IMK9?SeUiETunp8FP5q`zcMGa#d1(|R)T7&82IxJ?XT)GLNNDWdPMgena!gA
>z$R<1^@eG{yDr0c;#7Spr&z^MF+3#lDw38W5?@y<PGfrx$wi5UgML+N>r97%$CBHDS
>zIzE;f%Q~r|lPY-CwIIJxDF@}1YHB4oQwru&H7~d5R{{IYP%}bJmYUH`Wn^U;S&*MY
>zU$M8gw7khHExpn$$JG4jQRzS0Fl3nqZKOfVHfW;&N2jI^IJ@rhb0seb$0U{Bx7!#U
>zIhAl-$UvOEAH-^}80_w_FX0Nf5pntv$)n-Uc<<AZsh;*}bU=)3caRaOmHpw++|vH|
>zzu4FR!Oo^l3`@uR;kUmw84$p=*bGLzzM32C7FwE-#W#>?v!LNLs+%p4H$q)kf(4yQ
>zY~WTnVhtE67HF5MX0-rMrzT`U_a<}zk%abLyVzqp@;+^R<eVZl#<<cIKf(A`<U|TD
>zpiFs&B)Wm0W1P+?BIVz5Q^=id?hLh)Ok5oYCf@4!Q2W;OS33_c)v!!?b1Ru3HKKH*
>z)9jtXA5kJ?(x33~bpp_D;J>g=^FTx^QeS<$B^tgM^izCB!zurdtkXOYsd_>U;4BLY
>ze~<My3>+^INpBnYW!B#}@SUuGXW$>Qe$Bu?VSSjlo~q{|#wQpjrmya4!#~IRaRYZ)
>zzmGXF8c(&(&KUj=uzt?KKjV5XFiwo}3^D$u;jij@$H4cn|GSJ6qdaP#d}H{Jv;Mt-
>zPjQ}~7$-(~)V}E=1xbykd>x{mFz{cc7p_CbRUU>L*15_-Hu>U1Zu(Dfpou$-Q#ag|
>zL?#Vf)ql#s)%ct@@FCV;HgGk+;8xd`@Ww`7tAu<(Ni~192JXC9^T5qjtKe4rMbDl0
>zb1TKUQcx(PP0C&_SMg6bn7xV@6!8(Dhu|y7S0UI1xcTzZl8=|Z8<cCli?5JWwdUm?
>z<sg4yu2Auod@lTdvO$ceG1t!`N;ydllKI9XU!7<i2{UiBGrE%|oAI}yjm{%8{yfL1
>zdxTYRl)yZV+{{U)^VA?6)E5ovrZ3Gm{nwkspH5f#ki5$Av~I~yot^Y9CZcs~<|O<w
>z$NfNxMyBs9+Dwex*LgsOwOA5O-+K^iR{s^=5GtPPr`Lj6|9q322*DTJ{{kr*nf=e9
>zty%x-LQ~^J{mc7{b18fob;_ygWHV$bDH8o()Jds|KZeF;@vrltL+eW?6{q-9XjHQ9
>z_ql#(X$I5xC5|_i8C>D`542bk)%Yv;c@&zB-!^^_tg<7GAHBv@yc$2M$LyOzp5YDs
>pDHlxd2)YN&c$yP)?;!-U?DzsH8kyfud7H&w;`(3HVo5aP{|3Dhq4EF#
>
>delta 989
>zcmZ9LO=uHA6vyZ7=3^(D-ER7k%~~so#<mn2+q5+GD{TtZdeI_CwLPSvu1RQ2ViQ3x
>zB6#tb@DRb19~Bg;2t^dn_9*yqsW(Bv9=v$#!GklqlOh?|H}Cy_@6EhnW_J%g?`sT7
>zJG42JI=BjicnBW~^IAUl#E3PY6d-^q9M&}D0}}%rfR3l&fM0if6C0Ab&&TRJxRII#
>z;!|EQ<l=4zr#R-u#kclwh+}_&;{xC->E-?eOj)viTs@&4Q_Fe?I{nj;2QwCQu@po(
>z1~JTNrhkDgA(LVJD);9SqluLj-VE{f5-K?!GqM3*xa;jr?^#JE0HsPfAH7ko74v2L
>zO0-z66G!3B(fYM2sa075S+n!STHRjDR;sm8T=n+p+#$>)xfw(BEDU8AN5}q8OwNxx
>z33a<xA1>LstHV{>E)~o91fKWy;B6&>6_0}J-u2CUzDckmtjkWq4ljl(<`MkkH|ZRU
>zI1OV)i9u_LN!(EeA&VhuS|3GzkC^%GQ1<m=hMuuHg?g@y&f{Zh(u+cUhD~bXHZ`n8
>zp}cL&k0J!j;*3y#w9&sJ{v~F98h!;~5o_Xjt=`6k-rc6Z5qbo#t7bCYM*9VwZPR}W
>z{Vd>n^)#GB2xd1MK?T?|Y5KZwLyJQWKWkHt9l%LF63B^~3gDtX5qRfw<6Xdq`UL$X
>zcGz<2f9r92U%aRhM>*0FmtHG)p(9PV1XyynV&J=iUl@qW?vmYT87VXj1qPgyk`b4d
>zJna33@sZDi8%CTiyZ;}#iQkMA-BsMeIkZA4x-Hlpyv(9YqR(Y_w@{o;^Sfr_sjFzg
>Ef0VYQb^rhX
>
>-- 
>2.25.1.696.g5e7596f4ac-goog
>
  
Dodji Seketeli March 30, 2020, 10:36 a.m. UTC | #2
Matthias Maennich <maennich@google.com> a ?crit:

> On Thu, Mar 26, 2020 at 05:53:50PM +0000, Giuliano Procida wrote:
>>This is a follow-up to a recent commit. This patch adds more tests, as
>>suggested by a reviewer.
>>
>>	* src/abg-ir.cc (types_have_similar_structure): Update TODO
>>	regarding structure of arrays - multidimensional arrays are
>>	the issue.
>>	* tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
>>	Updated following changes.
>>	* tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc: Add
>>	more cases (see below).
>>	* tests/data/test-abidiff-exit/test-leaf-peeling-v0.o:
>>	Updated.
>>	* tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc: Add
>>	comment about a potential change to local-change semantics;
>>	add test cases to demonstrate that * and & and * and *** are
>>	structurally different; add a TODO regarding multidimensional
>>	arrays where changes are sometimes missed in leaf mode.
>>	* tests/data/test-abidiff-exit/test-leaf-peeling-v1.o
>>
>>Signed-off-by: Giuliano Procida <gprocida@google.com>
>
> Thanks for adding those additional test cases!
> Reviewed-by: Matthias Maennich <maennich@google.com>

Applied to master.

Thanks!
  

Patch

diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 5576c137..e1e757e9 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -22725,7 +22725,7 @@  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 uint32_t[10] vs uint64_t[5] better.
+      // TODO: Handle int[5][2] vs int[2][5] better.
       if (ty1->get_size_in_bits() != ty2->get_size_in_bits()
 	  || ty1->get_dimension_count() != ty2->get_dimension_count()
 	  || !types_have_similar_structure(ty1->get_element_type(),
diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
index eeffc7b5..119d01b4 100644
--- a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
@@ -1,9 +1,9 @@ 
-Leaf changes summary: 4 artifacts changed
-Changed leaf types summary: 4 leaf types changed
+Leaf changes summary: 6 artifacts changed
+Changed leaf types summary: 6 leaf types changed
 Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
 Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
 
-'struct foo at test-leaf-peeling.0.cc:1:1' changed:
+'struct foo at test-leaf-peeling-v0.cc:1:1' changed:
   type size changed from 32 to 64 (in bits)
   there are data member changes:
     type 'int' of 'foo::z' changed:
@@ -12,7 +12,7 @@  Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
     and size changed from 32 to 64 (in bits) (by +32 bits)
 
 
-'struct ops1 at test-leaf-peeling.0.cc:5:1' changed:
+'struct ops1 at test-leaf-peeling-v0.cc:5:1' changed:
   type size hasn't changed
   there are data member changes:
     type 'int*' of 'ops1::x' changed:
@@ -20,15 +20,32 @@  Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
 
 
 
-'struct ops2 at test-leaf-peeling.0.cc:9:1' changed:
+'struct ops2 at test-leaf-peeling-v0.cc:9:1' changed:
   type size changed from 320 to 640 (in bits)
   there are data member changes:
     'foo ops2::y[10]' size changed from 320 to 640 (in bits) (by +320 bits)
 
 
-'struct ops3 at test-leaf-peeling.0.cc:13:1' changed:
+'struct ops3 at test-leaf-peeling-v0.cc:13:1' changed:
   type size hasn't changed
   there are data member changes:
     type 'void (int&)*' of 'ops3::spong' changed:
       pointer type changed from: 'void (int&)*' to: 'void (int&&)*'
 
+
+
+'struct ops4 at test-leaf-peeling-v0.cc:17:1' changed:
+  type size hasn't changed
+  there are data member changes:
+    type 'int*' of 'ops4::x' changed:
+      entity changed from 'int*' to 'int&'
+      type size hasn't changed
+
+
+
+'struct ops5 at test-leaf-peeling-v0.cc:21:1' changed:
+  type size hasn't changed
+  there are data member changes:
+    type 'int*' of 'ops5::x' changed:
+      pointer type changed from: 'int*' to: 'int***'
+
diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc b/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc
index b927d15e..745d44f5 100644
--- a/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc
+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-v0.cc
@@ -14,11 +14,18 @@  struct ops3 {
   void (*spong)(int & wibble);
 };
 
-void register_ops1(ops1*) {
-}
+struct ops4 {
+  int * x;
+};
+
+struct ops5 {
+  int * x;
+};
 
-void register_ops2(ops2*) {
-}
+int var6[2][5];
 
-void register_ops3(ops3*) {
-}
+void register_ops1(ops1*) { }
+void register_ops2(ops2*) { }
+void register_ops3(ops3*) { }
+void register_ops4(ops4*) { }
+void register_ops5(ops5*) { }
diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v0.o b/tests/data/test-abidiff-exit/test-leaf-peeling-v0.o
index a556a63560122a5e926ab708bb13f0fd40714638..e6ed6218704e957cbac929f82d0a571fd181103f 100644
GIT binary patch
literal 4528
zcmbtXTW?!M5T5mk<KsAXotR#N+v1>Bv?_LE$4M`cHV|kV6{-rU;!Zemjvb5mII>UL
zq}(b5sC}YHrCd~`3f}q$C~pXmC_*Zgc;OEq5Krv`QYDaJX3wm<UfZCGk-RhW&3v<a
z_Uz^O&BvcO6cPfI2si+DJcR<>9K2&sOLH1hFbpS-ocra-xz}%>fAO2UF<Zs9h5-7c
zXUlE_yA=l5quHTyy$*ds*oP3bPJ;+V$ifPJ;i~6Dm~2~w?J{a>6-0lOEP#H=kl`I7
zYQIC_H4p<4vH&7kY(BuBO-&S{b_BI57q`DaXMhlq7`R3nVqar3NDahg?S7fW0~oS@
z#Q+Upm<Pat^)^i|1vYR%KEMV9W!FNseUPBSVkAu2QHvasb<~La`bjB?u^}!!jk!r`
zLhmo)1Jeis!qy&yvqYdKAY5+|w)7x;Nd#&F!lf2ra}UDrM4%=hEVT&f9)vWmh9ors
z;YlQb7!unPshE{WCx*muB)JizDPh^M*#6i<v3)U-!WoT&We4tTYA?=7-!_QF4#X@Q
z-y$X_^yL0^7@I{_M2OiG(d|)~P3=ZnlxaBs16VSlmyE3!lsg#4Tzu<EE|Mr}c?U;A
zn{e85;^2rPdves%n?8JWgy>14QxZIgKYUUY9Hb`tg4bYJK*xd{wnF@wh+`|aEzYh0
z-);KYCD$oumtA+M>Q%BQ$8x2Tq=(ZG4f>%1_loeAx5DYEu5HM|s`ce&zGuOPTd6jE
zw=pNPmg-(5v*I<Y70+GB&<<=aBLs8L<;V5R@#96z9|s+qfRj#R5)L0bk(t@GD__j)
z%N26@Okpg)FP|SPWU@7X#dD`BuIDzYC3KxeX>n?G?__qen8{W$*|O7I^Gb`2x>sLm
zW>>sZRc|5dJM&9!6R@`eHRIG2shQ{)R8bD92)q5-az`*PgZU1;An`&6J|XdmwkIsK
zdF@YM1UNi1vp=)rF?YV|c)`PG^Lut0(*XC#cZl!!^iv>KlOjDBvCrc>-;6l@NaPvt
zNFw=eXeQo&2on&a*uxY==@ow=2=|P?_8;*}{}Sg=hr*%bd4KEgjsOC<`nti0x7Kf4
z+`>pVvG_(YV-~a>JFTz<^0;sL%CPV%=q6S$V;vz2;vE<PR&bto#4OM*RgR9J9@3s`
z2jg2&4{P%y-zj2~j4NO1V~lS=O``BJ+Eiy$qId8&7^hQ-MCJE93FPiIH~QUf8+9<4
zb=2N8akb6xMh}K5uVrNuq-K<^YkGSgfha?PlKF(o3mNr#1HZ{Wy$2Fnk^1V}Dbe6_
zKtH!>zDZQSfd^1yL*d`D|B-?3V*gVE|DOFT20q39j|P67{of4yC-yh;7E|M?GQN#*
zQu->+8u`z2e$l|Uv%i-$DVjI6o?bHYpJm@Q@SnMUm2py3XO!{xjQpeQUo`L;&i{mQ
zQdCFnhp&wMDfYiJ@HqQFuqH)y)c&|-<Zr<k$!!DwBY@#AqYlGu>shT$iX}Ng^{evL
z`#13nC(sz_r(~K9g{$$uY~X5s&KUS8`<D$|y<f;R*J^nCqV^ksUR0LvuKJK$aC`@H
z^UWsY8t#&lTX5%BDsxq@T*sKSo%#8Od$KL;G#syj7rIWt?;BT9unWkQ>b07SA0Iid
z?z=hsqR2LVr}PSw+{L+a!>PGE@c*gO$fr5i&m1Z_O%00q^+vh6&^!`m-dv~jB+WMY
z4`7VW9g}~CIqJS(7c$CVokwlfq|g~^Q0~_k4f<v-y>I%DHjO`BuJU1cgZZ>>DUV^G
zFJ2N_w`NVkKVzQS|Adj4xyQR;ly{mZWQ-h5O#V@fbsPU#-VlmU<EO6zbNma4nKcoD
zE8PDgIhvTca~SK^|5ff^%@g%c_mtWHGGbIyjT22N!&DTdVsxlnrC-IIZu~VqbZC9)
zqIgQbjX`DWd6)G=OADB}A2Z)rW}x_2H7=Rz`4xNtjc)UIgXeFR6KVbu_)~l}uQVRB
vZ=yWI8~Pd#n7$Y29yIy%PRzZB5M(&<A~~9v&rfx`@sIF_%jX)GOq2f)#t@r1

delta 986
zcmZ`&&ubGw6rS15HZ$2~lTB<kORH@VH3Sk;1-ELO7C{<C3h`1gU=!Ol24b2t5$weT
zyoe{mL%c+65f2`ON-sGoB8V65#e*mP2So9r;K7;fY!8bL?ECS(`QE<w_U+E$UlR|K
z(t!3-R^%#1$cOkL%;GkbgVElz=N*l^*cjo;J*qHp|2MNjTu8keXoU2c<q!`1hmd0F
zLtHq7+CD-5H-YqvoKeK^P<k<u!Skl9omS7Nr_{Qp<A{GD{^+cUqwESa<pfT^k5R+F
z%(BWT4KL-R^O<aBb(Q%DDdUJrLyHMHj1_!W8PDxYNk$mg*6Wq2TlH48Qn%Nps`WO^
z5i{CqFc@vvm1?VPZ>+30TGOzk96Z7e#KdtfPN25kYNu;<`Fgrx+qG)FGJH35e9JJY
z#6eQV;0__MsT^Zvz!PP2_wvXLHsh?E2pW!kSe-lFEWGv`G%unt&`CmZGR~3nFd#|X
zg|Eagt3wt-V?qa*rtJ9!I$boc2=t~Wx&lpVm;ywPJ<&DTrbZ_v&<~#IMd5$);6H``
zS!mu9pLk68Zj)|>If45{^DZv(UO=h{xQX1+OnV4fk!Z$)M+80#O?45^!nSI_M>W5j
zA0?RWN+D?BGT#{33MPeB3Wni(Fiy*YdJXnM2K)&U$ZGMxme|rS7%pii=^OFBx6sop
z>7E4dwVc!?5D4YyK<ru(3L#6{A#k68QQ1kWvaFVL$OnONPCDmfHN>D4PSU*d6QbMD
k3tRLZac<{fk9n^|tP8qs(FIXS2@1>$^AFC>Q`XbuU(0!qTL1t6

diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc b/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc
index 7c88df9f..439f7c0b 100644
--- a/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc
+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-v1.cc
@@ -7,6 +7,10 @@  struct ops1 {
 };
 
 struct ops2 {
+  // A change to foo's size is currently considered local here.  Arguably this
+  // should be considered non-local as the change to foo is being reported
+  // independently.  If this happens, the test case will need to be updated (to
+  // remove the reporting of an ops5 diff).
   foo y[10];
 };
 
@@ -14,11 +18,19 @@  struct ops3 {
   void (*spong)(int && wibble);
 };
 
-void register_ops1(ops1*) {
-}
+struct ops4 {
+  int & x;
+};
+
+struct ops5 {
+  int *** x;
+};
 
-void register_ops2(ops2*) {
-}
+// TODO: This *should* be considered a local change, but currently is not.
+int var6[5][2];
 
-void register_ops3(ops3*) {
-}
+void register_ops1(ops1*) { }
+void register_ops2(ops2*) { }
+void register_ops3(ops3*) { }
+void register_ops4(ops4*) { }
+void register_ops5(ops5*) { }