[2/2] KMI Whitelists: Drop old whitelist extraction methods
Commit Message
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
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
@@ -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);
@@ -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);
@@ -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);
}
@@ -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);
@@ -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;
@@ -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;