* Re: [PATCH] symtab-reader: Setup aliases before checking ppc64 opd function entries
2022-05-17 9:59 ` Mark Wielaard
@ 2022-05-17 13:51 ` Giuliano Procida
2022-05-17 14:03 ` Mark Wielaard
2022-05-27 16:33 ` Dodji Seketeli
1 sibling, 1 reply; 8+ messages in thread
From: Giuliano Procida @ 2022-05-17 13:51 UTC (permalink / raw)
To: Mark Wielaard; +Cc: Dodji Seketeli, libabigail, Matthias Männich
Hi.
On Tue, 17 May 2022 at 10:59, Mark Wielaard <mark@klomp.org> wrote:
>
> Hi Dodji,
>
> On Mon, 2022-05-16 at 11:43 +0200, Dodji Seketeli wrote:
> > 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?
>
> It looks good to me, some small comments below. I don't have access to
> a ppc64[be] setup anymore, but the buildbot has:
> https://builder.sourceware.org/buildbot/#/builders/libabigail-debian-ppc64
> So hopefully after this patch that will turn green.
>
I have some minor comments. Other than that, it looks good to me.
I'm CCing Matthias as he did a lot of the reworking into the current
structure, I think.
> > 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.
>
> Is that second line too long?
>
> > 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.
>
> Same for the first line here.
>
> > 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
and and / and
> > +/// only once during the execution of the former.
I've lost a bit of the context, but where comments say "only once"
should that be "only once per symbol"?
> > +///
> > /// @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);
>
> This part is new. I think it is correct to do this, before setting up
> the aliases, just like we mask off the zero bit for arm32. But it would
> be good if Giuliano could confirm.
>
The two ARM adjustments belong together. I wasn't sure about PPC, so
I'm glad that it's getting moved to the right place.
> > + 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)
IIRC, someone said that this should apply to PPC32 as well, perhaps you, Mark?
> > + update_function_entry_address_symbol_map(elf_handle, elf_symbol,
> > + symbol_sptr);
>
> I would add a comment here to say something like:
>
> // Note, update_function_entry_address_symbol_map depends
> // on the symbol aliases been setup.
>
> So it is clear why this needs to be done after the add_alias call
> above.
>
> > 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,
> > --
>
> Looks good.
>
> Thanks,
>
> Mark
Cheers,
Giuliano.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] symtab-reader: Setup aliases before checking ppc64 opd function entries
2022-05-17 9:59 ` Mark Wielaard
2022-05-17 13:51 ` Giuliano Procida
@ 2022-05-27 16:33 ` Dodji Seketeli
2022-05-27 23:05 ` Mark Wielaard
1 sibling, 1 reply; 8+ messages in thread
From: Dodji Seketeli @ 2022-05-27 16:33 UTC (permalink / raw)
To: Mark Wielaard; +Cc: gprocida, maennich, libabigail
Hello Mark & Giuliano and Matthias!
Mark Wielaard <mark@klomp.org> a écrit:
[...]
> It looks good to me, some small comments below. I don't have access to
> a ppc64[be] setup anymore, but the buildbot has:
> https://builder.sourceware.org/buildbot/#/builders/libabigail-debian-ppc64
> So hopefully after this patch that will turn green.
Thank you for the review! I have updated the patch accordingly and I am
posting an updated version below.
[...]
>> 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.
>
> Is that second line too long?
Yes it is. Fixed.
>> 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.
>
> Same for the first line here.
Hmmh, that one is 70 columns wide, so less than 80, as far as I can
tell.
[...]
>> @@ -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);
>
> This part is new. I think it is correct to do this, before setting up
> the aliases, just like we mask off the zero bit for arm32. But it would
> be good if Giuliano could confirm.
Thanks for commenting on this. We'll see how Giuliano thinks.
[...]
>
>> + 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);
>
> I would add a comment here to say something like:
>
> // Note, update_function_entry_address_symbol_map depends
> // on the symbol aliases been setup.
>
> So it is clear why this needs to be done after the add_alias call
> above.
Absolutely. Fixed. Thanks.
[...]
> Looks good.
Thanks!
[...]
Giuliano Procida <gprocida@google.com> a écrit:
> Hi.
Hey Giuliano,
> I have some minor comments. Other than that, it looks good to me.
>
> I'm CCing Matthias as he did a lot of the reworking into the current
> structure, I think.
Thank you for the review. I have updated the patch according to your
comments. Let's look at them once again.
[...]
>> > @@ -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
>
> and and / and
Spot on. Thanks. Fixed.
[...]
>> > +/// only once during the execution of the former.
>
> I've lost a bit of the context, but where comments say "only once"
> should that be "only once per symbol"?
You are right. Fixed.
[...]
>> > /// @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);
>>
>> This part is new. I think it is correct to do this, before setting up
>> the aliases, just like we mask off the zero bit for arm32. But it would
>> be good if Giuliano could confirm.
>>
>
> The two ARM adjustments belong together. I wasn't sure about PPC, so
> I'm glad that it's getting moved to the right place.
Great, thanks to both of you.
[...]
>
>> > + 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)
>
> IIRC, someone said that this should apply to PPC32 as well, perhaps you, Mark?
[...]
Mark Wielaard <mark@klomp.org> a écrit:
> Yes, in theory both ppc32 and ppc64 use function descriptors (and both
> are big endian). But I haven't seen a real ppc32 setup in years.
>
> If you want to support that then it should be as simple as defining a
> architecture_is_ppc32 which is precisely like architecture_is_ppc64
> with s/EM_PPC64/EM_PPC/
OK, thanks Giuliano and Mark.
I have added support for ppc32 as well, as outlined above.
Please find below the updated patch that I am proposing.
Thanks!
From 8d44b582529ccd02689043918ef3ec3dd85155bc 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-elf-helpers.h (architecture_is_ppc32): Declare new
function.
* src/abg-elf-helpers.cc (architecture_is_ppc32): Define it.
* 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 ppc{32,64} 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-elf-helpers.cc | 13 +++++++++
src/abg-elf-helpers.h | 3 ++
src/abg-symtab-reader.cc | 62 ++++++++++++++++++++++------------------
src/abg-symtab-reader.h | 6 ++--
4 files changed, 53 insertions(+), 31 deletions(-)
diff --git a/src/abg-elf-helpers.cc b/src/abg-elf-helpers.cc
index c41e339e..2a2cc6c2 100644
--- a/src/abg-elf-helpers.cc
+++ b/src/abg-elf-helpers.cc
@@ -918,6 +918,19 @@ architecture_is_ppc64(Elf* elf_handle)
return (elf_header && elf_header->e_machine == EM_PPC64);
}
+/// Test if the architecture of the current binary is ppc32.
+///
+/// @param elf_handle the ELF handle to consider.
+///
+/// @return true iff the architecture of the current binary is ppc32.
+bool
+architecture_is_ppc32(Elf* elf_handle)
+{
+ GElf_Ehdr eh_mem;
+ GElf_Ehdr* elf_header = gelf_getehdr(elf_handle, &eh_mem);
+ return (elf_header && elf_header->e_machine == EM_PPC);
+}
+
/// Test if the architecture of the current binary is arm32.
///
/// @param elf_handle the ELF handle to consider.
diff --git a/src/abg-elf-helpers.h b/src/abg-elf-helpers.h
index ee406863..0dc987d4 100644
--- a/src/abg-elf-helpers.h
+++ b/src/abg-elf-helpers.h
@@ -147,6 +147,9 @@ get_version_for_symbol(Elf* elf_handle,
bool
architecture_is_ppc64(Elf* elf_handle);
+bool
+architecture_is_ppc32(Elf* elf_handle);
+
bool
architecture_is_arm32(Elf* elf_handle);
diff --git a/src/abg-symtab-reader.cc b/src/abg-symtab-reader.cc
index 3740cb7a..0c2f8ccc 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 must be called only
+/// once (per symbol) during the execution of the former.
+///
/// @param elf_handle the ELF handle
///
/// @param elf_symbol the ELF symbol
@@ -476,35 +470,49 @@ 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);
const bool is_ppc64 = elf_helpers::architecture_is_ppc64(elf_handle);
+ const bool is_ppc32 = elf_helpers::architecture_is_ppc32(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);
- }
+ if (is_arm32 && symbol_sptr->is_function())
+ // 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);
+ }
+
+ // Please note that update_function_entry_address_symbol_map depends
+ // on the symbol aliases been setup. This is why, the
+ // elf_symbol::add_alias call is done above BEFORE this point.
+ if ((is_ppc64 || is_ppc32) && symbol_sptr->is_function())
+ update_function_entry_address_symbol_map(elf_handle, elf_symbol,
+ symbol_sptr);
+
return symbol_value;
}
@@ -656,9 +664,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.36.1
Cheers,
--
Dodji
^ permalink raw reply [flat|nested] 8+ messages in thread