diff mbox series

[2/2] KMI Whitelists: Drop old whitelist extraction methods

Message ID 20200114173450.137714-2-maennich@google.com
State Superseded
Headers
Series None |

Commit Message

Aleksei Vetrov via Libabigail Jan. 1, 2020, midnight UTC 
  The previous commit introduces a new (tested) way of creating function
and variable suppressions from multiple whitelist definitions. Migrate
to this new way of processing KMI whitelists.

	* include/abg-tools-utils.h
	(gen_suppr_spec_from_kernel_abi_whitelist): Delete declaration.
	* src/abg-tools-utils.cc
	(gen_suppr_spec_from_kernel_abi_whitelist): Delete definition
	and migrate users to gen_suppr_spec_from_kernel_abi_whitelists.
	* tools/abidiff.cc (set_suppressions): Migrate from using
	gen_suppr_spec_from_kernel_abi_whitelist to
	gen_suppr_spec_from_kernel_abi_whitelists.
	* tools/abidw.cc (set_suppressions): Likewise.
	* tools/abipkgdiff.cc: Drop unused using definition.
	* tools/kmidiff.cc: Likewise.

Signed-off-by: Matthias Maennich <maennich@google.com>
---
 include/abg-tools-utils.h |   4 --
 src/abg-tools-utils.cc    | 117 ++------------------------------------
 tools/abidiff.cc          |  12 ++--
 tools/abidw.cc            |  12 ++--
 tools/abipkgdiff.cc       |   1 -
 tools/kmidiff.cc          |   1 -
 6 files changed, 18 insertions(+), 129 deletions(-)

Comments

Aleksei Vetrov via Libabigail Jan. 1, 2020, midnight UTC | #1 
On Tue, Jan 21, 2020 at 05:24:11PM +0100, Dodji Seketeli wrote:
>Matthias Maennich <maennich@google.com> a ?crit:
>
>> The previous commit introduces a new (tested) way of creating function
>> and variable suppressions from multiple whitelist definitions. Migrate
>> to this new way of processing KMI whitelists.
>>
>> 	* include/abg-tools-utils.h
>> 	(gen_suppr_spec_from_kernel_abi_whitelist): Delete declaration.
>> 	* src/abg-tools-utils.cc
>> 	(gen_suppr_spec_from_kernel_abi_whitelist): Delete definition
>> 	and migrate users to gen_suppr_spec_from_kernel_abi_whitelists.
>> 	* tools/abidiff.cc (set_suppressions): Migrate from using
>> 	gen_suppr_spec_from_kernel_abi_whitelist to
>> 	gen_suppr_spec_from_kernel_abi_whitelists.
>> 	* tools/abidw.cc (set_suppressions): Likewise.
>> 	* tools/abipkgdiff.cc: Drop unused using definition.
>> 	* tools/kmidiff.cc: Likewise.
>
>This is OK to commit to master once the first patch of the series is in.

Hi Dodji!

Thanks for the review!
I applied the patches after correcting them both(!) according to your
comments.

Cheers,
Matthias

>
>Thank you for working on this!
>
>Cheers,
>
>-- 
>		Dodji
diff mbox series

Patch

