[v2,6/8] abg-dwarf-reader: migrate more ELF helpers to elf-helpers

Message ID 20200421063551.222511-7-maennich@google.com
State Committed
Headers
Series Refactor dwarf-reader: split out ELF helpers |

Commit Message

Matthias Männich April 21, 2020, 6:35 a.m. UTC
  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(-)
  

Comments

Dodji Seketeli April 22, 2020, 9:53 a.m. UTC | #1
Matthias Maennich <maennich@google.com> a ?crit:

> 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.

I am correcting the typo in "declation" here.  Likewise for the
lines below, until ...

> 	(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.

... here.

[...]

> @@ -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_;
>    }

I am declaring the read_context::symtab_section_ as mutable to avoid the
const_cast here.

>  
> @@ -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_;
>    }

Likewise.

>  
> @@ -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_;
>    }


Likewise.

[...]

>    Elf_Scn*
> -  find_relocation_section(Elf_Scn* target_section) const
> +  find_ksymtab_gpl_section() const
>    {

[...]

> -    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_;
>    }

Likewise.

>  
>    /// 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_;
>    }

Likewise.

[...]

> @@ -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());
>        }

Likewise.

[...]

Applying to master with those changes, thanks!

Cheers,
  

Patch

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.
+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