Bug 19596 - Suppressed removed symbol changes still considered incompatible

Libabigail considers a removed function (or global variable) as being
an incompatible change, whether it has been suppressed or not.
Likewise, even if all function sub-type changes have been suppressed,
changed virtual offset on functions are still considered incompatible.

Thus, abidiff still returns an exit code that reflects an incompatible
change even if the removed symbol change was suppressed.

The rule should rather be that if incompatible changes have been
suppressed, abidiff (and the other tools) should take that into
account and not return an exit code that reflects incompatible
changes.

This patch implements that rule, at least for the incompatible changes
that are detected so far.

	* src/abg-comparison.cc (corpus_diff::has_incompatible_changes):
	Consider the *net* number of removed function and variable
	symbols.  Also, if all function sub-type changes have been
	suppressed, then no virtual offset change should be considered
	incompatible.
	* tests/data/test-abidiff-exit/test1-voffset-change-report1.txt
	* tests/data/test-abidiff-exit/test1-voffset-change.abignore
	* tests/data/test-abidiff-exit/test2-filtered-removed-fns-report0.txt
	* tests/data/test-abidiff-exit/test2-filtered-removed-fns-report1.txt
	* tests/data/test-abidiff-exit/test2-filtered-removed-fns-v0.c
	* tests/data/test-abidiff-exit/test2-filtered-removed-fns-v1.c
	* tests/data/test-abidiff-exit/test2-filtered-removed-fns.abignore
	* tests/data/Makefile.am: Add the new test material above to
	source distribution.
	* tests/test-abidiff-exit.cc (InOutSpec::in_suppr_path): New data
	member.
	(in_out_specs): Adjust. Add new test inputs.
	(main): Adjust.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>

src/abg-comparison.cc

@@ -14624,11 +14624,16 @@ corpus_diff::has_incompatible_changes() const
     apply_filters_and_suppressions_before_reporting();
 
   return (soname_changed()
-	  || stats.num_func_removed() != 0
-	  || stats.num_func_with_virtual_offset_changes() != 0
-	  || stats.num_vars_removed() != 0
-	  || stats.num_func_syms_removed() != 0
-	  || stats.num_var_syms_removed() != 0);
+	  || stats.net_num_func_removed() != 0
+	  || (stats.num_func_with_virtual_offset_changes() != 0
+	      // If all reports about functions with sub-type changes
+	      // have been suppressd, then even those about functions
+	      // that are virtual don't matter anymore because the
+	      // user willingly requested to shut them down
+	      && stats.net_num_func_changed() != 0)
+	  || stats.net_num_vars_removed() != 0
+	  || stats.net_num_removed_func_syms() != 0
+	  || stats.net_num_removed_var_syms() != 0);
 }
 
 /// Test if the current instance of @ref corpus_diff carries subtype

tests/data/Makefile.am

@@ -76,10 +76,19 @@ test-abidiff/test-PR18791-v0.so.abi     \
 test-abidiff/test-PR18791-v1.so.abi     \
 \
 test-abidiff-exit/test1-voffset-change-report0.txt \
+test-abidiff-exit/test1-voffset-change-report1.txt \
+test-abidiff-exit/test1-voffset-change.abignore \
 test-abidiff-exit/test1-voffset-change-v0.cc \
 test-abidiff-exit/test1-voffset-change-v0.o \
 test-abidiff-exit/test1-voffset-change-v1.cc \
 test-abidiff-exit/test1-voffset-change-v1.o \
+test-abidiff-exit/test2-filtered-removed-fns-report0.txt \
+test-abidiff-exit/test2-filtered-removed-fns-report1.txt \
+test-abidiff-exit/test2-filtered-removed-fns-v0.c \
+test-abidiff-exit/test2-filtered-removed-fns-v1.c \
+test-abidiff-exit/test2-filtered-removed-fns-v0.o \
+test-abidiff-exit/test2-filtered-removed-fns-v1.o \
+test-abidiff-exit/test2-filtered-removed-fns.abignore \
 \
 test-diff-dwarf/test0-v0.cc		\
 test-diff-dwarf/test0-v0.o			\

tests/data/test-abidiff-exit/test1-voffset-change-report1.txt

@@ -0,0 +1,3 @@
+Functions changes summary: 0 Removed, 0 Changed (2 filtered out), 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+

tests/data/test-abidiff-exit/test1-voffset-change.abignore

@@ -0,0 +1,9 @@
+[suppress_function]
+  name = C::virtual_func0
+  change_kind = function-subtype-change
+
+[suppress_function]
+  name = C::virtual_func1
+  change_kind = function-subtype-change
+
+

tests/data/test-abidiff-exit/test2-filtered-removed-fns-report0.txt

@@ -0,0 +1,8 @@
+Functions changes summary: 1 Removed, 0 Changed, 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+1 Removed function:
+
+  'function void to_erase()'    {to_erase}
+
+

tests/data/test-abidiff-exit/test2-filtered-removed-fns-report1.txt

