[PATCH 6/8] abg-dwarf-reader: migrate more ELF helpers to elf-helpers
Giuliano Procida
gprocida@google.com
Tue Apr 21 11:02:55 GMT 2020
So a zero load address is OK. That's fine then.
I didn't understand your comment about memory management. I see no
allocation, free or change of ownership.
Giuliano.
On Tue, 21 Apr 2020 at 07:14, 'Matthias Maennich' via kernel-team
<kernel-team@android.com> wrote:
>
> On Mon, Apr 20, 2020 at 04:24:50PM +0100, Giuliano Procida wrote:
> >Hi.
> >
> >On Mon, 20 Apr 2020 at 12:09, Matthias Maennich <maennich@google.com> wrote:
> >>
> >> This change migrates all ELF helpers related to section lookup to
> >> abg-elf-helpers.{cc,h}. It also homogenizes the interface of those to
> >> always return Elf_Scn* and NULL in case that section can't be found.
> >> Though this smells like a functional change, this latter change is
> >> purely cosmetic.
> >>
> >> * src/abg-dwarf-reader.cc (read_context::find_symbol_table_section):
> >> adjust to new interface of elf_helpers::find_symbol_table_section.
> >> (find_opd_section): use elf_helpers::find_opd_section for lookup.
> >> (find_ksymtab_section): use elf_helpers::find_ksymtab_section.
> >> (find_ksymtab_gpl_section): use elf_helpers::find_ksymtab_gpl_section.
> >> (find_relocation_section): Move out function.
> >> (get_binary_load_address): Move out function.
> >> (find_ksymtab_reloc_section): use elf_helpers::find_relocation_section
> >> (find_ksymtab_gpl_reloc_section): use elf_helpers::find_relocation_section
> >> * src/elf-helpers.cc (find_symbol_table_section): change
> >> interface to match other find_*_section functions.
> >> (find_symbol_table_section_index): Adjust for the new interface
> >> of find_symbol_table_section.
> >> (find_opd_section): New function.
> >> (find_ksymtab_section): New function.
> >> (find_ksymtab_gpl_section): New function.
> >> (find_relocation_section): New function.
> >> (get_binary_load_address): New function.
> >> * src/elf-helpers.h (find_symbol_table_section): Change declaration.
> >> (find_opd_section): New function declation.
> >> (find_ksymtab_section): New function declation.
> >> (find_ksymtab_gpl_section): New function declation.
> >> (find_relocation_section): New function declation.
> >> (get_binary_load_address): New function declation.
> >>
> >> Signed-off-by: Matthias Maennich <maennich@google.com>
> >> ---
> >> src/abg-dwarf-reader.cc | 112 ++++++----------------------------
> >> src/abg-elf-helpers.cc | 129 +++++++++++++++++++++++++++++++++++++---
> >> src/abg-elf-helpers.h | 22 ++++++-
> >> 3 files changed, 157 insertions(+), 106 deletions(-)
> >>
> >> diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
> >> index ec1f9f3fe8f3..56da03a60940 100644
> >> --- a/src/abg-dwarf-reader.cc
> >> +++ b/src/abg-dwarf-reader.cc
> >> @@ -542,53 +542,6 @@ compare_dies(const read_context& ctxt,
> >> bool update_canonical_dies_on_the_fly);
> >>
> >>
> >> -/// Get the address at which a given binary is loaded in memory⋅
> >> -///
> >> -/// @param elf_handle the elf handle for the binary to consider.
> >> -///
> >> -/// @param load_address the address where the binary is loaded. This
> >> -/// is set by the function iff it returns true.
> >> -///
> >> -/// @return true if the function could get the binary load address
> >> -/// and assign @p load_address to it.
> >> -static bool
> >> -get_binary_load_address(Elf *elf_handle,
> >> - GElf_Addr &load_address)
> >> -{
> >> - GElf_Ehdr eh_mem;
> >> - GElf_Ehdr *elf_header = gelf_getehdr(elf_handle, &eh_mem);
> >> - size_t num_segments = elf_header->e_phnum;
> >> - GElf_Phdr *program_header = 0;
> >> - GElf_Addr result;
> >> - bool found_loaded_segment = false;
> >> - GElf_Phdr ph_mem;
> >> -
> >> - for (unsigned i = 0; i < num_segments; ++i)
> >> - {
> >> - program_header = gelf_getphdr(elf_handle, i, &ph_mem);
> >> - if (program_header && program_header->p_type == PT_LOAD)
> >> - {
> >> - if (!found_loaded_segment)
> >> - {
> >> - result = program_header->p_vaddr;
> >> - found_loaded_segment = true;
> >> - }
> >> -
> >> - if (program_header->p_vaddr < result)
> >> - // The resulting load address we want is the lowest
> >> - // load address of all the loaded segments.
> >> - result = program_header->p_vaddr;
> >> - }
> >> - }
> >> -
> >> - if (found_loaded_segment)
> >> - {
> >> - load_address = result;
> >> - return true;
> >> - }
> >> - return false;
> >> -}
> >> -
> >> /// Find the file name of the alternate debug info file.
> >> ///
> >> /// @param elf_module the elf module to consider.
> >> @@ -5282,8 +5235,8 @@ public:
> >> find_symbol_table_section() const
> >> {
> >> if (!symtab_section_)
> >> - dwarf_reader::find_symbol_table_section(elf_handle(),
> >> - const_cast<read_context*>(this)->symtab_section_);
> >> + const_cast<read_context*>(this)->symtab_section_ =
> >> + elf_helpers::find_symbol_table_section(elf_handle());
> >> return symtab_section_;
> >> }
> >>
> >> @@ -5296,8 +5249,8 @@ public:
> >> find_opd_section() const
> >> {
> >> if (!opd_section_)
> >> - const_cast<read_context*>(this)->opd_section_=
> >> - find_section(elf_handle(), ".opd", SHT_PROGBITS);
> >> + const_cast<read_context*>(this)->opd_section_ =
> >> + elf_helpers::find_opd_section(elf_handle());
> >> return opd_section_;
> >> }
> >>
> >> @@ -5310,39 +5263,21 @@ public:
> >> {
> >> if (!ksymtab_section_)
> >> const_cast<read_context*>(this)->ksymtab_section_ =
> >> - find_section(elf_handle(), "__ksymtab", SHT_PROGBITS);
> >> + elf_helpers::find_ksymtab_section(elf_handle());
> >> return ksymtab_section_;
> >> }
> >>
> >> - /// Return the .rel{a,} section corresponding to a given section.
> >> - ///
> >> - /// @param target_section the section to search the relocation section for
> >> + /// Return the __ksymtab_gpl section of a linux kernel ELF file
> >> + /// (either a vmlinux binary or a kernel module).
> >> ///
> >> - /// @return the .rel{a,} section if found, null otherwise.
> >> + /// @return the __ksymtab_gpl section if found, nil otherwise.
> >> Elf_Scn*
> >> - find_relocation_section(Elf_Scn* target_section) const
> >> + find_ksymtab_gpl_section() const
> >> {
> >> - if (target_section)
> >> - {
> >> - // the relo section we are searching for has this index as sh_info
> >> - size_t target_index = elf_ndxscn(target_section);
> >> -
> >> - // now iterate over all the sections, look for relocation sections and
> >> - // find the one that points to the section we are searching for
> >> - Elf_Scn* section = 0;
> >> - GElf_Shdr header_mem, *header;
> >> - while ((section = elf_nextscn(elf_handle(), section)) != 0)
> >> - {
> >> - header = gelf_getshdr(section, &header_mem);
> >> - if (header == NULL
> >> - || (header->sh_type != SHT_RELA && header->sh_type != SHT_REL))
> >> - continue;
> >> -
> >> - if (header->sh_info == target_index)
> >> - return section;
> >> - }
> >> - }
> >> - return NULL;
> >> + if (!ksymtab_gpl_section_)
> >> + const_cast<read_context*>(this)->ksymtab_gpl_section_ =
> >> + elf_helpers::find_ksymtab_gpl_section(elf_handle());
> >> + return ksymtab_gpl_section_;
> >> }
> >>
> >> /// Return the .rel{a,}__ksymtab section of a linux kernel ELF file (either
> >> @@ -5354,25 +5289,12 @@ public:
> >> {
> >> if (!ksymtab_reloc_section_)
> >> {
> >> - const_cast<read_context*>(this)->ksymtab_reloc_section_
> >> - = find_relocation_section(find_ksymtab_section());
> >> + const_cast<read_context*>(this)->ksymtab_reloc_section_ =
> >> + find_relocation_section(elf_handle(), find_ksymtab_section());
> >> }
> >> return ksymtab_reloc_section_;
> >> }
> >>
> >> - /// Return the __ksymtab_gpl section of a linux kernel ELF file
> >> - /// (either a vmlinux binary or a kernel module).
> >> - ///
> >> - /// @return the __ksymtab_gpl section if found, nil otherwise.
> >> - Elf_Scn*
> >> - find_ksymtab_gpl_section() const
> >> - {
> >> - if (!ksymtab_gpl_section_)
> >> - const_cast<read_context*>(this)->ksymtab_gpl_section_ =
> >> - find_section(elf_handle(), "__ksymtab_gpl", SHT_PROGBITS);
> >> - return ksymtab_gpl_section_;
> >> - }
> >> -
> >> /// Return the .rel{a,}__ksymtab_gpl section of a linux kernel ELF file
> >> /// (either a vmlinux binary or a kernel module).
> >> ///
> >> @@ -5382,8 +5304,8 @@ public:
> >> {
> >> if (!ksymtab_gpl_reloc_section_)
> >> {
> >> - const_cast<read_context*>(this)->ksymtab_gpl_reloc_section_
> >> - = find_relocation_section(find_ksymtab_gpl_section());
> >> + const_cast<read_context*>(this)->ksymtab_gpl_reloc_section_ =
> >> + find_relocation_section(elf_handle(), find_ksymtab_gpl_section());
> >> }
> >> return ksymtab_gpl_reloc_section_;
> >> }
> >> diff --git a/src/abg-elf-helpers.cc b/src/abg-elf-helpers.cc
> >> index ede191014369..b77440206fb0 100644
> >> --- a/src/abg-elf-helpers.cc
> >> +++ b/src/abg-elf-helpers.cc
> >> @@ -357,9 +357,9 @@ find_section(Elf* elf_handle, const std::string& name, Elf64_Word section_type)
> >> ///
> >> /// @param symtab the symbol table found.
> >> ///
> >> -/// @return true iff the symbol table is found.
> >> -bool
> >> -find_symbol_table_section(Elf* elf_handle, Elf_Scn*& symtab)
> >> +/// @return the symbol table section
> >> +Elf_Scn*
> >> +find_symbol_table_section(Elf* elf_handle)
> >> {
> >> Elf_Scn* section = 0, *dynsym = 0, *sym_tab = 0;
> >> while ((section = elf_nextscn(elf_handle, section)) != 0)
> >> @@ -378,12 +378,11 @@ find_symbol_table_section(Elf* elf_handle, Elf_Scn*& symtab)
> >> GElf_Ehdr* elf_header = gelf_getehdr(elf_handle, &eh_mem);
> >> if (elf_header->e_type == ET_REL
> >> || elf_header->e_type == ET_EXEC)
> >> - symtab = sym_tab ? sym_tab : dynsym;
> >> + return sym_tab ? sym_tab : dynsym;
> >> else
> >> - symtab = dynsym ? dynsym : sym_tab;
> >> - return true;
> >> + return dynsym ? dynsym : sym_tab;
> >> }
> >> - return false;
> >> + return NULL;
> >> }
> >>
> >> /// Find the index (in the section headers table) of the symbol table
> >> @@ -402,8 +401,9 @@ find_symbol_table_section(Elf* elf_handle, Elf_Scn*& symtab)
> >> bool
> >> find_symbol_table_section_index(Elf* elf_handle, size_t& symtab_index)
> >> {
> >> - Elf_Scn* section = 0;
> >> - if (!find_symbol_table_section(elf_handle, section))
> >> + Elf_Scn* section = find_symbol_table_section(elf_handle);
> >> +
> >> + if (!section)
> >> return false;
> >>
> >> symtab_index = elf_ndxscn(section);
> >> @@ -500,6 +500,17 @@ Elf_Scn*
> >> find_data1_section(Elf* elf_handle)
> >> {return find_section(elf_handle, ".data1", SHT_PROGBITS);}
> >>
> >> +/// Return the "Official Procedure descriptors section." This
> >> +/// section is named .opd, and is usually present only on PPC64
> >> +/// ELFv1 binaries.
> >> +///
> >> +/// @param elf_handle the elf handle to consider.
> >> +///
> >> +/// @return the .opd section, if found. Return nil otherwise.
> >> +Elf_Scn*
> >> +find_opd_section(Elf* elf_handle)
> >> +{return find_section(elf_handle, ".opd", SHT_PROGBITS);}
> >> +
> >> /// Return the SHT_GNU_versym, SHT_GNU_verdef and SHT_GNU_verneed
> >> /// sections that are involved in symbol versionning.
> >> ///
> >> @@ -548,6 +559,26 @@ get_symbol_versionning_sections(Elf* elf_handle,
> >> return false;
> >> }
> >>
> >> +/// Return the __ksymtab section of a linux kernel ELF file (either
> >> +/// a vmlinux binary or a kernel module).
> >> +///
> >> +/// @param elf_handle the elf handle to consider.
> >> +///
> >> +/// @return the __ksymtab section if found, nil otherwise.
> >> +Elf_Scn*
> >> +find_ksymtab_section(Elf* elf_handle)
> >> +{return find_section(elf_handle, "__ksymtab", SHT_PROGBITS);}
> >> +
> >> +/// Return the __ksymtab_gpl section of a linux kernel ELF file (either
> >> +/// a vmlinux binary or a kernel module).
> >> +///
> >> +/// @param elf_handle the elf handle to consider.
> >> +///
> >> +/// @return the __ksymtab section if found, nil otherwise.
> >> +Elf_Scn*
> >> +find_ksymtab_gpl_section(Elf* elf_handle)
> >> +{return find_section(elf_handle, "__ksymtab_gpl", SHT_PROGBITS);}
> >> +
> >> /// Find the __ksymtab_strings section of a Linux kernel binary.
> >> ///
> >> /// @param elf_handle the elf handle to use.
> >> @@ -563,6 +594,39 @@ find_ksymtab_strings_section(Elf *elf_handle)
> >> return 0;
> >> }
> >>
> >> +/// Return the .rel{a,} section corresponding to a given section.
> >> +///
> >> +/// @param elf_handle the elf handle to consider.
> >> +///
> >> +/// @param target_section the section to search the relocation section for
> >> +///
> >> +/// @return the .rel{a,} section if found, null otherwise.
> >> +Elf_Scn*
> >> +find_relocation_section(Elf* elf_handle, Elf_Scn* target_section)
> >> +{
> >> + if (target_section)
> >> + {
> >> + // the relo section we are searching for has this index as sh_info
> >> + size_t target_index = elf_ndxscn(target_section);
> >> +
> >> + // now iterate over all the sections, look for relocation sections and
> >> + // find the one that points to the section we are searching for
> >> + Elf_Scn* section = 0;
> >> + GElf_Shdr header_mem, *header;
> >> + while ((section = elf_nextscn(elf_handle, section)) != 0)
> >> + {
> >> + header = gelf_getshdr(section, &header_mem);
> >> + if (header == NULL
> >> + || (header->sh_type != SHT_RELA && header->sh_type != SHT_REL))
> >> + continue;
> >> +
> >> + if (header->sh_info == target_index)
> >> + return section;
> >> + }
> >> + }
> >> + return NULL;
> >> +}
> >> +
> >> /// Get the version definition (from the SHT_GNU_verdef section) of a
> >> /// given symbol represented by a pointer to GElf_Versym.
> >> ///
> >> @@ -797,5 +861,52 @@ is_linux_kernel(Elf *elf_handle)
> >> || is_linux_kernel_module(elf_handle));
> >> }
> >>
> >> +/// Get the address at which a given binary is loaded in memory⋅
> >> +///
> >> +/// @param elf_handle the elf handle for the binary to consider.
> >> +///
> >> +/// @param load_address the address where the binary is loaded. This
> >> +/// is set by the function iff it returns true.
> >> +///
> >> +/// @return true if the function could get the binary load address
> >> +/// and assign @p load_address to it.
> >
> >Should be returning nullable load_address.
>
> I thought of that as well, but decided against. The return value is not
> a pointer type where NULL would be indicative, 0 is a valid result for
> this function and we do not have the facilities like std::optional
> (yet). I would not want to burden the caller with freeing any memory
> either. Hence I stuck to the original interface. Did I miss an obvious
> other option?
>
> Cheers,
> Matthias
>
> >
> >
> >> +bool
> >> +get_binary_load_address(Elf* elf_handle, GElf_Addr& load_address)
> >> +{
> >> + GElf_Ehdr elf_header;
> >> + gelf_getehdr(elf_handle, &elf_header);
> >> + size_t num_segments = elf_header.e_phnum;
> >> + GElf_Phdr *program_header = NULL;
> >> + GElf_Addr result;
> >> + bool found_loaded_segment = false;
> >> + GElf_Phdr ph_mem;
> >> +
> >> + for (unsigned i = 0; i < num_segments; ++i)
> >> + {
> >> + program_header = gelf_getphdr(elf_handle, i, &ph_mem);
> >> + if (program_header && program_header->p_type == PT_LOAD)
> >> + {
> >> + if (!found_loaded_segment)
> >> + {
> >> + result = program_header->p_vaddr;
> >> + found_loaded_segment = true;
> >> + }
> >> +
> >> + if (program_header->p_vaddr < result)
> >> + // The resulting load address we want is the lowest
> >> + // load address of all the loaded segments.
> >> + result = program_header->p_vaddr;
> >> + }
> >> + }
> >> +
> >> + if (found_loaded_segment)
> >> + {
> >> + load_address = result;
> >> + 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 7ddd887de959..8a83bb4f2e95 100644
> >> --- a/src/abg-elf-helpers.h
> >> +++ b/src/abg-elf-helpers.h
> >> @@ -63,8 +63,8 @@ find_section(Elf* elf_handle,
> >> const std::string& name,
> >> Elf64_Word section_type);
> >>
> >> -bool
> >> -find_symbol_table_section(Elf* elf_handle, Elf_Scn*& symtab);
> >> +Elf_Scn*
> >> +find_symbol_table_section(Elf* elf_handle);
> >>
> >> bool
> >> find_symbol_table_section_index(Elf* elf_handle, size_t& symtab_index);
> >> @@ -96,15 +96,27 @@ find_data_section(Elf* elf_handle);
> >> Elf_Scn*
> >> find_data1_section(Elf* elf_handle);
> >>
> >> +Elf_Scn*
> >> +find_opd_section(Elf* elf_handle);
> >> +
> >> bool
> >> get_symbol_versionning_sections(Elf* elf_handle,
> >> Elf_Scn*& versym_section,
> >> Elf_Scn*& verdef_section,
> >> Elf_Scn*& verneed_section);
> >>
> >> +Elf_Scn*
> >> +find_ksymtab_section(Elf* elf_handle);
> >> +
> >> +Elf_Scn*
> >> +find_ksymtab_gpl_section(Elf* elf_handle);
> >> +
> >> Elf_Scn*
> >> find_ksymtab_strings_section(Elf *elf_handle);
> >>
> >> +Elf_Scn*
> >> +find_relocation_section(Elf* elf_handle, Elf_Scn* target_section);
> >> +
> >> //
> >> // Helpers for symbol versioning
> >> //
> >> @@ -137,6 +149,12 @@ is_linux_kernel_module(Elf *elf_handle);
> >> bool
> >> is_linux_kernel(Elf *elf_handle);
> >>
> >> +//
> >> +// Misc Helpers
> >> +//
> >> +
> >> +bool
> >> +get_binary_load_address(Elf* elf_handle, GElf_Addr& load_address);
> >>
> >> } // end namespace elf_helpers
> >> } // end namespace abigail
> >> --
> >> 2.26.1.301.g55bc3eb7cb9-goog
> >>
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>
More information about the Libabigail
mailing list