Re: [patch] Resolve ppc64 func descriptors as .func (via .opd)

[Mark Wielaard <mjw at redhat dot com>]

On Thu, 2012-12-06 at 15:59 +0100, Jan Kratochvil wrote:
> On Thu, 06 Dec 2012 15:27:06 +0100, Mark Wielaard wrote:
> > The root of our disagreement is really just what dwfl_addrsym() is about
> > 1) return just a name associated with a symbol (with possibly the
> > st_value adjusted for the module/code load address, so it matches) from
> > a known symbol table we have at hand, or 2) return a (synthetic) ELF
> > symbol that match some ABI for a particular arch. I think it should be
> > the first
> 
> You "think" but you give no use cases.  Even Roland was asking for "I think
> it's extremely helpful if you give [...] example of the API change you are
> proposing." (although one can find other statements supporting your API).

I admit am somewhat confused what we are now arguing about. Feel free to
skip the questions and just skip to the end of this email to see what I
thought I was saying. We might be talking past each other.

> > So to be explicit. If we are using the current interfaces
> > dwfl_module_addrname () and by extension dwfl_module_addrsym () for
> > returning a symbol name associated with an address then I think we
> > should be as conservative as possible. So that means not fabricating a
> > "synthetic" symbol,
> 
> So what would be the user/caller of such function?

You mean what is the user of the current dwfl_module_addrname () or
dwfl_module_addrsym () functions? Anybody wanting to associate a symbol
name with an address in the dwfl module from the available symbol table.
So the use case is having an address and wanting to know a name
associated with that address and possibly whether it is a function, data
and the offset. Like how you would use in a backtrace (it matches the
"DWARF way" of doing backtraces in gdb).

>   The facts are:
> 
> * Any current caller of dwfl_module_addrsym wants to call the "new" function
>   providing even the synthetic symbols.
> 
> * There is no use/caller of non-synthetic dwfl_module_addrsym.

Could you give examples of these?

> So the solutions are two:
> 
> (1) * Rename dwfl_module_addrsym to dwfl_module_addrsym_pure_elf.
>     * Create some ld script magic to keep ABI - but no API - compatibility.
>     * Create new dwfl_module_codesym function also decoding descriptors.
>     * Modify all the current callers of dwfl_module_addrsym to call
>       dwfl_module_codesym instead.
> 
> (2) * Extend dwfl_module_addrsym to decode also descriptors.

I think I gave a specification of how we should do (2) with a backend
hook that just matches the st_value to a function address.

> > but just return the data from the symbol table that
> 
> Why?  The caller wants more than just the data from the symbol table.

OK, then I am confused about your use case. Could you give an example of
how you want to use this function?

> > The current dwfl_module_addrname () and dwfl_module_addrsym () return
> > the name of that symbol and that seems as good a name as any to return,
> > so we should not tweak it. Nor should we tweak any other symbol values.
> 
> Why?  The caller wants them tweaked.

Because that is the contract of these functions, it seems we disagree on
that then?

> > I don't agree with you that we should generate some synthetic symbols,
> > names, etc, that is just not what these functions do.
> 
> It is not my idea, it is a requirement of existing callers, such as addr2line,
> my backtracer and in fact also any other caller.

I think the name of the symbol as is is fine for these callers.

> > And it opens a whole can of worms which values need to be adjusted so they
> > are "in sync" with the rest of the synthetic value derived from the symbol
> > we have.
> 
> It is nothing unknown, BFD IMO in fact defines the ABI for archs like ppc64
> and BFD generates the synthetic symbols.  You cannot create better ppc64 ABI
> than existing ppc64 ABI, which is defined by BFD.

But we are not talking about confirming to that. At least I am not.

> > Except for adjusting the st_value as we already do that is just
> > too painful and messy IMHO.
> 
> One could find adjusting st_value also messy in such case, bias could be
> applied by the caller.

But it is what is done now.

> > If you really want to return any other different values that describe
> > the function, or a different name or something that matches synthetic
> > symbol in agreement with some ppc64 ABI more closely from what we do
> > now, then we should design a new function.
> 
> That is the solution (1) above; yes, we can go that way.

I rather not.

> > But I don't think that is
> > really necessary. We tried, but the benefits seem not enough and it
> > would be confusing since then users would have two similar, but slightly
> > different functions to call for the same kind of functionality.
> 
> I agree, this is why I find (2) more feasible.  But I am OK with (1).

I am sorry if I gave the impression I would like (1), I don't. I just
tried to explain why.

> > It is
> > IMHO better to just use the current functions so that they return a name
> > also for arches that use function descriptor symbols (and the same name
> > we would return for any other arch), so that users can use the same
> > function on all arches to get the same names.
> 
> Please rephrase this paragraph, it seems to contradict everything you have
> said above so I probably do not understand it correctly.

OK. Lets try again. You said you liked me to be explicit and provide a
specification of what I would like to see implemented. That was this:

> So to be explicit. If we are using the current interfaces
> dwfl_module_addrname () and by extension dwfl_module_addrsym () for
> returning a symbol name associated with an address then I think we
> should be as conservative as possible. So that means not fabricating a
> "synthetic" symbol, but just return the data from the symbol table that
> we have at hand, where only the st_value is adjusted as documented for
> the module load address to match the given address. To make that work
> for architectures that use function descriptors add a backend hook that
> translates the st_value when appropriate if the original value doesn't
> match directly. And that IMHO is really all that is needed.

Everything else was just to explain why I think other options are not
appropriate. The above IMHO provides the simplest semantics for these
functions that are consistent with how they work now and give meaningful
results when used with architectures that happen to use function
descriptors in the symbol table.

Cheers,

Mark