diff --git a/include/abg-tools-utils.h b/include/abg-tools-utils.h
index ea44e4dd36aa..240e36b2f363 100644
--- a/include/abg-tools-utils.h
+++ b/include/abg-tools-utils.h
@@ -86,10 +86,6 @@  void convert_char_stars_to_char_star_stars(const vector<char*>&,
 suppr::type_suppression_sptr
 gen_suppr_spec_from_headers(const string& hdrs_root_dir);
 
-bool
-gen_suppr_spec_from_kernel_abi_whitelist(const string& abi_whitelist_path,
-					 suppr::suppressions_type& s);
-
 suppr::suppressions_type
 gen_suppr_spec_from_kernel_abi_whitelists(
     const std::vector<string>& abi_whitelist_paths);
diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc
index dcc302546beb..e833f91a99a8 100644
--- a/src/abg-tools-utils.cc
+++ b/src/abg-tools-utils.cc
@@ -1944,7 +1944,8 @@  gen_suppr_spec_from_kernel_abi_whitelists(
   suppressions_type result;
   if (!whitelisted_names.empty())
     {
-      // Drop duplicates
+      // Drop duplicates to simplify the regex we are generating
+      std::sort(whitelisted_names.begin(), whitelisted_names.end());
       whitelisted_names.erase(
 	  std::unique(whitelisted_names.begin(), whitelisted_names.end()),
 	  whitelisted_names.end());
@@ -1983,111 +1984,6 @@  gen_suppr_spec_from_kernel_abi_whitelists(
   return result;
 }
 
-/// Generate a suppression specification from kernel abi whitelist
-/// files.
-///
-/// A kernel ABI whitelist file is an INI file that usually has only
-/// one section.  The name of the section is a string that ends up
-/// with the sub-string "whitelist".  For instance
-/// RHEL7_x86_64_whitelist.
-///
-/// Then the content of the section is a set of function or variable
-/// names, one name per line.  Each function or variable name is the
-/// name of a function or a variable whose changes are to be keept.
-///
-/// This function reads the white list and generates a
-/// function_suppression_sptr or variable_suppression_sptr (or
-/// several, if there are more than one section) that is added to a
-/// vector of suppressions.
-///
-/// @param abi_whitelist_path the path to the Kernel ABI whitelist.
-///
-/// @param supprs the resulting vector of suppressions to which the
-/// new function suppressions resulting from reading the whitelist are
-/// added.  This vector is updated iff the function returns true.
-///
-/// @return true iff the abi whitelist file was read and function
-/// suppressions could be generated as a result.
-bool
-gen_suppr_spec_from_kernel_abi_whitelist(const string& abi_whitelist_path,
-					 suppressions_type& supprs)
-{
-  abigail::ini::config whitelist;
-  if (!read_config(abi_whitelist_path, whitelist))
-    return false;
-
-  bool created_a_suppr = false;
-
-  const ini::config::sections_type &whitelist_sections =
-    whitelist.get_sections();
-  for (ini::config::sections_type::const_iterator s =
-	 whitelist_sections.begin();
-       s != whitelist_sections.end();
-       ++s)
-    {
-      string section_name = (*s)->get_name();
-      if (!string_ends_with(section_name, "whitelist"))
-	continue;
-
-      function_suppression_sptr fn_suppr;
-      variable_suppression_sptr var_suppr;
-      string function_names_regexp, variable_names_regexp;
-      for (ini::config::properties_type::const_iterator p =
-	     (*s)->get_properties().begin();
-	   p != (*s)->get_properties().end();
-	   ++p)
-	{
-	  if (simple_property_sptr prop = is_simple_property(*p))
-	    if (prop->has_empty_value())
-	      {
-		const string &name = prop->get_name();
-		if (!name.empty())
-		  {
-		    if (!function_names_regexp.empty())
-		      function_names_regexp += "|";
-		    function_names_regexp += "^" + name + "$";
-
-		    if (!variable_names_regexp.empty())
-		      variable_names_regexp += "|";
-		    variable_names_regexp += "^" + name + "$";
-		  }
-	      }
-	}
-
-      if (!function_names_regexp.empty())
-	{
-	  // Build a suppression specification which *keeps* functions
-	  // whose ELF symbols match the regular expression contained
-	  // in function_names_regexp.  This will also keep the ELF
-	  // symbols (not designated by any debug info) whose names
-	  // match this regexp.
-	  fn_suppr.reset(new function_suppression);
-	  fn_suppr->set_label(section_name);
-	  fn_suppr->set_symbol_name_not_regex_str(function_names_regexp);
-	  fn_suppr->set_drops_artifact_from_ir(true);
-	  supprs.push_back(fn_suppr);
-	  created_a_suppr = true;
-	}
-
-      if (!variable_names_regexp.empty())
-	{
-	  // Build a suppression specification which *keeps* variables
-	  // whose ELF symbols match the regular expression contained
-	  // in function_names_regexp.  This will also keep the ELF
-	  // symbols (not designated by any debug info) whose names
-	  // match this regexp.
-	  var_suppr.reset(new variable_suppression);
-	  var_suppr->set_label(section_name);
-	  var_suppr->set_symbol_name_not_regex_str(variable_names_regexp);
-	  var_suppr->set_drops_artifact_from_ir(true);
-	  supprs.push_back(var_suppr);
-	  created_a_suppr = true;
-	}
-    }
-
-  return created_a_suppr;
-}
-
 /// Get the path to the default system suppression file.
 ///
 /// @return a copy of the default system suppression file.
@@ -2262,11 +2158,10 @@  load_generate_apply_suppressions(dwarf_reader::read_context &read_ctxt,
 	   ++i)
 	read_suppressions(*i, supprs);
 
-      for (vector<string>::const_iterator i =
-	     kabi_whitelist_paths.begin();
-	   i != kabi_whitelist_paths.end();
-	   ++i)
-	gen_suppr_spec_from_kernel_abi_whitelist(*i, supprs);
+      const suppressions_type& wl_suppr
+	  = gen_suppr_spec_from_kernel_abi_whitelists(kabi_whitelist_paths);
+
+      supprs.insert(supprs.end(), wl_suppr.begin(), wl_suppr.end());
     }
 
   abigail::dwarf_reader::add_read_context_suppressions(read_ctxt, supprs);
diff --git a/tools/abidiff.cc b/tools/abidiff.cc
index fda8d2f98565..7ca427446911 100644
--- a/tools/abidiff.cc
+++ b/tools/abidiff.cc
@@ -59,7 +59,7 @@  using abigail::tools_utils::emit_prefix;
 using abigail::tools_utils::check_file;
 using abigail::tools_utils::guess_file_type;
 using abigail::tools_utils::gen_suppr_spec_from_headers;
-using abigail::tools_utils::gen_suppr_spec_from_kernel_abi_whitelist;
+using abigail::tools_utils::gen_suppr_spec_from_kernel_abi_whitelists;
 using abigail::tools_utils::load_default_system_suppressions;
 using abigail::tools_utils::load_default_user_suppressions;
 using abigail::tools_utils::abidiff_status;
@@ -760,11 +760,11 @@  set_suppressions(ReadContextType& read_ctxt, const options& opts)
 	}
     }
 
