* [PATCH] suppression: Better handle soname/filename properties evaluation
@ 2020-01-01 0:00 Dodji Seketeli
0 siblings, 0 replies; only message in thread
From: Dodji Seketeli @ 2020-01-01 0:00 UTC (permalink / raw)
To: libabigail
At the moment, we don't make any difference between these two cases:
1/ A suppression specification has no soname or file name related
property.
2/ A suppression specification has a soname or file name related
property that doesn't match the soname or file name of a given
corpus.
In both cases 1/ and 2/ libabigail currently assumes that the
suppression specification does not match the given corpus we are
looking at. This can be wrong, especially if we are in the case 1/
and if the suppression does have other properties that should make it
match the corpus.
This patch fixes this issue.
* include/abg-suppression.h
(suppression_base::has_{soname,file_name}_related_property): Add
new member functions.
* src/abg-dwarf-reader.cc (read_context::suppression_can_match):
Fix the logic to make a difference between the case where the
suppression doesn't have any soname/filename property and the case
where the suppression does have a soname/filename property that
does not match the current binary.
* src/abg-reader.cc (read_context::suppression_can_match):
Likewise.
* src/abg-suppression-priv.h
(suppression_base::priv::matches_soname): If the suppression does
not have any soname related property then it doesn't match the
soname we are looking at.
(suppression_base::priv::matches_binary_name): If the suppression
does not have any filename related property then it doesn't match
the filename we are looking at.
* src/abg-suppression.cc
(suppression_base::has_{soname,file_name}_related_property):
Define new member functions.
(sonames_of_binaries_match): If the suppression does not have any
soname related property then it doesn't match the corpora of the
diff we are looking at.
(names_of_binaries_match): If the suppression does not have any
filename related property then it doesn't match the corpora of the
diff we are looking at.
(type_suppression::suppresses_type): Fix the logic to make a
difference between the case where the suppression doesn't have any
soname/filename property and the case where the suppression does
have a soname/filename property that does not match the current
binary.
(function_suppression::suppresses_{function, function_symbol}):
Likewise.
(variable_suppression::suppresses_{variable, variable_symbol}):
Likewise.
(file_suppression::suppresses_file): Likewise.
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
include/abg-suppression.h | 6 +++
src/abg-dwarf-reader.cc | 30 +++++++++---
src/abg-reader.cc | 20 ++++++--
src/abg-suppression-priv.h | 58 +++++++++++++++++++----
src/abg-suppression.cc | 113 ++++++++++++++++++++++++++++++++++-----------
5 files changed, 180 insertions(+), 47 deletions(-)
diff --git a/include/abg-suppression.h b/include/abg-suppression.h
index 02f3fc4..05709d6 100644
--- a/include/abg-suppression.h
+++ b/include/abg-suppression.h
@@ -97,6 +97,9 @@ public:
const string&
get_file_name_not_regex_str() const;
+ bool
+ has_file_name_related_property() const;
+
void
set_soname_regex_str(const string& regexp);
@@ -109,6 +112,9 @@ public:
const string&
get_soname_not_regex_str() const;
+ bool
+ has_soname_related_property() const;
+
virtual bool
suppresses_diff(const diff*) const = 0;
diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 436f610..0697c7a 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -8820,17 +8820,35 @@ public:
/// Tests if a suppression specification can match ABI artifacts
/// coming from the binary being analyzed.
///
- /// This tests if the suppression matches the soname of and binary
- /// name of the ELF binary being analyzed.
+ /// This tests if the suppression can match the soname of and binary
+ /// name of the ELF binary being analyzed. More precisely, if there
+ /// are any soname or file name property in the suppression and if
+ /// those do *NOT* match the current binary, then the function
+ /// returns false.
///
/// @param s the suppression specification to consider.
+ ///
+ /// @return true iff either there are no soname/filename related
+ /// property on the suppression, or if none of the soname/filename
+ /// properties of the suppression match the current binary.
bool
suppression_can_match(const suppr::suppression_base& s) const
{
- if (s.priv_->matches_soname(dt_soname())
- && s.priv_->matches_binary_name(elf_path()))
- return true;
- return false;
+ if (!s.priv_->matches_soname(dt_soname()))
+ if (s.has_soname_related_property())
+ // The suppression has some SONAME related properties, but
+ // none of them match the SONAME of the current binary. So
+ // the suppression cannot match the current binary.
+ return false;
+
+ if (!s.priv_->matches_binary_name(elf_path()))
+ if (s.has_file_name_related_property())
+ // The suppression has some file_name related properties, but
+ // none of them match the file name of the current binary. So
+ // the suppression cannot match the current binary.
+ return false;
+
+ return true;
}
/// Test whether if a given function suppression matches a function
diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 30b9abc..7c8a56a 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -948,10 +948,22 @@ public:
suppression_can_match(const suppr::suppression_base& s) const
{
corpus_sptr corp = get_corpus();
- if (s.priv_->matches_soname(corp->get_soname())
- && s.priv_->matches_binary_name(corp->get_path()))
- return true;
- return false;
+
+ if (!s.priv_->matches_soname(corp->get_soname()))
+ if (s.has_soname_related_property())
+ // The suppression has some SONAME related properties, but
+ // none of them match the SONAME of the current binary. So
+ // the suppression cannot match the current binary.
+ return false;
+
+ if (!s.priv_->matches_binary_name(corp->get_path()))
+ if (s.has_file_name_related_property())
+ // The suppression has some file_name related properties, but
+ // none of them match the file name of the current binary. So
+ // the suppression cannot match the current binary.
+ return false;
+
+ return true;
}
/// Test whether if a given function suppression matches a function
diff --git a/src/abg-suppression-priv.h b/src/abg-suppression-priv.h
index 449814c..d872168 100644
--- a/src/abg-suppression-priv.h
+++ b/src/abg-suppression-priv.h
@@ -179,32 +179,72 @@ public:
return soname_not_regex_;
}
+ /// Test if the current suppression matches a given SONAME.
+ ///
+ /// @param soname the SONAME to consider.
+ ///
+ /// @return true iff the suppression matches the SONAME denoted by
+ /// @p soname.
+ ///
+ /// Note that if the suppression contains no property that is
+ /// related to SONAMEs, the function returns false.
bool
matches_soname(const string& soname) const
{
+ bool has_regexp = false;
if (sptr_utils::regex_t_sptr regexp = get_soname_regex())
- if (regexec(regexp.get(), soname.c_str(), 0, NULL, 0) != 0)
- return false;
+ {
+ has_regexp = true;
+ if (regexec(regexp.get(), soname.c_str(), 0, NULL, 0) != 0)
+ return false;
+ }
if (sptr_utils::regex_t_sptr regexp = get_soname_not_regex())
- if (regexec(regexp.get(), soname.c_str(), 0, NULL, 0) == 0)
+ {
+ has_regexp = true;
+ if (regexec(regexp.get(), soname.c_str(), 0, NULL, 0) == 0)
+ return false;
+ }
+
+ if (!has_regexp)
return false;
return true;
}
+ /// Test if the current suppression matches the full file path to a
+ /// given binary.
+ ///
+ /// @param binary_name the full path to the binary.
+ ///
+ /// @return true iff the suppression matches the path denoted by @p
+ /// binary_name.
+ ///
+ /// Note that if the suppression contains no property that is
+ /// related to file name, the function returns false.
bool
matches_binary_name(const string& binary_name) const
{
+ bool has_regexp = false;
+
if (sptr_utils::regex_t_sptr regexp = get_file_name_regex())
- if (regexec(regexp.get(), binary_name.c_str(),
- 0, NULL, 0) != 0)
- return false;
+ {
+ has_regexp = true;
+ if (regexec(regexp.get(), binary_name.c_str(),
+ 0, NULL, 0) != 0)
+ return false;
+ }
if (sptr_utils::regex_t_sptr regexp = get_file_name_not_regex())
- if (regexec(regexp.get(), binary_name.c_str(),
- 0, NULL, 0) == 0)
- return false;
+ {
+ has_regexp = true;
+ if (regexec(regexp.get(), binary_name.c_str(),
+ 0, NULL, 0) == 0)
+ return false;
+ }
+
+ if (!has_regexp)
+ return false;
return true;
}
diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
index 05d3858..663749d 100644
--- a/src/abg-suppression.cc
+++ b/src/abg-suppression.cc
@@ -184,6 +184,18 @@ const string&
suppression_base::get_file_name_not_regex_str() const
{return priv_->file_name_not_regex_str_;}
+/// Test if the current suppression has a property related to file
+/// name.
+///
+/// @return true iff the current suppression has either a
+/// file_name_regex or a file_name_not_regex property.
+bool
+suppression_base::has_file_name_related_property() const
+{
+ return (!(get_file_name_regex_str().empty()
+ && get_file_name_not_regex_str().empty()));
+}
+
/// Setter of the "soname_regex_str property of the current instance
/// of @ref suppression_base.
///
@@ -234,6 +246,17 @@ const string&
suppression_base::get_soname_not_regex_str() const
{return priv_->soname_not_regex_str_;}
+/// Test if the current suppression has a property related to SONAMEs.
+///
+/// @return true iff the current suppression has either a soname_regex
+/// or a soname_not_regex property.
+bool
+suppression_base::has_soname_related_property() const
+{
+ return (!(get_soname_regex_str().empty()
+ && get_soname_not_regex_str().empty()));
+}
+
/// Check if the SONAMEs of the two binaries being compared match the
/// content of the properties "soname_regexp" and "soname_not_regexp"
/// of the current suppression specification.
@@ -254,6 +277,9 @@ sonames_of_binaries_match(const suppression_base& suppr,
string first_soname = ctxt.get_corpus_diff()->first_corpus()->get_soname(),
second_soname = ctxt.get_corpus_diff()->second_corpus()->get_soname();
+ if (!suppr.has_soname_related_property())
+ return false;
+
if (!suppr.priv_->matches_soname(first_soname)
&& !suppr.priv_->matches_soname(second_soname))
return false;
@@ -281,6 +307,9 @@ names_of_binaries_match(const suppression_base& suppr,
string first_binary_path = ctxt.get_corpus_diff()->first_corpus()->get_path(),
second_binary_path = ctxt.get_corpus_diff()->second_corpus()->get_path();
+ if (!suppr.has_file_name_related_property())
+ return false;
+
if (!suppr.priv_->matches_binary_name(first_binary_path)
&& !suppr.priv_->matches_binary_name(second_binary_path))
return false;
@@ -850,17 +879,18 @@ type_suppression::suppresses_type(const type_base_sptr& type,
{
if (ctxt)
{
- // Check if the names of the binaries match
+ // Check if the names of the binaries match the suppression
if (!names_of_binaries_match(*this, *ctxt))
- return false;
+ if (has_file_name_related_property())
+ return false;
- // Check if the sonames of the binaries match
+ // Check if the sonames of the binaries match the suppression
if (!sonames_of_binaries_match(*this, *ctxt))
- return false;
+ if (has_soname_related_property())
+ return false;
}
return suppresses_type(type);
-
}
/// Test if an instance of @ref type_suppression matches a given type.
@@ -2419,16 +2449,19 @@ function_suppression::suppresses_function(const function_decl* fn,
if (!(get_change_kind() & k))
return false;
- // Check if the name and soname of the binaries match
+ // Check if the name and soname of the binaries match the current
+ // suppr spec
if (ctxt)
{
- // Check if the name of the binaries match
+ // Check if the name of the binaries match the current suppr spec
if (!names_of_binaries_match(*this, *ctxt))
- return false;
+ if (has_file_name_related_property())
+ return false;
- // Check if the soname of the binaries match
+ // Check if the soname of the binaries match the current suppr spec
if (!sonames_of_binaries_match(*this, *ctxt))
- return false;
+ if (has_soname_related_property())
+ return false;
}
string fname = fn->get_qualified_name();
@@ -2752,16 +2785,21 @@ function_suppression::suppresses_function_symbol(const elf_symbol* sym,
ABG_ASSERT(k & function_suppression::ADDED_FUNCTION_CHANGE_KIND
|| k & function_suppression::DELETED_FUNCTION_CHANGE_KIND);
- // Check if the name and soname of the binaries match
+ // Check if the name and soname of the binaries match the current
+ // suppr spect
if (ctxt)
{
- // Check if the name of the binaries match
+ // Check if the name of the binaries match the current
+ // suppr spect
if (!names_of_binaries_match(*this, *ctxt))
- return false;
+ if (has_file_name_related_property())
+ return false;
- // Check if the soname of the binaries match
+ // Check if the soname of the binaries match the current
+ // suppr spect
if (!sonames_of_binaries_match(*this, *ctxt))
- return false;
+ if (has_soname_related_property())
+ return false;
}
string sym_name = sym->get_name(), sym_version = sym->get_version().str();
@@ -3715,13 +3753,17 @@ variable_suppression::suppresses_variable(const var_decl* var,
// Check if the name and soname of the binaries match
if (ctxt)
{
- // Check if the name of the binaries match
+ // Check if the name of the binaries match the current
+ // suppr spec
if (!names_of_binaries_match(*this, *ctxt))
- return false;
+ if (has_file_name_related_property())
+ return false;
- // Check if the soname of the binaries match
+ // Check if the soname of the binaries match the current suppr
+ // spec
if (!sonames_of_binaries_match(*this, *ctxt))
- return false;
+ if (has_soname_related_property())
+ return false;
}
string var_name = var->get_qualified_name();
@@ -3871,16 +3913,20 @@ variable_suppression::suppresses_variable_symbol(const elf_symbol* sym,
ABG_ASSERT(k & ADDED_VARIABLE_CHANGE_KIND
|| k & DELETED_VARIABLE_CHANGE_KIND);
- // Check if the name and soname of the binaries match
+ // Check if the name and soname of the binaries match the current
+ // suppr spec.
if (ctxt)
{
- // Check if the name of the binaries match
+ // Check if the name of the binaries match the current suppr
+ // spec
if (!names_of_binaries_match(*this, *ctxt))
- return false;
+ if (has_file_name_related_property())
+ return false;
- // Check if the soname of the binaries match
+ // Check if the soname of the binaries match the current suppr spec
if (!sonames_of_binaries_match(*this, *ctxt))
- return false;
+ if (has_soname_related_property())
+ return false;
}
string sym_name = sym->get_name(), sym_version = sym->get_version().str();
@@ -4227,15 +4273,26 @@ file_suppression::suppresses_file(const string& file_path)
string fname;
tools_utils::base_name(file_path, fname);
+ bool has_regexp = false;
+
if (sptr_utils::regex_t_sptr regexp =
suppression_base::priv_->get_file_name_regex())
- if (regexec(regexp.get(), fname.c_str(), 0, NULL, 0) != 0)
- return false;
+ {
+ has_regexp = true;
+ if (regexec(regexp.get(), fname.c_str(), 0, NULL, 0) != 0)
+ return false;
+ }
if (sptr_utils::regex_t_sptr regexp =
suppression_base::priv_->get_file_name_not_regex())
- if (regexec(regexp.get(), fname.c_str(), 0, NULL, 0) == 0)
- return false;
+ {
+ has_regexp = true;
+ if (regexec(regexp.get(), fname.c_str(), 0, NULL, 0) == 0)
+ return false;
+ }
+
+ if (!has_regexp)
+ return false;
return true;
}
--
1.8.3.1
--
Dodji
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2020-02-21 16:11 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-01 0:00 [PATCH] suppression: Better handle soname/filename properties evaluation Dodji Seketeli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).