[PATCH v3 06/21] diff suppression: Fix handling of change kinds.

[Giuliano Procida <gprocida@google.com>]

When parsing suppression specifications, libabigail attempts to detect
and ignore suppressions that are empty and so would match everything
in their category by default.

Unfortunately, with the way the parser currently works, these checks
have to be against an exhaustive list of fields that matter, rather
than by listing the fields that don't (like label). They are fragile
in the face of changes that add new fields.

Two of the short-cut checks are in fact buggy, missing out the
change_kind field. One of the checks also risks a null pointer
dereference as it doesn't actually trigger a return from the function.

This patch eliminates (rather than fixes up) this short-cutting on the
grounds that it is a maintenance burden and inconsistent behaviour.
Users should be able to do this:

[suppress_variable]
  label = Suppress all changes to variables

We could reinstate the logic when the code has global knowledge of
which fields are present and which have no suppression (restriction)
semantics, or perhaps just emit a warning message to the user if they
have supplied a completely empty (no label even) specification.

The patch also corrects 4 affected test cases to reflect that
suppression is actually happening (according to change_kind).

	* src/abg-suppression.cc (read_type_suppression): Remove
	short-circuiting of useless suppressions.
	(read_function_suppression): Ditto.
	(read_variable_suppression: Ditto.
	(read_file_suppression): Ditto.
	tests/data/test-diff-suppr/test15-suppr-added-fn-report-5.txt:
	Fix test - something is actually suppressed.
	* tests/data/test-diff-suppr/test16-suppr-removed-fn-report-5.txt:
	Ditto.
	* tests/data/test-diff-suppr/test17-suppr-added-var-report-5.txt:
	Ditto.
	* tests/data/test-diff-suppr/test18-suppr-removed-var-report-5.txt:
	Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-suppression.cc                        | 91 +++++--------------
 .../test15-suppr-added-fn-report-5.txt        |  6 +-
 .../test16-suppr-removed-fn-report-5.txt      | 15 +--
 .../test17-suppr-added-var-report-5.txt       | 15 +--
 .../test18-suppr-removed-var-report-5.txt     | 15 +--
 5 files changed, 26 insertions(+), 116 deletions(-)

diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
index 2fbbd61b..831a61ef 100644
--- a/src/abg-suppression.cc
+++ b/src/abg-suppression.cc
@@ -1840,19 +1840,6 @@ read_type_suppression(const ini::config::section& section)
 	changed_enumerator_names.push_back(p->get_value()->as_string());
     }
 
-  if (file_name_regex_str.empty()
-      && file_name_not_regex_str.empty()
-      && soname_regex_str.empty()
-      && soname_not_regex_str.empty()
-      && (!name_regex_prop || name_regex_prop->get_value()->as_string().empty())
-      && (!name_not_regex_prop
-	  || name_not_regex_prop->get_value()->as_string().empty())
-      && (!name_prop || name_prop->get_value()->as_string().empty())
-      && !consider_type_kind
-      && srcloc_not_regexp_str.empty()
-      && srcloc_not_in.empty())
-    return result;
-
   result.reset(new type_suppression(label_str, name_regex_str, name_str));
 
   if (consider_type_kind)
@@ -3288,32 +3275,16 @@ read_function_suppression(const ini::config::section& section)
 	  parms.push_back(parm);
       }
 
-  if (!label_str.empty()
-      || !name.empty()
-      || !name_regex_str.empty()
-      || !name_not_regex_str.empty()
-      || !file_name_regex_str.empty()
-      || !file_name_not_regex_str.empty()
-      || !soname_regex_str.empty()
-      || !soname_not_regex_str.empty()
-      || !return_type_name.empty()
-      || !return_type_regex_str.empty()
-      || !sym_name.empty()
-      || !sym_name_regex_str.empty()
-      || !sym_name_not_regex_str.empty()
-      || !sym_version.empty()
-      || !sym_ver_regex_str.empty()
-      || !parms.empty())
-
-    result.reset(new function_suppression(label_str, name,
-					  name_regex_str,
-					  return_type_name,
-					  return_type_regex_str,
-					  parms,
-					  sym_name,
-					  sym_name_regex_str,
-					  sym_version,
-					  sym_ver_regex_str));
+  result.reset(new function_suppression(label_str,
+					name,
+					name_regex_str,
+					return_type_name,
+					return_type_regex_str,
+					parms,
+					sym_name,
+					sym_name_regex_str,
+					sym_version,
+					sym_ver_regex_str));
 
   if ((drop_artifact_str == "yes" || drop_artifact_str == "true")
       && (!name.empty()
@@ -3324,11 +3295,11 @@ read_function_suppression(const ini::config::section& section)
 	  || !sym_name_not_regex_str.empty()))
     result->set_drops_artifact_from_ir(true);
 
-  if (result && !change_kind_str.empty())
+  if (!change_kind_str.empty())
     result->set_change_kind
       (function_suppression::parse_change_kind(change_kind_str));
 
-  if (result && !allow_other_aliases.empty())
+  if (!allow_other_aliases.empty())
     result->set_allow_other_aliases(allow_other_aliases == "yes"
 				    || allow_other_aliases == "true");
 
@@ -4157,27 +4128,15 @@ read_variable_suppression(const ini::config::section& section)
     ? type_name_regex_prop->get_value()->as_string()
      : "";
 
