From patchwork Thu May 14 08:04:15 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 39244 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 89C753897829; Thu, 14 May 2020 08:04:23 +0000 (GMT) X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by sourceware.org (Postfix) with ESMTPS id BBFDD3897829 for ; Thu, 14 May 2020 08:04:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org BBFDD3897829 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=seketeli.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=dodji@seketeli.org X-Originating-IP: 91.166.131.130 Received: from localhost (91-166-131-130.subs.proxad.net [91.166.131.130]) (Authenticated sender: dodj@seketeli.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 2FA721C0008 for ; Thu, 14 May 2020 08:04:17 +0000 (UTC) Received: by localhost (Postfix, from userid 1001) id E41A01A4BA1; Thu, 14 May 2020 10:04:15 +0200 (CEST) From: Dodji Seketeli To: libabigail@sourceware.org Subject: [PATCH] Bug 25986 - Wrong name of function type used in change report Organization: Me, myself and I X-Operating-System: Red Hat Enterprise Linux Server 7.7 X-URL: http://www.seketeli.net/~dodji X-Patchwork-State: Committed Date: Thu, 14 May 2020 10:04:15 +0200 Message-ID: <861rnnc58g.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-Spam-Status: No, score=-7.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, KAM_LOTSOFHASH, LIKELY_SPAM_BODY, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP 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: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , Errors-To: libabigail-bounces@sourceware.org Sender: "Libabigail" Hello, We have introduced type name caching long ago to speed up operations involving type names. Then last year, I was looking at some speed optimization in the xml-writer and I started playing with the caching to quantify the speed impact that early caching could have on emitting writting out the abixml, independantly from the potential accuracy issues that could have. And somehow I accidentally committed one of those experimental patches: https://sourceware.org/git/?p=libabigail.git;a=commit;h=7699dfc921d248fa6d5cbdbeab9d501b17ef3f52. This commit reverts that bogus patch. There should be no speed impact because the writter now de-duplicates types at writting time by "simply" writting out the canonical types, as opposed to the naive approach we were using then. This fixes the bug reported at https://sourceware.org/bugzilla/show_bug.cgi?id=25986 which shows the impact of caching the wrong name of the type, which happens when caching occurs before the type is canonicalized. * src/abg-ir.cc (function_type::get_cached_name): Don't cache names for non-canonicalized types. * tests/data/test-abidiff-exit/test-fun-param-report.txt: Add reference output for new test. * tests/data/test-abidiff-exit/test-fun-param-v{0,1}.abi: Add new test input files. * tests/data/test-abidiff-exit/test-fun-param-v{0,1}.c: Add source files for the above. * tests/data/test-abidiff-exit/test-fun-param-v{0,1}.o: Add binaries for the above. Signed-off-by: Dodji Seketeli Applied to master. --- src/abg-ir.cc | 4 +- tests/data/Makefile.am | 7 ++++ .../test-abidiff-exit/test-fun-param-report.txt | 15 +++++++ tests/data/test-abidiff-exit/test-fun-param-v0.abi | 44 ++++++++++++++++++++ tests/data/test-abidiff-exit/test-fun-param-v0.c | 7 ++++ tests/data/test-abidiff-exit/test-fun-param-v0.o | Bin 0 -> 2992 bytes tests/data/test-abidiff-exit/test-fun-param-v1.abi | 46 +++++++++++++++++++++ tests/data/test-abidiff-exit/test-fun-param-v1.c | 7 ++++ tests/data/test-abidiff-exit/test-fun-param-v1.o | Bin 0 -> 3000 bytes tests/test-abidiff-exit.cc | 9 ++++ 10 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 tests/data/test-abidiff-exit/test-fun-param-report.txt create mode 100644 tests/data/test-abidiff-exit/test-fun-param-v0.abi create mode 100644 tests/data/test-abidiff-exit/test-fun-param-v0.c create mode 100644 tests/data/test-abidiff-exit/test-fun-param-v0.o create mode 100644 tests/data/test-abidiff-exit/test-fun-param-v1.abi create mode 100644 tests/data/test-abidiff-exit/test-fun-param-v1.c create mode 100644 tests/data/test-abidiff-exit/test-fun-param-v1.o diff --git a/src/abg-ir.cc b/src/abg-ir.cc index bfcaf5d..7b1f460 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -16411,13 +16411,13 @@ function_type::get_cached_name(bool internal) const { if (internal) { - if (priv_->internal_cached_name_.empty()) + if (!get_naked_canonical_type() || priv_->internal_cached_name_.empty()) priv_->internal_cached_name_ = get_function_type_name(this, internal); return priv_->internal_cached_name_; } - if (priv_->cached_name_.empty()) + if (!get_naked_canonical_type() || priv_->cached_name_.empty()) priv_->cached_name_ = get_function_type_name(this, internal); return priv_->cached_name_; diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index a1b9bf6..7e91f52 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -151,6 +151,13 @@ test-abidiff-exit/test-decl-struct-v0.o \ test-abidiff-exit/test-decl-struct-v1.c \ test-abidiff-exit/test-decl-struct-v1.o \ test-abidiff-exit/test-decl-struct-report.txt \ +test-abidiff-exit/test-fun-param-report.txt \ +test-abidiff-exit/test-fun-param-v0.abi \ +test-abidiff-exit/test-fun-param-v0.c \ +test-abidiff-exit/test-fun-param-v0.o \ +test-abidiff-exit/test-fun-param-v1.abi \ +test-abidiff-exit/test-fun-param-v1.c \ +test-abidiff-exit/test-fun-param-v1.o \ \ test-diff-dwarf/test0-v0.cc \ test-diff-dwarf/test0-v0.o \ diff --git a/tests/data/test-abidiff-exit/test-fun-param-report.txt b/tests/data/test-abidiff-exit/test-fun-param-report.txt new file mode 100644 index 0000000..295cce8 --- /dev/null +++ b/tests/data/test-abidiff-exit/test-fun-param-report.txt @@ -0,0 +1,15 @@ +Functions changes summary: 0 Removed, 1 Changed, 0 Added function +Variables changes summary: 0 Removed, 0 Changed, 0 Added variable + +1 function with some indirect sub-type change: + + [C] 'function void reg(ops*)' at test-fun-param-v1.c:7:1 has some indirect sub-type changes: + parameter 1 of type 'ops*' has sub-type changes: + in pointed to type 'struct ops' at test-fun-param-v1.c:1:1: + type size hasn't changed + 1 data member change: + type of 'void (void*, unsigned int, unsigned long int)* ops::bind_class' changed: + in pointed to type 'function type void (void*, unsigned int, unsigned long int)': + parameter 4 of type 'void*' was added + parameter 5 of type 'unsigned long int' was added + diff --git a/tests/data/test-abidiff-exit/test-fun-param-v0.abi b/tests/data/test-abidiff-exit/test-fun-param-v0.abi new file mode 100644 index 0000000..816f74c --- /dev/null +++ b/tests/data/test-abidiff-exit/test-fun-param-v0.abi @@ -0,0 +1,44 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/test-abidiff-exit/test-fun-param-v0.c b/tests/data/test-abidiff-exit/test-fun-param-v0.c new file mode 100644 index 0000000..aa1198c --- /dev/null +++ b/tests/data/test-abidiff-exit/test-fun-param-v0.c @@ -0,0 +1,7 @@ +struct ops { + void(*foo)(void); + void(*bind_class)(void *, unsigned int, unsigned long); + int(*bar)(int); +}; + +void reg(struct ops* o) { (void)o; } diff --git a/tests/data/test-abidiff-exit/test-fun-param-v0.o b/tests/data/test-abidiff-exit/test-fun-param-v0.o new file mode 100644 index 0000000000000000000000000000000000000000..64218730b3fa188f61752ebbc917f49a75a92785 GIT binary patch literal 2992 zcmbtW&2QsW5T6(4!>OBW+E7R+5=LTG7GWnXXlWO92^6|5yINI9dqkC;IF3bPSGH5S zAU*^LSPp2BkPs3l_J+ir0~f?U!2yW_LPDID+e#c@#(ppByt)?{HSf**X6DVu`*7cV z`Py!QF%ZPyGTfI03UIf4pYJ7X51OzHH}-D*zIW^Gd$(Wv=~sk{iAqgYSWNiLpk??% za1=p{G1W#;83qBcLbXo@$r4PdeuliN*j5OvsD6t=14`w@yz&mIlHon#KE?s6>O!$X z%z~OsWz|DLyMqJ$R2pEhSn~WB%IX&=)I!GvR+$wh(qmu$g4#M}RC$oN!sjFOFpvYb z#Lm~7no?h_FR|rH;~~^GnWAdiCG9zFOJhwerUr_diu=t?T(pQlJqwgpV66>MJ_nk% zj~(BwFD$rNr8ntj?-lfhGme&ev@n#IjUj-CVRduVy(3)qh9OF7n7yxrA!uu z7UupGl|n@Tx@Jg}oq-Z|7e@u2#F4ST>opvcV}e`DAa#Yn7JXFDU3Y^&`*qoqxt2GEE7x!6JL}KtTjqwju5WZVy7>3BZcO5-?`*q{?}T0-MJw!&wof)M z8W*3@4OcgYR&>+vkHWwYrjarA-|+l_5nJ}yiNHqwus^nVCaDTSnK4PSso|5NRo$K;YkTnc@=({Lxedyx$4Y4rO=a z0>{ZdyYG7$oZ^1NZxbA~-kBu-`fZ*Evo#W$OYnx;eqy zgmWCG^GI;9a}t9&uwn~L8?TrdI%CTmIQG;%^!#Cfx`elEJ9Lic)cCykE?&^A%h>ZB z;esK+><5#HgHMa;2eD(~`(Q+|)qkBkIitfNK2i=({C`fX=uf>B{S?dfI5RtcT$XR> zV~7(b-^8ojUY=zCH8h}e^I!ef(N|uRVv6{`k&q$TmcAnNh3;i+h`pD7`Qa|d>0A*c z&NzL6h_>-3uSvMc!5>J-kZgN^IvFGLKRMXV_!2AIQk{AE|G_VX@TdIgm6P)y={y!b7CphTZ2KD`5S{3FC@O_3+E2@QDs9|gc?5ns%2A)e=do*yVW zUs)1DvNFunKmrpo@bCwZm*Oi3Y$cR!U*_lnld^WWq7-w}>{mizZFd^q39 literal 0 HcmV?d00001 diff --git a/tests/data/test-abidiff-exit/test-fun-param-v1.abi b/tests/data/test-abidiff-exit/test-fun-param-v1.abi new file mode 100644 index 0000000..dcc91d1 --- /dev/null +++ b/tests/data/test-abidiff-exit/test-fun-param-v1.abi @@ -0,0 +1,46 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/test-abidiff-exit/test-fun-param-v1.c b/tests/data/test-abidiff-exit/test-fun-param-v1.c new file mode 100644 index 0000000..e47f49c --- /dev/null +++ b/tests/data/test-abidiff-exit/test-fun-param-v1.c @@ -0,0 +1,7 @@ +struct ops { + void(*foo)(void); + void(*bind_class)(void *, unsigned int, unsigned long, void *, unsigned long); + int(*bar)(int); +}; + +void reg(struct ops* o) { (void) o; } diff --git a/tests/data/test-abidiff-exit/test-fun-param-v1.o b/tests/data/test-abidiff-exit/test-fun-param-v1.o new file mode 100644 index 0000000000000000000000000000000000000000..c84e4f0fe209d872c2309acefceba2cce51a998f GIT binary patch literal 3000 zcmbtV-D@LN6hAkUk4dMAX=Amef+MWd6=%{`S<{uS(Q3EVWhugnFH&Zb$z(8@DKnGW zRYVXGR}i`&s31P*vu{57qW&YkxZs1Zf)5KmxaUmn&EDKjU-XdNd(Q8ibI-^9m=9mQ zwp(Bf1TnY*4D#~lfp9TX>DdbN87~cn#22EY z1eO`oT?Dm$6agz#yJV0p!L;gE$m^Qx#K4OBk0>;tRL;yRpOPvYJ|ph-R3r|n`dqO> z%$%N0Wj#Q__yGs{>omZ8u@rsKuTStTHW8B+9 zA3Z8dY;A>H^%9U=B?T680jDb%49g&k{U+JWb5zFyZ1&=ch_$vt8FiMwd^uaHUXrpX zEMxA^P$^UdpsPfpYzfNPT^u!d8b`+dsn>AK4h(KGgVak>t3M8{BPVu7*71g&Z6cIb zpgM_xZp)>Gj|v+4jl(4HL$7C2?}t(7n=?pjoW!kR)OCg}KZ^XJ*BV44ujL=bQ8(y0 zt)6$>8V0W82EG#v;p+7p=FZ0R=9ax_ZE2iqs@ zOV*|5Ov^W|zLVSxyMs6iqj6%5!*_$QXQhri^b&B9KkN>jB*`!B>dww(^Ql)nH*ms9 zU~98|QAq(yei;8m?7cUDoiy0$d__OSjy;AL{fV-Ezf^C0QrM}LchO)1=Z4qQr-cj>&{73Hixp{NhKl>PSso|5NTmDfxxrJ1Tl(FB=E;4BH-=Lh;~Tc zjf)&7dwJjYB%I=Y$K}rzoQv>o!lJZ zUBWqyC-XpXsdtis-E&e0Y!@$>9eYE^?s@LmKMcZtgu0A(TsQWPXVmz(gg#zR*<~1n zo^ZhsV0WX@$is)l4x`kw@pZ70)aky%oxH(eAD<|XC;mUDRrIIcihhdaI-HrFKQ5~` z^a;cXQ*Ywy++Lkj|1~tAbMs&QH_%sIlVXbaf07`PYD?b``cii!n~dd$>MqCWT#=nP z3b>`)Nm*+41DSvw9)ckunYa#>} z_+@yF1c}u6hp5YoZ}I~r`b6>R9Z=&RAx3M8Jdw>P_!1V6r^{<`NKl+LYf6-UUM~zKu spYr$zJYjn8=}lGrX;1P>{f&}B67T**I@>E+H_!io=l{KM