public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
From: Giuliano Procida <gprocida@google.com>
To: libabigail@sourceware.org
Cc: dodji@seketeli.org, kernel-team@android.com, gprocida@google.com,
	 maennich@google.com, willdeacon@google.com
Subject: [PATCH 1/2] symtab: refactor ELF symbol value tweaks
Date: Thu,  7 Apr 2022 17:29:25 +0100	[thread overview]
Message-ID: <20220407162926.2076100-1-gprocida@google.com> (raw)

A previous changes duplicated some logic for tweaking ELF symbol
values (and possibly updating some bookkeeping information).

This change refactors the code so the logic is in one place, in
symtab::get_symbol_value.

	* src/abg-symtab-reader.cc (symtab::load_): Replace address
	tweaking logic with a call to get_symbol_value.
	(symtab::add_alternative_address_lookups): Likewise.
	(symtab::get_symbol_value): New function containing address
	tweaking logic for PPC and ARM.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-symtab-reader.cc | 81 ++++++++++++++++++++--------------------
 src/abg-symtab-reader.h  |  5 +++
 2 files changed, 46 insertions(+), 40 deletions(-)

diff --git a/src/abg-symtab-reader.cc b/src/abg-symtab-reader.cc
index b42ce87d..026988ee 100644
--- a/src/abg-symtab-reader.cc
+++ b/src/abg-symtab-reader.cc
@@ -236,9 +236,6 @@ symtab::load_(Elf*	       elf_handle,
   std::unordered_set<std::string> exported_kernel_symbols;
   std::unordered_map<std::string, uint64_t> crc_values;
 
-  const bool is_arm32 = elf_helpers::architecture_is_arm32(elf_handle);
-  const bool is_ppc64 = elf_helpers::architecture_is_ppc64(elf_handle);
-
   for (size_t i = 0; i < number_syms; ++i)
     {
       GElf_Sym *sym, sym_mem;
@@ -347,23 +344,8 @@ symtab::load_(Elf*	       elf_handle,
 	}
       else if (symbol_sptr->is_defined())
 	{
-	  GElf_Addr symbol_value =
-	      elf_helpers::maybe_adjust_et_rel_sym_addr_to_abs_addr(elf_handle,
-								    sym);
-
-	  // See also symtab::add_alternative_address_lookups.
-	  if (symbol_sptr->is_function())
-	    {
-	      if (is_arm32)
-		// Clear bit zero of ARM32 addresses as per "ELF for the Arm
-		// Architecture" section 5.5.3.
-		// https://static.docs.arm.com/ihi0044/g/aaelf32.pdf
-		symbol_value &= ~1;
-	      else if (is_ppc64)
-		update_function_entry_address_symbol_map(elf_handle, sym,
-							 symbol_sptr);
-	    }
-
+	  const GElf_Addr symbol_value =
+	    get_symbol_value(elf_handle, sym, symbol_sptr);
 	  const auto result =
 	    addr_symbol_map_.emplace(symbol_value, symbol_sptr);
 	  if (!result.second)
@@ -483,6 +465,43 @@ symtab::update_main_symbol(GElf_Addr addr, const std::string& name)
     addr_symbol_map_[addr] = new_main;
 }
 
+/// Various adjustments and bookkeeping may be needed to provide a correct
+/// interpretation (one that matches DWARF addresses) of raw symbol values.
+///
+/// @param elf_handle the ELF handle
+///
+/// @param elf_symbol the ELF symbol
+///
+/// @param symbol_sptr the libabigail symbol
+///
+/// @return a possibly-adjusted symbol value
+GElf_Addr
+symtab::get_symbol_value(Elf* elf_handle,
+			 GElf_Sym* elf_symbol,
+			 const elf_symbol_sptr& symbol_sptr)
+{
+  const bool is_arm32 = elf_helpers::architecture_is_arm32(elf_handle);
+  const bool is_ppc64 = elf_helpers::architecture_is_ppc64(elf_handle);
+
+  GElf_Addr symbol_value =
+    elf_helpers::maybe_adjust_et_rel_sym_addr_to_abs_addr(elf_handle,
+							  elf_symbol);
+
+  if (symbol_sptr->is_function())
+    {
+      if (is_arm32)
+	// Clear bit zero of ARM32 addresses as per "ELF for the Arm
+	// Architecture" section 5.5.3.
+	// https://static.docs.arm.com/ihi0044/g/aaelf32.pdf
+	symbol_value &= ~1;
+      else if (is_ppc64)
+	update_function_entry_address_symbol_map(elf_handle, elf_symbol,
+						 symbol_sptr);
+    }
+
+  return symbol_value;
+}
+
 /// Update the function entry symbol map to later allow lookups of this symbol
 /// by entry address as well. This is relevant for ppc64 ELFv1 binaries.
 ///
@@ -582,9 +601,6 @@ symtab::update_function_entry_address_symbol_map(
 void
 symtab::add_alternative_address_lookups(Elf* elf_handle)
 {
-  const bool is_arm32 = elf_helpers::architecture_is_arm32(elf_handle);
-  const bool is_ppc64 = elf_helpers::architecture_is_ppc64(elf_handle);
-
   Elf_Scn* symtab_section = elf_helpers::find_symtab_section(elf_handle);
   if (!symtab_section)
     return;
@@ -634,23 +650,8 @@ symtab::add_alternative_address_lookups(Elf* elf_handle)
 	  if (symbols.size() == 1)
 	    {
 	      const auto& symbol_sptr = symbols[0];
-	      GElf_Addr	  symbol_value =
-		  elf_helpers::maybe_adjust_et_rel_sym_addr_to_abs_addr(
-		      elf_handle, sym);
-
-	      // See also symtab::load_.
-	      if (symbol_sptr->is_function())
-		{
-		  if (is_arm32)
-		    // Clear bit zero of ARM32 addresses as per "ELF for the Arm
-		    // Architecture" section 5.5.3.
-		    // https://static.docs.arm.com/ihi0044/g/aaelf32.pdf
-		    symbol_value &= ~1;
-		  else if (is_ppc64)
-		    update_function_entry_address_symbol_map(elf_handle, sym,
-							     symbol_sptr);
-		}
-
+	      const GElf_Addr symbol_value =
+		get_symbol_value(elf_handle, sym, symbol_sptr);
 	      addr_symbol_map_.emplace(symbol_value, symbol_sptr);
 	    }
 	}
diff --git a/src/abg-symtab-reader.h b/src/abg-symtab-reader.h
index 7ac15352..bddde2f6 100644
--- a/src/abg-symtab-reader.h
+++ b/src/abg-symtab-reader.h
@@ -289,6 +289,11 @@ private:
   load_(string_elf_symbols_map_sptr function_symbol_map,
        string_elf_symbols_map_sptr variables_symbol_map);
 
+  GElf_Addr
+  get_symbol_value(Elf* elf_handle,
+		   GElf_Sym* elf_symbol,
+		   const elf_symbol_sptr& symbol_sptr);
+
   void
   update_function_entry_address_symbol_map(Elf*	     elf_handle,
 					   GElf_Sym* native_symbol,
-- 
2.35.1.1094.g7c7d902a7c-goog


             reply	other threads:[~2022-04-07 16:29 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-07 16:29 Giuliano Procida [this message]
2022-04-07 16:29 ` [PATCH 2/2] symtab: fix up 64-bit ARM address which may contain tags Giuliano Procida
2022-05-04 10:05   ` Dodji Seketeli
2022-05-04 10:00 ` [PATCH 1/2] symtab: refactor ELF symbol value tweaks Dodji Seketeli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220407162926.2076100-1-gprocida@google.com \
    --to=gprocida@google.com \
    --cc=dodji@seketeli.org \
    --cc=kernel-team@android.com \
    --cc=libabigail@sourceware.org \
    --cc=maennich@google.com \
    --cc=willdeacon@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).