Re: [patchv4 4/4] Provide virtual symbols for ppc64 function descriptors

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi Mark,

On Wed, 04 Dec 2013 20:02:33 +0100, Mark Wielaard wrote:
> No, I suggest just NOT adding extra names, dot mangled, or not. But just
> keep the function symbol names as is. Just resolve the values (which we
> already adjust anyway so they map into Dwfl address space) without any
> extra magic.

I have found only addr2line as a common binutils<->elfutils application
affected by such change.

Other elfutils utilities (such as eu-readelf or eu-nm) use directly libelf.

And for addr2line/eu-addr2line the behavior from your patch match that of
binutils:

addr2line -S -e tests/testfile66 0x103d0 0x2d8 # without your patch
_start
??:0
()+0x2d8
??:0

addr2line -S -e tests/testfile66 0x103d0 0x2d8 # with your patch
()+0x103d0
??:0
_start
??:0

addr2line -f -e tests/testfile66 0x103d0 0x2d8 # binutils without ppc64 target
_start
??:?
??
??:0

addr2line -f -e tests/testfile66 0x103d0 0x2d8 # binutils with ppc64 target
??
??:0
_start
??:?

The implementation difference is that in binutils the function descriptors
resolving comes from:
	bfd_find_nearest_line_discriminator
	-> elf_find_function
	-> ppc64_elf_maybe_function_sym

while in elfutils libdwfl dwfl_module_getsym deals with GElf_Sym which no
longer represent ELF symbols with your patch, which I find a bit confusing.

There is moreover the confusion that dwfl_module_getsym looks like returning
an ELF symbol table including the first null entry but such returned data no
longer have ELF symbol table semantics.  But that cannot be changed due to
API/ABI compatibility.

binutils additionally provides also bfd_get_synthetic_symtab which contains
the '.entry' addresses.  This was provided in my patch but it is no longer
provided in your patch.  This way GDB can display:

(gdb) info sym 0x0000000010000504
.func2 in section .text
(gdb) info sym 0x0000000010010a40
func2 in section .opd

and addr2line displayed with my patch:

addr2line -S -e tests/testfile66 0x103d0 0x2d8
_start
??:0
._start
??:0

But having this functionality together with your patch address "corruption"
does not seem to be possible with the current elfutils API.  There would be
clashing two symbol names for the same address etc.

Asking the ppc64 ABI authority I found irrelevant.  Our disagreement here is
whether dwfl_module_getsym returns an ELF symbol or not.  You say it is not an
ELF symbol, it is just coincidentally stored in GElf_Sym.  Such disagreement
is apparently outside of the scope of ppc64 ABI.


> As I said before I don't think the ABI for object file link editor
> really matters much in the case of the libdwfl interfaces. The don't
> need extra real synthetic ELF symbols.

The functionality of GDB cannot be achieved with elfutils and your patch.


> If dot-prefixed symbols are
> present as in ET_REL files just leave them alone (kernel modules are
> special anyway). But adding lots of additional synthetic symbols to the
> dwfl_module_*sym* calls where they didn't exist before really is not the
> way to go. We really need something simpler.

binutils does it and thanks to it binutils provides better functionality as it
can resolve addr->sym both addresses, as shown by GDB.


> What we have are functions that match function symbol names and values
> to/from the Dwfl_Module address space layout. For that we already adjust
> the values as found in the ELF files. All we need to do is resolve any
> function descriptor entry values. We don't need to introduce an extra
> synthetic symbol table for that.

By not introducing the extra synthetic symbol table you can no longer resolve
function descriptor addresses to names.


> I wrote a patch to just do that which is attached. I did keep your tests
> and adjusted the expected outputs a little to remove the extra
> dot-prefixed symbols. It probably needs a little more testing, but seems
> to work just fine.

I no longer think it violates the ppc64 ABI.  Utilities like eu-readelf are
not affected by it.  Behavior of command-line utilities affected by your patch
fortunately matches that of binutils, which I find as the excuse for
acceptability of your patch.

I still do not agree with the patch as it provides worse functionality than
the patch of mine without any benefit.  But as it does not seem possible to
convince you to get the better functionality than binutils has I no longer
want to block this patch.


> I'll try it out with your ppc unwinder backend later.

ppc (32-bit) does not use function descriptors.


Regards,
Jan