From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x341.google.com (mail-wm1-x341.google.com [IPv6:2a00:1450:4864:20::341]) by sourceware.org (Postfix) with ESMTPS id 92913385B836 for ; Sun, 29 Mar 2020 15:13:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 92913385B836 Received: by mail-wm1-x341.google.com with SMTP id d198so16977150wmd.0 for ; Sun, 29 Mar 2020 08:13:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=euCNTNGZvCUy3krvamKIng8TVilNCrMd++xma4hIcqM=; b=VwPTREhhKKibT5PtYFwbEzotx9x7ky0wNl/nPHxZfgJ3TXH59BxQyUt4X8b9CO6SY6 z/+5kiGxNtBpjC+AsWcBtF+B7MCylDKd/IBfScOOw4Kperk1F1quXxolvrdUB6/6RmQ4 HQgTqAWVVYKj42vV9yheuaomq6262xDDfkSgZz4QoPDin/HUvQUL7HQ9amVolhABdyRM b1q3uNRmv4PR6m4/JniyZdMmFDjBQ4RU44MIQTHg0rqMg1Wj+ez9cs8TOt/9X37jrGT0 oeBT2g/EQsQEjb/UeFj5ivhMhHfvu0Ai4YZ+tc3WMZ0U2waCwkNSIguUM5Us7QpMgKr3 ocTg== X-Gm-Message-State: ANhLgQ3fqXtG/c5/qxkaU/xo0lRh1LzQ9giMDt4o9yv/iSSS2/N+pncw ADxHvWTPjEz6H7j1gNxKcBZVoA== X-Google-Smtp-Source: ADFU+vupg7c7UBsv0UfVbuDntqyD5UBWT3Mf6TeuUPsVtXWP9cCzf5CHjMxdTlKaUMthtD+De7iSdw== X-Received: by 2002:a1c:ba04:: with SMTP id k4mr9071908wmf.165.1585494836186; Sun, 29 Mar 2020 08:13:56 -0700 (PDT) Received: from google.com ([2a00:79e0:d:210:e8f7:125b:61e9:733d]) by smtp.gmail.com with ESMTPSA id a16sm16095111wmm.20.2020.03.29.08.13.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 29 Mar 2020 08:13:55 -0700 (PDT) Date: Sun, 29 Mar 2020 17:13:55 +0200 From: Matthias Maennich To: Giuliano Procida Cc: libabigail@sourceware.org, dodji@seketeli.org, kernel-team@android.com Subject: Re: [PATCH] abg-ir.cc: Add types_have_similar_structure tests. Message-ID: <20200329151355.GC101337@google.com> References: <20200326175350.144389-1-gprocida@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20200326175350.144389-1-gprocida@google.com> User-Agent: Mutt/1.12.2 (2019-09-21) X-Spam-Status: No, score=-39.9 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, LOTS_OF_MONEY, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libabigail mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Mar 2020 15:13:59 -0000 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 Thanks for adding those additional test cases! Reviewed-by: Matthias Maennich 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?!M5T5mkBv?_LE$4M`cHV|kV6{-rU;!Zemjvb5mII>UL >zq}(b5sC}YHrCd~`3f}q$C~pXmC_*Zgc;OEq5Krv`QYDaJX3wmz_Uz^O&BvcO6cPfI2si+DJcR<>9K2&sOLH1hFbpS-ocra-xz}%>fAO2UFzXUlE_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+UpmzLhmo)1Jeis!qy&yvqYdKAY5+|w)7x;Nd#&F!lf2ra}UDrM4%=hEVT&f9)vWmh9ors >z;YlQb7!unPshE{WCx*muB)JizDPh^M*#6iz-y$X_^yL0^7@I{_M2OiG(d|)~P3=ZnlxaBs16VSlmyE3!lsg#4Tzuzn{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*Iz%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;vEz2jg2&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&|-shT$iX}Ng^{evL >z`#13nC(sz_r(~K9g{$$uY~X5s&KUS8`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~_k4fI%DHjO`BuJU1cgZZ>>DUV^G >zFJ2N_w`NVkKVzQS|Adj4xyQR;ly{mZWQ-h5O#V@fbsPU#-VlmUzE8PDgIhvTca~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(& >delta 986 >zcmZ`&&ubGw6rS15HZ$2~lTBzyoe{mL%c+65f2`ON-sGoB8V65#e*mP2So9r;K7;fY!8bL?ECS(`QEz(t!3-R^%#1$cOkL%;GkbgVElz=N*l^*cjo;J*qHp|2MNjTu8keXoU2czLtHq7+CD-5H-YqvoKeK^Pz%(BWT4KL-R^Oz5i{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!?5D4YyKD4PSU*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^6Gz9lq>R=}ur$LCBBxULxf|=xA%+Q4?<9fzzaWS1C_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?@yr97%$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<zzu4FR!Oo^l3`@uR;kUmw84$p=*bGLzzM32C7FwE-#W#>?v!LNLs+%p4H$q)kf(4yQ >zY~WTnVhtE67HF5MX0-rMrzT`U_a<}zk%abLyVzqp@;+^RzpiFs&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~+^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=|Z8zzdxTYRl)yZV+{{U)^VA?6)E5ovrZ3Gm{nwkspH5f#ki5$Av~I~yot^Y9CZcs~<|Oz$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#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;!|EQE-?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 >z33aLstHV{>E)~o91fKWy;B6&>6_0}J-u2CUzDckmtjkWq4ljl(<`MkkH|ZRU >zI1OV)i9u_LN!(EeA&VhuS|3GzkC^%GQ1zp}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 >