KMI Whitelists: Add functionality to make whitelists additive
authorMatthias Maennich <maennich@google.com>
Tue, 14 Jan 2020 17:34:49 +0000 (17:34 +0000)
committerMatthias Maennich <maennich@google.com>
Tue, 21 Jan 2020 18:37:43 +0000 (18:37 +0000)
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.

Reviewed-by: Dodji Seketeli <dodji@seketeli.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
include/abg-tools-utils.h
src/abg-tools-utils.cc
tests/.gitignore
tests/Makefile.am
tests/data/Makefile.am
tests/data/test-kmi-whitelist/whitelist-with-another-single-entry [new file with mode: 0644]
tests/data/test-kmi-whitelist/whitelist-with-duplicate-entry [new file with mode: 0644]
tests/data/test-kmi-whitelist/whitelist-with-single-entry [new file with mode: 0644]
tests/data/test-kmi-whitelist/whitelist-with-two-sections [new file with mode: 0644]
tests/test-kmi-whitelist.cc [new file with mode: 0644]

index a153af689740236ad444778c46f3ea9cc5b6f473..e78dadf247f75a986b54926edbb368094a3f6b09 100644 (file)
@@ -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 vector<string>& abi_whitelist_paths);
+
 bool
 get_vmlinux_path_from_kernel_dist(const string&        from,
                                  string&               vmlinux_path);
index 0495905253ed1e9a6e8aeb88338176fffd08ea7e..2d396981c7667fb9c9e860be68ceabbcbd395e7e 100644 (file)
 #include <sys/time.h>
 #include <dirent.h>
 #include <time.h>
+#include <algorithm>
 #include <cstdlib>
 #include <cstring>
+#include <iterator>
 #include <ctype.h>
 #include <errno.h>
 #include <libgen.h>
@@ -1870,6 +1872,122 @@ 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<std::string>& abi_whitelist_paths)
+{
+
+  std::vector<std::string> whitelisted_names;
+  for (std::vector<std::string>::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 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());
+
+      // Build a regular expression representing the union of all
+      // the function and variable names expressed in the white list.
+      std::stringstream regex_ss;
+      regex_ss << "^";
+      std::copy(whitelisted_names.begin(), whitelisted_names.end(),
+               std::ostream_iterator<std::string>(regex_ss, "$|^"));
+      regex_ss.seekp(0, std::ios::end);
+      const std::string& regex =
+         regex_ss.str().substr(0, static_cast<size_t>(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.
 ///
index 05451408c4f7b68af58e1eb4280148b8863d4b0e..dec5f38802a7a2a2eae55b10496ea10a6618b447 100644 (file)
@@ -21,6 +21,7 @@ runtestdifffilter
 runtestdiffpkg
 runtestdiffsuppr
 runtestini
+runtestkmiwhitelist
 runtestlookupsyms
 runtestreaddwarf
 runtestreadwrite
index 14e83608b0e530dd0e4d225cef8a2489f693b0e6..7baa623a0289e7808afe6f50f78dcda334ae33fa 100644 (file)
@@ -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
 
index 41d9c25afdf2d7862dc749b927b9b9ff05d73c93..1ae9d31a4cd424330ae3ed3acd40d96f6e0aff25 100644 (file)
@@ -1624,4 +1624,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 (file)
index 0000000..dc601b8
--- /dev/null
@@ -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 (file)
index 0000000..1721e76
--- /dev/null
@@ -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 (file)
index 0000000..748db09
--- /dev/null
@@ -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 (file)
index 0000000..7efeef2
--- /dev/null
@@ -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 (file)
index 0000000..c8c43b9
--- /dev/null
@@ -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 <http://www.gnu.org/licenses/>.
+
+// Author: Matthias Maennich
+
+/// @file
+///
+/// This program tests suppression generation from KMI whitelists.
+
+#include <string>
+
+#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<std::string> abi_whitelist_paths;
+  suppressions_type             suppr
+      = gen_suppr_spec_from_kernel_abi_whitelists(abi_whitelist_paths);
+  return suppr.empty();
+}
+
+bool
+testSingleEntryWhitelist()
+{
+  std::vector<std::string> 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<std::string> 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<std::string> 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_another_symbol$|^test_symbol$");
+}
+
+bool
+testTwoWhitelistsWithDuplicates()
+{
+  std::vector<std::string> 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_another_symbol$|^test_symbol$");
+}
+
+bool
+testWhitelistWithTwoSections()
+{
+  std::vector<std::string> 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;
+}