-  if (label_str.empty()
-      && name_str.empty()
-      && name_regex_str.empty()
-      && name_not_regex_str.empty()
-      && file_name_regex_str.empty()
-      && file_name_not_regex_str.empty()
-      && soname_regex_str.empty()
-      && soname_not_regex_str.empty()
-      && symbol_name.empty()
-      && symbol_name_regex_str.empty()
-      && symbol_name_not_regex_str.empty()
-      && symbol_version.empty()
-      && symbol_version_regex_str.empty()
-      && type_name_str.empty()
-      && type_name_regex_str.empty())
-    return result;
-
-  result.reset(new variable_suppression(label_str, name_str, name_regex_str,
-					symbol_name, symbol_name_regex_str,
-					symbol_version, symbol_version_regex_str,
-					type_name_str, type_name_regex_str));
+  result.reset(new variable_suppression(label_str,
+					name_str,
+					name_regex_str,
+					symbol_name,
+					symbol_name_regex_str,
+					symbol_version,
+					symbol_version_regex_str,
+					type_name_str,
+					type_name_regex_str));
 
   if ((drop_artifact_str == "yes" || drop_artifact_str == "true")
       && (!name_str.empty()
@@ -4194,7 +4153,7 @@ read_variable_suppression(const ini::config::section& section)
   if (!symbol_name_not_regex_str.empty())
     result->set_symbol_name_not_regex_str(symbol_name_not_regex_str);
 
-  if (result && !change_kind_str.empty())
+  if (!change_kind_str.empty())
     result->set_change_kind
       (variable_suppression::parse_change_kind(change_kind_str));
 
@@ -4337,12 +4296,6 @@ read_file_suppression(const ini::config::section& section)
     ? soname_not_regex_prop->get_value()->as_string()
     : "";
 
-  if (file_name_regex_str.empty()
-      && file_name_not_regex_str.empty()
-      && soname_regex_str.empty()
-      && soname_not_regex_str.empty())
-    return result;
-
   result.reset(new file_suppression(label_str,
 				    file_name_regex_str,
 				    file_name_not_regex_str));
diff --git a/tests/data/test-diff-suppr/test15-suppr-added-fn-report-5.txt b/tests/data/test-diff-suppr/test15-suppr-added-fn-report-5.txt
index 4eaba5b7..83dfe326 100644
--- a/tests/data/test-diff-suppr/test15-suppr-added-fn-report-5.txt
+++ b/tests/data/test-diff-suppr/test15-suppr-added-fn-report-5.txt
@@ -1,10 +1,6 @@
-Functions changes summary: 0 Removed, 1 Changed, 1 Added functions
+Functions changes summary: 0 Removed, 1 Changed, 0 Added (1 filtered out) functions
 Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
 
-1 Added function:
-
-  [A] 'function void bar()'    {_Z3barv}
-
 1 function with some indirect sub-type change:
 
   [C] 'function void bar(S&)' has some indirect sub-type changes:
diff --git a/tests/data/test-diff-suppr/test16-suppr-removed-fn-report-5.txt b/tests/data/test-diff-suppr/test16-suppr-removed-fn-report-5.txt
index b28fbd16..851f7728 100644
--- a/tests/data/test-diff-suppr/test16-suppr-removed-fn-report-5.txt
+++ b/tests/data/test-diff-suppr/test16-suppr-removed-fn-report-5.txt
@@ -1,16 +1,3 @@
-Functions changes summary: 1 Removed, 1 Changed, 0 Added functions
+Functions changes summary: 0 Removed (1 filtered out), 0 Changed (1 filtered out), 0 Added functions
 Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
 
-1 Removed function:
-
-  [D] 'function void bar()'    {_Z3barv}
-
-1 function with some indirect sub-type change:
-
-  [C] 'function void bar(S*)' has some indirect sub-type changes:
-    parameter 1 of type 'S*' has sub-type changes:
-      in pointed to type 'struct S':
-        type size changed from 32 to 64 (in bits)
-        1 data member insertion:
-          'unsigned int S::bar', at offset 32 (in bits)
-
diff --git a/tests/data/test-diff-suppr/test17-suppr-added-var-report-5.txt b/tests/data/test-diff-suppr/test17-suppr-added-var-report-5.txt
index 6965a151..f4e0aa29 100644
--- a/tests/data/test-diff-suppr/test17-suppr-added-var-report-5.txt
+++ b/tests/data/test-diff-suppr/test17-suppr-added-var-report-5.txt
@@ -1,16 +1,3 @@
 Functions changes summary: 0 Removed, 0 Changed, 0 Added function
-Variables changes summary: 0 Removed, 1 Changed, 1 Added variables
-
-1 Added variable:
-
-  [A] 'int var1'    {var1}
-
-1 Changed variable:
-
-  [C] 'S* var0' was changed:
-    type of variable changed:
-      in pointed to type 'struct S':
-        type size changed from 32 to 64 (in bits)
-        1 data member insertion:
-          'char S::m1', at offset 32 (in bits)
+Variables changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added (1 filtered out) variables
 
diff --git a/tests/data/test-diff-suppr/test18-suppr-removed-var-report-5.txt b/tests/data/test-diff-suppr/test18-suppr-removed-var-report-5.txt
index 3edf2bd1..ac380a4a 100644
--- a/tests/data/test-diff-suppr/test18-suppr-removed-var-report-5.txt
+++ b/tests/data/test-diff-suppr/test18-suppr-removed-var-report-5.txt
@@ -1,16 +1,3 @@
 Functions changes summary: 0 Removed, 0 Changed, 0 Added function
-Variables changes summary: 1 Removed, 1 Changed, 0 Added variables
-
-1 Removed variable:
-
-  [D] 'int var1'    {var1}
-
-1 Changed variable:
-
-  [C] 'S* var0' was changed:
-    type of variable changed:
-      in pointed to type 'struct S':
-        type size changed from 32 to 64 (in bits)
-        1 data member insertion:
-          'char S::m1', at offset 32 (in bits)
+Variables changes summary: 0 Removed (1 filtered out), 0 Changed (1 filtered out), 0 Added variables
 
-- 
2.26.2.303.gf8c07b1a785-goog