From patchwork Thu Apr 23 15:44:35 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 39142 From: gprocida@google.com (Giuliano Procida) Date: Thu, 23 Apr 2020 16:44:35 +0100 Subject: [PATCH 15/21] abg-suppression.cc: More consistent regex matching. In-Reply-To: <20200423154441.170531-1-gprocida@google.com> References: <20200423154441.170531-1-gprocida@google.com> Message-ID: <20200423154441.170531-16-gprocida@google.com> Following previous patches, various minor inconsistencies in the suppression regex matching code were more readily apparent. - regex_t_sptr values are fetched both by reference and by value (which has a reference count mutation cost) - there's a mixture of code styles for testing regex presence - regex = ...; if (regex && ...) - if (regex = ...) if (...) the latter has the advantage that the variable has a smaller scope and can be given a shorter name - in some cases there are redundant tests (always true due to previous conditional logic) This patch ensures all shared pointers to compiled regexes are fetched by const reference and that the code uses the nested if style of presence checking where possible. It simplifies some logic where there's redundancy. There are no behavioural changes. There may be performance improvements. * src/abg-suppression.cc (suppression_matches_type_name): Get regexes by const reference. (suppression_matches_type_location): Get regexes by const reference. (function_suppression::suppresses_function): Get regexes by const reference; use nested if checks; simplify logic around symbol version checks. (function_suppression::suppresses_function_symbol): Get regexes by const reference; use nested if checks; remove redundant regex presence checks. (suppression_matches_function_name): Get regexes by const reference. (suppression_matches_function_sym_name): Get regexes by const reference. (suppression_matches_variable_name): Get regexes by const reference. (suppression_matches_variable_sym_name): Get regexes by const reference. (suppression_matches_type): Get regexes by const reference. (variable_suppression::suppresses_variable): Get regexes by const reference; use nested if checks; remove redundant type_name empty check. (variable_suppression::suppresses_variable_symbol): Get regexes by const reference; use nested if checks. Signed-off-by: Giuliano Procida --- src/abg-suppression.cc | 167 +++++++++++++++++++---------------------- 1 file changed, 76 insertions(+), 91 deletions(-) diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc index 33bccbb6..fa382e02 100644 --- a/src/abg-suppression.cc +++ b/src/abg-suppression.cc @@ -1057,15 +1057,14 @@ suppression_matches_type_name(const type_suppression& s, // If the qualified name of the considered type doesn't match // the regular expression of the type name, then this // suppression doesn't apply. - if (const regex_t_sptr& type_name_regex = - s.get_type_name_regex()) + if (const regex_t_sptr& type_name_regex = s.get_type_name_regex()) { if (!regex::match(type_name_regex, type_name)) return false; } - if (const regex_t_sptr type_name_not_regex = - s.get_type_name_not_regex()) + if (const regex_t_sptr& type_name_not_regex = + s.get_type_name_not_regex()) { if (regex::match(type_name_not_regex, type_name)) return false; @@ -1113,7 +1112,7 @@ suppression_matches_type_location(const type_suppression& s, unsigned loc_line = 0, loc_column = 0; loc.expand(loc_path, loc_line, loc_column); - if (regex_t_sptr regexp = s.get_source_location_to_keep_regex()) + if (const regex_t_sptr& regexp = s.get_source_location_to_keep_regex()) if (regex::match(regexp, loc_path)) return false; @@ -2553,7 +2552,7 @@ function_suppression::suppresses_function(const function_decl* fn, } // check if the "name_regexp" property matches. - const regex_t_sptr name_regex = get_name_regex(); + const regex_t_sptr& name_regex = get_name_regex(); if (name_regex) { if (!regex::match(name_regex, fname)) @@ -2584,7 +2583,7 @@ function_suppression::suppresses_function(const function_decl* fn, } // check if the "name_not_regexp" property matches. - const regex_t_sptr name_not_regex = get_name_not_regex(); + const regex_t_sptr& name_not_regex = get_name_not_regex(); if (name_not_regex) { if (regex::match(name_not_regex, fname)) @@ -2628,11 +2627,9 @@ function_suppression::suppresses_function(const function_decl* fn, if (fn_return_type_name != get_return_type_name()) return false; } - else + else if (const regex_t_sptr& regex = get_return_type_regex()) { - const regex_t_sptr return_type_regex = get_return_type_regex(); - if (return_type_regex - && !regex::match(return_type_regex, fn_return_type_name)) + if (!regex::match(regex, fn_return_type_name)) return false; } @@ -2668,14 +2665,15 @@ function_suppression::suppresses_function(const function_decl* fn, } else if (sym) { - const regex_t_sptr symbol_name_regex = get_symbol_name_regex(); - if (symbol_name_regex && !regex::match(symbol_name_regex, fn_sym_name)) - return false; + const regex_t_sptr& symbol_name_regex = get_symbol_name_regex(); + const regex_t_sptr& symbol_name_not_regex = get_symbol_name_not_regex(); - const regex_t_sptr symbol_name_not_regex = get_symbol_name_not_regex(); - if (symbol_name_not_regex - && regex::match(symbol_name_not_regex, fn_sym_name)) - return false; + if (symbol_name_regex) + if (!regex::match(symbol_name_regex, fn_sym_name)) + return false; + if (symbol_name_not_regex) + if (regex::match(symbol_name_not_regex, fn_sym_name)) + return false; if (get_allow_other_aliases()) { @@ -2688,13 +2686,13 @@ function_suppression::suppresses_function(const function_decl* fn, a && !a->is_main_symbol(); a = a->get_next_alias()) { - if (symbol_name_regex - && !regex::match(symbol_name_regex, a->get_name())) - return false; - - if (symbol_name_not_regex - && regex::match(symbol_name_not_regex, a->get_name())) - return false; + const std::string& alias_name = a->get_name(); + if (symbol_name_regex) + if (!regex::match(symbol_name_regex, alias_name)) + return false; + if (symbol_name_not_regex) + if (regex::match(symbol_name_not_regex, alias_name)) + return false; } } } @@ -2702,17 +2700,19 @@ function_suppression::suppresses_function(const function_decl* fn, // Check if the "symbol_version" and "symbol_version_regexp" // properties match. - if (sym && !get_symbol_version().empty()) - { - if (fn_sym_version != get_symbol_version()) - return false; - } - else if (sym) + if (sym) { - const regex_t_sptr symbol_version_regex = get_symbol_version_regex(); - if (symbol_version_regex - && !regex::match(symbol_version_regex, fn_sym_version)) - return false; + if (!get_symbol_version().empty()) + { + if (fn_sym_version != get_symbol_version()) + return false; + } + else + { + if (const regex_t_sptr& regex = get_symbol_version_regex()) + if (!regex::match(regex, fn_sym_version)) + return false; + } } // Check the 'parameter' property. @@ -2746,16 +2746,11 @@ function_suppression::suppresses_function(const function_decl* fn, if (tn != fn_parm_type_qualified_name) return false; } - else + else if (const regex_t_sptr& regex = + (*p)->get_parameter_type_name_regex()) { - const regex_t_sptr parm_type_name_regex = - (*p)->get_parameter_type_name_regex(); - if (parm_type_name_regex) - { - if (!regex::match(parm_type_name_regex, - fn_parm_type_qualified_name)) - return false; - } + if (!regex::match(regex, fn_parm_type_qualified_name)) + return false; } } } @@ -2838,10 +2833,9 @@ function_suppression::suppresses_function_symbol(const elf_symbol* sym, if (sym_name != get_symbol_name()) return false; } - else if (get_symbol_name_regex()) + else if (const regex_t_sptr& regex = get_symbol_name_regex()) { - const regex_t_sptr symbol_name_regex = get_symbol_name_regex(); - if (symbol_name_regex && !regex::match(symbol_name_regex, sym_name)) + if (!regex::match(regex, sym_name)) return false; } else @@ -2853,11 +2847,9 @@ function_suppression::suppresses_function_symbol(const elf_symbol* sym, if (sym_version != get_symbol_version()) return false; } - else if (get_symbol_version_regex()) + else if (const regex_t_sptr& regex = get_symbol_version_regex()) { - const regex_t_sptr symbol_version_regex = get_symbol_version_regex(); - if (symbol_version_regex - && !regex::match(symbol_version_regex, sym_version)) + if (!regex::match(regex, sym_version)) return false; } else @@ -2947,12 +2939,12 @@ bool suppression_matches_function_name(const suppr::function_suppression& s, const string& fn_name) { - if (regex_t_sptr regexp = s.get_name_regex()) + if (const regex_t_sptr& regexp = s.get_name_regex()) { if (!regex::match(regexp, fn_name)) return false; } - else if (regex_t_sptr regexp = s.get_name_not_regex()) + else if (const regex_t_sptr& regexp = s.get_name_not_regex()) { if (regex::match(regexp, fn_name)) return false; @@ -2983,12 +2975,12 @@ bool suppression_matches_function_sym_name(const suppr::function_suppression& s, const string& fn_linkage_name) { - if (regex_t_sptr regexp = s.get_symbol_name_regex()) + if (const regex_t_sptr& regexp = s.get_symbol_name_regex()) { if (!regex::match(regexp, fn_linkage_name)) return false; } - else if (regex_t_sptr regexp = s.get_symbol_name_not_regex()) + else if (const regex_t_sptr& regexp = s.get_symbol_name_not_regex()) { if (regex::match(regexp, fn_linkage_name)) return false; @@ -3016,12 +3008,12 @@ bool suppression_matches_variable_name(const suppr::variable_suppression& s, const string& var_name) { - if (regex_t_sptr regexp = s.get_name_regex()) + if (const regex_t_sptr& regexp = s.get_name_regex()) { if (!regex::match(regexp, var_name)) return false; } - else if (regex_t_sptr regexp = s.get_name_not_regex()) + else if (const regex_t_sptr& regexp = s.get_name_not_regex()) { if (regex::match(regexp, var_name)) return false; @@ -3050,12 +3042,12 @@ bool suppression_matches_variable_sym_name(const suppr::variable_suppression& s, const string& var_linkage_name) { - if (regex_t_sptr regexp = s.get_symbol_name_regex()) + if (const regex_t_sptr& regexp = s.get_symbol_name_regex()) { if (!regex::match(regexp, var_linkage_name)) return false; } - else if (regex_t_sptr regexp = s.get_symbol_name_not_regex()) + else if (const regex_t_sptr& regexp = s.get_symbol_name_not_regex()) { if (regex::match(regexp, var_linkage_name)) return false; @@ -3084,7 +3076,7 @@ bool suppression_matches_type(const suppr::type_suppression& s, const string& type_name) { - if (regex_t_sptr regexp = s.get_type_name_regex()) + if (const regex_t_sptr& regexp = s.get_type_name_regex()) { if (!regex::match(regexp, type_name)) return false; @@ -3793,13 +3785,13 @@ variable_suppression::suppresses_variable(const var_decl* var, // "name_regex" and "name_not_regex" properties match if (get_name().empty()) { - const regex_t_sptr name_regex = get_name_regex(); - if (name_regex && !regex::match(name_regex, var_name)) - return false; + if (const regex_t_sptr& regex = get_name_regex()) + if (!regex::match(regex, var_name)) + return false; - const regex_t_sptr name_not_regex = get_name_not_regex(); - if (name_not_regex && regex::match(name_not_regex, var_name)) - return false; + if (const regex_t_sptr& regex = get_name_not_regex()) + if (regex::match(regex, var_name)) + return false; } } @@ -3813,13 +3805,13 @@ variable_suppression::suppresses_variable(const var_decl* var, } else { - const regex_t_sptr sym_name_regex = get_symbol_name_regex(); - if (sym_name_regex && !regex::match(sym_name_regex, var_sym_name)) - return false; + if (const regex_t_sptr& regex = get_symbol_name_regex()) + if (!regex::match(regex, var_sym_name)) + return false; - const regex_t_sptr sym_name_not_regex = get_symbol_name_not_regex(); - if (sym_name_not_regex && regex::match(sym_name_not_regex, var_sym_name)) - return false; + if (const regex_t_sptr& regex = get_symbol_name_not_regex()) + if (regex::match(regex, var_sym_name)) + return false; } // Check for symbol_version and symbol_version_regexp property match @@ -3832,10 +3824,9 @@ variable_suppression::suppresses_variable(const var_decl* var, } else { - const regex_t_sptr symbol_version_regex = get_symbol_version_regex(); - if (symbol_version_regex - && !regex::match(symbol_version_regex, var_sym_version)) - return false; + if (const regex_t_sptr& regex = get_symbol_version_regex()) + if (!regex::match(regex, var_sym_version)) + return false; } // Check for the "type_name" and type_name_regex properties match. @@ -3849,12 +3840,9 @@ variable_suppression::suppresses_variable(const var_decl* var, } else { - if (get_type_name().empty()) - { - const regex_t_sptr type_name_regex = get_type_name_regex(); - if (type_name_regex && !regex::match(type_name_regex, var_type_name)) - return false; - } + if (const regex_t_sptr& regex = get_type_name_regex()) + if (!regex::match(regex, var_type_name)) + return false; } return true; @@ -3940,10 +3928,9 @@ variable_suppression::suppresses_variable_symbol(const elf_symbol* sym, if (get_symbol_name() != sym_name) return false; } - else if (get_symbol_name_regex()) + else if (const regex_t_sptr& regex = get_symbol_name_regex()) { - const regex_t_sptr sym_name_regex = get_symbol_name_regex(); - if (sym_name_regex && !regex::match(sym_name_regex, sym_name)) + if (!regex::match(regex, sym_name)) return false; } else @@ -3956,11 +3943,9 @@ variable_suppression::suppresses_variable_symbol(const elf_symbol* sym, if (get_symbol_version() != sym_version) return false; } - else if (get_symbol_version_regex()) + else if (const regex_t_sptr& regex = get_symbol_version_regex()) { - const regex_t_sptr symbol_version_regex = get_symbol_version_regex(); - if (symbol_version_regex - && !regex::match(symbol_version_regex, sym_version)) + if (!regex::match(regex, sym_version)) return false; } else @@ -4261,14 +4246,14 @@ file_suppression::suppresses_file(const string& file_path) bool has_regexp = false; - if (regex_t_sptr regexp = get_file_name_regex()) + if (const regex_t_sptr& regexp = get_file_name_regex()) { has_regexp = true; if (!regex::match(regexp, fname)) return false; } - if (regex_t_sptr regexp = get_file_name_not_regex()) + if (const regex_t_sptr& regexp = get_file_name_not_regex()) { has_regexp = true; if (regex::match(regexp, fname))