abidiff/kmidiff: do not default-suppress added symbols

kmidiff and abidiff do filter out added symbols (vars, functions and
symbols without debug info) by default when dealing with kernel
binaries. The reason for this is that the ABI could be considered
compatible and not broken when adding symbols. In practice, this is
confusing as there is no possibility for a symmetric comparison (i.e. a
deleted function when comparing left to right is an added function when
comparing right to left). Furthermore, there is no option available to
actually report these added symbols. I thought of adding an option to
report added symbols, but in the end came to the conclusion that we
should behave consistent across the various ways you can diff an ABI
with abidiff and kmidiff and should not change default behaviour for a
particular type of binary. Hence, remove the default behaviour of
filtering out added symbols when comparing kernel binaries. To restore
the current behaviour, the user needs to parametrize with the tools with
--no-added-syms --no-unreferenced-symbols.

Adjusted test cases accordingly and add a new test that covers the old
behaviour new available with additional flags to abidiff.

	* tools/abidiff.cc (adjust_diff_context_for_kmidiff): Drop
	default suppression of added symbols.
	* tools/kmidiff.cc (set_diff_context): Likewise.
	* tests/data/test-diff-suppr/test46-PR25128-report-1.txt: Adjust
	test expectation.
	* tests/data/test-diff-suppr/test46-PR25128-report-2.txt: Add
	test case for abidiff with flag --no-added-syms.
	* tests/data/Makefile.am: add new testcase.

Reviewed-by: Dodji Seketeli <dodji@seketeli.org>
Signed-off-by: Matthias Maennich <maennich@google.com>

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 16026545fff4e0482ea5d0da62a13a9f91eb8e61..27a2a3652c9f35a8a810ae19a0b26d9d74e97bd1 100644 (file)
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -1240,6 +1240,7 @@ test-diff-suppr/test45-abi.xml \
 test-diff-suppr/test46-PR25128-base.xml \
 test-diff-suppr/test46-PR25128-new.xml \
 test-diff-suppr/test46-PR25128-report-1.txt \
+test-diff-suppr/test46-PR25128-report-2.txt \
 test-diff-suppr/test47-non-reachable-types-report-1.txt \
 test-diff-suppr/test47-non-reachable-types-report-2.txt \
 test-diff-suppr/test47-non-reachable-types-report-3.txt \

diff --git a/tests/data/test-diff-suppr/test46-PR25128-report-1.txt b/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
index 558530e1e7f931ed5ff2356774ebc508ce1f7f36..b08460d4799bc503bd501d366c4efbb6922cda08 100644 (file)
--- a/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
+++ b/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
@@ -1,8 +1,12 @@
-Leaf changes summary: 1 artifact changed
+Leaf changes summary: 2 artifacts changed
 Changed leaf types summary: 1 leaf type changed
-Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function (1 filtered out)
+Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 1 Added function
 Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
 
+1 Added function:
+
+  [A] 'function void arch_set_max_freq_scale(cpumask*, unsigned long int)'
+
 'struct root_domain at sched.h:732:1' changed:
   type size changed from 14592 to 14720 (in bits)
   there are data member changes:

diff --git a/tests/data/test-diff-suppr/test46-PR25128-report-2.txt b/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
new file mode 100644 (file)
index 0000000..558530e
--- /dev/null
+++ b/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
@@ -0,0 +1,14 @@
+Leaf changes summary: 1 artifact changed
+Changed leaf types summary: 1 leaf type changed
+Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function (1 filtered out)
+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
+
+'struct root_domain at sched.h:732:1' changed:
+  type size changed from 14592 to 14720 (in bits)
+  there are data member changes:
+   type 'unsigned long int' of 'root_domain::max_cpu_capacity' changed:
+     entity changed from 'unsigned long int' to 'struct max_cpu_capacity' at sched.h:722:1
+     type size changed from 64 to 192 (in bits)
+, size changed from 64 to 192 (in bits) (by +128 bits)
+   'perf_domain* root_domain::pd' offset changed from 14528 to 14656 (in bits) (by +128 bits)
+

diff --git a/tests/test-diff-suppr.cc b/tests/test-diff-suppr.cc
index 5d9528fe0bd9656308f03859415fecc021b719b2..b513a4b4180b98f4e42cdd551cf6d2bb9d4bbaed 100644 (file)
--- a/tests/test-diff-suppr.cc
+++ b/tests/test-diff-suppr.cc
@@ -1838,6 +1838,16 @@ InOutSpec in_out_specs[] =
     "data/test-diff-suppr/test46-PR25128-report-1.txt",
     "output/test-diff-suppr/test46-PR25128-report-1.txt"
   },
+  {
+    "data/test-diff-suppr/test46-PR25128-base.xml",
+    "data/test-diff-suppr/test46-PR25128-new.xml",
+    "",
+    "",
+    "",
+    "--no-default-suppression --leaf-changes-only --no-added-syms",
+    "data/test-diff-suppr/test46-PR25128-report-2.txt",
+    "output/test-diff-suppr/test46-PR25128-report-2.txt"
+  },
   {
     "data/test-diff-suppr/test47-non-reachable-types-v0.o",
     "data/test-diff-suppr/test47-non-reachable-types-v1.o",

diff --git a/tools/abidiff.cc b/tools/abidiff.cc
index 1edb1b401cc4cdc39d70d2a221e87b68eac35242..77efcd167544c5aea939956332ffdb9e9312a1de 100644 (file)
--- a/tools/abidiff.cc
+++ b/tools/abidiff.cc
@@ -837,10 +837,6 @@ static void
 adjust_diff_context_for_kmidiff(diff_context &ctxt)
 {
   ctxt.show_linkage_names(false);
-  ctxt.show_added_fns(false);
-  ctxt.show_added_vars(false);
-  ctxt.show_added_symbols_unreferenced_by_debug_info
-    (false);
 }
 
 /// Convert options::di_root_paths{1,2} into

diff --git a/tools/kmidiff.cc b/tools/kmidiff.cc
index 842381f8917ebc5609fc23df2fb925db49e350dc..9a1645284a013f7fe78935beee62ca5d1cd17d53 100644 (file)
--- a/tools/kmidiff.cc
+++ b/tools/kmidiff.cc
@@ -322,10 +322,6 @@ set_diff_context(diff_context_sptr ctxt, const options& opts)
   ctxt->show_redundant_changes(false);
   ctxt->show_locs(true);
   ctxt->show_linkage_names(false);
-  ctxt->show_added_fns(false);
-  ctxt->show_added_vars(false);
-  ctxt->show_added_symbols_unreferenced_by_debug_info
-    (false);
   ctxt->show_symbols_unreferenced_by_debug_info
     (true);
   ctxt->show_leaf_changes_only(opts.leaf_changes_only);