diff --git a/src/abg-ir-priv.h b/src/abg-ir-priv.h index 640f6de0..24da0f02 100644 --- a/src/abg-ir-priv.h +++ b/src/abg-ir-priv.h @@ -28,6 +28,7 @@ namespace ir { using std::string; +using std::unordered_set; using abg_compat::optional; /// The result of structural comparison of type ABI artifacts. @@ -1601,7 +1602,9 @@ struct class_or_union::priv member_function_templates member_function_templates_; member_class_templates member_class_templates_; bool is_printing_flat_representation_ = false; - + // The set of classes which layouts are currently being compared + // against this one. This is to avoid endless loops. + unordered_set comparing_class_layouts_; priv() {} diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 697466e4..0a2ce5a1 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -10261,6 +10261,37 @@ get_type_declaration(const type_base_sptr t) bool classes_have_same_layout(const type_base_sptr& f, const type_base_sptr& s) { +#ifdef RETURN_FROM_CLASSES_HAVE_SAME_LAYOUT +#undef RETURN_FROM_CLASSES_HAVE_SAME_LAYOUT +#endif + +#ifdef ENSURE_NO_ENDLESS_LOOP +#undef ENSURE_NO_ENDLESS_LOOP +#endif + +#define RETURN_FROM_CLASSES_HAVE_SAME_LAYOUT(VALUE) \ + do \ + { \ + auto t1 = is_class_or_union_type(f); \ + auto t2 = is_class_or_union_type(s); \ + t1->priv_->comparing_class_layouts_.erase(t2.get()); \ + t2->priv_->comparing_class_layouts_.erase(t1.get()); \ + return VALUE; \ + } while (false) + +#define ENSURE_NO_ENDLESS_LOOP \ + do \ + { \ + auto t1 = is_class_or_union_type(f); \ + auto t2 = is_class_or_union_type(s); \ + const auto& END = t1->priv_->comparing_class_layouts_.end(); \ + if (t1->priv_->comparing_class_layouts_.find(t2.get()) != END \ + || t2->priv_->comparing_class_layouts_.find(t1.get()) != END) \ + return true; \ + t1->priv_->comparing_class_layouts_.insert(t2.get()); \ + t2->priv_->comparing_class_layouts_.insert(t1.get()); \ + } while (false) + class_decl_sptr fc = is_class_type(peel_qualified_or_typedef_type(f)), sc = is_class_type(peel_qualified_or_typedef_type(s)); @@ -10272,7 +10303,7 @@ classes_have_same_layout(const type_base_sptr& f, const type_base_sptr& s) return false; if (*fc == *sc) - return true; + RETURN_FROM_CLASSES_HAVE_SAME_LAYOUT(true); // Compare the types and offsets of data members one by one. for (auto f_decl_it = fc->get_data_members().begin(), @@ -10286,7 +10317,7 @@ classes_have_same_layout(const type_base_sptr& f, const type_base_sptr& s) if (*dm1_type != *dm2_type || get_data_member_offset(dm1) != get_data_member_offset(dm2)) - return false; + RETURN_FROM_CLASSES_HAVE_SAME_LAYOUT(false); } // Compare the layout of base types @@ -10299,22 +10330,23 @@ classes_have_same_layout(const type_base_sptr& f, const type_base_sptr& s) class_decl::base_spec_sptr f_bs = *f_bs_it, s_bs = *s_bs_it; if ((f_bs->get_is_virtual() != s_bs->get_is_virtual()) || (f_bs->get_offset_in_bits() != s_bs->get_offset_in_bits())) - return false; + RETURN_FROM_CLASSES_HAVE_SAME_LAYOUT(false); class_decl_sptr fb = f_bs->get_base_class(), sb = s_bs->get_base_class(); if (!classes_have_same_layout(fb, sb)) - return false; + RETURN_FROM_CLASSES_HAVE_SAME_LAYOUT(false); } if (fc->has_vtable() != sc->has_vtable()) - return false; + RETURN_FROM_CLASSES_HAVE_SAME_LAYOUT(false); // Compare virtual function types + ENSURE_NO_ENDLESS_LOOP; if (fc->has_vtable()) { if (fc->get_virtual_mem_fns().size() > sc->get_virtual_mem_fns().size()) // Some virtual member function got removed. Bad. - return false; + RETURN_FROM_CLASSES_HAVE_SAME_LAYOUT(false); for (auto it1 = fc->get_virtual_mem_fns().begin(), it2 = sc->get_virtual_mem_fns().begin(); @@ -10329,11 +10361,19 @@ classes_have_same_layout(const type_base_sptr& f, const type_base_sptr& s) != get_member_function_vtable_offset(method2)) || !types_are_compatible(method1->get_type(), method2->get_type())) - return false; + RETURN_FROM_CLASSES_HAVE_SAME_LAYOUT(false); } } - return true; + RETURN_FROM_CLASSES_HAVE_SAME_LAYOUT(true); + +#ifdef RETURN_FROM_CLASSES_HAVE_SAME_LAYOUT +#undef RETURN_FROM_CLASSES_HAVE_SAME_LAYOUT +#endif + +#ifdef ENSURE_NO_ENDLESS_LOOP +#undef ENSURE_NO_ENDLESS_LOOP +#endif } /// Test if two types are equal modulo a typedef or CV qualifiers. diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 68811a5e..f6b77077 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -512,6 +512,12 @@ test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v0.o \ test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v1.c \ test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-v1.o \ test-abidiff-exit/replace-dm-with-compatible-anon-dm-1-report-1.txt \ +test-abidiff-exit/PR33055/Makefile \ +test-abidiff-exit/PR33055/PR33055-report-1.txt \ +test-abidiff-exit/PR33055/PR33055-report-2.txt \ +test-abidiff-exit/PR33055/file.cpp \ +test-abidiff-exit/PR33055/new-lib.so \ +test-abidiff-exit/PR33055/old-lib.so \ \ test-diff-dwarf/test0-v0.cc \ test-diff-dwarf/test0-v0.o \ diff --git a/tests/data/test-abidiff-exit/PR33055/Makefile b/tests/data/test-abidiff-exit/PR33055/Makefile new file mode 100644 index 00000000..b415ba14 --- /dev/null +++ b/tests/data/test-abidiff-exit/PR33055/Makefile @@ -0,0 +1,25 @@ +CXXFLAGS = -g -fPIC +LDFLAGS = ${CXXFLAGS} -shared + +all: old-lib.so new-lib.so +.PHONY: all + +old-file.o: file.cpp + g++ ${CXXFLAGS} -o "$@" -c "$<" + +new-file.o: file.cpp + g++ ${CXXFLAGS} -DNEW_VERSION -o "$@" -c "$<" + +old-lib.so: old-file.o + g++ ${LDFLAGS} -o "$@" "$<" + +new-lib.so: new-file.o + g++ ${LDFLAGS} -o "$@" "$<" + +check: old-lib.so new-lib.so + abidiff $^ +.PHONY: check + +clean: + ${RM} -f *.so *.o +.PHONY: clean diff --git a/tests/data/test-abidiff-exit/PR33055/PR33055-report-1.txt b/tests/data/test-abidiff-exit/PR33055/PR33055-report-1.txt new file mode 100644 index 00000000..ce294d74 --- /dev/null +++ b/tests/data/test-abidiff-exit/PR33055/PR33055-report-1.txt @@ -0,0 +1,7 @@ +Functions changes summary: 0 Removed, 0 Changed (3 filtered out), 1 Added functions +Variables changes summary: 0 Removed, 0 Changed, 0 Added variable + +1 Added function: + + [A] 'method void PublicImplementation::someNewMethod()' {_ZN20PublicImplementation13someNewMethodEv} + diff --git a/tests/data/test-abidiff-exit/PR33055/PR33055-report-2.txt b/tests/data/test-abidiff-exit/PR33055/PR33055-report-2.txt new file mode 100644 index 00000000..0a35da8a --- /dev/null +++ b/tests/data/test-abidiff-exit/PR33055/PR33055-report-2.txt @@ -0,0 +1,47 @@ +Functions changes summary: 0 Removed, 3 Changed, 1 Added functions +Variables changes summary: 0 Removed, 0 Changed, 0 Added variable + +1 Added function: + + [A] 'method void PublicImplementation::someNewMethod()' {_ZN20PublicImplementation13someNewMethodEv} + +3 functions with some indirect sub-type change: + + [C] 'method PublicImplementation::PublicImplementation()' at file.cpp:28:1 has some indirect sub-type changes: + implicit parameter 0 of type 'PublicImplementation*' has sub-type changes: + in pointed to type 'class PublicImplementation' at file.cpp:13:1: + type size hasn't changed + 3 member function changes: + 'method virtual PublicImplementation::~PublicImplementation()' has some sub-type changes: + implicit parameter 0 of type 'PublicImplementation*' has sub-type changes: + pointed to type 'class PublicImplementation' changed, as being reported + 'method virtual PublicImplementation::~PublicImplementation()' has some sub-type changes: + implicit parameter 0 of type 'PublicImplementation*' has sub-type changes: + pointed to type 'class PublicImplementation' changed, as being reported + 'method virtual PublicImplementation::~PublicImplementation(int)' has some sub-type changes: + implicit parameter 0 of type 'PublicImplementation*' has sub-type changes: + pointed to type 'class PublicImplementation' changed, as being reported + 1 data member change: + type of 'Interface* _pImpl' changed: + in pointed to type 'class Interface' at file.cpp:1:1: + type size hasn't changed + 1 member function insertion: + 'method virtual void Interface::someNewMethod()' at file.cpp:9:1, virtual at voffset 3/3 + 2 member function changes: + 'method virtual Interface* Interface::clone()' has some sub-type changes: + return type changed: + pointed to type 'class Interface' changed, as being reported + implicit parameter 0 of type 'Interface*' has sub-type changes: + pointed to type 'class Interface' changed, as being reported + 'method virtual Interface::~Interface(int)' has some sub-type changes: + implicit parameter 0 of type 'Interface*' has sub-type changes: + pointed to type 'class Interface' changed, as being reported + + [C] 'method virtual PublicImplementation::~PublicImplementation()' at file.cpp:29:1 has some indirect sub-type changes: + implicit parameter 0 of type 'PublicImplementation*' has sub-type changes: + pointed to type 'class PublicImplementation' changed at file.cpp:13:1, as reported earlier + + [C] 'method virtual PublicImplementation::~PublicImplementation()' at file.cpp:29:1 has some indirect sub-type changes: + implicit parameter 0 of type 'PublicImplementation*' has sub-type changes: + pointed to type 'class PublicImplementation' changed at file.cpp:13:1, as reported earlier + diff --git a/tests/data/test-abidiff-exit/PR33055/file.cpp b/tests/data/test-abidiff-exit/PR33055/file.cpp new file mode 100644 index 00000000..0d637f09 --- /dev/null +++ b/tests/data/test-abidiff-exit/PR33055/file.cpp @@ -0,0 +1,33 @@ +class Interface +{ +public: + virtual ~Interface() {}; + + virtual Interface* clone() = 0; + +#ifdef NEW_VERSION + virtual void someNewMethod() = 0; +#endif +}; + +class PublicImplementation +{ +public: + PublicImplementation(); + + virtual ~PublicImplementation(); + +#ifdef NEW_VERSION + void someNewMethod(); +#endif + +private: + Interface* _pImpl; +}; + +PublicImplementation::PublicImplementation() = default; +PublicImplementation::~PublicImplementation() = default; + +#ifdef NEW_VERSION +void PublicImplementation::someNewMethod() {} +#endif diff --git a/tests/data/test-abidiff-exit/PR33055/new-lib.so b/tests/data/test-abidiff-exit/PR33055/new-lib.so new file mode 100755 index 0000000000000000000000000000000000000000..372d3058462e11609f017fc15f73731d23e628f2 GIT binary patch literal 19280 zcmeHPeQ*@z8GrXSms~iw963G`0_Ng?gT`EPfk1!;67s=CNl0U0kkQ%PSCX@ryUg7t zButA8s8eL9)Q%l((Nb$WPRFVApVr!r;K$h7b|~UM)Tte8wL+^FTWzTwNuTH4_qptD zcB8g`_(%86>^|@Nd%oU%_T9aAci$&>c5d%-NfM(Xv&{@!gMJNZ6^yf6MFmJJYi6ae zzn0Z-*`gWNteVH8K}4Cbi$2`Q5^XbmSagXf@e$_!f~X87$oB=hNazvO>lK6Oi8@XG zM2nFgb7}1d1tmRfqTWTJcTwmOy)66@6^G0Fi2ZC9_RMsp=8h5N#{^j4n`#2<6+unF zOxJ2KBT9Oo2R-cPCoL5Ae&Nq9E`y@~M8)Cq<68vIWZ1U#?qoN-f77`~mVWc@=IW1X zzZ2a1*13iwcVl0WUjiHIVgGg<2(sTDdz51st1{T$$=`PH(MzY!yz$PdGqKi=ld(5X zo!NT(sWYLzk6t|#J$2^2pR{}r#4Jk}px0V(2EgG$3%nTcatj>e5wyTT+!DXh2Hyt? zHEcmaX{oUJOQ7Y<%bG=}RtWqF8+-$|Q_jwd@kZzCqLAMu?9>W7UlI5+ll?s4HQ@Ov zVJ9f$ciPDNz%Tamps?c=@=Q^NN3$6vuSRpKqA;aN3sd+D7tNrVqjNqxIuxSOh#j= ziEwy5)YH*?UQwsUl1eHwlkvM7*{(l+T9NF=&n!YQyoklE+pU@wkS! z@7~?EwOa{?)`T{&nFiaoy?blB(imzCHNp9I!BtE56$!3fzKXm7=HL3qIYC>Hg^#*7mvuP*eH0p3bU6B;Qqii_urK=K3hHoue^%>oyf-_=iGULUCjw3coCr7( z`2UN*-^;G~FmmW^PvnUE^?Jr4cb`^W(`O@xUhq86gE8Io2Y{zny#f2O>Q<;>djxrx zE=)sN^(wY;igD>IZ=b_9PMI#9hIZs=%S!-6j=G;ivf+xl7+htr4z5b4FO*du09p3D zpdkMcmv6cs$@Nzwhc8E-eRoUb*(-8HdNK0yRdo>;yI& zuiY6r)bh6wR3nGqROf@~XW+5zVmEZ_VghzAy6Zqrx*dEM{G z{TOUSj`sN@M>?-W4*QSY{=A<1kPY|H+kQ z&PU-40O%Aa0!{>+2sjaNBH%>8iGULUCjw3coCr7(a3b(OM!@FF5|gUrP56x&zd?8| zPfzy&b-gz|Jqa`j^g*DTfj$NF4xs0OV!?01SfqXZOq%pdRr5evS+?nhH7ogNT$F*wd3F$du0XjqpZ^|L`-0-#U|vqyW2W{GOl@4Vhh@G-3Jc+AE!#1%POnn%X&qmcXw#as zWui^%`S3Xo3-Ld=I-SM#AbvT9LTlev7in!;^Y*yl)7rj2i~iBty$Vg7uRG?|Rt*qp zEF$>yoJwo>Xr1n90TBLW;h6f_Ed0UZMPA4se9PtiZx-_K1(~vUH3S=2Kr zl??T@YK}frUmq4eVL<|4UrJX3t>4CV+g2q91Io=Lbk9n^1-qKC5VO)O#L9>$U z8?gT_2}tikjrlsT9V|YD!cRl}G`7(dE4vS|O5af+9_B(5GVWZ+^?T01ej(bxt(*ml z+^LMpT-cJQVYkS0AMF1KTX8K~kV;{Xwn|pP4tu}&>rj!3ya6Q0l{Uey3o9t6b#TdM zkd%s-TmyuoF29S*4ui-j$+D&VI7m;YJ3|u2~M+liNADw9-e$uM_W}Wi=_@wb6SCP ze4Sp_69>6=Kzd*+_+`sc2py+^()i(T-2ju0yHNvOG0Dxk18d(de=vpcZe+f$+SOA}aX6wtFF8AuBi2Bx)QUnRz?lH!g5Ulx+Wfs8K^ z&?LUdC8~^|rncM+!r@>lh#M}(8}Ot{(!YHRkl^5g4Jy@qoVlcGSDB07Z1~PCu~HX} z8M#V2=(+WvXZO?Y8$EBi>khhP(6mkOpgfEX-5JOo8H&b}O>s;g!E_18dQjpcxnaf; zW=N;7&%5YYtL1&nShUE*P$tw^S{YMwJiP+bmV)iIwc+)j;Fl~|H>!?jk}bo@Ofr{> zLo=F-kF-oSud7?Pwr-*^ST`K38|v$Bx9_6JY3ZHmqSomN=uoH-2&Rst@@&6VQp_Bt z#nD^8%x_=Y*?;pbkg_v|Nj7Z%j4%5Wq$Ru9B+6KXmLG&=r|U-`A*Mc&P27>HA5N+D zkc0~HAhWFf$DKN$g`E`0x8Gcb| z9%;m7M)OA_Uc_jgX~f-(=4(d0n9)4Xh?g*$FB)+Vqj{wfhgqbUD;V*4ENF>)3v(x< z{Cq}pAtOGMhi1fmh5SMzUdH-@dPJ+s&`Fxt>$x&y*wK96h?g_U$1&moW;NfG;j0nl z&lu$wGuEm{w7Sfeum($gu88K|h zjDAavJelFV!&vYhTg2YOIgX*=bC(P9@n{}POrOAynZ`+t!0CCB~VVva0zCL=JEDx zz+LdvYQ8ScaeVf5hch&Z%^shxbA0iv@%E0u&Er#s1GdWZ#2Jqjz6Eg1&!l`aI-Prj zym=Bh3HY_(+g!c|_(C%X?H9P6*^J)``}7M=BlVwvTjj5o!&J~Jj}#}LcwA;XHv=A= z?UoJO;1f1D=4D&vqsDV;UL7AA3dPw>8mgj>LINesB=RtkNMx1abT$@ED+x85%PZ0G zNfysyo~fElgc__RFgZ1)M02_5lmgkPxhXc3i;gCh#Q5mw6sYK0FjGmXMp63dQk1U# zww_LurwauHYpX8n zRN2F`X>|}}rP>}8Q~GzL`7X>^2Y znXH-&4QIwfvGG(oQI|?E-WrMKM_4E^l>tqSsyU4_k<8^GVcO78fR{_Aqo^QiV`-Iz zaJ;gRnw*3^A9A5wHW5{$ER-A(1A8O^y42D%v=OdpP#v|<1sLGniH@e?(1k1*hFyqz zi{IHw8DnANuTDmMCpAgi0~rJQIo$P#dnvaPxD`*K_N-~ zCpn_{Fp2L%sZH}_qJ2Ui`;Rize*yEgJ;Xzd=F>#!{VVw=eLDUO=%D^qRE9$HZlVzl z<}rfVWx+q`PXdN95wB@y=I7#v**pV8U;2`VhF+iM@kCqo!ceE*d!qYI`h$W&w9a(G zNRQ0kWzwhl4^et=Px9ve-z)U#{IQ^r6x2M!Hv7Ndq+cQQh~ld*eKGq#0ZcqZ@=wnr zL|@P=L!Hh)!QY1p&W&k&&~pp@e-pDjwTb=+bjx)5^nCMp9e_|sPHWAy34ab4*njd* z^U{+-znK&TB|U