@@ -0,0 +1,3 @@
+Functions changes summary: 0 Removed (1 filtered out), 0 Changed, 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+

tests/data/test-abidiff-exit/test2-filtered-removed-fns-v0.c

@@ -0,0 +1,11 @@
+/* Compile with: gcc -g -c -Wall foo-v0.c  */
+
+void
+to_erase()
+{
+}
+
+void
+to_keep()
+{
+}

tests/data/test-abidiff-exit/test2-filtered-removed-fns-v1.c

@@ -0,0 +1,6 @@
+/* Compile with: gcc -g -c -Wall foo-v1.c  */
+
+void
+to_keep()
+{
+}

tests/data/test-abidiff-exit/test2-filtered-removed-fns.abignore

@@ -0,0 +1,3 @@
+[suppress_function]
+  name = to_erase
+  change_kind = deleted-function

tests/test-abidiff-exit.cc

@@ -46,6 +46,7 @@ struct InOutSpec
 {
   const char*	in_elfv0_path;
   const char*	in_elfv1_path;
+  const char*	in_suppr_path;
   const char*	abidiff_options;
   abidiff_status status;
   const char*	in_report_path;
@@ -57,13 +58,42 @@ InOutSpec in_out_specs[] =
   {
     "data/test-abidiff-exit/test1-voffset-change-v0.o",
     "data/test-abidiff-exit/test1-voffset-change-v1.o",
+    "",
     "--no-show-locs",
     abigail::tools_utils::ABIDIFF_ABI_CHANGE
     | abigail::tools_utils::ABIDIFF_ABI_INCOMPATIBLE_CHANGE,
     "data/test-abidiff-exit/test1-voffset-change-report0.txt",
     "output/test-abidiff-exit/test1-voffset-change-report0.txt"
   },
-  {0, 0, 0 , abigail::tools_utils::ABIDIFF_OK, 0, 0}
+  {
+    "data/test-abidiff-exit/test1-voffset-change-v0.o",
+    "data/test-abidiff-exit/test1-voffset-change-v1.o",
+    "data/test-abidiff-exit/test1-voffset-change.abignore",
+    "--no-show-locs",
+    abigail::tools_utils::ABIDIFF_OK,
+    "data/test-abidiff-exit/test1-voffset-change-report1.txt",
+    "output/test-abidiff-exit/test1-voffset-change-report1.txt"
+  },
+  {
+    "data/test-abidiff-exit/test2-filtered-removed-fns-v0.o",
+    "data/test-abidiff-exit/test2-filtered-removed-fns-v1.o",
+    "",
+    "--no-show-locs",
+    abigail::tools_utils::ABIDIFF_ABI_CHANGE
+    | abigail::tools_utils::ABIDIFF_ABI_INCOMPATIBLE_CHANGE,
+    "data/test-abidiff-exit/test2-filtered-removed-fns-report0.txt",
+    "output/test-abidiff-exit/test2-filtered-removed-fns-report0.txt"
+  },
+  {
+    "data/test-abidiff-exit/test2-filtered-removed-fns-v0.o",
+    "data/test-abidiff-exit/test2-filtered-removed-fns-v1.o",
+    "data/test-abidiff-exit/test2-filtered-removed-fns.abignore",
+    "--no-show-locs",
+    abigail::tools_utils::ABIDIFF_OK,
+    "data/test-abidiff-exit/test2-filtered-removed-fns-report1.txt",
+    "output/test-abidiff-exit/test2-filtered-removed-fns-report1.txt"
+  },
+  {0, 0, 0 ,0,  abigail::tools_utils::ABIDIFF_OK, 0, 0}
 };
 
 int
@@ -78,13 +108,18 @@ main()
 
   bool is_ok = true;
   string in_elfv0_path, in_elfv1_path,
-    abidiff_options, abidiff, cmd,
+    in_suppression_path, abidiff_options, abidiff, cmd,
     ref_diff_report_path, out_diff_report_path;
 
     for (InOutSpec* s = in_out_specs; s->in_elfv0_path; ++s)
       {
 	in_elfv0_path = string(get_src_dir()) + "/tests/" + s->in_elfv0_path;
 	in_elfv1_path = string(get_src_dir()) + "/tests/" + s->in_elfv1_path;
+	if (s->in_suppr_path && strcmp(s->in_suppr_path, ""))
+	  in_suppression_path =
+	    string(get_src_dir()) + "/tests/" + s->in_suppr_path;
+	else
+	  in_suppression_path.clear();
 
 	abidiff_options = s->abidiff_options;
 	ref_diff_report_path =
@@ -104,6 +139,9 @@ main()
 	if (!abidiff_options.empty())
 	  abidiff += " " + abidiff_options;
 
+	if (!in_suppression_path.empty())
+	  abidiff += " --suppressions " + in_suppression_path;
+
 	cmd = abidiff + " " + in_elfv0_path + " " + in_elfv1_path;
 	cmd += " > " + out_diff_report_path;