From e2341a939c4058ba64d44375b13be302dac4e133 Mon Sep 17 00:00:00 2001
From: Dodji Seketeli <dodji@redhat.com>
Date: Wed, 13 May 2020 16:22:01 +0200
Subject: [PATCH] Bug 25986 - Wrong name of function type used in change report
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 <dodji@redhat.com>
---
src/abg-ir.cc | 4 +-
tests/data/Makefile.am | 7 +++
.../test-fun-param-report.txt | 15 ++++++
.../test-abidiff-exit/test-fun-param-v0.abi | 44 +++++++++++++++++
.../test-abidiff-exit/test-fun-param-v0.c | 7 +++
.../test-abidiff-exit/test-fun-param-v0.o | Bin 0 -> 2992 bytes
.../test-abidiff-exit/test-fun-param-v1.abi | 46 ++++++++++++++++++
.../test-abidiff-exit/test-fun-param-v1.c | 7 +++
.../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 bfcaf5d3..7b1f460a 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 a1b9bf64..7e91f52c 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 00000000..295cce84
--- /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 00000000..816f74c8
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-fun-param-v0.abi
@@ -0,0 +1,44 @@
+<abi-corpus path='test-fun-param-v0.o' architecture='elf-amd-x86_64'>
+ <elf-function-symbols>
+ <elf-symbol name='reg' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+ </elf-function-symbols>
+ <abi-instr version='1.0' address-size='64' path='test-fun-param-v0.c' comp-dir-path='/usr/local/google/home/gprocida/dev/libabigail' language='LANG_C99'>
+ <type-decl name='int' size-in-bits='32' id='type-id-1'/>
+ <type-decl name='unsigned int' size-in-bits='32' id='type-id-2'/>
+ <type-decl name='unsigned long int' size-in-bits='64' id='type-id-3'/>
+ <type-decl name='void' id='type-id-4'/>
+ <class-decl name='ops' size-in-bits='192' is-struct='yes' visibility='default' filepath='/usr/local/google/home/gprocida/dev/libabigail/test-fun-param-v0.c' line='1' column='1' id='type-id-5'>
+ <data-member access='public' layout-offset-in-bits='0'>
+ <var-decl name='foo' type-id='type-id-6' visibility='default' filepath='/usr/local/google/home/gprocida/dev/libabigail/test-fun-param-v0.c' line='2' column='1'/>
+ </data-member>
+ <data-member access='public' layout-offset-in-bits='64'>
+ <var-decl name='bind_class' type-id='type-id-7' visibility='default' filepath='/usr/local/google/home/gprocida/dev/libabigail/test-fun-param-v0.c' line='3' column='1'/>
+ </data-member>
+ <data-member access='public' layout-offset-in-bits='128'>
+ <var-decl name='bar' type-id='type-id-8' visibility='default' filepath='/usr/local/google/home/gprocida/dev/libabigail/test-fun-param-v0.c' line='4' column='1'/>
+ </data-member>
+ </class-decl>
+ <pointer-type-def type-id='type-id-9' size-in-bits='64' id='type-id-8'/>
+ <pointer-type-def type-id='type-id-5' size-in-bits='64' id='type-id-10'/>
+ <pointer-type-def type-id='type-id-11' size-in-bits='64' id='type-id-6'/>
+ <pointer-type-def type-id='type-id-12' size-in-bits='64' id='type-id-7'/>
+ <pointer-type-def type-id='type-id-4' size-in-bits='64' id='type-id-13'/>
+ <function-decl name='reg' mangled-name='reg' filepath='/usr/local/google/home/gprocida/dev/libabigail/test-fun-param-v0.c' line='7' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='reg'>
+ <parameter type-id='type-id-10' name='o' filepath='/usr/local/google/home/gprocida/dev/libabigail/test-fun-param-v0.c' line='7' column='1'/>
+ <return type-id='type-id-4'/>
+ </function-decl>
+ <function-type size-in-bits='64' id='type-id-9'>
+ <parameter type-id='type-id-1'/>
+ <return type-id='type-id-1'/>
+ </function-type>
+ <function-type size-in-bits='64' id='type-id-11'>
+ <return type-id='type-id-4'/>
+ </function-type>
+ <function-type size-in-bits='64' id='type-id-12'>
+ <parameter type-id='type-id-13'/>
+ <parameter type-id='type-id-2'/>
+ <parameter type-id='type-id-3'/>
+ <return type-id='type-id-4'/>
+ </function-type>
+ </abi-instr>
+</abi-corpus>
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 00000000..aa1198c9
--- /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~<m3w$>(BwFD$rNr8ntj?-lfhGme&ev@n#IjUj-CVRduVy(3)qh9OF7n7yxrA!uu
z7UupGl|n@Tx@Jg}oq-Z|7e@u2#F4ST>opvcV}e`DAa<hI7*2iT$O^5AaojbNt%Fh<
zs%Z>#Yn7JXFDU3Y^&`*qoq<lh9|yjx&mo;@6n4fz-x_z^AaKV{XB14Fj(ZdaeQ#iO
z2F`J3?Aey>xt2GEE7x!6JL}KtTjqwju5WZVy7>3BZcO5-?`*q{?}T0-MJw!&wof)M
z8W*3@4OcgYR&>+vkHWwYrjarA-|+l_5nJ}yiNHqwus^n<D8IBTJ3E*3Cth-F&+^m2
z#>VCaDTSnK4<sY@=*?Gwoiy0$Vnw};J$nRk`Vl1^f1%#^sIXHj@1j8p=Z<ufityJz
zc}nbU9*ny-+k-j1f|@*1__dXF3e4`toJxV@hRtClC{R6xrFi}sQ=oQ+Js$ZSQsC5+
z0xM{@oaw_XQr;1z3(rYZwt%wWNhKl>PSso|5NRo$K;YkTnc@=({Lxedyx$4Y4rO=a
z0>{ZdyYG7$oZ^1N<qssBi}0z0i%x#eaXGH2k>ZxbA~-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<gAGh
zJj^e{D<ot{j(>={y!b7CphTZ2KD`5S{3FC@O_3+E2@QDs9|gc?5ns%2A)e=do*yVW
zUs)1<V*U`7g3sDrlMf_U%C=u||GBpVu><bEnOzilQNMucAGf^zb#wYhUlHLi`bzo8
vv1#p3c>DvNFunKmrpo@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 00000000..dcc91d19
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-fun-param-v1.abi
@@ -0,0 +1,46 @@
+<abi-corpus path='test-fun-param-v1.o' architecture='elf-amd-x86_64'>
+ <elf-function-symbols>
+ <elf-symbol name='reg' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+ </elf-function-symbols>
+ <abi-instr version='1.0' address-size='64' path='test-fun-param-v1.c' comp-dir-path='/usr/local/google/home/gprocida/dev/libabigail' language='LANG_C99'>
+ <type-decl name='int' size-in-bits='32' id='type-id-1'/>
+ <type-decl name='unsigned int' size-in-bits='32' id='type-id-2'/>
+ <type-decl name='unsigned long int' size-in-bits='64' id='type-id-3'/>
+ <type-decl name='void' id='type-id-4'/>
+ <class-decl name='ops' size-in-bits='192' is-struct='yes' visibility='default' filepath='/usr/local/google/home/gprocida/dev/libabigail/test-fun-param-v1.c' line='1' column='1' id='type-id-5'>
+ <data-member access='public' layout-offset-in-bits='0'>
+ <var-decl name='foo' type-id='type-id-6' visibility='default' filepath='/usr/local/google/home/gprocida/dev/libabigail/test-fun-param-v1.c' line='2' column='1'/>
+ </data-member>
+ <data-member access='public' layout-offset-in-bits='64'>
+ <var-decl name='bind_class' type-id='type-id-7' visibility='default' filepath='/usr/local/google/home/gprocida/dev/libabigail/test-fun-param-v1.c' line='3' column='1'/>
+ </data-member>
+ <data-member access='public' layout-offset-in-bits='128'>
+ <var-decl name='bar' type-id='type-id-8' visibility='default' filepath='/usr/local/google/home/gprocida/dev/libabigail/test-fun-param-v1.c' line='4' column='1'/>
+ </data-member>
+ </class-decl>
+ <pointer-type-def type-id='type-id-9' size-in-bits='64' id='type-id-8'/>
+ <pointer-type-def type-id='type-id-5' size-in-bits='64' id='type-id-10'/>
+ <pointer-type-def type-id='type-id-11' size-in-bits='64' id='type-id-6'/>
+ <pointer-type-def type-id='type-id-12' size-in-bits='64' id='type-id-7'/>
+ <pointer-type-def type-id='type-id-4' size-in-bits='64' id='type-id-13'/>
+ <function-decl name='reg' mangled-name='reg' filepath='/usr/local/google/home/gprocida/dev/libabigail/test-fun-param-v1.c' line='7' column='1' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='reg'>
+ <parameter type-id='type-id-10' name='o' filepath='/usr/local/google/home/gprocida/dev/libabigail/test-fun-param-v1.c' line='7' column='1'/>
+ <return type-id='type-id-4'/>
+ </function-decl>
+ <function-type size-in-bits='64' id='type-id-9'>
+ <parameter type-id='type-id-1'/>
+ <return type-id='type-id-1'/>
+ </function-type>
+ <function-type size-in-bits='64' id='type-id-11'>
+ <return type-id='type-id-4'/>
+ </function-type>
+ <function-type size-in-bits='64' id='type-id-12'>
+ <parameter type-id='type-id-13'/>
+ <parameter type-id='type-id-2'/>
+ <parameter type-id='type-id-3'/>
+ <parameter type-id='type-id-13'/>
+ <parameter type-id='type-id-3'/>
+ <return type-id='type-id-4'/>
+ </function-type>
+ </abi-instr>
+</abi-corpus>
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 00000000..e47f49cc
--- /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*4<vyC+$%reI~m)7CM?2@y<5NU-TL6(>D#~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@r<Ul=W{>sKuTStTHW8B+9<IgW5XgR(XWD!tDWiSjYie
zU{BVYhE`v#FR;Z*<5AQ$nWh`YW#a{7%V13`s|K1r5%-&IT(pQlT?R@^u(kqFeh3U>
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_Z<w3wo9p;{*0e_HIP|uCFZAM|i=q>E2iqs@
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=Z<uvityLJ
zc}nbT9*lc7-GLdshMGLm`1MtF8cc7-j7o#-e$8McXiz<aP4N5^ra|o-dpz(tq`{e|
z1(wimQR>4qQr-cj>&{73Hixp{NhKl>PSso|5NTmDfxxrJ1Tl(FB=E;4BH-=Lh;~Tc
zjf)&7dwJjYB%I=Y$K}rzoQv><f{RZ6!f`dOsFC89#3DF1&avM*2RAuRHOX~>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
z<MjO@+Qz@SCgE)k{zQUAs_g;lRE*63=HR;IORQ>3b>`)Nm*+41DSvw9)ckunYa#>}
z_+@yF1c}u6hp5YoZ}I~r`b6>R9Z=&RAx3M8Jdw>P<nezN0G~yCF~5&^o_~uUC^}y<
z2|qD^j7q`Fc5kW&k}Fl)uetxs+kx0y+`la^ioB>_!1V6r^{<`NKl+LYf6-UUM~zKu
spYr$zJYjn8=}lGrX;1P>{f&}B67T**I@>E+H_!io=l{KM<g?np`#%ohBme*a
literal 0
HcmV?d00001
diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
index 5afc15bc..4d9c1943 100644
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -203,6 +203,15 @@ InOutSpec in_out_specs[] =
"data/test-abidiff-exit/test-decl-struct-report.txt",
"output/test-abidiff-exit/test-decl-struct-report.txt"
},
+ {
+ "data/test-abidiff-exit/test-fun-param-v0.abi",
+ "data/test-abidiff-exit/test-fun-param-v1.abi",
+ "",
+ "",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+ "data/test-abidiff-exit/test-fun-param-report.txt",
+ "output/test-abidiff-exit/test-fun-param-report.txt"
+ },
{0, 0, 0 ,0, abigail::tools_utils::ABIDIFF_OK, 0, 0}
};