From patchwork Thu Apr 23 18:07:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 39151 From: gprocida@google.com (Giuliano Procida) Date: Thu, 23 Apr 2020 19:07:22 +0100 Subject: [PATCH v2 07/21] Add POSIX regex wrapper functions. In-Reply-To: <20200423154441.170531-8-gprocida@google.com> References: <20200423154441.170531-8-gprocida@google.com> Message-ID: <20200423180722.70921-1-gprocida@google.com> libabigail code uses the POSIX regex library consistently: - compile std::string to regex, with the flag REG_EXTENDED - store regex using a shared pointer wrapper - check match of regex against std::string All the C string / std::string logic and so on is repeated at every call site. This patch introduces wrapper functions to take care of this logic. There are no behavioural changes. * include/abg-regex.h (compile): Declare new function. (match): Declare new function. * src/abg-regex.cc (compile): Add new function wrapping regcomp. (match): Add new function wrapping regexec. Signed-off-by: Giuliano Procida --- include/abg-regex.h | 6 ++++++ src/abg-regex.cc | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/include/abg-regex.h b/include/abg-regex.h index 59976794..c7951a8f 100644 --- a/include/abg-regex.h +++ b/include/abg-regex.h @@ -71,6 +71,12 @@ operator<<(std::ostream& os, const escape& esc); std::string generate_from_strings(const std::vector& strs); +regex_t_sptr +compile(const std::string& str); + +bool +match(const regex_t_sptr& r, const std::string& str); + }// end namespace regex /// Specialization of sptr_utils::build_sptr for regex_t. diff --git a/src/abg-regex.cc b/src/abg-regex.cc index 90e4d144..7e5277e2 100644 --- a/src/abg-regex.cc +++ b/src/abg-regex.cc @@ -23,11 +23,22 @@ /// Some specialization for shared pointer utility templates. /// +#include "config.h" + #include #include + +#include "abg-internal.h" + +// +ABG_BEGIN_EXPORT_DECLARATIONS + #include "abg-sptr-utils.h" #include "abg-regex.h" +ABG_END_EXPORT_DECLARATIONS +// + namespace abigail { @@ -101,6 +112,36 @@ generate_from_strings(const std::vector& strs) return os.str(); } +/// Compile a regex from a string. +/// +/// The result is held in a shared pointer. This will be null if regex +/// compilation fails. +/// +/// @param str the string representation of the regex. +/// +/// @return shared pointer holder of a compiled regex object. +regex_t_sptr +compile(const std::string& str) +{ + regex_t_sptr r = sptr_utils::build_sptr(new regex_t); + if (regcomp(r.get(), str.c_str(), REG_EXTENDED)) + r.reset(); + return r; +} + +/// See if a string matches a regex. +/// +/// @param r a shared pointer holder of a compiled regex object. +/// +/// @param str a string. +/// +/// @return whether there was a match. +bool +match(const regex_t_sptr& r, const std::string& str) +{ + return !regexec(r.get(), str.c_str(), 0, NULL, 0); +} + }//end namespace regex }//end namespace abigail From patchwork Thu Apr 23 18:02:44 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 39149 From: gprocida@google.com (Giuliano Procida) Date: Thu, 23 Apr 2020 19:02:44 +0100 Subject: [PATCH v2 09/21] Use regex::match wrapper instead of regexec. In-Reply-To: <20200423154441.170531-10-gprocida@google.com> References: <20200423154441.170531-10-gprocida@google.com> Message-ID: <20200423180244.68691-1-gprocida@google.com> This patch eliminates all calls to regexec except that by regex::match itself. There are no behavioural changes. * src/abg-corpus-priv.h: Mechanically substitute use of regexec with regex::match wrapper. * src/abg-suppression-priv.h: Ditto. * src/abg-suppression.cc: Ditto. Signed-off-by: Giuliano Procida --- src/abg-corpus-priv.h | 8 +-- src/abg-suppression-priv.h | 10 ++-- src/abg-suppression.cc | 119 +++++++++++-------------------------- 3 files changed, 44 insertions(+), 93 deletions(-) diff --git a/src/abg-corpus-priv.h b/src/abg-corpus-priv.h index 544fac54..2618e2d0 100644 --- a/src/abg-corpus-priv.h +++ b/src/abg-corpus-priv.h @@ -521,7 +521,7 @@ public: compiled_regex_fns_suppress().begin(); i != compiled_regex_fns_suppress().end(); ++i) - if (regexec(i->get(), frep.c_str(), 0, NULL, 0) == 0) + if (regex::match(*i, frep)) { keep = false; break; @@ -554,7 +554,7 @@ public: compiled_regex_fns_keep().begin(); i != compiled_regex_fns_keep().end(); ++i) - if (regexec(i->get(), frep.c_str(), 0, NULL, 0) == 0) + if (regex::match(*i, frep)) { keep = true; break; @@ -628,7 +628,7 @@ public: compiled_regex_vars_suppress().begin(); i != compiled_regex_vars_suppress().end(); ++i) - if (regexec(i->get(), frep.c_str(), 0, NULL, 0) == 0) + if (regex::match(*i, frep)) { keep = false; break; @@ -662,7 +662,7 @@ public: compiled_regex_vars_keep().begin(); i != compiled_regex_vars_keep().end(); ++i) - if (regexec(i->get(), frep.c_str(), 0, NULL, 0) == 0) + if (regex::match(*i, frep)) { keep = true; break; diff --git a/src/abg-suppression-priv.h b/src/abg-suppression-priv.h index 4959cdbb..8043ebd9 100644 --- a/src/abg-suppression-priv.h +++ b/src/abg-suppression-priv.h @@ -160,14 +160,14 @@ public: if (regex::regex_t_sptr regexp = get_soname_regex()) { has_regexp = true; - if (regexec(regexp.get(), soname.c_str(), 0, NULL, 0) != 0) + if (!regex::match(regexp, soname)) return false; } if (regex::regex_t_sptr regexp = get_soname_not_regex()) { has_regexp = true; - if (regexec(regexp.get(), soname.c_str(), 0, NULL, 0) == 0) + if (regex::match(regexp, soname)) return false; } @@ -195,16 +195,14 @@ public: if (regex::regex_t_sptr regexp = get_file_name_regex()) { has_regexp = true; - if (regexec(regexp.get(), binary_name.c_str(), - 0, NULL, 0) != 0) + if (!regex::match(regexp, binary_name)) return false; } if (regex::regex_t_sptr regexp = get_file_name_not_regex()) { has_regexp = true; - if (regexec(regexp.get(), binary_name.c_str(), - 0, NULL, 0) == 0) + if (regex::match(regexp, binary_name)) return false; } diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc index b600c88c..afedf6c7 100644 --- a/src/abg-suppression.cc +++ b/src/abg-suppression.cc @@ -994,18 +994,14 @@ suppression_matches_type_name(const type_suppression& s, if (const regex_t_sptr& type_name_regex = s.priv_->get_type_name_regex()) { - if (regexec(type_name_regex.get(), - type_name.c_str(), - 0, NULL, 0) != 0) + if (!regex::match(type_name_regex, type_name)) return false; } if (const regex_t_sptr type_name_not_regex = s.priv_->get_type_name_not_regex()) { - if (regexec(type_name_not_regex.get(), - type_name.c_str(), - 0, NULL, 0) == 0) + if (regex::match(type_name_not_regex, type_name)) return false; } } @@ -1052,7 +1048,7 @@ suppression_matches_type_location(const type_suppression& s, loc.expand(loc_path, loc_line, loc_column); if (regex_t_sptr regexp = s.priv_->get_source_location_to_keep_regex()) - if (regexec(regexp.get(), loc_path.c_str(), 0, NULL, 0) == 0) + if (regex::match(regexp, loc_path)) return false; tools_utils::base_name(loc_path, loc_path_base); @@ -2489,9 +2485,7 @@ function_suppression::suppresses_function(const function_decl* fn, const regex_t_sptr name_regex = priv_->get_name_regex(); if (name_regex) { - if (regexec(name_regex.get(), - fname.c_str(), - 0, NULL, 0) != 0) + if (!regex::match(name_regex, fname)) return false; if (get_allow_other_aliases() @@ -2512,9 +2506,7 @@ function_suppression::suppresses_function(const function_decl* fn, for (elf_symbol_sptr a = sym->get_next_alias(); a && !a->is_main_symbol(); a = a->get_next_alias()) - if (regexec(name_regex.get(), - a->get_name().c_str(), - 0, NULL, 0) != 0) + if (!regex::match(name_regex, a->get_name())) return false; } } @@ -2524,9 +2516,7 @@ function_suppression::suppresses_function(const function_decl* fn, const regex_t_sptr name_not_regex = priv_->get_name_not_regex(); if (name_not_regex) { - if (regexec(name_not_regex.get(), - fname.c_str(), - 0, NULL, 0) == 0) + if (regex::match(name_not_regex, fname)) return false; if (get_allow_other_aliases() @@ -2547,9 +2537,7 @@ function_suppression::suppresses_function(const function_decl* fn, for (elf_symbol_sptr a = sym->get_next_alias(); a && !a->is_main_symbol(); a = a->get_next_alias()) - if (regexec(name_regex.get(), - a->get_name().c_str(), - 0, NULL, 0) == 0) + if (regex::match(name_regex, a->get_name())) return false; } } @@ -2573,9 +2561,7 @@ function_suppression::suppresses_function(const function_decl* fn, { const regex_t_sptr return_type_regex = priv_->get_return_type_regex(); if (return_type_regex - && (regexec(return_type_regex.get(), - fn_return_type_name.c_str(), - 0, NULL, 0) != 0)) + && !regex::match(return_type_regex, fn_return_type_name)) return false; } @@ -2612,18 +2598,13 @@ function_suppression::suppresses_function(const function_decl* fn, else if (sym) { const regex_t_sptr symbol_name_regex = priv_->get_symbol_name_regex(); - if (symbol_name_regex - && (regexec(symbol_name_regex.get(), - fn_sym_name.c_str(), - 0, NULL, 0) != 0)) + if (symbol_name_regex && !regex::match(symbol_name_regex, fn_sym_name)) return false; const regex_t_sptr symbol_name_not_regex = priv_->get_symbol_name_not_regex(); if (symbol_name_not_regex - && (regexec(symbol_name_not_regex.get(), - fn_sym_name.c_str(), - 0, NULL, 0) == 0)) + && regex::match(symbol_name_not_regex, fn_sym_name)) return false; if (get_allow_other_aliases()) @@ -2638,15 +2619,11 @@ function_suppression::suppresses_function(const function_decl* fn, a = a->get_next_alias()) { if (symbol_name_regex - && (regexec(symbol_name_regex.get(), - a->get_name().c_str(), - 0, NULL, 0) != 0)) + && !regex::match(symbol_name_regex, a->get_name())) return false; if (symbol_name_not_regex - && (regexec(symbol_name_not_regex.get(), - a->get_name().c_str(), - 0, NULL, 0) == 0)) + && regex::match(symbol_name_not_regex, a->get_name())) return false; } } @@ -2665,9 +2642,7 @@ function_suppression::suppresses_function(const function_decl* fn, const regex_t_sptr symbol_version_regex = priv_->get_symbol_version_regex(); if (symbol_version_regex - && (regexec(symbol_version_regex.get(), - fn_sym_version.c_str(), - 0, NULL, 0) != 0)) + && !regex::match(symbol_version_regex, fn_sym_version)) return false; } @@ -2708,9 +2683,8 @@ function_suppression::suppresses_function(const function_decl* fn, (*p)->priv_->get_type_name_regex(); if (parm_type_name_regex) { - if ((regexec(parm_type_name_regex.get(), - fn_parm_type_qualified_name.c_str(), - 0, NULL, 0) != 0)) + if (!regex::match(parm_type_name_regex, + fn_parm_type_qualified_name)) return false; } } @@ -2798,10 +2772,7 @@ function_suppression::suppresses_function_symbol(const elf_symbol* sym, else if (!get_symbol_name_regex_str().empty()) { const regex_t_sptr symbol_name_regex = priv_->get_symbol_name_regex(); - if (symbol_name_regex - && (regexec(symbol_name_regex.get(), - sym_name.c_str(), - 0, NULL, 0) != 0)) + if (symbol_name_regex && !regex::match(symbol_name_regex, sym_name)) return false; } else @@ -2818,9 +2789,7 @@ function_suppression::suppresses_function_symbol(const elf_symbol* sym, const regex_t_sptr symbol_version_regex = priv_->get_symbol_version_regex(); if (symbol_version_regex - && (regexec(symbol_version_regex.get(), - sym_version.c_str(), - 0, NULL, 0) != 0)) + && !regex::match(symbol_version_regex, sym_version)) return false; } else @@ -2912,12 +2881,12 @@ suppression_matches_function_name(const suppr::function_suppression& s, { if (regex_t_sptr regexp = s.priv_->get_name_regex()) { - if (regexec(regexp.get(), fn_name.c_str(), 0, NULL, 0) != 0) + if (!regex::match(regexp, fn_name)) return false; } else if (regex_t_sptr regexp = s.priv_->get_name_not_regex()) { - if (regexec(regexp.get(), fn_name.c_str(), 0, NULL, 0) == 0) + if (regex::match(regexp, fn_name)) return false; } else if (s.priv_->name_.empty()) @@ -2948,12 +2917,12 @@ suppression_matches_function_sym_name(const suppr::function_suppression& s, { if (regex_t_sptr regexp = s.priv_->get_symbol_name_regex()) { - if (regexec(regexp.get(), fn_linkage_name.c_str(), 0, NULL, 0) != 0) + if (!regex::match(regexp, fn_linkage_name)) return false; } else if (regex_t_sptr regexp = s.priv_->get_symbol_name_not_regex()) { - if (regexec(regexp.get(), fn_linkage_name.c_str(), 0, NULL, 0) == 0) + if (regex::match(regexp, fn_linkage_name)) return false; } else if (s.priv_->symbol_name_.empty()) @@ -2981,12 +2950,12 @@ suppression_matches_variable_name(const suppr::variable_suppression& s, { if (regex_t_sptr regexp = s.priv_->get_name_regex()) { - if (regexec(regexp.get(), var_name.c_str(), 0, NULL, 0) != 0) + if (!regex::match(regexp, var_name)) return false; } else if (regex_t_sptr regexp = s.priv_->get_name_not_regex()) { - if (regexec(regexp.get(), var_name.c_str(), 0, NULL, 0) == 0) + if (regex::match(regexp, var_name)) return false; } else if (s.priv_->name_.empty()) @@ -3015,13 +2984,13 @@ suppression_matches_variable_sym_name(const suppr::variable_suppression& s, { if (regex_t_sptr regexp = s.priv_->get_symbol_name_regex()) { - if (regexec(regexp.get(), var_linkage_name.c_str(), 0, NULL, 0) != 0) + if (!regex::match(regexp, var_linkage_name)) return false; } else if (regex_t_sptr regexp = s.priv_->get_symbol_name_not_regex()) { - if (regexec(regexp.get(), var_linkage_name.c_str(), 0, NULL, 0) == 0) + if (regex::match(regexp, var_linkage_name)) return false; } else if (s.priv_->symbol_name_.empty()) @@ -3050,7 +3019,7 @@ suppression_matches_type(const suppr::type_suppression& s, { if (regex_t_sptr regexp = s.priv_->get_type_name_regex()) { - if (regexec(regexp.get(), type_name.c_str(), 0, NULL, 0) != 0) + if (!regex::match(regexp, type_name)) return false; } else if (!s.get_type_name().empty()) @@ -3748,15 +3717,11 @@ variable_suppression::suppresses_variable(const var_decl* var, if (get_name().empty()) { const regex_t_sptr name_regex = priv_->get_name_regex(); - if (name_regex - && (regexec(name_regex.get(), var_name.c_str(), - 0, NULL, 0) != 0)) + if (name_regex && !regex::match(name_regex, var_name)) return false; const regex_t_sptr name_not_regex = priv_->get_name_not_regex(); - if (name_not_regex - && (regexec(name_not_regex.get(), var_name.c_str(), - 0, NULL, 0) == 0)) + if (name_not_regex && regex::match(name_not_regex, var_name)) return false; } } @@ -3772,16 +3737,12 @@ variable_suppression::suppresses_variable(const var_decl* var, else { const regex_t_sptr sym_name_regex = priv_->get_symbol_name_regex(); - if (sym_name_regex - && (regexec(sym_name_regex.get(), var_sym_name.c_str(), - 0, NULL, 0) != 0)) + if (sym_name_regex && !regex::match(sym_name_regex, var_sym_name)) return false; const regex_t_sptr sym_name_not_regex = priv_->get_symbol_name_not_regex(); - if (sym_name_not_regex - && (regexec(sym_name_not_regex.get(), var_sym_name.c_str(), - 0, NULL, 0) == 0)) + if (sym_name_not_regex && regex::match(sym_name_not_regex, var_sym_name)) return false; } @@ -3798,9 +3759,7 @@ variable_suppression::suppresses_variable(const var_decl* var, const regex_t_sptr symbol_version_regex = priv_->get_symbol_version_regex(); if (symbol_version_regex - && (regexec(symbol_version_regex.get(), - var_sym_version.c_str(), - 0, NULL, 0) != 0)) + && !regex::match(symbol_version_regex, var_sym_version)) return false; } @@ -3818,9 +3777,7 @@ variable_suppression::suppresses_variable(const var_decl* var, if (get_type_name().empty()) { const regex_t_sptr type_name_regex = priv_->get_type_name_regex(); - if (type_name_regex - && (regexec(type_name_regex.get(), var_type_name.c_str(), - 0, NULL, 0) != 0)) + if (type_name_regex && !regex::match(type_name_regex, var_type_name)) return false; } } @@ -3911,9 +3868,7 @@ variable_suppression::suppresses_variable_symbol(const elf_symbol* sym, else if (!get_symbol_name_regex_str().empty()) { const regex_t_sptr sym_name_regex = priv_->get_symbol_name_regex(); - if (sym_name_regex - && (regexec(sym_name_regex.get(), sym_name.c_str(), - 0, NULL, 0) != 0)) + if (sym_name_regex && !regex::match(sym_name_regex, sym_name)) return false; } else @@ -3931,9 +3886,7 @@ variable_suppression::suppresses_variable_symbol(const elf_symbol* sym, const regex_t_sptr symbol_version_regex = priv_->get_symbol_version_regex(); if (symbol_version_regex - && (regexec(symbol_version_regex.get(), - sym_version.c_str(), - 0, NULL, 0) != 0)) + && !regex::match(symbol_version_regex, sym_version)) return false; } else @@ -4229,14 +4182,14 @@ file_suppression::suppresses_file(const string& file_path) if (regex_t_sptr regexp = suppression_base::priv_->get_file_name_regex()) { has_regexp = true; - if (regexec(regexp.get(), fname.c_str(), 0, NULL, 0) != 0) + if (!regex::match(regexp, fname)) return false; } if (regex_t_sptr regexp = suppression_base::priv_->get_file_name_not_regex()) { has_regexp = true; - if (regexec(regexp.get(), fname.c_str(), 0, NULL, 0) == 0) + if (regex::match(regexp, fname)) return false; } From patchwork Thu Apr 23 18:04:10 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 39150 From: gprocida@google.com (Giuliano Procida) Date: Thu, 23 Apr 2020 19:04:10 +0100 Subject: [PATCH v2 19/21] Warn if user-supplied regexes fail to compile. In-Reply-To: <20200423154441.170531-20-gprocida@google.com> References: <20200423154441.170531-20-gprocida@google.com> Message-ID: <20200423180410.69448-1-gprocida@google.com> There are not many calls to regex::compile in the code base. Two of these pass internally-generated regexes and are followed by assertions that the regexes compile. The remaining cases deal with user-supplied regexes. If these fail to compile they are currently silently ignored. This patch makes sure failures now result in warning messages on stderr, but otherwise does not change program behaviour. * src/abg-corpus-priv.h (corpus::exported_decls_builder::priv::compiled_regex_fns_suppress): Emit a warning message if regex::compile fails. (corpus::exported_decls_builder::priv::compiled_regex_fns_keep): Ditto. (corpus::exported_decls_builder::priv::compiled_regex_vars_suppress): Ditto. (corpus::exported_decls_builder::priv::compiled_regex_vars_keep): Ditto. * src/abg-suppression.cc (maybe_get_string_prop): Ditto. (read_parameter_spec_from_string): Ditto. Signed-off-by: Giuliano Procida --- src/abg-corpus-priv.h | 10 ++++++++++ src/abg-suppression.cc | 4 ++++ 2 files changed, 14 insertions(+) diff --git a/src/abg-corpus-priv.h b/src/abg-corpus-priv.h index 2618e2d0..a06a1a23 100644 --- a/src/abg-corpus-priv.h +++ b/src/abg-corpus-priv.h @@ -29,6 +29,8 @@ #ifndef __ABG_CORPUS_PRIV_H__ #define __ABG_CORPUS_PRIV_H__ +#include + #include "abg-sptr-utils.h" #include "abg-regex.h" #include "abg-internal.h" @@ -126,6 +128,8 @@ public: regex_t_sptr r = regex::compile(*i); if (r) compiled_fns_suppress_regexp_.push_back(r); + else + std::cerr << "warning: bad regex '" << *i << "'\n"; } } return compiled_fns_suppress_regexp_; @@ -148,6 +152,8 @@ public: regex_t_sptr r = regex::compile(*i); if (r) compiled_fns_keep_regexps_.push_back(r); + else + std::cerr << "warning: bad regex '" << *i << "'\n"; } } return compiled_fns_keep_regexps_; @@ -170,6 +176,8 @@ public: regex_t_sptr r = regex::compile(*i); if (r) compiled_vars_suppress_regexp_.push_back(r); + else + std::cerr << "warning: bad regex '" << *i << "'\n"; } } return compiled_vars_suppress_regexp_; @@ -192,6 +200,8 @@ public: regex_t_sptr r = regex::compile(*i); if (r) compiled_vars_keep_regexps_.push_back(r); + else + std::cerr << "warning: bad regex '" << *i << "'\n"; } } return compiled_vars_keep_regexps_; diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc index 3db2b7b1..8632d028 100644 --- a/src/abg-suppression.cc +++ b/src/abg-suppression.cc @@ -1644,6 +1644,8 @@ maybe_get_regex_prop(const ini::config::section& section, if (!maybe_get_string_prop(section, name, str)) return false; regex = regex::compile(str); + if (!regex) + std::cerr << "warning: bad regex '" << str << "'\n"; return true; } @@ -3161,6 +3163,8 @@ read_parameter_spec_from_string(const string& str) if (is_regex) { type_name_regex = regex::compile(type_name); + if (!type_name_regex) + std::cerr << "warning: bad regex '" << type_name << "'\n"; type_name.clear(); } function_suppression::parameter_spec* p =