From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-il1-x143.google.com (mail-il1-x143.google.com [IPv6:2607:f8b0:4864:20::143]) by sourceware.org (Postfix) with ESMTPS id 031E3385E825 for ; Mon, 20 Apr 2020 15:33:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 031E3385E825 Received: by mail-il1-x143.google.com with SMTP id w6so5845861ilg.1 for ; Mon, 20 Apr 2020 08:33:29 -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=hSg9Y2TWN61VEgq2QOUNtoVJ+oRgA5HQqoXT4sVwBYA=; b=KTWBf54yDMVieiVdTvCpjBRrIlg5yzYHB9X2KhvVF4mMn2YKxFiCL6DyOEsfRQQxir fHgsuUSH3UIspGaZ8GANHUoz/38iAZyJn4hLF2gbvNeXgIl7NMCvEpI4n4gCTu1TQ0YU /bO/dqTZMUxZ0uxdwa0R47wPT7z7zU0v38i3FlEdeFCFIbZd5YOabDL0xoc3LjffZvax SMVqMTAkbAFk42siH3kHNqw3aOVCOeECaeQNJDPsK8YzwNgvqmShTZoUqHjtKgZ2HMav 11LtGHalcJFjfFqoo29y+Yi0jVoxJnsxsXnWkL7CKiYY//68mfB3rX5dNo1fCQf6ua61 b+Xg== X-Gm-Message-State: AGi0PubrGoJQwZ/bKSGPmFHbYbKCMNnGJGg9/jr6WqzKcQggMt3cuAuf lf7z723yvY2xfI5spM3NZoFwahIhLMZB4JQh6dbEFg== X-Google-Smtp-Source: APiQypKIrcSpXIh2P/2UdxhdGXc3FNLEIogCqMxdrQ55NleB1bP6EV+dYuMOtrOTJxb2KjP7O9hqR17QdnEWFvnmBJY= X-Received: by 2002:a92:24f:: with SMTP id 76mr16697414ilc.178.1587396809044; Mon, 20 Apr 2020 08:33:29 -0700 (PDT) MIME-Version: 1.0 References: <20200420110846.218792-1-maennich@google.com> <20200420110846.218792-4-maennich@google.com> In-Reply-To: <20200420110846.218792-4-maennich@google.com> From: Giuliano Procida Date: Mon, 20 Apr 2020 16:33:12 +0100 Message-ID: Subject: Re: [PATCH 3/8] abg-elf-helpers: move some versioning helpers from abg-dwarf-reader 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=-39.0 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, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, 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: Libabigail mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 20 Apr 2020 15:33:32 -0000 Looks good. On Mon, 20 Apr 2020 at 12:09, Matthias Maennich wrote: > > Move some definitions from abg-dwarf-reader to abg-elf-helpers that are > strictly only related to ELF. > > * abg-dwarf-reader.cc(get_symbol_versionning_sections): Move function out. > (get_version_definition_for_versym): Likewise. > (get_version_needed_for_versym): Likewise. > (get_version_for_symbol): Likewise. > * abg-elf-helpers.cc(get_symbol_versionning_sections): Move function in. > (get_version_definition_for_versym): Likewise. > (get_version_needed_for_versym): Likewise. > (get_version_for_symbol): Likewise. > * abg-elf-helpers.cc(get_symbol_versionning_sections): Add declaration. > (get_version_definition_for_versym): Likewise. > (get_version_needed_for_versym): Likewise. > (get_version_for_symbol): Likewise. Reviewed-by: Giuliano Procida > Signed-off-by: Matthias Maennich > --- > src/abg-dwarf-reader.cc | 256 ---------------------------------------- > src/abg-elf-helpers.cc | 251 +++++++++++++++++++++++++++++++++++++++ > src/abg-elf-helpers.h | 28 +++++ > 3 files changed, 279 insertions(+), 256 deletions(-) > > diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc > index 6ec7e4b6e968..303c1f8df4c2 100644 > --- a/src/abg-dwarf-reader.cc > +++ b/src/abg-dwarf-reader.cc > @@ -294,12 +294,6 @@ static void > add_symbol_to_map(const elf_symbol_sptr& sym, > string_elf_symbols_map_type& map); > > -static bool > -get_symbol_versionning_sections(Elf* elf_handle, > - Elf_Scn*& versym_section, > - Elf_Scn*& verdef_section, > - Elf_Scn*& verneed_section); > - > static bool > get_parent_die(const read_context& ctxt, > const Dwarf_Die* die, > @@ -867,256 +861,6 @@ compare_symbol_name(const string& symbol_name, > return symbol_name == name; > } > > -/// Return the SHT_GNU_versym, SHT_GNU_verdef and SHT_GNU_verneed > -/// sections that are involved in symbol versionning. > -/// > -/// @param elf_handle the elf handle to use. > -/// > -/// @param versym_section the SHT_GNU_versym section found. If the > -/// section wasn't found, this is set to nil. > -/// > -/// @param verdef_section the SHT_GNU_verdef section found. If the > -/// section wasn't found, this is set to nil. > -/// > -/// @param verneed_section the SHT_GNU_verneed section found. If the > -/// section wasn't found, this is set to nil. > -/// > -/// @return true iff at least one of the sections where found. > -static bool > -get_symbol_versionning_sections(Elf* elf_handle, > - Elf_Scn*& versym_section, > - Elf_Scn*& verdef_section, > - Elf_Scn*& verneed_section) > -{ > - Elf_Scn* section = NULL; > - GElf_Shdr mem; > - Elf_Scn* versym = NULL, *verdef = NULL, *verneed = NULL; > - > - while ((section = elf_nextscn(elf_handle, section)) != NULL) > - { > - GElf_Shdr* h = gelf_getshdr(section, &mem); > - if (h->sh_type == SHT_GNU_versym) > - versym = section; > - else if (h->sh_type == SHT_GNU_verdef) > - verdef = section; > - else if (h->sh_type == SHT_GNU_verneed) > - verneed = section; > - } > - > - if (versym || verdef || verneed) > - { > - // At least one the versionning sections was found. Return it. > - versym_section = versym; > - verdef_section = verdef; > - verneed_section = verneed; > - return true; > - } > - > - return false; > -} > - > -/// Get the version definition (from the SHT_GNU_verdef section) of a > -/// given symbol represented by a pointer to GElf_Versym. > -/// > -/// @param elf_hande the elf handle to use. > -/// > -/// @param versym the symbol to get the version definition for. > -/// > -/// @param verdef_section the SHT_GNU_verdef section. > -/// > -/// @param version the resulting version definition. This is set iff > -/// the function returns true. > -/// > -/// @return true upon successful completion, false otherwise. > -static bool > -get_version_definition_for_versym(Elf* elf_handle, > - GElf_Versym* versym, > - Elf_Scn* verdef_section, > - elf_symbol::version& version) > -{ > - Elf_Data* verdef_data = elf_getdata(verdef_section, NULL); > - GElf_Verdef verdef_mem; > - GElf_Verdef* verdef = gelf_getverdef(verdef_data, 0, &verdef_mem); > - size_t vd_offset = 0; > - > - for (;; vd_offset += verdef->vd_next) > - { > - for (;verdef != 0;) > - { > - if (verdef->vd_ndx == (*versym & 0x7fff)) > - // Found the version of the symbol. > - break; > - vd_offset += verdef->vd_next; > - verdef = (verdef->vd_next == 0 > - ? 0 > - : gelf_getverdef(verdef_data, vd_offset, &verdef_mem)); > - } > - > - if (verdef != 0) > - { > - GElf_Verdaux verdaux_mem; > - GElf_Verdaux *verdaux = gelf_getverdaux(verdef_data, > - vd_offset + verdef->vd_aux, > - &verdaux_mem); > - GElf_Shdr header_mem; > - GElf_Shdr* verdef_section_header = gelf_getshdr(verdef_section, > - &header_mem); > - size_t verdef_stridx = verdef_section_header->sh_link; > - version.str(elf_strptr(elf_handle, verdef_stridx, verdaux->vda_name)); > - if (*versym & 0x8000) > - version.is_default(false); > - else > - version.is_default(true); > - return true; > - } > - if (!verdef || verdef->vd_next == 0) > - break; > - } > - return false; > -} > - > -/// Get the version needed (from the SHT_GNU_verneed section) to > -/// resolve an undefined symbol represented by a pointer to > -/// GElf_Versym. > -/// > -/// @param elf_hande the elf handle to use. > -/// > -/// @param versym the symbol to get the version definition for. > -/// > -/// @param verneed_section the SHT_GNU_verneed section. > -/// > -/// @param version the resulting version definition. This is set iff > -/// the function returns true. > -/// > -/// @return true upon successful completion, false otherwise. > -static bool > -get_version_needed_for_versym(Elf* elf_handle, > - GElf_Versym* versym, > - Elf_Scn* verneed_section, > - elf_symbol::version& version) > -{ > - if (versym == 0 || elf_handle == 0 || verneed_section == 0) > - return false; > - > - size_t vn_offset = 0; > - Elf_Data* verneed_data = elf_getdata(verneed_section, NULL); > - GElf_Verneed verneed_mem; > - GElf_Verneed* verneed = gelf_getverneed(verneed_data, 0, &verneed_mem); > - > - for (;verneed; vn_offset += verneed->vn_next) > - { > - size_t vna_offset = vn_offset; > - GElf_Vernaux vernaux_mem; > - GElf_Vernaux *vernaux = gelf_getvernaux(verneed_data, > - vn_offset + verneed->vn_aux, > - &vernaux_mem); > - for (;vernaux != 0 && verneed;) > - { > - if (vernaux->vna_other == *versym) > - // Found the version of the symbol. > - break; > - vna_offset += verneed->vn_next; > - verneed = (verneed->vn_next == 0 > - ? 0 > - : gelf_getverneed(verneed_data, vna_offset, &verneed_mem)); > - } > - > - if (verneed != 0 && vernaux != 0 && vernaux->vna_other == *versym) > - { > - GElf_Shdr header_mem; > - GElf_Shdr* verneed_section_header = gelf_getshdr(verneed_section, > - &header_mem); > - size_t verneed_stridx = verneed_section_header->sh_link; > - version.str(elf_strptr(elf_handle, > - verneed_stridx, > - vernaux->vna_name)); > - if (*versym & 0x8000) > - version.is_default(false); > - else > - version.is_default(true); > - return true; > - } > - > - if (!verneed || verneed->vn_next == 0) > - break; > - } > - return false; > -} > - > -/// Return the version for a symbol that is at a given index in its > -/// SHT_SYMTAB section. > -/// > -/// @param elf_handle the elf handle to use. > -/// > -/// @param symbol_index the index of the symbol to consider. > -/// > -/// @param get_def_version if this is true, it means that that we want > -/// the version for a defined symbol; in that case, the version is > -/// looked for in a section of type SHT_GNU_verdef. Otherwise, if > -/// this parameter is false, this means that we want the version for > -/// an undefined symbol; in that case, the version is the needed one > -/// for the symbol to be resolved; so the version is looked fo in a > -/// section of type SHT_GNU_verneed. > -/// > -/// @param version the version found for symbol at @p symbol_index. > -/// > -/// @return true iff a version was found for symbol at index @p > -/// symbol_index. > -static bool > -get_version_for_symbol(Elf* elf_handle, > - size_t symbol_index, > - bool get_def_version, > - elf_symbol::version& version) > -{ > - Elf_Scn *versym_section = NULL, > - *verdef_section = NULL, > - *verneed_section = NULL; > - > - if (!get_symbol_versionning_sections(elf_handle, > - versym_section, > - verdef_section, > - verneed_section)) > - return false; > - > - GElf_Versym versym_mem; > - Elf_Data* versym_data = (versym_section) > - ? elf_getdata(versym_section, NULL) > - : NULL; > - GElf_Versym* versym = (versym_data) > - ? gelf_getversym(versym_data, symbol_index, &versym_mem) > - : NULL; > - > - if (versym == 0 || *versym <= 1) > - // I got these value from the code of readelf.c in elfutils. > - // Apparently, if the symbol version entry has these values, the > - // symbol must be discarded. This is not documented in the > - // official specification. > - return false; > - > - if (get_def_version) > - { > - if (*versym == 0x8001) > - // I got this value from the code of readelf.c in elfutils > - // too. It's not really documented in the official > - // specification. > - return false; > - > - if (verdef_section > - && get_version_definition_for_versym(elf_handle, versym, > - verdef_section, version)) > - return true; > - } > - else > - { > - if (verneed_section > - && get_version_needed_for_versym(elf_handle, versym, > - verneed_section, version)) > - return true; > - } > - > - return false; > -} > - > /// Lookup a symbol using the SysV ELF hash table. > /// > /// Note that this function hasn't been tested. So it hasn't been > diff --git a/src/abg-elf-helpers.cc b/src/abg-elf-helpers.cc > index b0113a4efd2c..b7b868a332ec 100644 > --- a/src/abg-elf-helpers.cc > +++ b/src/abg-elf-helpers.cc > @@ -500,6 +500,257 @@ Elf_Scn* > find_data1_section(Elf* elf_handle) > {return find_section(elf_handle, ".data1", SHT_PROGBITS);} > > +/// Return the SHT_GNU_versym, SHT_GNU_verdef and SHT_GNU_verneed > +/// sections that are involved in symbol versionning. > +/// > +/// @param elf_handle the elf handle to use. > +/// > +/// @param versym_section the SHT_GNU_versym section found. If the > +/// section wasn't found, this is set to nil. > +/// > +/// @param verdef_section the SHT_GNU_verdef section found. If the > +/// section wasn't found, this is set to nil. > +/// > +/// @param verneed_section the SHT_GNU_verneed section found. If the > +/// section wasn't found, this is set to nil. > +/// > +/// @return true iff at least one of the sections where found. > +bool > +get_symbol_versionning_sections(Elf* elf_handle, > + Elf_Scn*& versym_section, > + Elf_Scn*& verdef_section, > + Elf_Scn*& verneed_section) > +{ > + Elf_Scn* section = NULL; > + GElf_Shdr mem; > + Elf_Scn* versym = NULL, *verdef = NULL, *verneed = NULL; > + > + while ((section = elf_nextscn(elf_handle, section)) != NULL) > + { > + GElf_Shdr* h = gelf_getshdr(section, &mem); > + if (h->sh_type == SHT_GNU_versym) > + versym = section; > + else if (h->sh_type == SHT_GNU_verdef) > + verdef = section; > + else if (h->sh_type == SHT_GNU_verneed) > + verneed = section; > + } > + > + if (versym || verdef || verneed) > + { > + // At least one the versionning sections was found. Return it. > + versym_section = versym; > + verdef_section = verdef; > + verneed_section = verneed; > + return true; > + } > + > + return false; > +} > + > +/// Get the version definition (from the SHT_GNU_verdef section) of a > +/// given symbol represented by a pointer to GElf_Versym. > +/// > +/// @param elf_hande the elf handle to use. > +/// > +/// @param versym the symbol to get the version definition for. > +/// > +/// @param verdef_section the SHT_GNU_verdef section. > +/// > +/// @param version the resulting version definition. This is set iff > +/// the function returns true. > +/// > +/// @return true upon successful completion, false otherwise. > +bool > +get_version_definition_for_versym(Elf* elf_handle, > + GElf_Versym* versym, > + Elf_Scn* verdef_section, > + elf_symbol::version& version) > +{ > + Elf_Data* verdef_data = elf_getdata(verdef_section, NULL); > + GElf_Verdef verdef_mem; > + GElf_Verdef* verdef = gelf_getverdef(verdef_data, 0, &verdef_mem); > + size_t vd_offset = 0; > + > + for (;; vd_offset += verdef->vd_next) > + { > + for (;verdef != 0;) > + { > + if (verdef->vd_ndx == (*versym & 0x7fff)) > + // Found the version of the symbol. > + break; > + vd_offset += verdef->vd_next; > + verdef = (verdef->vd_next == 0 > + ? 0 > + : gelf_getverdef(verdef_data, vd_offset, &verdef_mem)); > + } > + > + if (verdef != 0) > + { > + GElf_Verdaux verdaux_mem; > + GElf_Verdaux *verdaux = gelf_getverdaux(verdef_data, > + vd_offset + verdef->vd_aux, > + &verdaux_mem); > + GElf_Shdr header_mem; > + GElf_Shdr* verdef_section_header = gelf_getshdr(verdef_section, > + &header_mem); > + size_t verdef_stridx = verdef_section_header->sh_link; > + version.str(elf_strptr(elf_handle, verdef_stridx, verdaux->vda_name)); > + if (*versym & 0x8000) > + version.is_default(false); > + else > + version.is_default(true); > + return true; > + } > + if (!verdef || verdef->vd_next == 0) > + break; > + } > + return false; > +} > + > +/// Get the version needed (from the SHT_GNU_verneed section) to > +/// resolve an undefined symbol represented by a pointer to > +/// GElf_Versym. > +/// > +/// @param elf_hande the elf handle to use. > +/// > +/// @param versym the symbol to get the version definition for. > +/// > +/// @param verneed_section the SHT_GNU_verneed section. > +/// > +/// @param version the resulting version definition. This is set iff > +/// the function returns true. > +/// > +/// @return true upon successful completion, false otherwise. > +bool > +get_version_needed_for_versym(Elf* elf_handle, > + GElf_Versym* versym, > + Elf_Scn* verneed_section, > + elf_symbol::version& version) > +{ > + if (versym == 0 || elf_handle == 0 || verneed_section == 0) > + return false; > + > + size_t vn_offset = 0; > + Elf_Data* verneed_data = elf_getdata(verneed_section, NULL); > + GElf_Verneed verneed_mem; > + GElf_Verneed* verneed = gelf_getverneed(verneed_data, 0, &verneed_mem); > + > + for (;verneed; vn_offset += verneed->vn_next) > + { > + size_t vna_offset = vn_offset; > + GElf_Vernaux vernaux_mem; > + GElf_Vernaux *vernaux = gelf_getvernaux(verneed_data, > + vn_offset + verneed->vn_aux, > + &vernaux_mem); > + for (;vernaux != 0 && verneed;) > + { > + if (vernaux->vna_other == *versym) > + // Found the version of the symbol. > + break; > + vna_offset += verneed->vn_next; > + verneed = (verneed->vn_next == 0 > + ? 0 > + : gelf_getverneed(verneed_data, vna_offset, &verneed_mem)); > + } > + > + if (verneed != 0 && vernaux != 0 && vernaux->vna_other == *versym) > + { > + GElf_Shdr header_mem; > + GElf_Shdr* verneed_section_header = gelf_getshdr(verneed_section, > + &header_mem); > + size_t verneed_stridx = verneed_section_header->sh_link; > + version.str(elf_strptr(elf_handle, > + verneed_stridx, > + vernaux->vna_name)); > + if (*versym & 0x8000) > + version.is_default(false); > + else > + version.is_default(true); > + return true; > + } > + > + if (!verneed || verneed->vn_next == 0) > + break; > + } > + return false; > +} > + > +/// Return the version for a symbol that is at a given index in its > +/// SHT_SYMTAB section. > +/// > +/// @param elf_handle the elf handle to use. > +/// > +/// @param symbol_index the index of the symbol to consider. > +/// > +/// @param get_def_version if this is true, it means that that we want > +/// the version for a defined symbol; in that case, the version is > +/// looked for in a section of type SHT_GNU_verdef. Otherwise, if > +/// this parameter is false, this means that we want the version for > +/// an undefined symbol; in that case, the version is the needed one > +/// for the symbol to be resolved; so the version is looked fo in a > +/// section of type SHT_GNU_verneed. > +/// > +/// @param version the version found for symbol at @p symbol_index. > +/// > +/// @return true iff a version was found for symbol at index @p > +/// symbol_index. > +bool > +get_version_for_symbol(Elf* elf_handle, > + size_t symbol_index, > + bool get_def_version, > + elf_symbol::version& version) > +{ > + Elf_Scn *versym_section = NULL, > + *verdef_section = NULL, > + *verneed_section = NULL; > + > + if (!get_symbol_versionning_sections(elf_handle, > + versym_section, > + verdef_section, > + verneed_section)) > + return false; > + > + GElf_Versym versym_mem; > + Elf_Data* versym_data = (versym_section) > + ? elf_getdata(versym_section, NULL) > + : NULL; > + GElf_Versym* versym = (versym_data) > + ? gelf_getversym(versym_data, symbol_index, &versym_mem) > + : NULL; > + > + if (versym == 0 || *versym <= 1) > + // I got these value from the code of readelf.c in elfutils. > + // Apparently, if the symbol version entry has these values, the > + // symbol must be discarded. This is not documented in the > + // official specification. > + return false; > + > + if (get_def_version) > + { > + if (*versym == 0x8001) > + // I got this value from the code of readelf.c in elfutils > + // too. It's not really documented in the official > + // specification. > + return false; > + > + if (verdef_section > + && get_version_definition_for_versym(elf_handle, versym, > + verdef_section, version)) > + return true; > + } > + else > + { > + if (verneed_section > + && get_version_needed_for_versym(elf_handle, versym, > + verneed_section, version)) > + return true; > + } > + > + return false; > +} > + > + > > } // end namespace elf_helpers > } // end namespace abigail > diff --git a/src/abg-elf-helpers.h b/src/abg-elf-helpers.h > index 58720da0fa9e..7e1c231ccc4e 100644 > --- a/src/abg-elf-helpers.h > +++ b/src/abg-elf-helpers.h > @@ -96,6 +96,34 @@ find_data_section(Elf* elf_handle); > Elf_Scn* > find_data1_section(Elf* elf_handle); > > +bool > +get_symbol_versionning_sections(Elf* elf_handle, > + Elf_Scn*& versym_section, > + Elf_Scn*& verdef_section, > + Elf_Scn*& verneed_section); > + > +// > +// Helpers for symbol versioning > +// > + > +bool > +get_version_definition_for_versym(Elf* elf_handle, > + GElf_Versym* versym, > + Elf_Scn* verdef_section, > + elf_symbol::version& version); > + > +bool > +get_version_needed_for_versym(Elf* elf_handle, > + GElf_Versym* versym, > + Elf_Scn* verneed_section, > + elf_symbol::version& version); > + > +bool > +get_version_for_symbol(Elf* elf_handle, > + size_t symbol_index, > + bool get_def_version, > + elf_symbol::version& version); > + > } // end namespace elf_helpers > } // end namespace abigail > > -- > 2.26.1.301.g55bc3eb7cb9-goog >