From f2437aabad085e9f9c99d28e94ac0f046e224763 Mon Sep 17 00:00:00 2001 From: Dodji Seketeli Date: Fri, 19 Jul 2019 17:45:27 +0200 Subject: [PATCH] Bug 24787 - Filter out enum changes into compatible integer types Libabigail's filtering engine fails to recognize an enum changing into a compatible integer (or vice versa) as a harmless change. This patch fixes that. * include/abg-comparison.h (peel_typedef_or_qualified_type_diff): Declare new function. (peel_pointer_or_qualified_type_diff): Rename peel_pointer_or_qualified_type into this. * include/abg-fwd.h (is_enum_type): Declare a new overload for type_or_decl_base*. * src/abg-comp-filter.cc (has_harmless_enum_to_int_change): Define new static function. * src/abg-comparison.cc (categorize_harmless_diff_node): Use the new has_harmless_enum_to_int_change here. (peel_pointer_or_qualified_type_diff): Renamed peel_pointer_or_qualified_type into this. (is_diff_of_basic_type): Adjust. (peel_typedef_or_qualified_type_diff): Define new function. * test-diff-filter/PR24787-lib{one, two}.so: New test input binaries. * test-diff-filter/PR24787-{one, two}.c: Source files of the test input binaries above. * test-diff-filter/PR24787-report-0.txt: Test output reference. * tests/data/Makefile.am: Add the new testing material to source distribution. * tests/test-diff-filter.cc (in_out_specs): Add the new test to the test harness. Signed-off-by: Dodji Seketeli --- include/abg-comparison.h | 5 ++- include/abg-fwd.h | 3 ++ src/abg-comp-filter.cc | 51 ++++++++++++++++++++++- src/abg-comparison.cc | 31 +++++++++++++- tests/data/Makefile.am | 5 +++ tests/data/test-diff-filter/PR24787-libone.so | Bin 0 -> 9336 bytes tests/data/test-diff-filter/PR24787-libtwo.so | Bin 0 -> 9200 bytes tests/data/test-diff-filter/PR24787-one.c | 13 ++++++ tests/data/test-diff-filter/PR24787-report-0.txt | 3 ++ tests/data/test-diff-filter/PR24787-two.c | 13 ++++++ tests/test-diff-filter.cc | 7 ++++ 11 files changed, 126 insertions(+), 5 deletions(-) create mode 100755 tests/data/test-diff-filter/PR24787-libone.so create mode 100755 tests/data/test-diff-filter/PR24787-libtwo.so create mode 100644 tests/data/test-diff-filter/PR24787-one.c create mode 100644 tests/data/test-diff-filter/PR24787-report-0.txt create mode 100644 tests/data/test-diff-filter/PR24787-two.c diff --git a/include/abg-comparison.h b/include/abg-comparison.h index 66392b9..8537a43 100644 --- a/include/abg-comparison.h +++ b/include/abg-comparison.h @@ -2799,7 +2799,10 @@ const diff* peel_qualified_diff(const diff* dif); const diff* -peel_pointer_or_qualified_type(const diff*dif); +peel_pointer_or_qualified_type_diff(const diff* dif); + +const diff* +peel_typedef_or_qualified_type_diff(const diff* dif); }// end namespace comparison }// end namespace abigail diff --git a/include/abg-fwd.h b/include/abg-fwd.h index 3a71743..ec37af6 100644 --- a/include/abg-fwd.h +++ b/include/abg-fwd.h @@ -423,6 +423,9 @@ is_typedef(type_base*); enum_type_decl_sptr is_enum_type(const type_or_decl_base_sptr&); +const enum_type_decl* +is_enum_type(const type_or_decl_base*); + bool is_class_type(const type_or_decl_base&); diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc index 5362b53..d412bca 100644 --- a/src/abg-comp-filter.cc +++ b/src/abg-comp-filter.cc @@ -1035,6 +1035,52 @@ has_harmful_enum_change(const diff* diff) return false; } +/// Test if a diff node carries a harmless change of an enum into an +/// integer (or vice-versa). +/// +/// The test takes into account the fact change we care about might be +/// wrapped into a typedef or qualified type diff. +/// +/// @param diff the diff node to consider. +/// +/// @return true if @p diff is a harmless enum to integer change. +static bool +has_harmless_enum_to_int_change(const diff* diff) +{ + if (!diff) + return false; + + diff = peel_typedef_or_qualified_type_diff(diff); + + if (const distinct_diff *d = is_distinct_diff(diff)) + { + const enum_type_decl *enum_type = 0; + const type_base *integer_type = 0; + + type_base *first_type = + peel_qualified_or_typedef_type(is_type(d->first().get())); + type_base *second_type = + peel_qualified_or_typedef_type(is_type(d->second().get())); + + if (const enum_type_decl *e = is_enum_type(first_type)) + enum_type = e; + else if (const enum_type_decl *e = is_enum_type(second_type)) + enum_type = e; + + if (const type_base * i = is_type_decl(first_type)) + integer_type = i; + else if (const type_base *i = is_type_decl(second_type)) + integer_type = i; + + if (enum_type + && integer_type + && enum_type->get_size_in_bits() == integer_type->get_size_in_bits()) + return true; + } + + return false; +} + /// Test if an @ref fn_parm_diff node has a top cv qualifier change on /// the type of the function parameter. /// @@ -1385,8 +1431,9 @@ categorize_harmless_diff_node(diff *d, bool pre) || static_data_member_type_size_changed(f, s)) category |= STATIC_DATA_MEMBER_CHANGE_CATEGORY; - if (has_enumerator_insertion(d) - && !has_harmful_enum_change(d)) + if ((has_enumerator_insertion(d) + && !has_harmful_enum_change(d)) + || has_harmless_enum_to_int_change(d)) category |= HARMLESS_ENUM_CHANGE_CATEGORY; if (function_name_changed_but_not_symbol(d)) diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc index feae5f9..55033b0 100644 --- a/src/abg-comparison.cc +++ b/src/abg-comparison.cc @@ -11551,7 +11551,7 @@ const type_decl_diff* is_diff_of_basic_type(const diff* diff, bool allow_indirect_type) { if (allow_indirect_type) - diff = peel_pointer_or_qualified_type(diff); + diff = peel_pointer_or_qualified_type_diff(diff); return is_diff_of_basic_type(diff); } @@ -11646,7 +11646,7 @@ peel_qualified_diff(const diff* dif) /// @return the underlying diff node of @p dif, or just return @p dif /// if it's not a pointer, reference or qualified diff node. const diff* -peel_pointer_or_qualified_type(const diff*dif) +peel_pointer_or_qualified_type_diff(const diff*dif) { while (true) { @@ -11662,6 +11662,33 @@ peel_pointer_or_qualified_type(const diff*dif) return dif; } +/// If a diff node is about changes between two typedefs or qualified +/// types, get the diff node about changes between the underlying +/// types. +/// +/// Note that this function walks the tree of underlying diff nodes +/// returns the first diff node about types that are not typedef or +/// qualified types. +/// +/// @param dif the dif node to consider. +/// +/// @return the underlying diff node of @p dif, or just return @p dif +/// if it's not typedef or qualified diff node. +const diff* +peel_typedef_or_qualified_type_diff(const diff *dif) +{ + while (true) + { + if (const typedef_diff *d = is_typedef_diff(dif)) + dif = peel_typedef_diff(d); + else if (const qualified_type_diff *d = is_qualified_type_diff(dif)) + dif = peel_qualified_diff(d); + else + break; + } + return dif; +} + /// Test if a diff node represents a diff between two class or union /// types. /// diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 982d32e..3474b2e 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -704,6 +704,11 @@ test-diff-filter/test-PR24731-v0.c \ test-diff-filter/test-PR24731-v0.o \ test-diff-filter/test-PR24731-v1.c \ test-diff-filter/test-PR24731-v1.o \ +test-diff-filter/PR24787-libone.so \ +test-diff-filter/PR24787-libtwo.so \ +test-diff-filter/PR24787-one.c \ +test-diff-filter/PR24787-two.c \ +test-diff-filter/PR24787-report-0.txt \ \ test-diff-suppr/test0-type-suppr-v0.cc \ test-diff-suppr/test0-type-suppr-v1.cc \ diff --git a/tests/data/test-diff-filter/PR24787-libone.so b/tests/data/test-diff-filter/PR24787-libone.so new file mode 100755 index 0000000000000000000000000000000000000000..d86352bee69e9941f10a775f91036eab6ddfbdf8 GIT binary patch literal 9336 zcmeHNZ*Wvs6~Avcn_WoQEKq9rQ+c+b1gCkyK+;%DSrRsEFc4hARB-U+?e5#%$Lt@v z?=4Ljs|?uM*gyQL5X`O?9UxKG&w}uK zD|;8$6{^=2JiR-DE<=<0NGhDMvI15ze9ip8+7vCr-*&&N|?O|8m9#(mW=#01( zq9Ip}H37xzMIo1IhtMO6y4dzya?;p;3I}aK*SIhvN_HOrJ7FR;ZcHLVoDk#97nhKT z2T^f5{N8Q?AAY!Y?B2(A96WHEeJ1hJBTLGkPR5`3g?j0`OZK_<0Pl2zv)pBWj)W>l|-qof7_efy?^%T$Vv(`lUr);dm>nh=+U| z@YrsKYZ+U@J|{nXUC$OuMcsCcvZL!vw~Cg-^b8QDkBslq(`MPsTDD`B$9E0oOGR_s zNajsf)>xvaCJl68KRm`%(Wvl39Sl&veG`pnr zG2KiXj=^+2Y1{6+04k>0j?t09A-zZI(R%qUF|p);DxQC?43TN*()z8evS1nc*we1K z_FmI_7^S#~_m*(}QllRUC;md4$~~evt{rL@JvgpyYL`7Yy^|=r=E1#>bk>8@6j8rj ztP^}cC=Q6nPh1Vf9}YaRSY`1CW}Lv~O&ok7bQY!8TebHqpi*7;B>c9mjY1Fk94cJ7 zScSIk0PvYp)pdtBjh_shf=>M8z;Ph^;u~h;)0g5WX2bXS4xWVpJ5UG%zIL#B5&ePw zepf%?(SL5%AD@0c{@fq8#+CE&pUhUc@rvpqG=3S3uCuf1})u`do7B#PnO5V56DbH(c`JNFL;Q0Ni+l zc$}0)K)wZFBST*(m`DFg=(9+C!K+A`g7gszCfr0XXU~UnuZZ1Vh0#aO+c= zzXV*+0LAc4gJB%$|`Uma+11 zYSlA&BWttau`zvkWOO{1(8uG6Sd76@Hmg>Owv{cydco&1Tq+H7llt>%O(&Hz%B~q| zcjrn4vpZc%AF{f$meURAS0ia<4J+TBu_jHs`*7JYZO883ljzxeXaAjbGq`7r*|}2L zsh@xgp>sD-Hk_ZGsl%q%bZmFb4d4xa{Sa!ZBYD;Q@L*^ktH=F}_OE)piP1h+j|Uj- z7xj2Ei;8`u9uKmKhIok4zE-b4x2}P5KU>TuXan{b_+ghl)(~&4)eAl9;HNP( zFVlwZG4QjtTAj@UgP#a{v7!FbT798jA1Y4bJ5!G@XQ77pO||b{z5WWQGpb&Q_x(`i zp?ZW@=*O>!R98Z|AHEWDUCG7itLgQE`n9iRoxsoNt6B$xK7CDP{FZt^aDMjAIXLbe zP3%?NKI>X5a0;6nH=MZ-vI43TlM&7k@fm5IXac7?59x1*j^c?A&I8+CxD>1$O7TAm zIQp;UFQ^RdLAVq)cRhR!@J9DF-~B$#{X|$NB>=!j?Z0J zzeGn3*8iWm{@gm|KLswYw+P(WGKdfD`nRE5Xtw~a@?QPVdjW4$CyjDHbKl=#j@SD@ z?f~HGT$ovaN8tI&_12=m<@e&~Jbpek5B?P3ynrK({Q}^P>b5LZc5J7T$-oCv*LMyj z^wE*M<4};jUVqh{f}X+(tPL7aLe1t&Nh7bPol@D>jmjiTl?sRRremhHzD>Qojg@$1 zS2xOKV+x9@PI-!D%0|J|)0IMD3QRm4F9p}Bx}POoA5LuB71Lw8w`0MWPnP;=yQJrg zVj3&N+xPF@whO+)x(?rUJr);c@$CtKpqNY!c8rbeKQXypZzXCOM4QNBd{yi7rSIn=d~zc=$q^* z|04Pf$h=Lst3h$-;TezYRdItv=?hNtCq1J3z#h+x#7BiaQB{m2&tJeS4IPw`{e-Y5 zIwKTmeEI&1!hTTbQGQ62;)M2))y#?CBw!d5O3Uc)V?^oxL4`LlJz^Q2u)OvYB3~e? z)->G?{e6ijKc|6+#y=+PiIN_s{}N`z z1EaXpylEXBm&T{_#03lrv`&}Qe#xHb??K|d|5_d>22g93ZinoMz5+1zY5tUlsluN0 z$c?=I{t5!bHj;v%WZxy>WKZol1#S0eV*~p2z&Z|ksH~*N7&2v`MR)QFBuT)rL=d@7)aISE_Yt+hbpi=aMAOm keI55FdiKz*wQEA*M>LR-NG!u?HnRV&*!Tw|1Bqq-0c+1fEdT%j literal 0 HcmV?d00001 diff --git a/tests/data/test-diff-filter/PR24787-libtwo.so b/tests/data/test-diff-filter/PR24787-libtwo.so new file mode 100755 index 0000000000000000000000000000000000000000..c18d26a211586bf5e7e3bd9f44d8c8251653c029 GIT binary patch literal 9200 zcmeHNeQX>@6`#HH#h09Oz9bDeNlTX2X&qFrQzv%a1h*GEcFw{1avVV_rR(+G-rYv` zVRrWtJEAm7AYe*R0I5Wz{83a&X@!IY5{>u+mz1bbA%!AP1VRc$fdr~HEgv#adb~F~ z@3J>uCl!D22lAwQZ|3(tX6Ehe%$w&vHZVLGQWQod%pPReIvw$lq+m>}5fvaw*25a% z_fGafE-OTD2>5FEHhK-BjHw*JJ{G|(64R0rYkcW<*JEFX#ysA#rn(5bP1r-+X?F z3V#q4)8TWw4Qzzr*}iK(d!!mWdG?ia_dW2~?oHp@{LSj#b7x<}E|8C}g#7>688xCd zBo`yL7@inBeFMLI{RhAQ(kCu`{A0JDJM~ul=_h_Q^ygo^DLf7W0R`u;2H?j5TONR8 z|7rkk%!BvLgO31yXMmkA1D0U#XKF&N<37Xj7S<-=Ulq74kMCtUc&0~Mv0Ty}*RWk(XS!9gT&8D%Fnwrrr=BrwGiN!jX^-ygFO*B> zs4-qJJzg+JPfr?Xz$jQpL2$=meMDI3FBpzvIxJJxopRB1^HwRxCTy$ZW?8`+PiszD z>t?!{FD+)%__Hu>bQ2OT=wC(wy9k8;q*-+@4657ztUMBPD4cb z7O_t7`yf9co;vY*H1&AoYfDs?`t*z&xw?^qFGbHI_eQC9eH}!q>z;<+=Cw(vVLOik z*DhC~ta}*R+0)f^hd520ikyZ@>QwJ>Ap2A6XH(NxQYU8PM?weBLx-)%gbv?6SiOw) zK>mOyKjM>raWelRDA+ zM_@yj(^uRk=>P3Tm0dMqgje(MbMQjsIjC;Vrnw;5&E$Byd0Y8JHRaiUs#REXvUlrc5T3HBF7t)38ul0Ykk> z&{2W1cw#W#vZFb6BzlZJw0g^jHn#HZ17)_sgA?Z8^456bWT=1XqDR0G>MfK74=i|K z!2=5(Sn$CAfd^`Fm_OKoc~QJYF&4&lSeiUii-X0rtrq955`4P9(RGo|&eej?M0|(m zEf(Vc?cHh_+t)?M6r1HmP)Eb0`ydYrK>quv1SRGX)*@&v78LOt#csr>ctKoG;iljf zRdh`Fm_sh@fERfd`*0rvb-??3nbLaVzCDSf-XcA z=GmYr53LHvH^;Wb?pcMR$(ASj;B$yXn1>Fn2;nN}LCZWARg{0lmofzkc8UMFN6K0n z8c-I8Xl&sn%D1BjLU8L+>Mn9{In1e*l4Ipc@CI}RKhB<_jntEQ?wpOMyy zCUDBHko-2NC_ew-II!%4N5PDp){)K2hD4~`99C>Bv>0A zywJvS4xS)CATP-NkGcHZb@eL8=dP>YprL^E{|1+zn?L-gz~%LpfQc;!|Ip@r7n+4~ z2jD8N)&D#Icu;;a$?eSjeg`>TZv$Ho1Fp`6nFV+P-mjeJED2oxE{@J)=L_@T&jQX9 zCsN-p0UneuWog@W+)6eJ=bWzZ=pWIChxUy^BJ@VyQ+JDc8Z(~`NWh7gE0o8Lf}U~9 zwxb)BNtP}bCkm!(X0+~&U0uOKJfo@`wrxy7GSjuESk^X*rk<%3i&LQDYw;9pov3#V z>-ylx)|~_Tz^-kWsO5vDdd4a1d83rU%$?XB_YI8dqg(ri2VmO%)a*?Yz#>b_GsRvGH<|ROXywE*rf}aRkZPnRmikk8OB?A!^tx(GgSgjkGi(UIc(aF zRW8*vbl}-$!9WF3n<%(UigRP|Wn03K@IO#q_2tz5~UNI#!qrY z_k%v()u^2m`b1UHlRSO_vkX*_NBUzzpXiJbr2ggkmxO+wkfZpJDESHX@lGo1PXdNM zp|FhpE=82)4bsr?yApW+yz zQR#)t_D)Iq^mjI*bT6m=3(Pn?w7w|cLVquo<*7~dhoIZ+)33#OXcP*`d9535!e0Rf`h@zYxITIdeX@5^ z(x-e=;yvpB7W8rLQ~z{Np#L9WM!YccJB^#x(Q&DNx=&m}r$A}*NbQ&OiT(jN{`uG9 zK+%C()ATB&NAyjAu}ba + + +typedef enum FooFilter { + FOO_FILTER_ONE = 1 << 0, + FOO_FILTER_TWO = 1 << 1, + FOO_FILTER_THREE = 1 << 2, +} FooFilter; + + +void do_something(FooFilter filter_flags) { + printf("%d\n", filter_flags); +} diff --git a/tests/data/test-diff-filter/PR24787-report-0.txt b/tests/data/test-diff-filter/PR24787-report-0.txt new file mode 100644 index 0000000..9666a8f --- /dev/null +++ b/tests/data/test-diff-filter/PR24787-report-0.txt @@ -0,0 +1,3 @@ +Functions changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added function +Variables changes summary: 0 Removed, 0 Changed, 0 Added variable + diff --git a/tests/data/test-diff-filter/PR24787-two.c b/tests/data/test-diff-filter/PR24787-two.c new file mode 100644 index 0000000..4308181 --- /dev/null +++ b/tests/data/test-diff-filter/PR24787-two.c @@ -0,0 +1,13 @@ +#include + + +typedef enum FooFilter { + FOO_FILTER_ONE = 1 << 0, + FOO_FILTER_TWO = 1 << 1, + FOO_FILTER_THREE = 1 << 2, +} FooFilter; + + +void do_something(unsigned int filter_flags) { + printf("%d\n", filter_flags); +} diff --git a/tests/test-diff-filter.cc b/tests/test-diff-filter.cc index 04a1f67..f8879d3 100644 --- a/tests/test-diff-filter.cc +++ b/tests/test-diff-filter.cc @@ -563,6 +563,13 @@ InOutSpec in_out_specs[] = "data/test-diff-filter/test-PR24731-report-1.txt", "output/test-diff-filter/test-PR24731-report-1.txt", }, + { + "data/test-diff-filter/PR24787-libone.so", + "data/test-diff-filter/PR24787-libtwo.so", + "--no-default-suppression", + "data/test-diff-filter/PR24787-report-0.txt", + "output/test-diff-filter/PR24787-report-0.txt", + }, // This should be the last entry {NULL, NULL, NULL, NULL, NULL} }; -- 2.7.4