This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
Re: [patchv3 4/4] Provide virtual symbols for ppc64 function descriptors
- From: Mark Wielaard <mjw at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Wed, 27 Nov 2013 13:27:01 +0100
- Subject: Re: [patchv3 4/4] Provide virtual symbols for ppc64 function descriptors
On Thu, 2013-11-14 at 20:49 +0100, Jan Kratochvil wrote:
> On Thu, 14 Nov 2013 14:42:34 +0100, Mark Wielaard wrote:
> > As far as I understand from skimming the code, you want to provide an
> > (extra) table of (function) symbols from ebl for the elf file it is
> > backing. These symbols are matched against the .odp (in this ppc64 case)
> > section data.
>
> Yes. Not "for the elf file" but for the whole Dwfl_Module where symbols can
> come from two different places (aux_sym).
Right. So you need a symbol table and at least the main Elf .odp section
(none of the other Elf files contain the .odp bits). And it is the Ebl
of the main Elf file that is responsible for the table.
> > Could you please add a little (high-level) description of the ppc64 .odp
> > section data and how it relates to the symbol table data?
>
> Added there:
>
> /* Scan all the symbols of EBL and create table of struct sym_entry entries for
> every function symbol pointing to the .opd section. This includes automatic
> derefencing of the symbols via the .opd section content to create the
> virtual symbols for the struct sym_entry entries. As GETSYM points to
> dwfl_module_getsym we scan all the available symbols, either
> Dwfl_Module.main plus Dwfl_Module.aux_sym or Dwfl_Module.debug.
> Symbols from all the available files are included in the SYMENTS count. */
>
>
> > For each symbol, in the original elf file symbol table, whose st_value
> > points to an .odp entry, you store a new derived symbol with an adjust
> > value, name,
>
> Yes.
It looks like this could create duplicate symbols, at least symbols with
the same st_value. So you might need to also check such a symbol doesn't
exist yet. The relocatable kernel modules for example seem to already
contain such symbols, plus their .odp variants. See for example the
hello_ppc64.ko in the testsuite.
> The trick is that even if the symbol comes from aux_sym its VMA still has to
> point into the main dwfl_file. Also returning dwfl_file * does not make
> sense, the target virtual symbol always has to point into .opd, therefore into
> mod->main file and nowhere else.
That makes sense.
> > You don't directly read the symbol table (using for example
> > gelf_getsymshndx), but use a callback for that in the ebl hook. Why is
> > that?
>
> Because dwfl_module_getsym does all the work of merging various symbol sources
> (main, debug, aux_sym) together with their proper relocation. It would mean
> just reimplementing dwfl_module_getsym in backends/ppc64_get_symbol.c.
OK.
And one extra complication is that we present the dwfl_module_symtab as
a "real symbol table". That means it has a zero entry first and all
LOCAL symbols come before the GLOBAL ones.
[Side note. But unlike a real symbol table we don't provide a way to get
at the first GLOBAL, as a real symbol table would through sh_info. This
prevents users to actually use this as an optimization like
dwfl_module_addrsym does, by searching GLOBALs before LOCALs, which
might be desirable. So we really should provide an interface for the
index of the first GLOBAL for use with dwfl_module_getsymtab. Or is it
acceptable for a user to just walk the table once till they hit the
first GLOBAL and cache that themselves?]
> Provide virtual symbols for ppc64 function descriptors
>
> backends/
> 2013-11-14 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> Provide virtual symbols for ppc64 function descriptors.
> * Makefile.am (ppc64_SRCS): Add ppc64_get_symbol.c.
> * ppc64_get_symbol.c: New file.
> * ppc64_init.c (ppc64_init): Install init_symbols, get_symbol and
> destr.
> libdwfl/
> 2013-11-14 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * dwfl_module_addrsym.c (dwfl_module_addrsym): Adjust FIRST_GLOBAL also
> for EBL_FIRST_GLOBAL.
> * dwfl_module_getdwarf.c (getsym_helper): New function.
> (find_symtab): Call also ebl_init_symbols.
> (dwfl_module_getsymtab): Count also EBL_SYMENTS.
> * dwfl_module_getsym.c (__libdwfl_module_getsym): Count also
> EBL_FIRST_GLOBAL, EBL_SYMENTS. Call also ebl_get_symbol.
> * libdwflP.h (DWFL_ERRORS): Add INVALID_INDEX.
> (struct Dwfl_Module): Add fields ebl_syments and ebl_first_global.
>
> libebl/
> 2013-11-14 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> Provide virtual symbols for ppc64 function descriptors.
> * Makefile.am (gen_SOURCES): Add eblgetsymbol.c.
> * ebl-hooks.h (init_symbols, get_symbol): New.
> * eblgetsymbol.c: New file.
> * libebl.h (ebl_getsym_t): New definition.
> (ebl_init_symbols, ebl_get_symbol): New declarations.
> * libeblP.h (struct ebl): New field backend.
>
> tests/
> 2013-11-06 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> Provide virtual symbols for ppc64 function descriptors.
> * Makefile.am (EXTRA_DIST): Add testfile66.bz2 and testfile66.core.bz2.
> * run-addrname-test.sh (testfile66, testfile66.core): New tests.
> * testfile66.bz2: New file.
> * testfile66.core.bz2: New file.
>
> Signed-off-by: Jan Kratochvil <jan.kratochvil@redhat.com>
> ---
> backends/Makefile.am | 2 +-
> backends/ppc64_get_symbol.c | 196 +++++++++++++++++++++++++++++++++++++++++
> backends/ppc64_init.c | 3 +
> libdwfl/dwfl_module_addrsym.c | 21 ++---
> libdwfl/dwfl_module_getdwarf.c | 19 +++-
> libdwfl/dwfl_module_getsym.c | 48 ++++++++--
> libdwfl/libdwflP.h | 5 +-
> libebl/Makefile.am | 2 +-
> libebl/ebl-hooks.h | 10 +++
> libebl/eblgetsymbol.c | 57 ++++++++++++
> libebl/libebl.h | 26 ++++++
> libebl/libeblP.h | 3 +
> tests/Makefile.am | 3 +-
> tests/run-addrname-test.sh | 39 ++++++++
> tests/testfile66.bz2 | Bin 0 -> 741 bytes
> tests/testfile66.core.bz2 | Bin 0 -> 56448 bytes
> 16 files changed, 412 insertions(+), 22 deletions(-)
> create mode 100644 backends/ppc64_get_symbol.c
> create mode 100644 libebl/eblgetsymbol.c
> create mode 100755 tests/testfile66.bz2
> create mode 100644 tests/testfile66.core.bz2
>
> diff --git a/backends/Makefile.am b/backends/Makefile.am
> index 5b5e067..0e00329 100644
> --- a/backends/Makefile.am
> +++ b/backends/Makefile.am
> @@ -94,7 +94,7 @@ am_libebl_ppc_pic_a_OBJECTS = $(ppc_SRCS:.c=.os)
>
> ppc64_SRCS = ppc64_init.c ppc64_symbol.c ppc64_retval.c \
> ppc64_corenote.c ppc_regs.c ppc_auxv.c ppc_attrs.c ppc_syscall.c \
> - ppc_cfi.c
> + ppc_cfi.c ppc64_get_symbol.c
> libebl_ppc64_pic_a_SOURCES = $(ppc64_SRCS)
> am_libebl_ppc64_pic_a_OBJECTS = $(ppc64_SRCS:.c=.os)
OK.
> diff --git a/backends/ppc64_get_symbol.c b/backends/ppc64_get_symbol.c
> [...]
> +/* Pointer to the first entry is stored into ebl->backend.
> + It has Dwfl_Module->ebl_syments entries and its memory is followed by
> + strings for the NAME entries. */
> +
> +struct sym_entry
> +{
> + GElf_Sym sym;
> + GElf_Word shndx;
> + const char *name;
> +};
It might be slightly less memory to store only the original sym index
and new value instead of the full GElf_Sym. Although that might make the
code more complicated, having to pass the ebl_getsym_t also to this
function.
> +static Elf_Scn *
> +scnfindvma (Elf *elf, GElf_Addr vma)
> +{
> + Elf_Scn *scn = NULL;
> + while ((scn = elf_nextscn (elf, scn)) != NULL)
> + {
> + GElf_Shdr shdr_mem;
> + GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem);
> + if (likely (shdr != NULL)
> + && vma >= shdr->sh_addr
> + && vma < shdr->sh_addr + shdr->sh_size)
> + break;
> + }
> + return scn;
> +}
What does vma stand for in this case? Could use a little comment.
Should this just return the section index? Which is all you seem to need
below.
> +/* Scan all the symbols of EBL and create table of struct sym_entry entries for
> + every function symbol pointing to the .opd section. This includes automatic
> + derefencing of the symbols via the .opd section content to create the
> + virtual symbols for the struct sym_entry entries. As GETSYM points to
> + dwfl_module_getsym we scan all the available symbols, either
> + Dwfl_Module.main plus Dwfl_Module.aux_sym or Dwfl_Module.debug.
> + Symbols from all the available files are included in the SYMENTS count. */
In theory this is just a generic function. In goes a symbol table of
size syments, accessed by the getsym function (that takes arg as
callback value). Out comes a symbol table based on that of size
ebl_syments accessed through ebl_get_symbol, with ebl_first_global as
its first global.
> +void
> +ppc64_init_symbols (Ebl *ebl, size_t syments, int first_global,
> + GElf_Addr main_bias, ebl_getsym_t *getsym, void *arg,
> + size_t *ebl_symentsp, int *ebl_first_globalp)
You don't really need the first_global, see below.
main_bias is the bias used in the given symbol table st_values compared
to the load addresses used in the main/ebl elf file. I wonder whether
the getsym callback doesn't just need to compensate for that in this
case, so you don't have to do that here?
> +{
> + assert (ebl != NULL);
> + assert (ebl->backend == NULL);
You could also allow multiple calls and just see the values are already
there and returned the cached values. But do document whether it can
only be called once or multiple times.
> + Elf *elf = ebl->elf;
> + assert (*ebl_symentsp == 0);
> + assert (*ebl_first_globalp == 0);
Why do you care what values were passed in? These are return values.
Just initialize them to zero so an early return makes sure they are set
to zero.
> + GElf_Ehdr ehdr_mem, *ehdr = gelf_getehdr (elf, &ehdr_mem);
> + if (ehdr == NULL)
> + return;
We don't have a good ebl error story, but maybe make this function
return a boolean, so the caller knows whether zero ebl_syments means
error or no-extra-symbols?
> + GElf_Shdr opd_shdr_mem, *opd_shdr;
> + Elf_Data *opd_data = NULL;
> + Elf_Scn *scn = NULL;
> + while ((scn = elf_nextscn (elf, scn)) != NULL)
> + {
> + opd_shdr = gelf_getshdr (scn, &opd_shdr_mem);
> + if (opd_shdr == NULL)
> + continue;
> + if (strcmp (elf_strptr (elf, ehdr->e_shstrndx, opd_shdr->sh_name), ".opd")
> + != 0)
> + continue;
> + opd_data = elf_getdata (scn, NULL);
> + /* SHT_NOBITS will produce NULL D_BUF. */
> + if (opd_data == NULL || opd_data->d_buf == NULL)
> + return;
> + assert (opd_data->d_size == opd_shdr->sh_size);
> + break;
> + }
I could not find a reference to this .odp section. Please add a
reference to the standard that describes it if there is any.
> + if (opd_data == NULL)
> + return;
> + size_t names_size = 0;
> + size_t ebl_syments = 0;
> + int ebl_first_global = 0;
> + for (size_t symi = 0; symi < syments; symi++)
> + {
> + GElf_Sym sym;
> + const char *symname = getsym (arg, symi, &sym, NULL);
> + /* Do not check SHNDX as it may correspond to aux_sym and we do not have
> + access to its Elf * to find its the aux_sym .opd section. */
We could (and should IMHO) provide the Elf a symbol comes from if you
need it.
> + if (symname == NULL)
> + continue;
> + Elf64_Addr val;
> + if (sym.st_value < opd_shdr->sh_addr + main_bias
> + || sym.st_value > (opd_shdr->sh_addr + main_bias
> + + opd_shdr->sh_size - sizeof (val)))
> + continue;
> + ebl_syments++;
> + if ((ssize_t) symi < first_global)
> + ebl_first_global++;
Since the symbols are given in normal order, you can just look for the
first GLOBAL sym to record your own first global.
> + names_size += 1 + strlen (symname) + 1;
> + }
> + if (ebl_syments == 0)
> + return;
> + struct sym_entry *sym_table;
> + sym_table = malloc (ebl_syments * sizeof (*sym_table) + names_size);
> + if (sym_table == NULL)
> + return;
> + struct sym_entry *dest = sym_table;
> + char *names = (void *) &sym_table[ebl_syments];
> + char *names_dest = names;
> + for (size_t symi = 0; symi < syments; symi++)
> + {
> + GElf_Sym sym;
> + const char *symname = getsym (arg, symi, &sym, NULL);
> + /* Do not check SHNDX as it may correspond to aux_sym and we do not have
> + access to its Elf * to find its the aux_sym .opd section. */
> + if (symname == NULL)
> + continue;
> + uint64_t val64;
> + if (sym.st_value < opd_shdr->sh_addr + main_bias
> + || sym.st_value > (opd_shdr->sh_addr + main_bias
> + + opd_shdr->sh_size - sizeof (val64)))
> + continue;
> + val64 = *(const uint64_t *) (opd_data->d_buf + sym.st_value
> + - (opd_shdr->sh_addr + main_bias));
> + val64 = (elf_getident (elf, NULL)[EI_DATA] == ELFDATA2MSB
> + ? be64toh (val64) : le64toh (val64));
A description of the layout of the .odp section, or a reference to it,
would help here.
> + assert ((char *) dest < names);
> + dest->sym = sym;
> + dest->sym.st_value = val64 + main_bias;
> + Elf_Scn *sym_scn = scnfindvma (elf, dest->sym.st_value);
> + dest->shndx = sym_scn == NULL ? SHN_ABS : elf_ndxscn (sym_scn);
When does it happen that no real section index can be found. Isn't that
always an error?
> + dest->sym.st_shndx = SHN_XINDEX;
That doesn't seem correct, unless the section index is really too large
to fit. Especially not if SHN_ABS is a valid value.
> + dest->name = names_dest;
> + *names_dest++ = '.';
> + names_dest = stpcpy (names_dest, symname) + 1;
> + dest++;
The mangling of names by prepending a dot is somewhat ugly and requires
lots of extra memory. Can't we just keep the proper name of the original
symbol without the extra dot added. That would look much cleaner.
Or more radical, do we even have to keep the old symbols? Can't we just
adjust their st_value and section index and be done with it? That would
simplify things a lot.
> + }
> + assert ((char *) dest == names);
> + assert (names_dest == names + names_size);
> + *ebl_symentsp = ebl_syments;
> + *ebl_first_globalp = ebl_first_global;
> + ebl->backend = sym_table;
> +}
> +
> +const char *
> +ppc64_get_symbol (Ebl *ebl, size_t ndx, GElf_Sym *symp, GElf_Word *shndxp)
> +{
> + assert (ebl != NULL);
> + if (ebl->backend == NULL)
> + return NULL;
> + struct sym_entry *sym_table = ebl->backend;
> + const struct sym_entry *found = &sym_table[ndx];
> + *symp = found->sym;
> + if (shndxp)
> + *shndxp = found->shndx;
> + return found->name;
> +}
> +
> +void
> +ppc64_destr (Ebl *ebl)
> +{
> + if (ebl->backend == NULL)
> + return;
> + struct sym_entry *sym_table = ebl->backend;
> + free (sym_table);
> + ebl->backend = NULL;
> +}
You can just free (ebl->backend).
> diff --git a/backends/ppc64_init.c b/backends/ppc64_init.c
> index 1435875..3ed882b 100644
> --- a/backends/ppc64_init.c
> +++ b/backends/ppc64_init.c
> @@ -65,6 +65,9 @@ ppc64_init (elf, machine, eh, ehlen)
> HOOK (eh, core_note);
> HOOK (eh, auxv_info);
> HOOK (eh, abi_cfi);
> + HOOK (eh, init_symbols);
> + HOOK (eh, get_symbol);
> + HOOK (eh, destr);
OK.
> diff --git a/libdwfl/dwfl_module_addrsym.c b/libdwfl/dwfl_module_addrsym.c
> index d9eb0a2..6a39346 100644
> --- a/libdwfl/dwfl_module_addrsym.c
> +++ b/libdwfl/dwfl_module_addrsym.c
> @@ -173,16 +173,17 @@ dwfl_module_addrsym (Dwfl_Module *mod, GElf_Addr addr,
> }
> }
>
> - /* First go through global symbols. mod->first_global and
> - mod->aux_first_global are setup by dwfl_module_getsymtab to the
> - index of the first global symbol in those symbol tables. Both
> - are non-zero when the table exist, except when there is only a
> - dynsym table loaded through phdrs, then first_global is zero and
> - there will be no auxiliary table. All symbols with local binding
> - come first in the symbol table, then all globals. The zeroth,
> - null entry, in the auxiliary table is skipped if there is a main
> - table. */
> - int first_global = mod->first_global + mod->aux_first_global;
> + /* First go through global symbols. mod->first_global,
> + mod->aux_first_global and mod->ebl_first_global are setup by
> + dwfl_module_getsymtab to the index of the first global symbol in
> + those symbol tables. Both are non-zero when the table exist,
> + except when there is only a dynsym table loaded through phdrs, then
> + first_global is zero and there will be no auxiliary table. All
> + symbols with local binding come first in the symbol table, then all
> + globals. The zeroth, null entry, in the auxiliary table is skipped
> + if there is a main table. */
> + int first_global = (mod->first_global + mod->aux_first_global
> + + mod->ebl_first_global);
> if (mod->syments > 0 && mod->aux_syments > 0)
> first_global--;
> search_table (first_global == 0 ? 1 : first_global, syments);
OK.
> diff --git a/libdwfl/dwfl_module_getdwarf.c b/libdwfl/dwfl_module_getdwarf.c
> [...]
> +static const char *
> +getsym_helper (void *arg, int ndx, GElf_Sym *sym, GElf_Word *shndxp)
> +{
> + Dwfl_Module *mod = arg;
> + return dwfl_module_getsym (mod, ndx, sym, shndxp);
> +}
So, here you could already adjust the sym st_value by any bias if that
makes things easier, and/or even provide the actual Elf or bias itself.
> /* Try to find a symbol table in either MOD->main.elf or MOD->debug.elf. */
> static void
> find_symtab (Dwfl_Module *mod)
> @@ -1064,7 +1071,7 @@ find_symtab (Dwfl_Module *mod)
> mod->aux_syments = 0;
> elf_end (mod->aux_sym.elf);
> mod->aux_sym.elf = NULL;
> - return;
> + goto aux_done;
> }
>
> mod->aux_symstrdata = elf_getdata (elf_getscn (mod->aux_sym.elf,
> @@ -1085,7 +1092,15 @@ find_symtab (Dwfl_Module *mod)
> mod->aux_symdata = elf_getdata (aux_symscn, NULL);
> if (mod->aux_symdata == NULL)
> goto aux_cleanup;
> + aux_done:;
> }
> +
> + Dwfl_Error error = __libdwfl_module_getebl (mod);
> + if (error == DWFL_E_NOERROR)
> + ebl_init_symbols (mod->ebl, mod->syments + mod->aux_syments,
> + mod->first_global + mod->aux_first_global,
> + mod->main_bias, getsym_helper, mod,
> + &mod->ebl_syments, &mod->ebl_first_global);
> }
Aha. This makes sure ebl_init_symbols is only called once because
find_symtab caches its values and returns early when already called
before.
> @@ -1242,7 +1257,7 @@ dwfl_module_getsymtab (Dwfl_Module *mod)
> find_symtab (mod);
> if (mod->symerr == DWFL_E_NOERROR)
> /* We will skip the auxiliary zero entry if there is another one. */
> - return (mod->syments + mod->aux_syments
> + return (mod->syments + mod->aux_syments + mod->ebl_syments
> - (mod->syments > 0 && mod->aux_syments > 0 ? 1 : 0));
OK.
> __libdwfl_seterrno (mod->symerr);
> diff --git a/libdwfl/dwfl_module_getsym.c b/libdwfl/dwfl_module_getsym.c
> index 0f5dd37..e159850 100644
> --- a/libdwfl/dwfl_module_getsym.c
> +++ b/libdwfl/dwfl_module_getsym.c
> @@ -55,8 +55,7 @@ __libdwfl_module_getsym (Dwfl_Module *mod, int ndx,
> Elf_Data *symdata;
> Elf_Data *symxndxdata;
> Elf_Data *symstrdata;
> - if (mod->aux_symdata == NULL
> - || ndx < mod->first_global)
> + if (ndx < mod->first_global)
> {
> /* main symbol table (locals). */
> tndx = ndx;
> @@ -74,24 +73,61 @@ __libdwfl_module_getsym (Dwfl_Module *mod, int ndx,
> symxndxdata = mod->aux_symxndxdata;
> symstrdata = mod->aux_symstrdata;
> }
> - else if ((size_t) ndx < mod->syments + mod->aux_first_global - skip_aux_zero)
> + else if (ndx < (mod->first_global + mod->aux_first_global
> + + mod->ebl_first_global - skip_aux_zero))
> + {
> + /* ebl symbol lookup (locals). */
> + symdata = NULL;
> + tndx = ndx - (mod->first_global + mod->aux_first_global - skip_aux_zero);
> + }
> + else if ((size_t) ndx < (mod->syments + mod->aux_first_global
> + + mod->ebl_first_global - skip_aux_zero))
> {
> /* main symbol table (globals). */
> - tndx = ndx - mod->aux_first_global + skip_aux_zero;
> + tndx = ndx - (mod->aux_first_global + mod->ebl_first_global
> + - skip_aux_zero);
> file = mod->symfile;
> symdata = mod->symdata;
> symxndxdata = mod->symxndxdata;
> symstrdata = mod->symstrdata;
> }
> - else
> + else if ((size_t) ndx < (mod->syments + mod->aux_syments
> + + mod->ebl_first_global - skip_aux_zero))
> {
> /* aux symbol table (globals). */
> - tndx = ndx - mod->syments + skip_aux_zero;
> + tndx = ndx - (mod->syments + mod->ebl_first_global - skip_aux_zero);
> file = &mod->aux_sym;
> symdata = mod->aux_symdata;
> symxndxdata = mod->aux_symxndxdata;
> symstrdata = mod->aux_symstrdata;
> }
> + else if ((size_t) ndx < (mod->syments + mod->aux_syments
> + + mod->ebl_syments - skip_aux_zero))
> + {
> + /* ebl symbol lookup (globals). */
> + symdata = NULL;
> + tndx = ndx - (mod->syments + mod->aux_syments - skip_aux_zero);
> + }
> + else
> + {
> + /* out of range NDX. */
> + __libdwfl_seterrno (DWFL_E_INVALID_INDEX);
> + return NULL;
> + }
> +
> + if (symdata == NULL)
> + {
> + const char *name = ebl_get_symbol (mod->ebl, tndx, sym, &shndx);
> + if (likely (name != NULL))
> + {
> + if (filep)
> + *filep = &mod->main;
> + return name;
> + }
> + __libdwfl_seterrno (DWFL_E_LIBEBL);
> + return NULL;
> + }
OK, lots of index adjustments/checks that make my head spin, but they
look OK. We better make sure we have some extra tests to check all the
corner cases (see also below, I think tests/dwflsyms might be a good way
to check against a couple of ppc64 ELF files with/without separate
debug/full-symtab/mini-symtab/dynsym-only, etc.
> diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
> index 0c862b3..ee89344 100644
> --- a/libdwfl/libdwflP.h
> +++ b/libdwfl/libdwflP.h
> @@ -89,7 +89,8 @@ typedef struct Dwfl_Process Dwfl_Process;
> DWFL_ERROR (ATTACH_STATE_CONFLICT, N_("Dwfl already has attached state")) \
> DWFL_ERROR (NO_ATTACH_STATE, N_("Dwfl has no attached state")) \
> DWFL_ERROR (NO_UNWIND, N_("Unwinding not supported for this architecture")) \
> - DWFL_ERROR (INVALID_ARGUMENT, N_("Invalid argument"))
> + DWFL_ERROR (INVALID_ARGUMENT, N_("Invalid argument")) \
> + DWFL_ERROR (INVALID_INDEX, N_("invalid section index"))
>
> #define DWFL_ERROR(name, text) DWFL_E_##name,
> typedef enum { DWFL_ERRORS DWFL_E_NUM } Dwfl_Error;
> @@ -172,8 +173,10 @@ struct Dwfl_Module
> Elf_Data *aux_symdata; /* Data in the auxiliary ELF symbol table. */
> size_t syments; /* sh_size / sh_entsize of that section. */
> size_t aux_syments; /* sh_size / sh_entsize of aux_sym section. */
> + size_t ebl_syments; /* Number of symbols from ebl_getsym. */
> int first_global; /* Index of first global symbol of table. */
> int aux_first_global; /* Index of first global of aux_sym table. */
> + int ebl_first_global; /* Index of first global from ebl_getsym. */
> Elf_Data *symstrdata; /* Data for its string table. */
> Elf_Data *aux_symstrdata; /* Data for aux_sym string table. */
> Elf_Data *symxndxdata; /* Data in the extended section index table. */
OK.
> diff --git a/libebl/Makefile.am b/libebl/Makefile.am
> index 4487c5f..1fb3da3 100644
> --- a/libebl/Makefile.am
> +++ b/libebl/Makefile.am
> @@ -54,7 +54,7 @@ gen_SOURCES = eblopenbackend.c eblclosebackend.c eblstrtab.c \
> eblreginfo.c eblnonerelocp.c eblrelativerelocp.c \
> eblsysvhashentrysize.c eblauxvinfo.c eblcheckobjattr.c \
> ebl_check_special_section.c ebl_syscall_abi.c eblabicfi.c \
> - eblstother.c eblinitreg.c
> + eblstother.c eblinitreg.c eblgetsymbol.c
OK.
> diff --git a/libebl/ebl-hooks.h b/libebl/ebl-hooks.h
> index cb52fee..2ad18ec 100644
> --- a/libebl/ebl-hooks.h
> +++ b/libebl/ebl-hooks.h
> @@ -162,5 +162,15 @@ bool EBLHOOK(set_initial_registers_tid) (pid_t tid,
> ebl_tid_registers_t *setfunc,
> void *arg);
>
> +/* See ebl_init_symbols. */
> +void EBLHOOK(init_symbols) (Ebl *ebl, size_t syments, int first_global,
> + GElf_Addr main_bias,
> + ebl_getsym_t *getsym, void *arg,
> + size_t *ebl_symentsp, int *ebl_first_globalp);
> +
> +/* See ebl_get_symbol. */
> +const char *EBLHOOK(get_symbol) (Ebl *ebl, size_t ndx, GElf_Sym *symp,
> + GElf_Word *shndxp);
OK.
> diff --git a/libebl/eblgetsymbol.c b/libebl/eblgetsymbol.c
> [...]
> +void
> +ebl_init_symbols (Ebl *ebl, size_t syments, int first_global,
> + GElf_Addr main_bias, ebl_getsym_t *getsym, void *arg,
> + size_t *ebl_symentsp, int *ebl_first_globalp)
> +{
> + if (ebl == NULL)
> + return;
> + if (ebl->init_symbols == NULL)
> + return;
> + ebl->init_symbols (ebl, syments, first_global, main_bias, getsym, arg,
> + ebl_symentsp, ebl_first_globalp);
> +}
> +
> +const char *
> +ebl_get_symbol (Ebl *ebl, size_t ndx, GElf_Sym *symp, GElf_Word *shndxp)
> +{
> + if (ebl == NULL)
> + return NULL;
> + if (ebl->get_symbol == NULL)
> + return NULL;
> + return ebl->get_symbol (ebl, ndx, symp, shndxp);
> +}
Nitpicks. if (ebl == NULL || ebl->xxx_symbol == NULL) return;
is shorter.
> diff --git a/libebl/libebl.h b/libebl/libebl.h
> +/* Callback type for ebl_init_symbols,
> + it is forwarded to dwfl_module_getsym. */
> +typedef const char *(ebl_getsym_t) (void *arg, int ndx, GElf_Sym *symp,
> + GElf_Word *shndxp)
> + __nonnull_attribute__ (3);
Above you claim to never need the shndx.
> +/* Initialize virtual backend symbol table for EBL->elf currently containing
> + SYMENTS symbols, FIRST_GLOBAL of them are local, EBL->elf is using
> + MAIN_BIAS. GETSYM is a callback to fetch the existing EBL->elf symbols,
> + ARG is an opaque parameter for GETSYM. Fill in *EBL_SYMENTSP with the total
> + number of virtual symbols found, *EBL_FIRST_GLOBALP of them are local.
> + Both return values have to be initialized to zero by caller. */
> +extern void ebl_init_symbols (Ebl *ebl, size_t syments, int first_global,
> + Dwarf_Addr symbias, ebl_getsym_t *getsym,
> + void *arg, size_t *ebl_symentsp,
> + int *ebl_first_globalp)
> + __nonnull_attribute__ (1, 5, 7, 8);
> +
> +/* Return NDXth virtual backend symbol from MOD, store it to *SYM and its
> + section to *SHNDX. Return its name. NDX must be less than *EBL_SYMENTSP
> + returned by init_symbols above. SHNDXP may be NULL. Returned name is valid
> + as long as EBL is valid. */
> +extern const char *ebl_get_symbol (Ebl *ebl, size_t ndx, GElf_Sym *symp,
> + GElf_Word *shndxp)
> + __nonnull_attribute__ (1, 3);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/libebl/libeblP.h b/libebl/libeblP.h
> index 4f4137d..3282050 100644
> --- a/libebl/libeblP.h
> +++ b/libebl/libeblP.h
> @@ -66,6 +66,9 @@ struct ebl
>
> /* Internal data. */
> void *dlhandle;
> +
> + /* Data specific to the backend. */
> + void *backend;
> };
OK.
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index bc97523..ce80a92 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -214,7 +214,8 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh \
> testfile_implicit_pointer.c testfile_implicit_pointer.bz2 \
> testfile_parameter_ref.c testfile_parameter_ref.bz2 \
> testfile_entry_value.c testfile_entry_value.bz2 \
> - testfile_implicit_value.c testfile_implicit_value.bz2
> + testfile_implicit_value.c testfile_implicit_value.bz2 \
> + testfile66.bz2 testfile66.core.bz2
>
> if USE_VALGRIND
> valgrind_cmd='valgrind -q --trace-children=yes --error-exitcode=1 --run-libc-freeres=no'
> diff --git a/tests/run-addrname-test.sh b/tests/run-addrname-test.sh
> index 8624074..bdf8ea6 100755
> --- a/tests/run-addrname-test.sh
> +++ b/tests/run-addrname-test.sh
> @@ -298,6 +298,45 @@ __vdso_time
> ??:0
> EOF
>
> +# .section ".text"
> +# .globl _start
> +# .section ".opd","aw"
> +#_start: .quad .L._start,.TOC.(a)tocbase
> +# .previous
> +# .type _start, @function
> +#.L._start:
> +# .byte 0x7d, 0x82, 0x10, 0x08
> +# .size _start,.-.L._start
> +testfiles testfile66 testfile66.core
> +testrun_compare ${abs_top_builddir}/src/addr2line -S -e testfile66 _start 0x103d0 0x103d3 0x103d4 ._start 0x2d8 0x2db 0x2dc <<\EOF
> +_start
> +??:0
> +_start
> +??:0
> +_start+0x3
> +??:0
> +()+0x103d4
> +??:0
> +._start
> +??:0
> +._start
> +??:0
> +._start+0x3
> +??:0
> +()+0x2dc
> +??:0
> +EOF
> +testrun_compare ${abs_top_builddir}/src/addr2line -S -e testfile66 --core=testfile66.core _start 0x461c03d0 ._start 0x461b02d8 <<\EOF
> +_start
> +??:0
> +_start
> +??:0
> +._start
> +??:0
> +._start
> +??:0
> +EOF
> +
> testfiles testfile69.core testfile69.so
> testrun_compare ${abs_top_builddir}/src/addr2line --core=./testfile69.core -S 0x7f0bc6a33535 0x7f0bc6a33546 <<\EOF
> libstatic+0x9
It would be good to also add a testcase based on run-dwflsym.sh that
shows the generated tables in a couple of cases. That will also test
some of the getsym/addrsym matching. See run-readelf-s.sh how to
generate the example files on ppc.
Thanks,
Mark