From patchwork Wed Jan 1 00:00:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aleksei Vetrov via Libabigail X-Patchwork-Id: 38986 X-Patchwork-Original-From: libabigail@sourceware.org (Matthias Maennich via libabigail) From: Aleksei Vetrov via Libabigail Date: Wed, 01 Jan 2020 00:00:00 -0000 Subject: [PATCH 1/2] KMI Whitelists: Add functionality to make whitelists additive Message-ID: <20200114173450.137714-1-maennich@google.com> If multiple KMI whitelists are specified, either by passing --kmi-whitelist several times or by having multiple whitelist sections in the whitelist files, the generated suppressions are created as an intersection of symbols. That is rather unusual, as whitelisting should rather work additive. That means that the symbols (or expressions thereof) defined across several sections or files shall be considered a union of symbols. This patch combines the whitelist parsing to create exactly one function_suppression and one variable suppression. A test case has been added to ensure the functionality is working. Please note, migrating the existing code to this new functionality is done in a separate commit. * include/abg-tools-utils.h (gen_suppr_spec_from_kernel_abi_whitelists): New function. * src/abg-tools-utils.cc (gen_suppr_spec_from_kernel_abi_whitelists): Likewise. * tests/.gitignore: Ignore new test executable. * tests/Makefile.am: Add new test executable. * tests/data/test-kmi-whitelist/whitelist-with-another-single-entry: New test input file. * tests/data/test-kmi-whitelist/whitelist-with-duplicate-entry: Likewise. * tests/data/test-kmi-whitelist/whitelist-with-single-entry: Likewise. * tests/data/test-kmi-whitelist/whitelist-with-two-sections: Likewise. * tests/data/Makefile.am: Add above test material. * tests/test-kmi-whitelist.cc: Add new test executable. Signed-off-by: Matthias Maennich --- include/abg-tools-utils.h | 4 + src/abg-tools-utils.cc | 113 ++++++++++++ tests/.gitignore | 1 + tests/Makefile.am | 4 + tests/data/Makefile.am | 7 +- .../whitelist-with-another-single-entry | 2 + .../whitelist-with-duplicate-entry | 3 + .../whitelist-with-single-entry | 2 + .../whitelist-with-two-sections | 5 + tests/test-kmi-whitelist.cc | 162 ++++++++++++++++++ 10 files changed, 302 insertions(+), 1 deletion(-) create mode 100644 tests/data/test-kmi-whitelist/whitelist-with-another-single-entry create mode 100644 tests/data/test-kmi-whitelist/whitelist-with-duplicate-entry create mode 100644 tests/data/test-kmi-whitelist/whitelist-with-single-entry create mode 100644 tests/data/test-kmi-whitelist/whitelist-with-two-sections create mode 100644 tests/test-kmi-whitelist.cc diff --git a/include/abg-tools-utils.h b/include/abg-tools-utils.h index a153af689740..ea44e4dd36aa 100644 --- a/include/abg-tools-utils.h +++ b/include/abg-tools-utils.h @@ -90,6 +90,10 @@ 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& abi_whitelist_paths); + bool get_vmlinux_path_from_kernel_dist(const string& from, string& vmlinux_path); diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc index 0495905253ed..dcc302546beb 100644 --- a/src/abg-tools-utils.cc +++ b/src/abg-tools-utils.cc @@ -38,8 +38,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -1870,6 +1872,117 @@ gen_suppr_spec_from_headers(const string& headers_root_dir) 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. +/// +/// A whitelist file can have multiple sections (adhering to the naming +/// conventions and multiple files can be passed. The suppression that +/// is created takes all whitelist sections from all files into account. +/// Symbols (or expression of such) are deduplicated in the final +/// suppression expression. +/// +/// This function reads the white lists and generates a +/// function_suppression_sptr and variable_suppression_sptr and returns +/// a vector containing those. +/// +/// @param abi_whitelist_paths a vector of KMI whitelist paths +/// +/// @return a vector or suppressions +suppressions_type +gen_suppr_spec_from_kernel_abi_whitelists( + const std::vector& abi_whitelist_paths) +{ + + std::vector whitelisted_names; + for (std::vector::const_iterator path_iter + = abi_whitelist_paths.begin(), + path_end = abi_whitelist_paths.end(); + path_iter != path_end; ++path_iter) + { + + abigail::ini::config whitelist; + if (!read_config(*path_iter, whitelist)) + continue; + + const ini::config::sections_type& whitelist_sections + = whitelist.get_sections(); + + for (ini::config::sections_type::const_iterator section_iter + = whitelist_sections.begin(), + section_end = whitelist_sections.end(); + section_iter != section_end; ++section_iter) + { + std::string section_name = (*section_iter)->get_name(); + if (!string_ends_with(section_name, "whitelist")) + continue; + for (ini::config::properties_type::const_iterator prop_iter + = (*section_iter)->get_properties().begin(), + prop_end = (*section_iter)->get_properties().end(); + prop_iter != prop_end; ++prop_iter) + { + if (const simple_property_sptr& prop + = is_simple_property(*prop_iter)) + if (prop->has_empty_value()) + { + const std::string& name = prop->get_name(); + if (!name.empty()) + whitelisted_names.push_back(name); + } + } + } + } + + suppressions_type result; + if (!whitelisted_names.empty()) + { + // Drop duplicates + whitelisted_names.erase( + std::unique(whitelisted_names.begin(), whitelisted_names.end()), + whitelisted_names.end()); + + // Build the regular expression to suppress non whitelisted symbols. + std::stringstream regex_ss; + regex_ss << "^"; + std::copy(whitelisted_names.begin(), whitelisted_names.end(), + std::ostream_iterator(regex_ss, "$|^")); + regex_ss.seekp(0, std::ios::end); + const std::string& regex = regex_ss.str().substr( + 0, static_cast(regex_ss.tellp()) - 2); + + // 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. + function_suppression_sptr fn_suppr(new function_suppression); + fn_suppr->set_label("whitelist"); + fn_suppr->set_symbol_name_not_regex_str(regex); + fn_suppr->set_drops_artifact_from_ir(true); + result.push_back(fn_suppr); + + // 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. + variable_suppression_sptr var_suppr(new variable_suppression); + var_suppr->set_label("whitelist"); + var_suppr->set_symbol_name_not_regex_str(regex); + var_suppr->set_drops_artifact_from_ir(true); + result.push_back(var_suppr); + } + return result; +} + /// Generate a suppression specification from kernel abi whitelist /// files. /// diff --git a/tests/.gitignore b/tests/.gitignore index 05451408c4f7..dec5f38802a7 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -21,6 +21,7 @@ runtestdifffilter runtestdiffpkg runtestdiffsuppr runtestini +runtestkmiwhitelist runtestlookupsyms runtestreaddwarf runtestreadwrite diff --git a/tests/Makefile.am b/tests/Makefile.am index 14e83608b0e5..7baa623a0289 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -45,6 +45,7 @@ runtestcorediff \ runtestabidiffexit \ runtestini \ runtesttoolsutils \ +runtestkmiwhitelist \ $(CXX11_TESTS) if ENABLE_RUNNING_TESTS_WITH_PY3 @@ -136,6 +137,9 @@ runtestini_LDADD = libtestutils.la $(top_builddir)/src/libabigail.la runtesttoolsutils_SOURCES = test-tools-utils.cc runtesttoolsutils_LDADD = libtestutils.la $(top_builddir)/src/libabigail.la +runtestkmiwhitelist_SOURCES = test-kmi-whitelist.cc +runtestkmiwhitelist_LDADD = libtestutils.la $(top_builddir)/src/libabigail.la + runtestsvg_SOURCES=test-svg.cc runtestsvg_LDADD=$(top_builddir)/src/libabigail.la diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 214a110a7595..c5ac1ff35189 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -1590,4 +1590,9 @@ test-fedabipkgdiff/nss-util/nss-util-3.12.6-1.fc14.x86_64.rpm \ test-fedabipkgdiff/nss-util/nss-util-3.24.0-2.0.fc25.x86_64.rpm \ \ test-ini/test01-equal-in-property-string.abignore.expected \ -test-ini/test01-equal-in-property-string.abignore +test-ini/test01-equal-in-property-string.abignore \ +\ +test-kmi-whitelist/whitelist-with-single-entry \ +test-kmi-whitelist/whitelist-with-another-single-entry \ +test-kmi-whitelist/whitelist-with-duplicate-entry \ +test-kmi-whitelist/whitelist-with-two-sections diff --git a/tests/data/test-kmi-whitelist/whitelist-with-another-single-entry b/tests/data/test-kmi-whitelist/whitelist-with-another-single-entry new file mode 100644 index 000000000000..dc601b8b8119 --- /dev/null +++ b/tests/data/test-kmi-whitelist/whitelist-with-another-single-entry @@ -0,0 +1,2 @@ +[abi_whitelist] + test_another_symbol diff --git a/tests/data/test-kmi-whitelist/whitelist-with-duplicate-entry b/tests/data/test-kmi-whitelist/whitelist-with-duplicate-entry new file mode 100644 index 000000000000..1721e76c946b --- /dev/null +++ b/tests/data/test-kmi-whitelist/whitelist-with-duplicate-entry @@ -0,0 +1,3 @@ +[abi_whitelist] + test_symbol + test_symbol diff --git a/tests/data/test-kmi-whitelist/whitelist-with-single-entry b/tests/data/test-kmi-whitelist/whitelist-with-single-entry new file mode 100644 index 000000000000..748db09e09f2 --- /dev/null +++ b/tests/data/test-kmi-whitelist/whitelist-with-single-entry @@ -0,0 +1,2 @@ +[abi_whitelist] + test_symbol diff --git a/tests/data/test-kmi-whitelist/whitelist-with-two-sections b/tests/data/test-kmi-whitelist/whitelist-with-two-sections new file mode 100644 index 000000000000..7efeef273c9f --- /dev/null +++ b/tests/data/test-kmi-whitelist/whitelist-with-two-sections @@ -0,0 +1,5 @@ +[abi_whitelist] + test_symbol1 + +[abi2_whitelist] + test_symbol2 diff --git a/tests/test-kmi-whitelist.cc b/tests/test-kmi-whitelist.cc new file mode 100644 index 000000000000..355ee2a650d7 --- /dev/null +++ b/tests/test-kmi-whitelist.cc @@ -0,0 +1,162 @@ +// -*- Mode: C++ -*- +// +// Copyright (C) 2020 Google, Inc. +// +// This file is part of the GNU Application Binary Interface Generic +// Analysis and Instrumentation Library (libabigail). This library is +// free software; you can redistribute it and/or modify it under the +// terms of the GNU Lesser General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) any +// later version. + +// This library is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Lesser Public License for more details. + +// You should have received a copy of the GNU Lesser General Public +// License along with this program; see the file COPYING-LGPLV3. If +// not, see . + +// Author: Matthias Maennich + +/// @file +/// +/// This program tests suppression generation from KMI whitelists. + +#include + +#include "abg-fwd.h" +#include "abg-suppression.h" +#include "abg-tools-utils.h" +#include "test-utils.h" + +using abigail::tools_utils::gen_suppr_spec_from_kernel_abi_whitelists; +using abigail::suppr::suppression_sptr; +using abigail::suppr::suppressions_type; +using abigail::suppr::function_suppression_sptr; +using abigail::suppr::variable_suppression_sptr; +using abigail::suppr::is_function_suppression; +using abigail::suppr::is_variable_suppression; + +const static std::string whitelist_with_single_entry + = std::string(abigail::tests::get_src_dir()) + + "/tests/data/test-kmi-whitelist/whitelist-with-single-entry"; + +const static std::string whitelist_with_another_single_entry + = std::string(abigail::tests::get_src_dir()) + + "/tests/data/test-kmi-whitelist/whitelist-with-another-single-entry"; + +const static std::string whitelist_with_two_sections + = std::string(abigail::tests::get_src_dir()) + + "/tests/data/test-kmi-whitelist/whitelist-with-two-sections"; + +const static std::string whitelist_with_duplicate_entry + = std::string(abigail::tests::get_src_dir()) + + "/tests/data/test-kmi-whitelist/whitelist-with-duplicate-entry"; + +bool +suppressions_are_consistent(const suppressions_type& suppr, + const std::string& expr) +{ + if (suppr.size() != 2) + return false; + + function_suppression_sptr left = is_function_suppression(suppr[0]); + variable_suppression_sptr right = is_variable_suppression(suppr[1]); + + return // correctly casted + (left && right) + // same label + && (left->get_label() == right->get_label()) + // same mode + && (left->get_drops_artifact_from_ir() + == right->get_drops_artifact_from_ir()) + // same regex + && (left->get_symbol_name_not_regex_str() + == right->get_symbol_name_not_regex_str()) + // regex as expected + && (left->get_symbol_name_not_regex_str() == expr); +} + +bool +testNoWhitelist() +{ + const std::vector abi_whitelist_paths; + suppressions_type suppr + = gen_suppr_spec_from_kernel_abi_whitelists(abi_whitelist_paths); + return suppr.empty(); +} + +bool +testSingleEntryWhitelist() +{ + std::vector abi_whitelist_paths; + abi_whitelist_paths.push_back(whitelist_with_single_entry); + suppressions_type suppr + = gen_suppr_spec_from_kernel_abi_whitelists(abi_whitelist_paths); + return !suppr.empty() && suppressions_are_consistent(suppr, "^test_symbol$"); +} + +bool +testWhitelistWithDuplicateEntries() +{ + std::vector abi_whitelist_paths; + abi_whitelist_paths.push_back(whitelist_with_duplicate_entry); + suppressions_type suppr + = gen_suppr_spec_from_kernel_abi_whitelists(abi_whitelist_paths); + return !suppr.empty() && suppressions_are_consistent(suppr, "^test_symbol$"); +} + +bool +testTwoWhitelists() +{ + std::vector abi_whitelist_paths; + abi_whitelist_paths.push_back(whitelist_with_single_entry); + abi_whitelist_paths.push_back(whitelist_with_another_single_entry); + suppressions_type suppr + = gen_suppr_spec_from_kernel_abi_whitelists(abi_whitelist_paths); + return !suppr.empty() + && suppressions_are_consistent(suppr, + "^test_symbol$|^test_another_symbol$"); +} + +bool +testTwoWhitelistsWithDuplicates() +{ + std::vector abi_whitelist_paths; + abi_whitelist_paths.push_back(whitelist_with_duplicate_entry); + abi_whitelist_paths.push_back(whitelist_with_another_single_entry); + suppressions_type suppr + = gen_suppr_spec_from_kernel_abi_whitelists(abi_whitelist_paths); + return !suppr.empty() + && suppressions_are_consistent(suppr, + "^test_symbol$|^test_another_symbol$"); +} + +bool +testWhitelistWithTwoSections() +{ + std::vector abi_whitelist_paths; + abi_whitelist_paths.push_back(whitelist_with_two_sections); + suppressions_type suppr + = gen_suppr_spec_from_kernel_abi_whitelists(abi_whitelist_paths); + return !suppr.empty() + && suppressions_are_consistent(suppr, + "^test_symbol1$|^test_symbol2$"); +} + +int +main(int, char*[]) +{ + bool is_ok = true; + + is_ok = is_ok && testNoWhitelist(); + is_ok = is_ok && testSingleEntryWhitelist(); + is_ok = is_ok && testWhitelistWithDuplicateEntries(); + is_ok = is_ok && testTwoWhitelists(); + is_ok = is_ok && testTwoWhitelistsWithDuplicates(); + is_ok = is_ok && testWhitelistWithTwoSections(); + + return !is_ok; +} From patchwork Wed Jan 1 00:00:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aleksei Vetrov via Libabigail X-Patchwork-Id: 38996 X-Patchwork-Original-From: libabigail@sourceware.org (Matthias Maennich via libabigail) From: Aleksei Vetrov via Libabigail Date: Wed, 01 Jan 2020 00:00:00 -0000 Subject: [PATCH 2/2] writer: completely skip over empty corpora In-Reply-To: <20200113144451.46359-1-maennich@google.com> References: <20200113144451.46359-1-maennich@google.com> Message-ID: <20200113144451.46359-3-maennich@google.com> A corpus that has no symbols contributing to the ABI surface (e.g. because of an exhaustive suppression), will not contribute in a later comparison via abidiff and friends. Hence, there is no need for such entries to appear in the ABI xml representation. This patch completely suppresses empty corpora. * src/abg-writer.cc (write_corpus): completely skip empty corpora rather than creating an empty entry for them. Signed-off-by: Matthias Maennich --- src/abg-writer.cc | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/abg-writer.cc b/src/abg-writer.cc index 994a21e28066..ca10af8d5e7e 100644 --- a/src/abg-writer.cc +++ b/src/abg-writer.cc @@ -4378,6 +4378,9 @@ write_corpus(write_context& ctxt, if (!corpus) return false; + if (corpus->is_empty()) + return true; + do_indent_to_level(ctxt, indent, 0); std::ostream& out = ctxt.get_ostream(); @@ -4411,12 +4414,6 @@ write_corpus(write_context& ctxt, write_tracking_non_reachable_types(corpus, out); - if (corpus->is_empty()) - { - out << "/>\n"; - return true; - } - out << ">\n"; // Write the list of needed corpora.