From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd44.google.com (mail-io1-xd44.google.com [IPv6:2607:f8b0:4864:20::d44]) by sourceware.org (Postfix) with ESMTPS id 53E0E385B834 for ; Mon, 22 Jun 2020 09:53:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 53E0E385B834 Received: by mail-io1-xd44.google.com with SMTP id y2so966148ioy.3 for ; Mon, 22 Jun 2020 02:53:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=uLu6z9I0Ki2/jm6NPQPienUeyysNUF8f1EOYwpeZ82c=; b=ngkJ9JNO3dd/D4rNmMG45VzmLiZO+pN3oS5Q9AFc9Nbvr9jJzp3/hPx8lj8LegF8FG K1mjI5Pc0Y78hgKbAH3ULri8btV2OOFP00Iu/dDKnV0lHYco6la0+Z5KDZ61pgkvX1u4 7q+IuqI1c5rixuuj3Zq8IeYnzZPnX6+UXGTr8iVSd/NG2t5GQjhgKwlZqDLqaQfHnaLn XKA6BYE0MOBE8+R2lK1GlPjW3c9Q439NFmtHglj5Bi4a1wWkFOsZPIqJBgUWCrfh7VKK OIAwGx2Wrc2JzZ2QTLFDeD0FkViJ8F6lj+iD2RNWrv/YkeOzxR/XUSsA6T+nVYfsg61F INBg== X-Gm-Message-State: AOAM533lz5bjHW2kzrv2YkjjID2loaypTLmtjaLKlCx0uPyLRZ+SjYEi 3Hc+kyG8YEKkTVdjlkWOhtFlehGuB3ZwTOVGWU+ynr31V11LMw== X-Google-Smtp-Source: ABdhPJwujQzwbFZJSpKckp/9MxuJ39kaHummSBe8vPevwJx+khp4RSmaRpzYNZzzHGNS6UIgT8Lhlqs6YSUDj2oCi6w= X-Received: by 2002:a05:6638:bd4:: with SMTP id g20mr16988473jad.92.1592819602408; Mon, 22 Jun 2020 02:53:22 -0700 (PDT) MIME-Version: 1.0 References: <20200619214305.562-1-maennich@google.com> <20200619214305.562-9-maennich@google.com> In-Reply-To: <20200619214305.562-9-maennich@google.com> From: Giuliano Procida Date: Mon, 22 Jun 2020 10:53:05 +0100 Message-ID: Subject: Re: [PATCH v1 08/16] corpus: make get_(undefined_)?_(var|fun)_symbols use the new symtab To: Matthias Maennich Cc: libabigail@sourceware.org, Dodji Seketeli , kernel-team@android.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-29.6 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Jun 2020 09:53:25 -0000 Hi. On Fri, 19 Jun 2020 at 22:43, Matthias Maennich wrote: > > Make the corresponding members an implementation detail of corpus::priv. > They get computed based on the new symtab whenever they are accessed > first with an atomic instantiation. That simplifies the implementation > and homogenizes the access to functions and variables. Sorting does not > need to be done as the symtab already gives a guarantee for that. > > Due to improved alias detection in the new symtab reader, ensure we only > write symbol aliases to ksymtab symbols if having a ksymtab main symbol. > > Test data needed to be adjusted as the new symtab reader is stricter in > regards to symbols listed in ksymtab. I.e. init_module is not an > exported symbol in the ksymtab of a kernel module. > > * src/abg-corpus-priv.h (corpus::priv::sorted_var_symbols): make > private, mutable and optional. > (corpus::sorted_undefined_var_symbols): Likewise. > (corpus::sorted_fun_symbols): Likewise. > (corpus::sorted_undefined_fun_symbols): Likewise. > (corpus::priv::get_sorted_fun_symbols): New method declaration. > (corpus::priv::get_sorted_undefined_fun_symbols): Likewise. > (corpus::priv::get_sorted_var_symbols): Likewise. > (corpus::priv::get_sorted_undefined_var_symbols): Likewise. > * src/abg-corpus.cc > (corpus::elf_symbol_comp_functor): Delete struct. > (corpus::priv::get_sorted_fun_symbols): New method implementation. > (corpus::priv::get_sorted_undefined_fun_symbols): Likewise. > (corpus::priv::get_sorted_var_symbols): Likewise. > (corpus::priv::get_sorted_undefined_var_symbols): Likewise. > (corpus::get_sorted_fun_symbols): Proxy call to corpus::priv. > (corpus::get_sorted_undefined_fun_symbols): Likewise. > (corpus::get_sorted_var_symbols): Likewise. > (corpus::get_sorted_undefined_var_symbols): Likewise. > * src/abg-writer.cc (write_elf_symbol_aliases): When emitting > aliases for a kernel symbol, ensure to only emit exported aliases. > * tests/data/test-read-dwarf/PR25007-sdhci.ko.abi: update test > data. > Reviewed-by: Giuliano Procida > Signed-off-by: Matthias Maennich > --- > src/abg-corpus-priv.h | 21 +- > src/abg-corpus.cc | 242 +++++++----------- > src/abg-writer.cc | 38 ++- > .../data/test-read-dwarf/PR25007-sdhci.ko.abi | 3 - > 4 files changed, 135 insertions(+), 169 deletions(-) > > diff --git a/src/abg-corpus-priv.h b/src/abg-corpus-priv.h > index 5c1e915ad2f3..ad96f260aa89 100644 > --- a/src/abg-corpus-priv.h > +++ b/src/abg-corpus-priv.h > @@ -699,13 +699,9 @@ struct corpus::priv > vector vars; > string_elf_symbols_map_sptr var_symbol_map; > string_elf_symbols_map_sptr undefined_var_symbol_map; > - elf_symbols sorted_var_symbols; > - elf_symbols sorted_undefined_var_symbols; > symtab_reader::symtab_sptr symtab_; > string_elf_symbols_map_sptr fun_symbol_map; > string_elf_symbols_map_sptr undefined_fun_symbol_map; > - elf_symbols sorted_fun_symbols; > - elf_symbols sorted_undefined_fun_symbols; > elf_symbols unrefed_fun_symbols; > elf_symbols unrefed_var_symbols; > // The type maps contained in this data member are populated if the > @@ -727,6 +723,11 @@ struct corpus::priv > private: > priv(); > > + mutable abg_compat::optional sorted_var_symbols; > + mutable abg_compat::optional sorted_undefined_var_symbols; > + mutable abg_compat::optional sorted_fun_symbols; > + mutable abg_compat::optional sorted_undefined_fun_symbols; > + > public: > priv(const string & p, > environment* e) > @@ -746,6 +747,18 @@ public: > const type_maps& > get_types() const; > > + const elf_symbols& > + get_sorted_fun_symbols() const; > + > + const elf_symbols& > + get_sorted_undefined_fun_symbols() const; > + > + const elf_symbols& > + get_sorted_var_symbols() const; > + > + const elf_symbols& > + get_sorted_undefined_var_symbols() const; > + > unordered_set* > get_public_types_pretty_representations(); > > diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc > index 94702047dd82..6d4bedfd57b7 100644 > --- a/src/abg-corpus.cc > +++ b/src/abg-corpus.cc > @@ -453,6 +453,88 @@ const type_maps& > corpus::priv::get_types() const > {return types_;} > > +/// Return a sorted vector of function symbols for this corpus. > +/// > +/// Note that the first time this function is called, the symbols are > +/// sorted and cached. Subsequent invocations of this function return > +/// the cached vector that was built previously. > +/// > +/// @return the sorted list of function symbols. > +const elf_symbols& > +corpus::priv::get_sorted_fun_symbols() const > +{ > + if (!sorted_fun_symbols) > + { > + const symtab_reader::symtab_filter filter = > + symtab_->make_filter().functions(); > + > + sorted_fun_symbols = elf_symbols(symtab_->begin(filter), symtab_->end()); > + } > + return *sorted_fun_symbols; > +} > + > +/// Getter for a sorted vector of the function symbols undefined in > +/// this corpus. > +/// > +/// @return a vector of the function symbols undefined in this corpus, > +/// sorted by name and then version. > +const elf_symbols& > +corpus::priv::get_sorted_undefined_fun_symbols() const > +{ > + if (!sorted_undefined_fun_symbols) > + { > + const symtab_reader::symtab_filter filter = symtab_->make_filter() > + .functions() > + .undefined_symbols() > + .public_symbols(false); > + > + sorted_undefined_fun_symbols = > + elf_symbols(symtab_->begin(filter), symtab_->end()); > + } > + return *sorted_undefined_fun_symbols; > +} > + > +/// Getter for the sorted vector of variable symbols for this corpus. > +/// > +/// Note that the first time this function is called, it computes the > +/// sorted vector, caches the result and returns it. Subsequent > +/// invocations of this function just return the cached vector. > +/// > +/// @return the sorted vector of variable symbols for this corpus. > +const elf_symbols& > +corpus::priv::get_sorted_var_symbols() const > +{ > + if (!sorted_var_symbols) > + { > + const symtab_reader::symtab_filter filter = > + symtab_->make_filter().variables(); > + > + sorted_var_symbols = elf_symbols(symtab_->begin(filter), symtab_->end()); > + } > + return *sorted_var_symbols; > +} > + > +/// Getter for a sorted vector of the variable symbols undefined in > +/// this corpus. > +/// > +/// @return a vector of the variable symbols undefined in this corpus, > +/// sorted by name and then version. > +const elf_symbols& > +corpus::priv::get_sorted_undefined_var_symbols() const > +{ > + if (!sorted_undefined_var_symbols) > + { > + const symtab_reader::symtab_filter filter = symtab_->make_filter() > + .variables() > + .undefined_symbols() > + .public_symbols(false); > + > + sorted_undefined_var_symbols = > + elf_symbols(symtab_->begin(filter), symtab_->end()); > + } > + return *sorted_undefined_var_symbols; > +} > + > /// Getter of the set of pretty representation of types that are > /// reachable from public interfaces (global functions and variables). > /// > @@ -988,104 +1070,21 @@ const string_elf_symbols_map_type& > corpus::get_undefined_fun_symbol_map() const > {return *get_undefined_fun_symbol_map_sptr();} > > -/// Functor to sort instances of @ref elf_symbol. > -struct elf_symbol_comp_functor > -{ > - > - /// Return true if the first argument is less than the second one. > - /// > - /// @param l the first parameter to consider. > - /// > - /// @param r the second parameter to consider. > - /// > - /// @return true if @p l is less than @p r > - bool > - operator()(elf_symbol& l, elf_symbol& r) > - {return (l.get_id_string() < r.get_id_string());} > - > - /// Return true if the first argument is less than the second one. > - /// > - /// @param l the first parameter to consider. > - /// > - /// @param r the second parameter to consider. > - /// > - /// @return true if @p l is less than @p r > - bool > - operator()(elf_symbol* l, elf_symbol* r) > - {return operator()(*l, *r);} > - > - /// Return true if the first argument is less than the second one. > - /// > - /// @param l the first parameter to consider. > - /// > - /// @param r the second parameter to consider. > - /// > - /// @return true if @p l is less than @p r > - bool > - operator()(elf_symbol_sptr l, elf_symbol_sptr r) > - {return operator()(*l, *r);} > -}; // end struct elf_symbol_comp_functor > - > -/// Return a sorted vector of function symbols for this corpus. > -/// > -/// Note that the first time this function is called, the symbols are > -/// sorted and cached. Subsequent invocations of this function return > -/// the cached vector that was built previously. > -/// > -/// @return the sorted list of function symbols. > const elf_symbols& > corpus::get_sorted_fun_symbols() const > -{ > - if (priv_->sorted_fun_symbols.empty() > - && !get_fun_symbol_map().empty()) > - { > - priv_->sorted_fun_symbols.reserve(get_fun_symbol_map().size()); > - for (string_elf_symbols_map_type::const_iterator i = > - get_fun_symbol_map().begin(); > - i != get_fun_symbol_map().end(); > - ++i) > - for (elf_symbols::const_iterator s = i->second.begin(); > - s != i->second.end(); > - ++s) > - priv_->sorted_fun_symbols.push_back(*s); > +{ return priv_->get_sorted_fun_symbols(); } > > - elf_symbol_comp_functor comp; > - std::sort(priv_->sorted_fun_symbols.begin(), > - priv_->sorted_fun_symbols.end(), > - comp); > - } > - return priv_->sorted_fun_symbols; > -} > - > -/// Getter for a sorted vector of the function symbols undefined in > -/// this corpus. > -/// > -/// @return a vector of the function symbols undefined in this corpus, > -/// sorted by name and then version. > const elf_symbols& > corpus::get_sorted_undefined_fun_symbols() const > -{ > - if (priv_->sorted_undefined_fun_symbols.empty() > - && !get_undefined_fun_symbol_map().empty()) > - { > - priv_->sorted_undefined_fun_symbols.reserve > - (get_undefined_fun_symbol_map().size()); > - for (string_elf_symbols_map_type::const_iterator i = > - get_undefined_fun_symbol_map().begin(); > - i != get_undefined_fun_symbol_map().end(); > - ++i) > - for (elf_symbols::const_iterator s = i->second.begin(); > - s != i->second.end(); > - ++s) > - priv_->sorted_undefined_fun_symbols.push_back(*s); > +{ return priv_->get_sorted_undefined_fun_symbols(); } > > - elf_symbol_comp_functor comp; > - std::sort(priv_->sorted_undefined_fun_symbols.begin(), > - priv_->sorted_undefined_fun_symbols.end(), > - comp); > - } > - return priv_->sorted_undefined_fun_symbols; > -} > +const elf_symbols& > +corpus::get_sorted_var_symbols() const > +{ return priv_->get_sorted_var_symbols(); } > + > +const elf_symbols& > +corpus::get_sorted_undefined_var_symbols() const > +{ return priv_->get_sorted_undefined_var_symbols(); } > > /// Getter for the variable symbols map. > /// > @@ -1125,65 +1124,6 @@ const string_elf_symbols_map_type& > corpus::get_undefined_var_symbol_map() const > {return *get_undefined_var_symbol_map_sptr();} > > -/// Getter for the sorted vector of variable symbols for this corpus. > -/// > -/// Note that the first time this function is called, it computes the > -/// sorted vector, caches the result and returns it. Subsequent > -/// invocations of this function just return the cached vector. > -/// > -/// @return the sorted vector of variable symbols for this corpus. > -const elf_symbols& > -corpus::get_sorted_var_symbols() const > -{ > - if (priv_->sorted_var_symbols.empty() > - && !get_var_symbol_map().empty()) > - { > - priv_->sorted_var_symbols.reserve(get_var_symbol_map().size()); > - for (string_elf_symbols_map_type::const_iterator i = > - get_var_symbol_map().begin(); > - i != get_var_symbol_map().end(); > - ++i) > - for (elf_symbols::const_iterator s = i->second.begin(); > - s != i->second.end(); ++s) > - priv_->sorted_var_symbols.push_back(*s); > - > - elf_symbol_comp_functor comp; > - std::sort(priv_->sorted_var_symbols.begin(), > - priv_->sorted_var_symbols.end(), > - comp); > - } > - return priv_->sorted_var_symbols; > -} > - > -/// Getter for a sorted vector of the variable symbols undefined in > -/// this corpus. > -/// > -/// @return a vector of the variable symbols undefined in this corpus, > -/// sorted by name and then version. > -const elf_symbols& > -corpus::get_sorted_undefined_var_symbols() const > -{ > - if (priv_->sorted_undefined_var_symbols.empty() > - && !get_undefined_var_symbol_map().empty()) > - { > - priv_->sorted_undefined_var_symbols.reserve > - (get_undefined_var_symbol_map().size()); > - for (string_elf_symbols_map_type::const_iterator i = > - get_undefined_var_symbol_map().begin(); > - i != get_undefined_var_symbol_map().end(); > - ++i) > - for (elf_symbols::const_iterator s = i->second.begin(); > - s != i->second.end(); ++s) > - priv_->sorted_undefined_var_symbols.push_back(*s); > - > - elf_symbol_comp_functor comp; > - std::sort(priv_->sorted_undefined_var_symbols.begin(), > - priv_->sorted_undefined_var_symbols.end(), > - comp); > - } > - return priv_->sorted_undefined_var_symbols; > -} > - > /// Look in the function symbols map for a symbol with a given name. > /// > /// @param n the name of the symbol to look for. > diff --git a/src/abg-writer.cc b/src/abg-writer.cc > index ce0bae2d5cfd..c5be11b26072 100644 > --- a/src/abg-writer.cc > +++ b/src/abg-writer.cc > @@ -1693,26 +1693,42 @@ write_elf_symbol_visibility(elf_symbol::visibility v, ostream& o) > /// > /// @return true upon successful completion. > static bool > -write_elf_symbol_aliases(const elf_symbol& sym, ostream& o) > +write_elf_symbol_aliases(const elf_symbol& sym, ostream& out) > { > if (!sym.is_main_symbol() || !sym.has_aliases()) > return false; > > - bool emitted = false; > - o << " alias='"; > - for (elf_symbol_sptr s = sym.get_next_alias(); > - !s->is_main_symbol(); > + > + std::vector aliases; > + for (elf_symbol_sptr s = sym.get_next_alias(); s && !s->is_main_symbol(); > s = s->get_next_alias()) > { > - if (s->get_next_alias()->is_main_symbol()) > - o << s->get_id_string() << "'"; > - else > - o << s->get_id_string() << ","; > + if (s->is_suppressed()) > + continue; > > - emitted = true; > + if (sym.is_in_ksymtab() != s->is_in_ksymtab()) > + continue; > + > + aliases.push_back(s->get_id_string()); > } > > - return emitted; > + if (!aliases.empty()) > + { > + out << " alias='"; > + std::string separator; > + for (std::vector::const_iterator it = aliases.begin(), > + end = aliases.end(); > + it != end; ++it) > + { if (it != aliases.begin()) // you can put begin in a variable as well, if you like, but it's all compiled away anyway out << ','; > + out << separator << *it; > + separator = ","; > + } > + > + out << "'"; > + return true; > + } > + > + return false; > } > > /// Write an XML attribute for the reference to a symbol for the > diff --git a/tests/data/test-read-dwarf/PR25007-sdhci.ko.abi b/tests/data/test-read-dwarf/PR25007-sdhci.ko.abi > index 755ea6dc433e..d5af7183095f 100644 > --- a/tests/data/test-read-dwarf/PR25007-sdhci.ko.abi > +++ b/tests/data/test-read-dwarf/PR25007-sdhci.ko.abi > @@ -2,8 +2,6 @@ > > > > - > - > > > > @@ -40,7 +38,6 @@ > > > > - > > > > -- > 2.27.0.111.gc72c7da667-goog >