* [PATCH 0/4] Fix incomplete function type bug in abg-reader
@ 2020-05-05 18:06 Giuliano Procida
2020-05-05 18:06 ` [PATCH 1/4] Add XML reader test for incomplete function types Giuliano Procida
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Giuliano Procida @ 2020-05-05 18:06 UTC (permalink / raw)
To: libabigail; +Cc: dodji, kernel-team, gprocida, maennich
This series is to be applied in sequence. The test can be moved after
the other changes and folded into one commit, if you prefer.
This seemed liked the simplest way to fix the issue, side-stepping
some uses of type names completely.
Regards,
Giuliano.
Giuliano Procida (4):
Add XML reader test for incomplete function types.
abg-reader.cc: Late canonicalise all types.
abg-reader.cc: Strip out WIP type tracking.
Pass bind_function_type_life_time a complete type.
src/abg-reader.cc | 173 ++++--------------
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, 168 insertions(+), 140 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
--
2.26.2.526.g744177e7f7-goog
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/4] Add XML reader test for incomplete function types.
2020-05-05 18:06 [PATCH 0/4] Fix incomplete function type bug in abg-reader Giuliano Procida
@ 2020-05-05 18:06 ` Giuliano Procida
2020-05-12 16:22 ` Matthias Maennich
2020-05-05 18:06 ` [PATCH 2/4] abg-reader.cc: Late canonicalise all types Giuliano Procida
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Giuliano Procida @ 2020-05-05 18:06 UTC (permalink / raw)
To: libabigail; +Cc: dodji, kernel-team, gprocida, maennich
This commit illustrates a bug caused by early caching of type names.
* tests/data/Makefile.am: Add new test files.
* tests/data/test-abidiff-exit/test-fun-param-v0.c: New test.
* tests/data/test-abidiff-exit/test-fun-param-v0.o: Ditto.
* tests/data/test-abidiff-exit/test-fun-param-v0.abi: Ditto.
* tests/data/test-abidiff-exit/test-fun-param-v1.c: Ditto.
* tests/data/test-abidiff-exit/test-fun-param-v1.o: Ditto.
* tests/data/test-abidiff-exit/test-fun-param-v1.abi: Ditto.
* tests/data/test-abidiff-exit/test-fun-param-report.txt: New
test report showing incorrect type "void ()*:.
* tests/test-abidiff-exit.cc: Run new test case.
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
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 ++++
9 files changed, 135 insertions(+)
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/tests/data/Makefile.am b/tests/data/Makefile.am
index a1b9bf64..461d9481 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-v0.c \
+test-abidiff-exit/test-fun-param-v0.o \
+test-abidiff-exit/test-fun-param-v0.abi \
+test-abidiff-exit/test-fun-param-v1.c \
+test-abidiff-exit/test-fun-param-v1.o \
+test-abidiff-exit/test-fun-param-v1.abi \
+test-abidiff-exit/test-fun-param-report.txt \
\
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..6bfbcf0d
--- /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 ()* 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}
};
--
2.26.2.526.g744177e7f7-goog
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/4] abg-reader.cc: Late canonicalise all types.
2020-05-05 18:06 [PATCH 0/4] Fix incomplete function type bug in abg-reader Giuliano Procida
2020-05-05 18:06 ` [PATCH 1/4] Add XML reader test for incomplete function types Giuliano Procida
@ 2020-05-05 18:06 ` Giuliano Procida
2020-05-05 18:06 ` [PATCH 3/4] abg-reader.cc: Strip out WIP type tracking Giuliano Procida
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Giuliano Procida @ 2020-05-05 18:06 UTC (permalink / raw)
To: libabigail; +Cc: dodji, kernel-team, gprocida, maennich
The XML reader has ability to canonicalise types both immediately
after creation and after a translation unit (or corpus) has been
read. The latter is referred to as late canonicalisation and needs to
be chosen for a variety of reasons.
Early canonicalisation doesn't appear to bring any performance
advantages and doing things with incomplete types is the cause of at
least one diff reporting bug.
* src/abg-reader.cc: (maybe_canonicalize_type): Rename this
function to canonicalize_type_later and make it
unconditionally schedule its type argument for later
canonicalisation.
(perform_late_type_canonicalizing): Update doc comment.
(read_context::build_or_get_type_decl): Substitute call to
maybe_canonicalize_type with call to canonicalize_type_later.
(build_function_decl): Ditto.
(build_class_decl): Ditto.
(build_union_decl): Ditto.
(build_class_tdecl): Ditto.
(build_type_tparameter): Ditto.
(build_type_composition): Ditto.
(build_template_tparameter): Ditto.
(handle_type_decl): Ditto.
(handle_qualified_type_decl): Ditto.
(handle_pointer_type_def): Ditto.
(handle_reference_type_def): Ditto.
(handle_function_type): Avoid canonicalising null type.
Substitute call to maybe_canonicalize_type with call to
canonicalize_type_later.
(handle_array_type_def): Ditto.
(handle_enum_type_decl): Substitute call to
maybe_canonicalize_type with call to canonicalize_type_later.
(handle_typedef_decl): Ditto.
(handle_class_decl): Ditto.
(handle_union_decl): Ditto.
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
src/abg-reader.cc | 104 ++++++++++++++--------------------------------
1 file changed, 32 insertions(+), 72 deletions(-)
diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 255a200f..a2bd3c5c 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -889,62 +889,21 @@ public:
clear_decls_stack();
}
- /// Test if a type should be canonicalized early. If so,
- /// canonicalize it right away. Otherwise, schedule it for late
- /// canonicalizing; that is, schedule it so that it's going to be
- /// canonicalized when the translation unit is fully read.
+ /// Schedule all types for canonicalizing once the translation unit
+ /// is fully read.
///
/// @param t the type to consider for canonicalizing.
void
- maybe_canonicalize_type(type_base_sptr t,
- bool force_delay = false)
+ canonicalize_type_later(type_base_sptr t)
{
- if (!t)
- return;
+ ABG_ASSERT(t);
+ ABG_ASSERT(!t->get_canonical_type());
- if (t->get_canonical_type())
- return;
+ // Check that class types have been properly added to their contexts.
+ if (class_decl_sptr c = is_class_type(t))
+ ABG_ASSERT(c->get_scope());
- // If this class has some non-canonicalized sub type, then wait
- // for the when we've read all the translation unit to
- // canonicalize all of its non-canonicalized sub types and then we
- // can canonicalize this one.
- //
- // Also, if this is a declaration-only class, wait for the end of
- // the translation unit reading so that we have its definition and
- // then we'll use that for canonicalizing it.
- if (!force_delay
- && !type_has_non_canonicalized_subtype(t)
- && !is_class_type(t)
- && !is_union_type(t)
- && !is_wip_type(t)
- // Below are types that *must* be canonicalized only after
- // they are added to their context; but then this function
- // might be called to early, before they are actually added to
- // their context.
- //
- // TODO: make sure this function is called after types are
- // added to their context, so that we can try to
- // early-canonicalize some of these types, reducing the size
- // of the set of types to put on the side, waiting for being
- // canonicalized.
- && !is_method_type(t)
- && !is_reference_type(t)
- && !is_pointer_type(t)
- && !is_qualified_type(t)
- && !is_typedef(t)
- && !is_enum_type(t)
- && !is_function_type(t))
- canonicalize(t);
- else
- {
- // We do not want to try to canonicalize a class type that
- // hasn't been properly added to its context.
- if (class_decl_sptr c = is_class_type(t))
- ABG_ASSERT(c->get_scope());
-
- schedule_type_for_late_canonicalizing(t);
- }
+ schedule_type_for_late_canonicalizing(t);
}
/// Schedule a type for being canonicalized after the current
@@ -955,9 +914,9 @@ public:
schedule_type_for_late_canonicalizing(type_base_sptr t)
{m_types_to_canonicalize.push_back(t);}
- /// Perform the canonicalizing of types that ought to be done after
- /// the current translation unit is read. This function is called
- /// when the current corpus is fully built.
+ /// Perform the canonicalizing of types after the current
+ /// translation unit is read. This function is called when the
+ /// current corpus is fully built.
void
perform_late_type_canonicalizing()
{
@@ -1413,7 +1372,7 @@ read_context::build_or_get_type_decl(const string& id,
if (add_decl_to_scope)
pop_scope_or_abort(scope);
- maybe_canonicalize_type(t, !add_decl_to_scope);
+ canonicalize_type_later(t);
}
return t;
}
@@ -3217,7 +3176,7 @@ build_function_decl(read_context& ctxt,
ctxt.get_translation_unit()->bind_function_type_life_time(fn_type);
- ctxt.maybe_canonicalize_type(fn_type, !add_to_current_scope);
+ ctxt.canonicalize_type_later(fn_type);
ctxt.maybe_add_fn_to_exported_decls(fn_decl.get());
@@ -4653,7 +4612,7 @@ build_class_decl(read_context& ctxt,
decl_base_sptr td = get_type_declaration(t);
ABG_ASSERT(td);
set_member_access_specifier(td, access);
- ctxt.maybe_canonicalize_type(t, !add_to_current_scope);
+ ctxt.canonicalize_type_later(t);
xml_char_sptr i= XML_NODE_GET_ATTRIBUTE(p, "id");
string id = CHAR_STR(i);
ABG_ASSERT(!id.empty());
@@ -5012,7 +4971,7 @@ build_union_decl(read_context& ctxt,
decl_base_sptr td = get_type_declaration(t);
ABG_ASSERT(td);
set_member_access_specifier(td, access);
- ctxt.maybe_canonicalize_type(t, !add_to_current_scope);
+ ctxt.canonicalize_type_later(t);
xml_char_sptr i= XML_NODE_GET_ATTRIBUTE(p, "id");
string id = CHAR_STR(i);
ABG_ASSERT(!id.empty());
@@ -5268,7 +5227,7 @@ build_class_tdecl(read_context& ctxt,
add_to_current_scope))
{
if (c->get_scope())
- ctxt.maybe_canonicalize_type(c, /*force_delay=*/false);
+ ctxt.canonicalize_type_later(c);
class_tmpl->set_pattern(c);
}
}
@@ -5335,7 +5294,7 @@ build_type_tparameter(read_context& ctxt,
ABG_ASSERT(result->get_environment());
- ctxt.maybe_canonicalize_type(result, /*force_delay=*/false);
+ ctxt.canonicalize_type_later(result);
return result;
}
@@ -5389,8 +5348,7 @@ build_type_composition(read_context& ctxt,
build_qualified_type_decl(ctxt, n,
/*add_to_current_scope=*/true)))
{
- ctxt.maybe_canonicalize_type(composed_type,
- /*force_delay=*/true);
+ ctxt.canonicalize_type_later(composed_type);
result->set_composed_type(composed_type);
break;
}
@@ -5517,7 +5475,7 @@ build_template_tparameter(read_context& ctxt,
if (result)
{
ctxt.key_type_decl(result, id);
- ctxt.maybe_canonicalize_type(result, /*force_delay=*/false);
+ ctxt.canonicalize_type_later(result);
}
return result;
@@ -5607,7 +5565,7 @@ handle_type_decl(read_context& ctxt,
{
type_decl_sptr decl = build_type_decl(ctxt, node, add_to_current_scope);
if (decl && decl->get_scope())
- ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+ ctxt.canonicalize_type_later(decl);
return decl;
}
@@ -5640,7 +5598,7 @@ handle_qualified_type_decl(read_context& ctxt,
build_qualified_type_decl(ctxt, node,
add_to_current_scope);
if (decl && decl->get_scope())
- ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+ ctxt.canonicalize_type_later(decl);
return decl;
}
@@ -5657,7 +5615,7 @@ handle_pointer_type_def(read_context& ctxt,
pointer_type_def_sptr decl = build_pointer_type_def(ctxt, node,
add_to_current_scope);
if (decl && decl->get_scope())
- ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+ ctxt.canonicalize_type_later(decl);
return decl;
}
@@ -5674,7 +5632,7 @@ handle_reference_type_def(read_context& ctxt,
reference_type_def_sptr decl = build_reference_type_def(ctxt, node,
add_to_current_scope);
if (decl && decl->get_scope())
- ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+ ctxt.canonicalize_type_later(decl);
return decl;
}
@@ -5690,7 +5648,8 @@ handle_function_type(read_context& ctxt,
{
function_type_sptr type = build_function_type(ctxt, node,
add_to_current_scope);
- ctxt.maybe_canonicalize_type(type, /*force_delay=*/true);
+ if (type)
+ ctxt.canonicalize_type_later(type);
return type;
}
@@ -5706,7 +5665,8 @@ handle_array_type_def(read_context& ctxt,
{
array_type_def_sptr decl = build_array_type_def(ctxt, node,
add_to_current_scope);
- ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+ if (decl)
+ ctxt.canonicalize_type_later(decl);
return decl;
}
@@ -5722,7 +5682,7 @@ handle_enum_type_decl(read_context& ctxt,
build_enum_type_decl_if_not_suppressed(ctxt, node,
add_to_current_scope);
if (decl && decl->get_scope())
- ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+ ctxt.canonicalize_type_later(decl);
return decl;
}
@@ -5737,7 +5697,7 @@ handle_typedef_decl(read_context& ctxt,
typedef_decl_sptr decl = build_typedef_decl(ctxt, node,
add_to_current_scope);
if (decl && decl->get_scope())
- ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+ ctxt.canonicalize_type_later(decl);
return decl;
}
@@ -5788,7 +5748,7 @@ handle_class_decl(read_context& ctxt,
class_decl_sptr decl =
build_class_decl_if_not_suppressed(ctxt, node, add_to_current_scope);
if (decl && decl->get_scope())
- ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+ ctxt.canonicalize_type_later(decl);
return decl;
}
@@ -5806,7 +5766,7 @@ handle_union_decl(read_context& ctxt,
union_decl_sptr decl =
build_union_decl_if_not_suppressed(ctxt, node, add_to_current_scope);
if (decl && decl->get_scope())
- ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+ ctxt.canonicalize_type_later(decl);
return decl;
}
--
2.26.2.526.g744177e7f7-goog
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] abg-reader.cc: Strip out WIP type tracking.
2020-05-05 18:06 [PATCH 0/4] Fix incomplete function type bug in abg-reader Giuliano Procida
2020-05-05 18:06 ` [PATCH 1/4] Add XML reader test for incomplete function types Giuliano Procida
2020-05-05 18:06 ` [PATCH 2/4] abg-reader.cc: Late canonicalise all types Giuliano Procida
@ 2020-05-05 18:06 ` Giuliano Procida
2020-05-05 18:06 ` [PATCH 4/4] Pass bind_function_type_life_time a complete type Giuliano Procida
2020-05-13 14:38 ` [PATCH 0/4] Fix incomplete function type bug in abg-reader Dodji Seketeli
4 siblings, 0 replies; 10+ messages in thread
From: Giuliano Procida @ 2020-05-05 18:06 UTC (permalink / raw)
To: libabigail; +Cc: dodji, kernel-team, gprocida, maennich
This functionality was needed only for early canonicalisation of types
and is now no longer used.
* src/abg-reader.cc (read_context): Drop m_wip_types_map
member. Drop clear_wip_classes_map, mark_type_as_wip,
unmark_type_as_wip and is_wip_type methods.
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
src/abg-reader.cc | 67 -----------------------------------------------
1 file changed, 67 deletions(-)
diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index a2bd3c5c..27bc9d51 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -119,7 +119,6 @@ private:
unordered_map<string, vector<type_base_sptr> > m_types_map;
unordered_map<string, shared_ptr<function_tdecl> > m_fn_tmpl_map;
unordered_map<string, shared_ptr<class_tdecl> > m_class_tmpl_map;
- unordered_map<string, size_t> m_wip_types_map;
vector<type_base_sptr> m_types_to_canonicalize;
string_xml_node_map m_id_xml_node_map;
xml_node_decl_base_sptr_map m_xml_node_decl_map;
@@ -518,66 +517,6 @@ public:
clear_types_to_canonicalize()
{m_types_to_canonicalize.clear();}
- /// Clean the map of classes that are "Work In Progress"; that is,
- /// the map of the class that are currently being built, but at not
- /// yet fully built.
- void
- clear_wip_classes_map()
- {m_wip_types_map.clear();}
-
- /// Mark a given type as being "Work In Progress"; that is, mark it
- /// as being currently built.
- ///
- /// @param t the class to mark as being "Work In Progress".
- void
- mark_type_as_wip(const type_base_sptr t)
- {
- if (!t)
- return;
- string qname = get_type_name(t, /*qualified=*/true);
- unordered_map<string, size_t>::iterator it = m_wip_types_map.find(qname);
- if (it == m_wip_types_map.end())
- m_wip_types_map[qname] = 1;
- else
- ++it->second;
- }
-
- /// Mark a given class as being *NOT* "Work In Progress" anymore;
- /// that is, mark it as being fully built.
- ///
- /// @param t the type to mark as being built.
- void
- unmark_type_as_wip(const type_base_sptr t)
- {
- if (!t)
- return;
-
- string qname = get_type_name(t, /*qualified=*/true);
- unordered_map<string, size_t>::iterator it = m_wip_types_map.find(qname);
- if (it == m_wip_types_map.end())
- return;
- if (it->second)
- --it->second;
- if (it->second == 0)
- m_wip_types_map.erase(it);
- }
-
- /// Test if a type is being currently built; that is, if it's "Work
- /// In Progress".
- ///
- /// @param t the type to consider.
- bool
- is_wip_type(const type_base_sptr t) const
- {
- if (!t)
- return false;
-
- string qname = get_type_name(t, /*qualified=*/true);
- unordered_map<string, size_t>::const_iterator i =
- m_wip_types_map.find(qname);
- return i != m_wip_types_map.end();
- }
-
/// Test if two types are equal, without comparing them structurally.
///
/// This either tests that type pointers are equal, or it tests
@@ -3815,7 +3754,6 @@ build_function_type(read_context& ctxt,
parms, size, align));
ctxt.get_translation_unit()->bind_function_type_life_time(fn_type);
- ctxt.mark_type_as_wip(fn_type);
ctxt.key_type_decl(fn_type, id);
for (xmlNodePtr n = node->children; n ; n = n->next)
@@ -3842,7 +3780,6 @@ build_function_type(read_context& ctxt,
}
fn_type->set_parameters(parms);
- ctxt.unmark_type_as_wip(fn_type);
return fn_type;
}
@@ -4538,7 +4475,6 @@ build_class_decl(read_context& ctxt,
ctxt.push_decl_to_current_scope(decl, add_to_current_scope);
ctxt.map_xml_node_to_decl(node, decl);
- ctxt.mark_type_as_wip(decl);
ctxt.key_type_decl(decl, id);
// If this class has a naming typedef, get it and refer to it.
@@ -4762,7 +4698,6 @@ build_class_decl(read_context& ctxt,
}
ctxt.pop_scope_or_abort(decl);
- ctxt.unmark_type_as_wip(decl);
return decl;
}
@@ -4945,7 +4880,6 @@ build_union_decl(read_context& ctxt,
ctxt.push_decl_to_current_scope(decl, add_to_current_scope);
ctxt.map_xml_node_to_decl(node, decl);
- ctxt.mark_type_as_wip(decl);
ctxt.key_type_decl(decl, id);
for (xmlNodePtr n = node->children; !is_decl_only && n; n = n->next)
@@ -5097,7 +5031,6 @@ build_union_decl(read_context& ctxt,
}
ctxt.pop_scope_or_abort(decl);
- ctxt.unmark_type_as_wip(decl);
return decl;
}
--
2.26.2.526.g744177e7f7-goog
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] Pass bind_function_type_life_time a complete type.
2020-05-05 18:06 [PATCH 0/4] Fix incomplete function type bug in abg-reader Giuliano Procida
` (2 preceding siblings ...)
2020-05-05 18:06 ` [PATCH 3/4] abg-reader.cc: Strip out WIP type tracking Giuliano Procida
@ 2020-05-05 18:06 ` Giuliano Procida
2020-05-13 14:38 ` [PATCH 0/4] Fix incomplete function type bug in abg-reader Dodji Seketeli
4 siblings, 0 replies; 10+ messages in thread
From: Giuliano Procida @ 2020-05-05 18:06 UTC (permalink / raw)
To: libabigail; +Cc: dodji, kernel-team, gprocida, maennich
Calling bind_function_type_life_time with an incomplete function type
is remaining cause of incomplete pretty representations of types being
generated and cached and then later used in reports.
This patch moves the call to this function to after the type is
complete. Update test that illustrated incomplete function type.
* src/abg-reader.cc: Move call to bind_function_type_life_time
to end of function.
* tests/data/test-abidiff-exit/test-fun-param-report.txt: Fix
incomplete function type.
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
src/abg-reader.cc | 2 +-
tests/data/test-abidiff-exit/test-fun-param-report.txt | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 27bc9d51..d7c48919 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -3753,7 +3753,6 @@ build_function_type(read_context& ctxt,
: new function_type(return_type,
parms, size, align));
- ctxt.get_translation_unit()->bind_function_type_life_time(fn_type);
ctxt.key_type_decl(fn_type, id);
for (xmlNodePtr n = node->children; n ; n = n->next)
@@ -3780,6 +3779,7 @@ build_function_type(read_context& ctxt,
}
fn_type->set_parameters(parms);
+ ctxt.get_translation_unit()->bind_function_type_life_time(fn_type);
return fn_type;
}
diff --git a/tests/data/test-abidiff-exit/test-fun-param-report.txt b/tests/data/test-abidiff-exit/test-fun-param-report.txt
index 6bfbcf0d..295cce84 100644
--- a/tests/data/test-abidiff-exit/test-fun-param-report.txt
+++ b/tests/data/test-abidiff-exit/test-fun-param-report.txt
@@ -8,7 +8,7 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
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 ()* ops::bind_class' changed:
+ 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
--
2.26.2.526.g744177e7f7-goog
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] Add XML reader test for incomplete function types.
2020-05-05 18:06 ` [PATCH 1/4] Add XML reader test for incomplete function types Giuliano Procida
@ 2020-05-12 16:22 ` Matthias Maennich
2020-05-13 18:39 ` Giuliano Procida
0 siblings, 1 reply; 10+ messages in thread
From: Matthias Maennich @ 2020-05-12 16:22 UTC (permalink / raw)
To: Giuliano Procida; +Cc: libabigail, dodji, kernel-team
On Tue, May 05, 2020 at 07:06:09PM +0100, Giuliano Procida wrote:
>This commit illustrates a bug caused by early caching of type names.
>
> * tests/data/Makefile.am: Add new test files.
> * tests/data/test-abidiff-exit/test-fun-param-v0.c: New test.
> * tests/data/test-abidiff-exit/test-fun-param-v0.o: Ditto.
> * tests/data/test-abidiff-exit/test-fun-param-v0.abi: Ditto.
> * tests/data/test-abidiff-exit/test-fun-param-v1.c: Ditto.
> * tests/data/test-abidiff-exit/test-fun-param-v1.o: Ditto.
> * tests/data/test-abidiff-exit/test-fun-param-v1.abi: Ditto.
> * tests/data/test-abidiff-exit/test-fun-param-report.txt: New
> test report showing incorrect type "void ()*:.
> * tests/test-abidiff-exit.cc: Run new test case.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>
>---
> 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 ++++
> 9 files changed, 135 insertions(+)
> 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/tests/data/Makefile.am b/tests/data/Makefile.am
>index a1b9bf64..461d9481 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-v0.c \
>+test-abidiff-exit/test-fun-param-v0.o \
>+test-abidiff-exit/test-fun-param-v0.abi \
>+test-abidiff-exit/test-fun-param-v1.c \
>+test-abidiff-exit/test-fun-param-v1.o \
>+test-abidiff-exit/test-fun-param-v1.abi \
>+test-abidiff-exit/test-fun-param-report.txt \
> \
> 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..6bfbcf0d
>--- /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 ()* 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
You might want to create those with --no-show-locs or --short-locs and
--no-comp-dir-path to avoid too much friction when regenerating them at
some point.
Looks otherwise good to me.
Reviewed-by: Matthias Maennich <maennich@google.com>
Cheers,
Matthias
>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}
> };
>
>--
>2.26.2.526.g744177e7f7-goog
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/4] Fix incomplete function type bug in abg-reader
2020-05-05 18:06 [PATCH 0/4] Fix incomplete function type bug in abg-reader Giuliano Procida
` (3 preceding siblings ...)
2020-05-05 18:06 ` [PATCH 4/4] Pass bind_function_type_life_time a complete type Giuliano Procida
@ 2020-05-13 14:38 ` Dodji Seketeli
2020-05-13 18:30 ` Giuliano Procida
4 siblings, 1 reply; 10+ messages in thread
From: Dodji Seketeli @ 2020-05-13 14:38 UTC (permalink / raw)
To: Giuliano Procida; +Cc: libabigail, kernel-team, maennich
Giuliano Procida <gprocida@google.com> a écrit:
> This series is to be applied in sequence. The test can be moved after
> the other changes and folded into one commit, if you prefer.
>
> This seemed liked the simplest way to fix the issue, side-stepping
> some uses of type names completely.
I don't think the early canonicalization is the problem. I think the
problem is that I wrongly introduced a change that does type name
caching too early. That is, it caches type names on non-canonicalized
types. The commit that introduced that change should never have went
inn to begin with. My bad.
Below is what I think is the proper fix.
Also, I filed this bug to track this issue
https://sourceware.org/bugzilla/show_bug.cgi?id=25986, so that I can
refer to it in the commit.
Here is the patch that should fix your issue. Please tell me if it
fixes your issue. If it does, then I'd prefer to apply this one.
From 15f19a0622daa51d2233e21405b1478b9437d6c3 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-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 @@
+<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 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~<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 0000000..dcc91d1
--- /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 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*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 5afc15b..4d9c194 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}
};
--
1.8.3.1
--
Dodji
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/4] Fix incomplete function type bug in abg-reader
2020-05-13 14:38 ` [PATCH 0/4] Fix incomplete function type bug in abg-reader Dodji Seketeli
@ 2020-05-13 18:30 ` Giuliano Procida
2020-05-14 8:14 ` Dodji Seketeli
0 siblings, 1 reply; 10+ messages in thread
From: Giuliano Procida @ 2020-05-13 18:30 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: libabigail, kernel-team, Matthias Männich
Hi.
On Wed, 13 May 2020 at 15:38, Dodji Seketeli <dodji@seketeli.org> wrote:
> Giuliano Procida <gprocida@google.com> a écrit:
>
> > This series is to be applied in sequence. The test can be moved after
> > the other changes and folded into one commit, if you prefer.
> >
> > This seemed liked the simplest way to fix the issue, side-stepping
> > some uses of type names completely.
>
> I don't think the early canonicalization is the problem. I think the
> problem is that I wrongly introduced a change that does type name
> caching too early. That is, it caches type names on non-canonicalized
> types. The commit that introduced that change should never have went
> inn to begin with. My bad.
>
The change to eliminate early canonicalisation has some residual value in
terms of code simplification. Let me know if you'd like me to adjust it and
repost it with that as the aim. Otherwise, I'll leave it be.
> Below is what I think is the proper fix.
>
> Also, I filed this bug to track this issue
> https://sourceware.org/bugzilla/show_bug.cgi?id=25986, so that I can
> refer to it in the commit.
>
> Here is the patch that should fix your issue. Please tell me if it
> fixes your issue. If it does, then I'd prefer to apply this one.
>
It clearly fixes the minimal test case. I've also run the code on some much
larger XML-XML comparisons both with and without --leaf-changes-only and it
appears to fix the issue and not do anything else. So, yes, it does.
Regards,
Giuliano.
> From 15f19a0622daa51d2233e21405b1478b9437d6c3 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-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 @@
> +<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 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~<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 0000000..dcc91d1
> --- /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 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*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 5afc15b..4d9c194 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}
> };
>
> --
> 1.8.3.1
>
> --
> Dodji
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] Add XML reader test for incomplete function types.
2020-05-12 16:22 ` Matthias Maennich
@ 2020-05-13 18:39 ` Giuliano Procida
0 siblings, 0 replies; 10+ messages in thread
From: Giuliano Procida @ 2020-05-13 18:39 UTC (permalink / raw)
To: Matthias Maennich; +Cc: libabigail, Dodji Seketeli, kernel-team
I'd rather not add non-default options for testing as this makes the tests
a little further away from what will usually happen in practice.
I normally have a little one line of script dance to cp the files to /tmp,
cd and compile them there, and copy them back, but I forgot on this
occasion.
Dodji has copied the test cases verbatim, your comments are transportable
to his commit.
Giuliano.
On Tue, 12 May 2020 at 17:22, Matthias Maennich <maennich@google.com> wrote:
> On Tue, May 05, 2020 at 07:06:09PM +0100, Giuliano Procida wrote:
> >This commit illustrates a bug caused by early caching of type names.
> >
> > * tests/data/Makefile.am: Add new test files.
> > * tests/data/test-abidiff-exit/test-fun-param-v0.c: New test.
> > * tests/data/test-abidiff-exit/test-fun-param-v0.o: Ditto.
> > * tests/data/test-abidiff-exit/test-fun-param-v0.abi: Ditto.
> > * tests/data/test-abidiff-exit/test-fun-param-v1.c: Ditto.
> > * tests/data/test-abidiff-exit/test-fun-param-v1.o: Ditto.
> > * tests/data/test-abidiff-exit/test-fun-param-v1.abi: Ditto.
> > * tests/data/test-abidiff-exit/test-fun-param-report.txt: New
> > test report showing incorrect type "void ()*:.
> > * tests/test-abidiff-exit.cc: Run new test case.
> >
> >Signed-off-by: Giuliano Procida <gprocida@google.com>
> >---
> > 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 ++++
> > 9 files changed, 135 insertions(+)
> > 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/tests/data/Makefile.am b/tests/data/Makefile.am
> >index a1b9bf64..461d9481 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-v0.c \
> >+test-abidiff-exit/test-fun-param-v0.o \
> >+test-abidiff-exit/test-fun-param-v0.abi \
> >+test-abidiff-exit/test-fun-param-v1.c \
> >+test-abidiff-exit/test-fun-param-v1.o \
> >+test-abidiff-exit/test-fun-param-v1.abi \
> >+test-abidiff-exit/test-fun-param-report.txt \
> > \
> > 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..6bfbcf0d
> >--- /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 ()* 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
>
> You might want to create those with --no-show-locs or --short-locs and
> --no-comp-dir-path to avoid too much friction when regenerating them at
> some point.
>
> Looks otherwise good to me.
>
> Reviewed-by: Matthias Maennich <maennich@google.com>
>
> Cheers,
> Matthias
>
> >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}
> > };
> >
> >--
> >2.26.2.526.g744177e7f7-goog
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/4] Fix incomplete function type bug in abg-reader
2020-05-13 18:30 ` Giuliano Procida
@ 2020-05-14 8:14 ` Dodji Seketeli
0 siblings, 0 replies; 10+ messages in thread
From: Dodji Seketeli @ 2020-05-14 8:14 UTC (permalink / raw)
To: Giuliano Procida; +Cc: libabigail, kernel-team, Matthias Männich
Hello,
Giuliano Procida <gprocida@google.com> a écrit:
> The change to eliminate early canonicalisation has some residual value in
> terms of code simplification. Let me know if you'd like me to adjust it and
> repost it with that as the aim. Otherwise, I'll leave it be.
Early canonicalization was introduced because it does save time during
canonicalization of big binaries. Presumably, one reason why you don't
see its advantage when reading abixml files in practice at the moment is
because abixml files are now (since quite recently) de-duplicated;
meaning they contain each type just once. This de-duplication happens
thanks to the type canonicalization that is done when reading binaries
and the de-duplication work done at abixml writting time. But then, we
still want to be able to read non-de-duplicated abixml files,
canonicalize them efficiently and write them back in a de-duplicated
form.
So I prefer not applying that early canonicalization elimitation patch.
It does introduce complexity, but that complexity is there for a reason.
>> Below is what I think is the proper fix.
>>
>> Also, I filed this bug to track this issue
>> https://sourceware.org/bugzilla/show_bug.cgi?id=25986, so that I can
>> refer to it in the commit.
>>
>> Here is the patch that should fix your issue. Please tell me if it
>> fixes your issue. If it does, then I'd prefer to apply this one.
>>
>
> It clearly fixes the minimal test case. I've also run the code on some much
> larger XML-XML comparisons both with and without --leaf-changes-only and it
> appears to fix the issue and not do anything else. So, yes, it does.
Thank you for having tested it. I've applied it and closed the
associated bug report.
[...]
Cheers,
--
Dodji
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-05-14 8:14 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-05 18:06 [PATCH 0/4] Fix incomplete function type bug in abg-reader Giuliano Procida
2020-05-05 18:06 ` [PATCH 1/4] Add XML reader test for incomplete function types Giuliano Procida
2020-05-12 16:22 ` Matthias Maennich
2020-05-13 18:39 ` Giuliano Procida
2020-05-05 18:06 ` [PATCH 2/4] abg-reader.cc: Late canonicalise all types Giuliano Procida
2020-05-05 18:06 ` [PATCH 3/4] abg-reader.cc: Strip out WIP type tracking Giuliano Procida
2020-05-05 18:06 ` [PATCH 4/4] Pass bind_function_type_life_time a complete type Giuliano Procida
2020-05-13 14:38 ` [PATCH 0/4] Fix incomplete function type bug in abg-reader Dodji Seketeli
2020-05-13 18:30 ` Giuliano Procida
2020-05-14 8:14 ` Dodji Seketeli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).