From patchwork Wed Mar 11 10:35:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 39023 From: gprocida@google.com (Giuliano Procida) Date: Wed, 11 Mar 2020 10:35:24 +0000 Subject: [PATCH 2/2] [abidiff] Add more leaf change reporting. In-Reply-To: <20200311103524.258203-1-gprocida@google.com> References: <20200311103524.258203-1-gprocida@google.com> Message-ID: <20200311103524.258203-2-gprocida@google.com> The leaf-changes-only reporting path does not report on all the same kinds of differences as the default reporting path does, such as reporting about changes to variables, even though they can be considered leaf changs. - The addition or removal of any symbol affects the ABI and is clearly a leaf change. - A change to a variable's declaration may be local rather than caused by a type change elsewhere. This patch adds these missing pieces and reorders some of the existing leaf reporting, bringing the default and leaf corpus_diff functions closer to the point where they can be trivially merged or refactored. * src/abg-default-reporter.cc (report): In the corpus_diff override, move some code and comments for clarity. * src/abg-leaf-reporter.cc (report): In the corpus_diff override, additionally report removed/added/changed variables and removed/added symbols absent from debug info. * tests/data/Makefile.am: Add new test case files. * tests/data/test-abidiff-exit/test-leaf0-report.txt: Update to include reporting of variable diff (change of type). * tests/data/test-abidiff-exit/test-leaf1-report.txt: New test case. * tests/data/test-abidiff-exit/test-leaf1-v0.cc: Ditto. * tests/data/test-abidiff-exit/test-leaf1-v0.o: Ditto. * tests/data/test-abidiff-exit/test-leaf1-v1.cc: Ditto. * tests/data/test-abidiff-exit/test-leaf1-v1.o: Ditto. * tests/test-abidiff-exit.cc: Run new test case. Signed-off-by: Giuliano Procida --- src/abg-default-reporter.cc | 12 +- src/abg-leaf-reporter.cc | 328 ++++++++++++++++-- tests/data/Makefile.am | 5 + .../test-abidiff-exit/test-leaf0-report.txt | 9 + .../test-abidiff-exit/test-leaf1-report.txt | 38 ++ tests/data/test-abidiff-exit/test-leaf1-v0.cc | 9 + tests/data/test-abidiff-exit/test-leaf1-v0.o | Bin 0 -> 3160 bytes tests/data/test-abidiff-exit/test-leaf1-v1.cc | 9 + tests/data/test-abidiff-exit/test-leaf1-v1.o | Bin 0 -> 3176 bytes tests/test-abidiff-exit.cc | 10 + 10 files changed, 383 insertions(+), 37 deletions(-) create mode 100644 tests/data/test-abidiff-exit/test-leaf1-report.txt create mode 100644 tests/data/test-abidiff-exit/test-leaf1-v0.cc create mode 100644 tests/data/test-abidiff-exit/test-leaf1-v0.o create mode 100644 tests/data/test-abidiff-exit/test-leaf1-v1.cc create mode 100644 tests/data/test-abidiff-exit/test-leaf1-v1.o diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc index 2ac93d41..3bfdfbc4 100644 --- a/src/abg-default-reporter.cc +++ b/src/abg-default-reporter.cc @@ -1761,18 +1761,12 @@ void default_reporter::report(const corpus_diff& d, ostream& out, const string& indent) const { - size_t total = 0; const corpus_diff::diff_stats &s = const_cast(d). apply_filters_and_suppressions_before_reporting(); const diff_context_sptr& ctxt = d.context(); - /// Report removed/added/changed functions. - total = s.net_num_func_removed() + s.net_num_func_added() + - s.net_num_func_changed(); - const unsigned large_num = 100; - d.priv_->emit_diff_stats(s, out, indent); if (ctxt->show_stats_only()) return; @@ -1790,6 +1784,11 @@ default_reporter::report(const corpus_diff& d, ostream& out, << d.first_corpus()->get_architecture_name() << "' to '" << d.second_corpus()->get_architecture_name() << "'\n\n"; + /// Report removed/added/changed functions. + size_t total = s.net_num_func_removed() + s.net_num_func_added() + + s.net_num_func_changed(); + const unsigned large_num = 100; + if (ctxt->show_deleted_fns()) { if (s.net_num_func_removed() == 1) @@ -2249,7 +2248,6 @@ default_reporter::report(const corpus_diff& d, ostream& out, // Report added/removed/changed types not reacheable from public // interfaces. - maybe_report_unreachable_type_changes(d, s, indent, out); d.priv_->maybe_dump_diff_tree(); diff --git a/src/abg-leaf-reporter.cc b/src/abg-leaf-reporter.cc index 4b85d816..878e4125 100644 --- a/src/abg-leaf-reporter.cc +++ b/src/abg-leaf-reporter.cc @@ -1034,16 +1034,15 @@ leaf_reporter::report(const corpus_diff& d, if (!d.has_changes()) return; - const diff_context_sptr& ctxt = d.context(); - const corpus_diff::diff_stats &s = const_cast(d). apply_filters_and_suppressions_before_reporting(); + const diff_context_sptr& ctxt = d.context(); + d.priv_->emit_diff_stats(s, out, indent); if (ctxt->show_stats_only()) return; - out << "\n"; if (ctxt->show_soname_change() @@ -1058,6 +1057,7 @@ leaf_reporter::report(const corpus_diff& d, << d.first_corpus()->get_architecture_name() << "' to '" << d.second_corpus()->get_architecture_name() << "'\n\n"; + /// Report removed/added/changed functions. if (ctxt->show_deleted_fns()) { if (s.net_num_func_removed() == 1) @@ -1104,6 +1104,55 @@ leaf_reporter::report(const corpus_diff& d, out << "\n"; } + if (ctxt->show_added_fns()) + { + if (s.net_num_func_added() == 1) + out << indent << "1 Added function:\n\n"; + else if (s.net_num_func_added() > 1) + out << indent << s.net_num_func_added() + << " Added functions:\n\n"; + bool emitted = false; + vector sorted_added_fns; + sort_string_function_ptr_map(d.priv_->added_fns_, sorted_added_fns); + for (vector::const_iterator i = sorted_added_fns.begin(); + i != sorted_added_fns.end(); + ++i) + { + if (d.priv_->added_function_is_suppressed(*i)) + continue; + + out + << indent + << " "; + out << "[A] "; + out << "'" + << (*i)->get_pretty_representation() + << "'"; + if (ctxt->show_linkage_names()) + { + out << " {"; + show_linkage_name_and_aliases + (out, "", *(*i)->get_symbol(), + d.second_corpus()->get_fun_symbol_map()); + out << "}"; + } + out << "\n"; + if (is_member_function(*i) && get_member_function_is_virtual(*i)) + { + class_decl_sptr c = + is_class_type(is_method_type((*i)->get_type())->get_class_type()); + out << indent + << " " + << "note that this adds a new entry to the vtable of " + << c->get_pretty_representation() + << "\n"; + } + emitted = true; + } + if (emitted) + out << "\n"; + } + if (ctxt->show_changed_fns()) { // Show changed functions. @@ -1174,55 +1223,274 @@ leaf_reporter::report(const corpus_diff& d, out << "\n"; } - if (ctxt->show_added_fns()) + // Report removed/added/changed variables. + if (ctxt->show_deleted_vars()) { - if (s.net_num_func_added() == 1) - out << indent << "1 Added function:\n\n"; - else if (s.net_num_func_added() > 1) - out << indent << s.net_num_func_added() - << " Added functions:\n\n"; + if (s.net_num_vars_removed() == 1) + out << indent << "1 Removed variable:\n\n"; + else if (s.net_num_vars_removed() > 1) + out << indent << s.net_num_vars_removed() + << " Removed variables:\n\n"; + string n; bool emitted = false; - vector sorted_added_fns; - sort_string_function_ptr_map(d.priv_->added_fns_, sorted_added_fns); - for (vector::const_iterator i = sorted_added_fns.begin(); - i != sorted_added_fns.end(); + vector sorted_deleted_vars; + sort_string_var_ptr_map(d.priv_->deleted_vars_, sorted_deleted_vars); + for (vector::const_iterator i = + sorted_deleted_vars.begin(); + i != sorted_deleted_vars.end(); ++i) { - if (d.priv_->added_function_is_suppressed(*i)) + if (d.priv_->deleted_variable_is_suppressed(*i)) continue; - out - << indent - << " "; - out << "[A] "; + n = (*i)->get_pretty_representation(); + + out << indent + << " "; + out << "[D] "; out << "'" - << (*i)->get_pretty_representation() + << n << "'"; if (ctxt->show_linkage_names()) { out << " {"; - show_linkage_name_and_aliases - (out, "", *(*i)->get_symbol(), - d.second_corpus()->get_fun_symbol_map()); + show_linkage_name_and_aliases(out, "", *(*i)->get_symbol(), + d.first_corpus()->get_var_symbol_map()); out << "}"; } out << "\n"; - if (is_member_function(*i) && get_member_function_is_virtual(*i)) + emitted = true; + } + if (emitted) + out << "\n"; + } + + if (ctxt->show_added_vars()) + { + if (s.net_num_vars_added() == 1) + out << indent << "1 Added variable:\n\n"; + else if (s.net_num_vars_added() > 1) + out << indent << s.net_num_vars_added() + << " Added variables:\n\n"; + string n; + bool emitted = false; + vector sorted_added_vars; + sort_string_var_ptr_map(d.priv_->added_vars_, sorted_added_vars); + for (vector::const_iterator i = + sorted_added_vars.begin(); + i != sorted_added_vars.end(); + ++i) + { + if (d.priv_->added_variable_is_suppressed(*i)) + continue; + + n = (*i)->get_pretty_representation(); + + out << indent + << " "; + out << "[A] "; + out << "'" << n << "'"; + if (ctxt->show_linkage_names()) { - class_decl_sptr c = - is_class_type(is_method_type((*i)->get_type())->get_class_type()); - out << indent - << " " - << "note that this adds a new entry to the vtable of " - << c->get_pretty_representation() - << "\n"; + out << " {"; + show_linkage_name_and_aliases(out, "", *(*i)->get_symbol(), + d.second_corpus()->get_var_symbol_map()); + out << "}"; } + out << "\n"; emitted = true; } if (emitted) out << "\n"; } + if (ctxt->show_changed_vars()) + { + size_t num_changed = + s.num_vars_changed() - s.num_changed_vars_filtered_out(); + if (num_changed == 1) + out << indent << "1 Changed variable:\n\n"; + else if (num_changed > 1) + out << indent << num_changed + << " Changed variables:\n\n"; + string n1, n2; + bool emitted = false; + for (var_diff_sptrs_type::const_iterator i = + d.priv_->sorted_changed_vars_.begin(); + i != d.priv_->sorted_changed_vars_.end(); + ++i) + { + diff_sptr diff = *i; + + if (!diff) + continue; + + if (!diff->to_be_reported()) + continue; + + n1 = diff->first_subject()->get_pretty_representation(); + n2 = diff->second_subject()->get_pretty_representation(); + + out << indent << " [C] '" << n1 << "' was changed"; + if (n1 != n2) + out << " to '" << n2 << "'"; + report_loc_info(diff->second_subject(), *ctxt, out); + out << ":\n"; + diff->report(out, indent + " "); + out << "\n"; + emitted = true; + } + if (emitted) + out << "\n"; + } + + // Report removed function symbols not referenced by any debug info. + if (ctxt->show_symbols_unreferenced_by_debug_info() + && d.priv_->deleted_unrefed_fn_syms_.size()) + { + if (s.net_num_removed_func_syms() == 1) + out << indent + << "1 Removed function symbol not referenced by debug info:\n\n"; + else if (s.net_num_removed_func_syms() > 0) + out << indent + << s.net_num_removed_func_syms() + << " Removed function symbols not referenced by debug info:\n\n"; + + bool emitted = false; + vector sorted_deleted_unrefed_fn_syms; + sort_string_elf_symbol_map(d.priv_->deleted_unrefed_fn_syms_, + sorted_deleted_unrefed_fn_syms); + for (vector::const_iterator i = + sorted_deleted_unrefed_fn_syms.begin(); + i != sorted_deleted_unrefed_fn_syms.end(); + ++i) + { + if (d.priv_->deleted_unrefed_fn_sym_is_suppressed((*i).get())) + continue; + + out << indent << " "; + out << "[D] "; + + show_linkage_name_and_aliases(out, "", **i, + d.first_corpus()->get_fun_symbol_map()); + out << "\n"; + emitted = true; + } + if (emitted) + out << "\n"; + } + + // Report added function symbols not referenced by any debug info. + if (ctxt->show_symbols_unreferenced_by_debug_info() + && ctxt->show_added_symbols_unreferenced_by_debug_info() + && d.priv_->added_unrefed_fn_syms_.size()) + { + if (s.net_num_added_func_syms() == 1) + out << indent + << "1 Added function symbol not referenced by debug info:\n\n"; + else if (s.net_num_added_func_syms() > 0) + out << indent + << s.net_num_added_func_syms() + << " Added function symbols not referenced by debug info:\n\n"; + + bool emitted = false; + vector sorted_added_unrefed_fn_syms; + sort_string_elf_symbol_map(d.priv_->added_unrefed_fn_syms_, + sorted_added_unrefed_fn_syms); + for (vector::const_iterator i = + sorted_added_unrefed_fn_syms.begin(); + i != sorted_added_unrefed_fn_syms.end(); + ++i) + { + if (d.priv_->added_unrefed_fn_sym_is_suppressed((*i).get())) + continue; + + out << indent << " "; + out << "[A] "; + show_linkage_name_and_aliases(out, "", + **i, + d.second_corpus()->get_fun_symbol_map()); + out << "\n"; + emitted = true; + } + if (emitted) + out << "\n"; + } + + // Report removed variable symbols not referenced by any debug info. + if (ctxt->show_symbols_unreferenced_by_debug_info() + && d.priv_->deleted_unrefed_var_syms_.size()) + { + if (s.net_num_removed_var_syms() == 1) + out << indent + << "1 Removed variable symbol not referenced by debug info:\n\n"; + else if (s.net_num_removed_var_syms() > 0) + out << indent + << s.net_num_removed_var_syms() + << " Removed variable symbols not referenced by debug info:\n\n"; + + bool emitted = false; + vector sorted_deleted_unrefed_var_syms; + sort_string_elf_symbol_map(d.priv_->deleted_unrefed_var_syms_, + sorted_deleted_unrefed_var_syms); + for (vector::const_iterator i = + sorted_deleted_unrefed_var_syms.begin(); + i != sorted_deleted_unrefed_var_syms.end(); + ++i) + { + if (d.priv_->deleted_unrefed_var_sym_is_suppressed((*i).get())) + continue; + + out << indent << " "; + out << "[D] "; + + show_linkage_name_and_aliases + (out, "", **i, + d.first_corpus()->get_fun_symbol_map()); + + out << "\n"; + emitted = true; + } + if (emitted) + out << "\n"; + } + + // Report added variable symbols not referenced by any debug info. + if (ctxt->show_symbols_unreferenced_by_debug_info() + && ctxt->show_added_symbols_unreferenced_by_debug_info() + && d.priv_->added_unrefed_var_syms_.size()) + { + if (s.net_num_added_var_syms() == 1) + out << indent + << "1 Added variable symbol not referenced by debug info:\n\n"; + else if (s.net_num_added_var_syms() > 0) + out << indent + << s.net_num_added_var_syms() + << " Added variable symbols not referenced by debug info:\n\n"; + + bool emitted = false; + vector sorted_added_unrefed_var_syms; + sort_string_elf_symbol_map(d.priv_->added_unrefed_var_syms_, + sorted_added_unrefed_var_syms); + for (vector::const_iterator i = + sorted_added_unrefed_var_syms.begin(); + i != sorted_added_unrefed_var_syms.end(); + ++i) + { + if (d.priv_->added_unrefed_var_sym_is_suppressed((*i).get())) + continue; + + out << indent << " "; + out << "[A] "; + show_linkage_name_and_aliases(out, "", **i, + d.second_corpus()->get_fun_symbol_map()); + out << "\n"; + emitted = true; + } + if (emitted) + out << "\n"; + } + // Now show the changed types. const diff_maps& leaf_diffs = d.get_leaf_diffs(); report_type_changes_from_diff_maps(*this, leaf_diffs, out, indent); diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 07077608..5dab8685 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -115,6 +115,11 @@ test-abidiff-exit/test-leaf0-v0.o \ test-abidiff-exit/test-leaf0-v1.cc \ test-abidiff-exit/test-leaf0-v1.o \ test-abidiff-exit/test-leaf0-report.txt \ +test-abidiff-exit/test-leaf1-v0.cc \ +test-abidiff-exit/test-leaf1-v0.o \ +test-abidiff-exit/test-leaf1-v1.cc \ +test-abidiff-exit/test-leaf1-v1.o \ +test-abidiff-exit/test-leaf1-report.txt \ \ test-diff-dwarf/test0-v0.cc \ test-diff-dwarf/test0-v0.o \ diff --git a/tests/data/test-abidiff-exit/test-leaf0-report.txt b/tests/data/test-abidiff-exit/test-leaf0-report.txt index f823789d..7d15e28f 100644 --- a/tests/data/test-abidiff-exit/test-leaf0-report.txt +++ b/tests/data/test-abidiff-exit/test-leaf0-report.txt @@ -11,3 +11,12 @@ Removed/Changed/Added variables summary: 0 Removed, 1 Changed, 0 Added variable type size changed from 32 to 64 (in bits) +1 Changed variable: + + [C] 'int changed_var' was changed to 'long int changed_var': + size of symbol changed from 4 to 8 + type of variable changed: + type name changed from 'int' to 'long int' + type size changed from 32 to 64 (in bits) + + diff --git a/tests/data/test-abidiff-exit/test-leaf1-report.txt b/tests/data/test-abidiff-exit/test-leaf1-report.txt new file mode 100644 index 00000000..9da015bf --- /dev/null +++ b/tests/data/test-abidiff-exit/test-leaf1-report.txt @@ -0,0 +1,38 @@ +Leaf changes summary: 6 artifacts changed +Changed leaf types summary: 0 leaf type changed +Removed/Changed/Added functions summary: 1 Removed, 1 Changed, 1 Added function +Removed/Changed/Added variables summary: 1 Removed, 1 Changed, 1 Added variable + +1 Removed function: + + [D] 'function int deleted_fun()' {_Z11deleted_funv} + +1 Added function: + + [A] 'function long int added_fun()' {_Z9added_funv} + +1 function with some sub-type change: + + [C] 'function int changed_fun()' has some sub-type changes: + return type changed: + type name changed from 'int' to 'long int' + type size changed from 32 to 64 (in bits) + + +1 Removed variable: + + [D] 'int deleted_var' {deleted_var} + +1 Added variable: + + [A] 'long int added_var' {added_var} + +1 Changed variable: + + [C] 'int changed_var' was changed to 'long int changed_var': + size of symbol changed from 4 to 8 + type of variable changed: + type name changed from 'int' to 'long int' + type size changed from 32 to 64 (in bits) + + diff --git a/tests/data/test-abidiff-exit/test-leaf1-v0.cc b/tests/data/test-abidiff-exit/test-leaf1-v0.cc new file mode 100644 index 00000000..ae395419 --- /dev/null +++ b/tests/data/test-abidiff-exit/test-leaf1-v0.cc @@ -0,0 +1,9 @@ +int deleted_var = 0; +int changed_var = 0; + +int deleted_fun() { + return 0; +} +int changed_fun() { + return 0; +} diff --git a/tests/data/test-abidiff-exit/test-leaf1-v0.o b/tests/data/test-abidiff-exit/test-leaf1-v0.o new file mode 100644 index 0000000000000000000000000000000000000000..fbdc54aa2dec2fe64aed01a82036e3f98ef8ff22 GIT binary patch literal 3160 zcmbtV-HRJl6hD*Mba$I>$Zo85S(ONiwsoeNx-9P2ZKKiFvZWQJMFk0ykd@<+d*YeBG%6#E5 zfXq6$`K{c!+-B}_@xp561c3WAkMfs`6jnL$%`2-o?DP2PVgWr3 zGG}VQa19j~p2ff8{FPtAzupI2E(c;YjJ>W3TY)z&`;CT<%NAiV835Je*`qsI=bdcv zOd@15n4tD<5H#9hw5yu6)+ic7Q*~9GDCwNyISvPK>FV`T_4Mgry|n3{^@Gycanyi z4K7B^B9~8$ZFAv>4Qbs4<4_tNE-V__RCJRWPXU%(SW008xR5^tKbFE*5Lq_nCFlJ~ z#zhx?3h@F$+YqRx>%BRSP)sY8K7Ojh)$Arme$Bb-)oi$J91gn{5s zuzuXae`K951S0B;$(cp@B!Nhj$Gj$djV3sa`xEQ59z>MK+@&3jPW>hRZN|yp+`Z89 zzs-8v!dZnq7NmaJhfgejS(h&?{4VQXS$SAJu>SgrW<%i+|BXZN663VS<~mm_T?+K<6+hH(gfeK-Vvpt_;oRQ1trtrNBSXw!07uMgDtfH@q{rF$@> z+liEf;1J+9`n{gQ7tN3QvGVc8^@efSc%KugcC9rCdx{JHpKJ}|Y47Ztqr!RZM~Qq* zdI}Z7?3?O_>14=uJUwgF-*&w8r`h|IM(zcYi30rWoP5Xq_jr6+CmNsbD|`Gs#3-lKiK3=od7uXZFqiR#-$Hy= z{Fi*lslQB;IN_h7QDoEaXY-OU47TsL9B;edXF&Q;Qpo&8{~Co^>-TWR{?R=x@v^Q| zk3BZ!F7Sc6!v)j*OYedmPkUmYJ%r#M6@_#qVUxc-f66;6{&%kb`$=RX*zx}YtC{4n literal 0 HcmV?d00001 diff --git a/tests/data/test-abidiff-exit/test-leaf1-v1.cc b/tests/data/test-abidiff-exit/test-leaf1-v1.cc new file mode 100644 index 00000000..099803ef --- /dev/null +++ b/tests/data/test-abidiff-exit/test-leaf1-v1.cc @@ -0,0 +1,9 @@ +long changed_var = 0; +long added_var = 0; + +long changed_fun() { + return 0; +} +long added_fun() { + return 0; +} diff --git a/tests/data/test-abidiff-exit/test-leaf1-v1.o b/tests/data/test-abidiff-exit/test-leaf1-v1.o new file mode 100644 index 0000000000000000000000000000000000000000..8b2fb3f775b0b8bb7d5a8019320154cd904dd860 GIT binary patch literal 3176 zcmbtW-HRJl6hD*M>_^iL*=(!3tjH)9Tk1@A>asMg+eV|imX#_>MGF!p$;}5QlVvhl zxAwsYMbw8PBKUzXqC#JM5eoY#_%8kvzUy-lJ?GwgvbQ%=5j>E4&iS2>d+xa(llNYD zX*=gQz~sPXIMNsexHESoSEX2mWhle-7eD^}ko32HF+_lG5t@;H^D>>1C7_0f3^_-# zk$GKNr#8K`8w#$K3j8eZZqIT%-he5L`WEk*y(X$r((G;>T-fwCH$1=N`R!=TP%H;t1ZrMqXS}OdxnfiS$1Wa9_bLzz@L9lTWAyI5uY-- z`Z(AI7bCLB)i=hrxgZa*4PzGLQW{nl77Xo3bk&NF04=$&IE4}50v`Dca^d(CT1I5a zSQlS`M>Wnm^V5izjlS`{t!SKe=Ku91Pu&3PfN>{};XEv(I9UZK$od0p9!X?cqkGVVB8fX9SwZ3cNoc9flQyz0S zBg_A5t#>W_C#~;k&g$=>h(m55PT*Q}50G zS1p|DhhTJbfbSLZG*P_EQYsEo2wG7ZLC_eDAV@?%3R$C>^S zR1F^z|B!zZ0KU`#R0lyk{|Y%FZQ;+(N%&thAYD^zGTOecpvA_>c~^t$lSn1lzI4B{ z>iSt=$+