From patchwork Wed Aug 14 12:56:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 95820 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6D9513858417 for ; Wed, 14 Aug 2024 12:57:24 +0000 (GMT) X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [217.70.183.201]) by sourceware.org (Postfix) with ESMTPS id 60FC63858C35 for ; Wed, 14 Aug 2024 12:56:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 60FC63858C35 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=seketeli.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 60FC63858C35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=217.70.183.201 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1723640215; cv=none; b=fnLmP48Ys3U09eDEktjz9i4Bcj4TYoWNNZR8aiUgpB/YJJhOySbxxDoRVpH7cDWPxOlF6B+uLqoaUJ6GBiAUSU5OG0ElEyLvP0actunep9Zw/1lLf4CmLZVxfhsKIBu9buH0CewY6OYvTAL4uwBcPVPEUAWXbyH3q/y9e10RH2o= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1723640215; c=relaxed/simple; bh=8CJgUcJMe6WcDbte21JY7IUr1SbGS/vKZ3/2+U3duu8=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=xntLktOWsPPFvTUuK0Ykw27JwaFeB+awG6ywQ/eMUwfd0g9dDB9zpS0AklnNLSUQ9AYGCK4nvgiZ9/N7CQIEAN2zq9Ey8QJ3akl5JznHuu6whMdj6nGWK8T5Lq9DV3PWJrphb605CdHNErGSz6zlDwEAkrtl1ytG6uV+LF48AAE= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail.gandi.net (Postfix) with ESMTPSA id C8C0C1BF203; Wed, 14 Aug 2024 12:56:49 +0000 (UTC) Received: by localhost (Postfix, from userid 1000) id 23D70507A61B; Wed, 14 Aug 2024 14:56:49 +0200 (CEST) From: dodji@redhat.com To: libabigail@sourceware.org Cc: dodji@redhat.com Subject: [PATCH 01/11] Use smart pointers for variables exported from the ABI corpus Date: Wed, 14 Aug 2024 14:56:39 +0200 Message-ID: <20240814125649.47119-1-dodji@redhat.com> X-Mailer: git-send-email 2.43.5 MIME-Version: 1.0 X-GND-Sasl: dodj@seketeli.org X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libabigail-bounces~patchwork=sourceware.org@sourceware.org From: Dodji Seketeli It turned out that for some applications using the library, being able to share the variables returned by libabigail::ir::corpus::lookup_variable is useful. This patch changes the API to use shared pointer for variables exported from the ABI corpus. For the sake of consistency, we should do the same for function decls, but given the churn, I thought I'd do it for variables first and see the impact. If it's OK then I'll do it for function decls too. * include/abg-comparison.h (string_var_ptr_map): Use a var_decl_sptr rather than a var_decl*. * include/abg-corpus.h (corpus::{variable, variables_set}): Likewise. (corpus::lookup_variable): Return a var_decl_sptr not a var_decl*. (corpus::exported_decls_builder::maybe_add_var_to_exported_vars): Take a var_decl_sptr, not a var_decl*. * include/abg-fe-iface.h (fe_iface::add_var_to_exported_or_undefined_decls): Likewise. * include/abg-ir.h (istring_var_decl_ptr_map_type): Use a var_decl_sptr rather than a var_decl*. * src/abg-btf-reader.cc (reader::build_ir_node_from_btf_type): Adjust call to add_var_to_exported_or_undefined_decls. * src/abg-comparison-priv.h (var_comp::operator()()): Add a new overload for var_decl_sptr. (corpus_diff::priv::{deleted_variable_is_suppressed, deleted_variable_is_suppressed}): Take a var_decl_sptr not a var_decl*. (sort_string_var_ptr_map): Take ... * src/abg-comparison.cc (sort_string_var_ptr_map): ... a vector rather than a vector. (corpus_diff::priv::ensure_lookup_tables_populated): Adjust. (variable_is_suppressed): Likewise. (corpus_diff::priv::{deleted_variable_is_suppressed, added_variable_is_suppressed}): Likewise. * src/abg-corpus-priv.h (str_var_ptr_map_type) (istr_var_ptr_map_type): Use a var_decl_sptr rather than a var_decl*. (corpus::exported_decls_builder::priv::{add_var_to_map, keep_wrt_id_of_vars_to_keep, keep_wrt_regex_of_vars_to_suppress, keep_wrt_regex_of_vars_to_keep}): Likewise. (corpus::exported_decls_builder::priv::vars): Likewise. * src/abg-corpus.cc (corpus::exported_decls_builder::maybe_add_var_to_exported_vars): Likewise. (var_comp::operator()()): Add an overload for var_decl_sptr. (corpus::lookup_variable): Return var_decl_sptr rather than var_decl*. * src/abg-ctf-reader.cc (reader::process_ctf_archive): Adjust. * src/abg-dwarf-reader.cc (build_ir_node_from_die): Adjust. * src/abg-fe-iface.cc (fe_iface::add_var_to_exported_or_undefined_decls): Take a var_decl_sptr rather than a var_decl*. * src/abg-reader.cc (build_class_decl, handle_var_decl): Adjust. * src/abg-writer.cc (write_translation_unit): Likewise. * tools/abicompat.cc (var_change::decl, var_change::var_change): Likewise. (compare_expected_against_provided_variables): Likewise. Signed-off-by: Dodji Seketeli --- include/abg-comparison.h | 2 +- include/abg-corpus.h | 8 ++++---- include/abg-fe-iface.h | 2 +- include/abg-ir.h | 2 +- src/abg-btf-reader.cc | 2 +- src/abg-comparison-priv.h | 10 +++++++--- src/abg-comparison.cc | 18 ++++++++---------- src/abg-corpus-priv.h | 20 ++++++++++---------- src/abg-corpus.cc | 10 +++++++--- src/abg-ctf-reader.cc | 2 +- src/abg-dwarf-reader.cc | 4 ++-- src/abg-fe-iface.cc | 2 +- src/abg-reader.cc | 4 ++-- src/abg-writer.cc | 3 +-- tools/abicompat.cc | 6 +++--- 15 files changed, 50 insertions(+), 45 deletions(-) diff --git a/include/abg-comparison.h b/include/abg-comparison.h index 3178c97a..b0527e79 100644 --- a/include/abg-comparison.h +++ b/include/abg-comparison.h @@ -225,7 +225,7 @@ typedef unordered_map string_member_function_sptr_map; /// Convenience typedef for a map which key is a string and which /// value is a point to @ref var_decl. -typedef unordered_map string_var_ptr_map; +typedef unordered_map string_var_ptr_map; /// Convenience typedef for a pair of pointer to @ref var_decl /// representing a @ref var_decl change. The first member of the pair diff --git a/include/abg-corpus.h b/include/abg-corpus.h index cbadbdb2..2cebad2e 100644 --- a/include/abg-corpus.h +++ b/include/abg-corpus.h @@ -34,10 +34,10 @@ public: typedef std::unordered_set functions_set; ///Convenience typedef for std::vector - typedef vector variables; + typedef vector variables; /// Convenience typedef for std::unordered_set. - typedef std::unordered_set variables_set; + typedef std::unordered_set variables_set; class exported_decls_builder; @@ -230,7 +230,7 @@ public: const std::unordered_set* lookup_functions(const char* id) const; - const var_decl* + const var_decl_sptr lookup_variable(const interned_string& id) const; void @@ -370,7 +370,7 @@ public: maybe_add_fn_to_exported_fns(function_decl*); bool - maybe_add_var_to_exported_vars(const var_decl*); + maybe_add_var_to_exported_vars(const var_decl_sptr&); }; //corpus::exported_decls_builder /// Abstraction of a group of corpora. diff --git a/include/abg-fe-iface.h b/include/abg-fe-iface.h index 119ab7ce..65bc0493 100644 --- a/include/abg-fe-iface.h +++ b/include/abg-fe-iface.h @@ -145,7 +145,7 @@ protected: add_fn_to_exported_or_undefined_decls(const function_decl* fn); void - add_var_to_exported_or_undefined_decls(const var_decl* var); + add_var_to_exported_or_undefined_decls(const var_decl_sptr& var); virtual ir::corpus_sptr read_corpus(status& status) = 0; diff --git a/include/abg-ir.h b/include/abg-ir.h index 348cce3c..1e89d06c 100644 --- a/include/abg-ir.h +++ b/include/abg-ir.h @@ -579,7 +579,7 @@ typedef unordered_map istring_function_decl_ptr_map_type; typedef unordered_map istring_var_decl_ptr_map_type; /// This is a type that aggregates maps of all the kinds of types that diff --git a/src/abg-btf-reader.cc b/src/abg-btf-reader.cc index ec13d267..ad610410 100644 --- a/src/abg-btf-reader.cc +++ b/src/abg-btf-reader.cc @@ -661,7 +661,7 @@ public: if (function_decl_sptr fn = is_function_decl(result)) add_fn_to_exported_or_undefined_decls(fn.get()); else if (var_decl_sptr var = is_var_decl(result)) - add_var_to_exported_or_undefined_decls(var.get()); + add_var_to_exported_or_undefined_decls(var); return result; } diff --git a/src/abg-comparison-priv.h b/src/abg-comparison-priv.h index db7946ae..1cd92790 100644 --- a/src/abg-comparison-priv.h +++ b/src/abg-comparison-priv.h @@ -1013,6 +1013,10 @@ struct var_comp bool operator() (const var_decl* l, const var_decl* r) const {return operator()(*l, *r);} + + bool + operator() (const var_decl_sptr& l, const var_decl_sptr& r) const + {return operator()(l.get(), r.get());} };// end struct var_comp /// A functor to compare instances of @ref elf_symbol base on their @@ -1147,10 +1151,10 @@ struct corpus_diff::priv added_function_is_suppressed(const function_decl* fn) const; bool - deleted_variable_is_suppressed(const var_decl* var) const; + deleted_variable_is_suppressed(const var_decl_sptr& var) const; bool - added_variable_is_suppressed(const var_decl* var) const; + added_variable_is_suppressed(const var_decl_sptr& var) const; bool added_unreachable_type_is_suppressed(const type_base *t)const ; @@ -1434,7 +1438,7 @@ sort_string_elf_symbol_map(const string_elf_symbol_map& map, void sort_string_var_ptr_map(const string_var_ptr_map& map, - vector& sorted); + vector& sorted); void sort_string_data_member_diff_sptr_map(const string_var_diff_sptr_map& map, diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc index 65996a84..87a2e4a9 100644 --- a/src/abg-comparison.cc +++ b/src/abg-comparison.cc @@ -324,7 +324,7 @@ sort_string_elf_symbol_map(const string_elf_symbol_map& map, /// @param sorted out parameter; the sorted vector of @ref var_decl. void sort_string_var_ptr_map(const string_var_ptr_map& map, - vector& sorted) + vector& sorted) { for (string_var_ptr_map::const_iterator i = map.begin(); i != map.end(); @@ -9685,7 +9685,7 @@ corpus_diff::priv::ensure_lookup_tables_populated() unsigned i = it->index(); ABG_ASSERT(i < first_->get_variables().size()); - const var_decl* deleted_var = first_->get_variables()[i]; + const var_decl_sptr deleted_var = first_->get_variables()[i]; string n = deleted_var->get_id(); ABG_ASSERT(!n.empty()); ABG_ASSERT(deleted_vars_.find(n) == deleted_vars_.end()); @@ -9702,7 +9702,7 @@ corpus_diff::priv::ensure_lookup_tables_populated() ++iit) { unsigned i = *iit; - const var_decl* added_var = second_->get_variables()[i]; + const var_decl_sptr added_var = second_->get_variables()[i]; string n = added_var->get_id(); ABG_ASSERT(!n.empty()); { @@ -9720,10 +9720,8 @@ corpus_diff::priv::ensure_lookup_tables_populated() { if (*j->second != *added_var) { - var_decl_sptr f(const_cast(j->second), - noop_deleter()); - var_decl_sptr s(const_cast(added_var), - noop_deleter()); + var_decl_sptr f = j->second; + var_decl_sptr s = added_var; changed_vars_map_[n] = compute_diff(f, s, ctxt); } deleted_vars_.erase(j); @@ -10173,7 +10171,7 @@ function_is_suppressed(const function_decl* fn, /// change reports about variable @p fn, if that variable changes in /// the way expressed by @p k. static bool -variable_is_suppressed(const var_decl* var, +variable_is_suppressed(const var_decl_sptr& var, const suppression_sptr suppr, variable_suppression::change_kind k, const diff_context_sptr ctxt) @@ -10425,7 +10423,7 @@ corpus_diff::priv::added_function_is_suppressed(const function_decl* fn) const /// @return true iff the change reports for a give given deleted /// variable has been deleted. bool -corpus_diff::priv::deleted_variable_is_suppressed(const var_decl* var) const +corpus_diff::priv::deleted_variable_is_suppressed(const var_decl_sptr& var) const { if (!var) return false; @@ -10444,7 +10442,7 @@ corpus_diff::priv::deleted_variable_is_suppressed(const var_decl* var) const /// @return true iff the change reports for a given deleted /// variable has been deleted. bool -corpus_diff::priv::added_variable_is_suppressed(const var_decl* var) const +corpus_diff::priv::added_variable_is_suppressed(const var_decl_sptr& var) const { if (!var) return false; diff --git a/src/abg-corpus-priv.h b/src/abg-corpus-priv.h index 485c77dc..100eaee7 100644 --- a/src/abg-corpus-priv.h +++ b/src/abg-corpus-priv.h @@ -55,12 +55,12 @@ typedef unordered_map str_var_ptr_map_type; +typedef unordered_map str_var_ptr_map_type; /// Convenience typedef for a hash map which key is an interned_string /// and which data is an abigail::ir::var_decl*. typedef unordered_map istr_var_ptr_map_type; /// The type of the private data of @ref @@ -449,7 +449,7 @@ public: /// /// @param id the variable to add to the map. void - add_var_to_map(var_decl* var) + add_var_to_map(const var_decl_sptr& var) { if (var) { @@ -475,13 +475,13 @@ public: /// /// @param fn the variable to add to the set of exported variables. void - add_var_to_exported(const var_decl* var) + add_var_to_exported(const var_decl_sptr& var) { const interned_string& id = get_id(*var); if (!var_id_is_in_id_var_map(id)) { - vars_.push_back(const_cast(var)); - add_var_to_map(const_cast(var)); + vars_.push_back(var); + add_var_to_map(var); } } @@ -617,7 +617,7 @@ public: /// /// @return true iff the variable is to be kept. bool - keep_wrt_id_of_vars_to_keep(const var_decl* var) + keep_wrt_id_of_vars_to_keep(const var_decl_sptr& var) { if (!var) return false; @@ -662,7 +662,7 @@ public: /// /// @return true iff the variable is to be kept. bool - keep_wrt_regex_of_vars_to_suppress(const var_decl *var) + keep_wrt_regex_of_vars_to_suppress(const var_decl_sptr var) { if (!var) return false; @@ -691,7 +691,7 @@ public: /// /// @return true iff the variable is to be kept. bool - keep_wrt_regex_of_vars_to_keep(const var_decl *var) + keep_wrt_regex_of_vars_to_keep(const var_decl_sptr& var) { if (!var) return false; @@ -743,7 +743,7 @@ struct corpus::priv translation_units members; string_tu_map_type path_tu_map; vector fns; - vector vars; + vector vars; functions_set undefined_fns; functions sorted_undefined_fns; variables_set undefined_vars; diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc index e9ba92ae..61c9b63d 100644 --- a/src/abg-corpus.cc +++ b/src/abg-corpus.cc @@ -190,7 +190,7 @@ corpus::exported_decls_builder::maybe_add_fn_to_exported_fns(function_decl* fn) /// @return true iff the variable was added to the set of exported /// variables. bool -corpus::exported_decls_builder::maybe_add_var_to_exported_vars(const var_decl* var) +corpus::exported_decls_builder::maybe_add_var_to_exported_vars(const var_decl_sptr& var) { if (!var->get_is_in_public_symbol_table()) return false; @@ -308,8 +308,12 @@ struct var_comp return first_name < second_name; } -}; + bool + operator()(const var_decl_sptr& first, + const var_decl_sptr& second) const + {return operator()(first.get(), second.get());} +}; /// A comparison functor to compare elf_symbols for the purpose of /// sorting. @@ -1411,7 +1415,7 @@ corpus::lookup_functions(const char* id) const /// /// @return the variable with ID @p id that was found or nil if none /// was found. -const var_decl* +const var_decl_sptr corpus::lookup_variable(const interned_string& id) const { exported_decls_builder_sptr b = get_exported_decls_builder(); diff --git a/src/abg-ctf-reader.cc b/src/abg-ctf-reader.cc index 6879c6e0..9f551eab 100644 --- a/src/abg-ctf-reader.cc +++ b/src/abg-ctf-reader.cc @@ -502,7 +502,7 @@ public: add_decl_to_scope(var_declaration, ir_translation_unit->get_global_scope()); var_declaration->set_is_in_public_symbol_table(true); - add_var_to_exported_or_undefined_decls(var_declaration.get()); + add_var_to_exported_or_undefined_decls(var_declaration); } else { diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc index 1da1fd06..70a44674 100644 --- a/src/abg-dwarf-reader.cc +++ b/src/abg-dwarf-reader.cc @@ -16300,7 +16300,7 @@ build_ir_node_from_die(reader& rdr, rdr.var_decls_to_re_add_to_tree().push_back(m); } ABG_ASSERT(m->get_scope()); - rdr.add_var_to_exported_or_undefined_decls(m.get()); + rdr.add_var_to_exported_or_undefined_decls(m); result = m; } } @@ -16318,7 +16318,7 @@ build_ir_node_from_die(reader& rdr, ABG_ASSERT(v); ABG_ASSERT(v->get_scope()); rdr.var_decls_to_re_add_to_tree().push_back(v); - rdr.add_var_to_exported_or_undefined_decls(v.get()); + rdr.add_var_to_exported_or_undefined_decls(v); } } break; diff --git a/src/abg-fe-iface.cc b/src/abg-fe-iface.cc index e44d1126..e13ba68e 100644 --- a/src/abg-fe-iface.cc +++ b/src/abg-fe-iface.cc @@ -334,7 +334,7 @@ fe_iface::add_fn_to_exported_or_undefined_decls(const function_decl* fn) /// /// @param var the internal representation of the ABI of a variable. void -fe_iface::add_var_to_exported_or_undefined_decls(const var_decl* var) +fe_iface::add_var_to_exported_or_undefined_decls(const var_decl_sptr& var) { bool added = false; if (var) diff --git a/src/abg-reader.cc b/src/abg-reader.cc index 2d06302e..2928f3b3 100644 --- a/src/abg-reader.cc +++ b/src/abg-reader.cc @@ -5375,7 +5375,7 @@ build_class_decl(reader& rdr, is_static, offset_in_bits); if (is_static) - rdr.add_var_to_exported_or_undefined_decls(v.get()); + rdr.add_var_to_exported_or_undefined_decls(v); // Now let's record the fact that the data // member uses its type and that the class being // built uses the data member. @@ -6469,7 +6469,7 @@ handle_var_decl(reader& rdr, { decl_base_sptr decl = build_var_decl_if_not_suppressed(rdr, node, add_to_current_scope); - rdr.add_var_to_exported_or_undefined_decls(is_var_decl(decl).get()); + rdr.add_var_to_exported_or_undefined_decls(is_var_decl(decl)); return decl; } diff --git a/src/abg-writer.cc b/src/abg-writer.cc index 935ab08f..c198a532 100644 --- a/src/abg-writer.cc +++ b/src/abg-writer.cc @@ -2548,8 +2548,7 @@ write_translation_unit(write_context& ctxt, if (const abigail::ir::corpus* abi = tu.get_corpus()) for (auto undefined_var : abi->get_sorted_undefined_variables()) { - var_decl_sptr v(const_cast(undefined_var), - noop_deleter()); + var_decl_sptr v = undefined_var; if (v->get_translation_unit() != &tu || ctxt.decl_is_emitted(v)) continue; diff --git a/tools/abicompat.cc b/tools/abicompat.cc index 718ddd9c..05ac2bdb 100644 --- a/tools/abicompat.cc +++ b/tools/abicompat.cc @@ -166,11 +166,11 @@ struct fn_change /// the differences found in the type of that variable. struct var_change { - const var_decl* decl = nullptr; + var_decl_sptr decl = nullptr; diff_sptr diff; bool reverse_direction = false; - var_change(const var_decl* var, + var_change(const var_decl_sptr& var, diff_sptr difference, bool reverse_dir) : decl(var), @@ -542,7 +542,7 @@ compare_expected_against_provided_variables(diff_context_sptr& ctxt, { interned_string var_id = expected_var->get_id(); // ... against the variables exported by the library! - const var_decl* exported_var = + const var_decl_sptr exported_var = reverse_direction ? app_corpus->lookup_variable(var_id) : lib_corpus->lookup_variable(var_id);