[PATCH] symtab-reader: Setup aliases before checking ppc64 opd function entries
Dodji Seketeli
dodji@seketeli.org
Mon May 16 09:43:57 GMT 2022
Mark Wielaard mark@klomp.org
Date: Mon, 16 May 2022 11:43:35 +0200
Message-ID: <87a6bhvmm0.fsf@seketeli.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
[This is email is in reply to the patch sent by Mark at https://sourceware.org/pipermail/libabigail/2022q2/004318.html]
Hello Mark, Giuliano,
For a reason, my email account was bouncing emails from sourceware.org
around the May 1st, so mailman stopped sending me emails around that
time. I re-enabled the service, but it seems I haven't received a
number of emails around that time.
So I am replying to the patch that Mark sent at
https://sourceware.org/pipermail/libabigail/2022q2/004318.html.
Mark, I applied two patches from Giuliano that changed the area that your
patch touches. Those patches are:
c96463e1 symtab: fix up 64-bit ARM address which may contain tags
5106149c symtab: refactor ELF symbol value tweaks
Basically, those patches create a new symtab::get_symbol_value and
performs the arm{32,64} and ppc64 ELFv1 addresses adjustments.
So, your patch won't apply anymore.
I have thus adjusted your patch to keep its spirit while making it apply
to the current code base.
Giuliano, Mark, would you guys please review the patch, when you have
time, and tell me what you think?
Thank you very much.
>From 768d13cb21a0d6c8ebd8ed0530a095a036c4c9b5 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Mon, 16 May 2022 10:53:55 +0200
Subject: [PATCH] symtab-reader: Setup aliases before checking ppc64 opd function entries
The update_function_entry_address_symbol_map function checks whether
the given symbol is an alias of another symbol or a special ppc64
ELFv1 function entry symbol. This requires the symbol aliases to
already been setup. But the alias entries were only setup after
calling update_function_entry_address_symbol_map. Make sure that
the symbol aliases have been setup and only then call the special
ppc64 update_function_entry_address_symbol_map function. But make
sure the arm32 function symbol entry address cleanup is done before
checking for aliases.
This patch renames symtab::get_symbol_value into
symtab::setup_symbol_lookup_tables to better reflect what it does.
It is in that function that the call to
update_function_entry_address_symbol_map is performed, the arm32 symbol address
cleanup is done and the ppc64 ELFv1 function address plumbing
is done.
So arranging for update_function_entry_address_symbol_map to be called
after symbol aliases are setup for ppc64 is also done in that
function. That way, symtab::load_ only have to call
symtab::setup_symbol_lookup_tables to have aliases setup.
This fixes runtestslowselfcompare.sh on ppc64 (ELFv1) with
ENABLE_SLOW_TEST=yes
* src/abg-symtab-reader.cc
(symtab::setup_symbol_lookup_tables): Rename
symtab::get_symbol_value into this. Setup symbol aliases before
setting up function entry address maps for ppc64 ELFv1 and after
fixing up arm32/64 addresses.
(symtab::load_): Invoke the new setup_symbol_lookup_tables.
update_function_entry_address_symbol_map after setting up aliases.
No need to setup symbol aliases anymore.
(symtab::add_alternative_address_lookups): Invoke the new
setup_symbol_lookup_tables.
Signed-off-by: Mark Wielaard <mark@klomp.org>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
src/abg-symtab-reader.cc | 58 +++++++++++++++++++++-------------------
src/abg-symtab-reader.h | 6 ++---
2 files changed, 33 insertions(+), 31 deletions(-)
diff --git a/src/abg-symtab-reader.cc b/src/abg-symtab-reader.cc
index 3740cb7a..05be4d6b 100644
--- a/src/abg-symtab-reader.cc
+++ b/src/abg-symtab-reader.cc
@@ -343,17 +343,7 @@ symtab::load_(Elf* elf_handle,
}
}
else if (symbol_sptr->is_defined())
- {
- 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)
- // A symbol with the same address already exists. This
- // means this symbol is an alias of the main symbol with
- // that address. So let's register this new alias as such.
- result.first->second->get_main_symbol()->add_alias(symbol_sptr);
- }
+ setup_symbol_lookup_tables(elf_handle, sym, symbol_sptr);
}
add_alternative_address_lookups(elf_handle);
@@ -468,6 +458,10 @@ symtab::update_main_symbol(GElf_Addr addr, const std::string& name)
/// Various adjustments and bookkeeping may be needed to provide a correct
/// interpretation (one that matches DWARF addresses) of raw symbol values.
///
+/// This is a sub-routine for symtab::load_and
+/// symtab::add_alternative_address_lookups and and must be called
+/// only once during the execution of the former.
+///
/// @param elf_handle the ELF handle
///
/// @param elf_symbol the ELF symbol
@@ -476,9 +470,9 @@ symtab::update_main_symbol(GElf_Addr addr, const std::string& name)
///
/// @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)
+symtab::setup_symbol_lookup_tables(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_arm64 = elf_helpers::architecture_is_arm64(elf_handle);
@@ -488,23 +482,33 @@ symtab::get_symbol_value(Elf* elf_handle,
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);
- }
+ if (symbol_sptr->is_function() && 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;
+
if (is_arm64)
// Copy bit 55 over bits 56 to 63 which may be tag information.
symbol_value = symbol_value & (1ULL<<55)
? symbol_value | (0xffULL<<56)
: symbol_value &~ (0xffULL<<56);
+ if (symbol_sptr->is_defined())
+ {
+ const auto result =
+ addr_symbol_map_.emplace(symbol_value, symbol_sptr);
+ if (!result.second)
+ // A symbol with the same address already exists. This
+ // means this symbol is an alias of the main symbol with
+ // that address. So let's register this new alias as such.
+ result.first->second->get_main_symbol()->add_alias(symbol_sptr);
+ }
+
+ if (symbol_sptr->is_function() && is_ppc64)
+ update_function_entry_address_symbol_map(elf_handle, elf_symbol,
+ symbol_sptr);
+
return symbol_value;
}
@@ -656,9 +660,7 @@ symtab::add_alternative_address_lookups(Elf* elf_handle)
if (symbols.size() == 1)
{
const auto& symbol_sptr = symbols[0];
- const GElf_Addr symbol_value =
- get_symbol_value(elf_handle, sym, symbol_sptr);
- addr_symbol_map_.emplace(symbol_value, symbol_sptr);
+ setup_symbol_lookup_tables(elf_handle, sym, symbol_sptr);
}
}
}
diff --git a/src/abg-symtab-reader.h b/src/abg-symtab-reader.h
index bddde2f6..948fc270 100644
--- a/src/abg-symtab-reader.h
+++ b/src/abg-symtab-reader.h
@@ -290,9 +290,9 @@ private:
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);
+ setup_symbol_lookup_tables(Elf* elf_handle,
+ GElf_Sym* elf_symbol,
+ const elf_symbol_sptr& symbol_sptr);
void
update_function_entry_address_symbol_map(Elf* elf_handle,
--
2.35.0.rc2
Cheers,
--
Dodji
More information about the Libabigail
mailing list