-  for (vector<string>::const_iterator i =
-	 opts.kernel_abi_whitelist_paths.begin();
-       i != opts.kernel_abi_whitelist_paths.end();
-       ++i)
-    gen_suppr_spec_from_kernel_abi_whitelist(*i, supprs);
+  const suppressions_type& wl_suppr
+      = gen_suppr_spec_from_kernel_abi_whitelists(
+	  opts.kernel_abi_whitelist_paths);
+
+  supprs.insert(supprs.end(), wl_suppr.begin(), wl_suppr.end());
 
   add_read_context_suppressions(read_ctxt, supprs);
 }
diff --git a/tools/abidw.cc b/tools/abidw.cc
index d520fc3d03a5..19ae9b0d2622 100644
--- a/tools/abidw.cc
+++ b/tools/abidw.cc
@@ -375,12 +375,12 @@  set_suppressions(read_context& read_ctxt, options& opts)
   if (suppr)
     supprs.push_back(suppr);
 
-  using abigail::tools_utils::gen_suppr_spec_from_kernel_abi_whitelist;
-  for (vector<string>::const_iterator i =
-	 opts.kabi_whitelist_paths.begin();
-       i != opts.kabi_whitelist_paths.end();
-       ++i)
-    gen_suppr_spec_from_kernel_abi_whitelist(*i, opts.kabi_whitelist_supprs);
+  using abigail::tools_utils::gen_suppr_spec_from_kernel_abi_whitelists;
+  const suppressions_type& wl_suppr
+      = gen_suppr_spec_from_kernel_abi_whitelists(opts.kabi_whitelist_paths);
+
+  opts.kabi_whitelist_supprs.insert(opts.kabi_whitelist_supprs.end(),
+				    wl_suppr.begin(), wl_suppr.end());
 
   add_read_context_suppressions(read_ctxt, supprs);
   add_read_context_suppressions(read_ctxt, opts.kabi_whitelist_supprs);
diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc
index f3065fdf05ac..e8352115ccc0 100644
--- a/tools/abipkgdiff.cc
+++ b/tools/abipkgdiff.cc
@@ -136,7 +136,6 @@  using abigail::tools_utils::base_name;
 using abigail::tools_utils::get_rpm_arch;
 using abigail::tools_utils::file_is_kernel_package;
 using abigail::tools_utils::gen_suppr_spec_from_headers;
-using abigail::tools_utils::gen_suppr_spec_from_kernel_abi_whitelist;
 using abigail::tools_utils::get_default_system_suppression_file_path;
 using abigail::tools_utils::get_default_user_suppression_file_path;
 using abigail::tools_utils::get_vmlinux_path_from_kernel_dist;
diff --git a/tools/kmidiff.cc b/tools/kmidiff.cc
index 9a1645284a01..86cfb3de9827 100644
--- a/tools/kmidiff.cc
+++ b/tools/kmidiff.cc
@@ -60,7 +60,6 @@  using abigail::comparison::get_default_harmful_categories_bitmap;
 using abigail::suppr::suppression_sptr;
 using abigail::suppr::suppressions_type;
 using abigail::suppr::read_suppressions;
-using abigail::tools_utils::gen_suppr_spec_from_kernel_abi_whitelist;
 using abigail::tools_utils::guess_file_type;
 using abigail::tools_utils::file_type;
 using abigail::xml_reader::read_corpus_group_from_native_xml_file;