dwfl_attach_state alternative taking Ebl?

dwfl_attach_state alternative taking Ebl?

[Milian Wolff]

Hey all,

would you be willing to accept a patch that adds an alternative to 
dwfl_attach_state taking an Ebl pointer instead of the Elf* to guess the 
architecture from?

This would simplify the code for cases where we know the architecture, but 
don't necessarily have a corresponding Elf* at hand (yet). If there'd be a 
version that takes the Ebl pointer directly, one could use e.g. 
ebl_openbackend_emulation or _machine to find the backend handle for the known 
target architecture directly.

If this would be acceptable, I will write a patch up for this. I would need 
suggestions for a fitting name though, in C++ I'd simply use an overload but 
here with plain C... ;-)

Thanks
-- 
Milian Wolff
mail@milianw.de
http://milianw.de

Re: dwfl_attach_state alternative taking Ebl?

[Mark Wielaard]

Hi Milian,

On Wed, 2017-03-29 at 16:50 +0200, Milian Wolff wrote:
> would you be willing to accept a patch that adds an alternative to 
> dwfl_attach_state taking an Ebl pointer instead of the Elf* to guess the 
> architecture from?

I rather not expose an Ebl handle to the public interface.
It is really an internal interface that we can change anytime.
There is also no way a user can create or get at an Ebl handle using the
public interfaces.

> This would simplify the code for cases where we know the architecture, but 
> don't necessarily have a corresponding Elf* at hand (yet). If there'd be a 
> version that takes the Ebl pointer directly, one could use e.g. 
> ebl_openbackend_emulation or _machine to find the backend handle for the known 
> target architecture directly.
> 
> If this would be acceptable, I will write a patch up for this. I would need 
> suggestions for a fitting name though, in C++ I'd simply use an overload but 
> here with plain C... ;-)

Would it help your use case if there was a dwfl_init_state (Dwfl *dwfl,
int e_machine, unsigned char ei_class, unsigned char ei_data, ...)?

And what exactly is your use case? Maybe we can come up with a better
interface.

Thanks,

Mark

Re: dwfl_attach_state alternative taking Ebl?

[Milian Wolff]

[-- Attachment #1: Type: text/plain, Size: 2032 bytes --]

On Mittwoch, 29. März 2017 21:48:08 CEST Mark Wielaard wrote:
> Hi Milian,
> 
> On Wed, 2017-03-29 at 16:50 +0200, Milian Wolff wrote:
> > would you be willing to accept a patch that adds an alternative to
> > dwfl_attach_state taking an Ebl pointer instead of the Elf* to guess the
> > architecture from?
> 
> I rather not expose an Ebl handle to the public interface.
> It is really an internal interface that we can change anytime.
> There is also no way a user can create or get at an Ebl handle using the
> public interfaces.

Ah right, I only looked at the implementation of dwfl_attach_state and related 
sources without reading the comment saying that Ebl is internal. No-go then.

> > This would simplify the code for cases where we know the architecture, but
> > don't necessarily have a corresponding Elf* at hand (yet). If there'd be a
> > version that takes the Ebl pointer directly, one could use e.g.
> > ebl_openbackend_emulation or _machine to find the backend handle for the
> > known target architecture directly.
> > 
> > If this would be acceptable, I will write a patch up for this. I would
> > need
> > suggestions for a fitting name though, in C++ I'd simply use an overload
> > but here with plain C... ;-)
> 
> Would it help your use case if there was a dwfl_init_state (Dwfl *dwfl,
> int e_machine, unsigned char ei_class, unsigned char ei_data, ...)?

What magic values do I pass to e_machine, ei_class, ei_data? I guess the ebl 
API that takes the Elf architecture or archicture name would be better.

> And what exactly is your use case? Maybe we can come up with a better
> interface.

The use-case is parsing profiler data, e.g. in perfparser by Ulf / TQC. We 
don't mess with Elf* anywhere, but need it to let dwfl_attach_state figure out 
the target architecture. We do know the architecture already so this is a lot 
of jumping through hoops, to find a fitting Elf* that can be used for dwfl 
then...

-- 
Milian Wolff
mail@milianw.de
http://milianw.de

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: dwfl_attach_state alternative taking Ebl?

[Mark Wielaard]

On Wed, 2017-03-29 at 23:57 +0200, Milian Wolff wrote:
> On Mittwoch, 29. März 2017 21:48:08 CEST Mark Wielaard wrote:
> > Would it help your use case if there was a dwfl_init_state (Dwfl *dwfl,
> > int e_machine, unsigned char ei_class, unsigned char ei_data, ...)?
> 
> What magic values do I pass to e_machine, ei_class, ei_data?

That would be one of the EM e_machine architecture constants, ELFCLASS32
or ELFCLASS64 for ei_class and ELFDATA2LSB or ELFDATA2MSB for ei_data.
(e_machine could arguably be a GElf_Half).

> I guess the ebl  API that takes the Elf architecture or archicture
> name would be better.

I think we should extend the ebl_openbackend calls with a variant that
takes all three machine/class/data constants. If you look at the
machines table in libebl/eblopenbackend.c you see that given just the EM
architecture constant or (emulation) name without an Elf handle given we
cannot distinguish between e.g. ppc64 (EM_PPC64/ELFCLASS64/ELFDATA2MSB)
and ppc64le (EM_PPC64/ELFCLASS64/ELFDATA2LSB). You may obviously counter
that just means that table isn't complete. But then we have to document
(and maybe export?) the emulation names that people can rely on. Which
is why I was suggesting we rely on the machine/class/data triple to
uniquely identify the architecture. Maybe that is inconvenient though?

> > And what exactly is your use case? Maybe we can come up with a better
> > interface.
> 
> The use-case is parsing profiler data, e.g. in perfparser by Ulf / TQC. We 
> don't mess with Elf* anywhere, but need it to let dwfl_attach_state figure out 
> the target architecture. We do know the architecture already so this is a lot 
> of jumping through hoops, to find a fitting Elf* that can be used for dwfl 
> then...

OK. How do you know the Elf architecture in that case? How and by what
is it given? Is that an EM constant or some architecture string?

Cheers,

Mark

Re: dwfl_attach_state alternative taking Ebl?

[Milian Wolff]

On Thursday, March 30, 2017 12:57:33 PM CEST Mark Wielaard wrote:
> On Wed, 2017-03-29 at 23:57 +0200, Milian Wolff wrote:
> > On Mittwoch, 29. März 2017 21:48:08 CEST Mark Wielaard wrote:
> > > Would it help your use case if there was a dwfl_init_state (Dwfl *dwfl,
> > > int e_machine, unsigned char ei_class, unsigned char ei_data, ...)?
> > 
> > What magic values do I pass to e_machine, ei_class, ei_data?
> 
> That would be one of the EM e_machine architecture constants, ELFCLASS32
> or ELFCLASS64 for ei_class and ELFDATA2LSB or ELFDATA2MSB for ei_data.
> (e_machine could arguably be a GElf_Half).
> 
> > I guess the ebl  API that takes the Elf architecture or archicture
> > name would be better.
> 
> I think we should extend the ebl_openbackend calls with a variant that
> takes all three machine/class/data constants. If you look at the
> machines table in libebl/eblopenbackend.c you see that given just the EM
> architecture constant or (emulation) name without an Elf handle given we
> cannot distinguish between e.g. ppc64 (EM_PPC64/ELFCLASS64/ELFDATA2MSB)
> and ppc64le (EM_PPC64/ELFCLASS64/ELFDATA2LSB). You may obviously counter
> that just means that table isn't complete. But then we have to document
> (and maybe export?) the emulation names that people can rely on. Which
> is why I was suggesting we rely on the machine/class/data triple to
> uniquely identify the architecture. Maybe that is inconvenient though?

Ah ok, such a triple would be OK as well, I guess. But see below.

> > > And what exactly is your use case? Maybe we can come up with a better
> > > interface.
> > 
> > The use-case is parsing profiler data, e.g. in perfparser by Ulf / TQC. We
> > don't mess with Elf* anywhere, but need it to let dwfl_attach_state figure
> > out the target architecture. We do know the architecture already so this
> > is a lot of jumping through hoops, to find a fitting Elf* that can be
> > used for dwfl then...
> 
> OK. How do you know the Elf architecture in that case? How and by what
> is it given? Is that an EM constant or some architecture string?

In our case we either get it from perf, or the user specifies it directly on 
the command line. In both cases it is a string like "x86_64", "arm" or 
"aarch64". We map that to a list of architectures we know about, see e.g.:

http://code.qt.io/cgit/qt-creator/perfparser.git/tree/app/
perfregisterinfo.h#n29

So, any API that would allow us to map these architectures directly to a dwfl/
elf counterpart would simplify our code, or at least would make it easier to 
understand, as we wouldn't have to wait for an Elf file we can open before 
calling dwfl_attach_state.

Bye

-- 
Milian Wolff
mail@milianw.de
http://milianw.de

Re: dwfl_attach_state alternative taking Ebl?

[Mark Wielaard]

On Thu, 2017-03-30 at 13:14 +0200, Milian Wolff wrote:
> > OK. How do you know the Elf architecture in that case? How and by what
> > is it given? Is that an EM constant or some architecture string?
> 
> In our case we either get it from perf, or the user specifies it directly on 
> the command line. In both cases it is a string like "x86_64", "arm" or 
> "aarch64". We map that to a list of architectures we know about, see e.g.:
> 
> http://code.qt.io/cgit/qt-creator/perfparser.git/tree/app/
> perfregisterinfo.h#n29
> 
> So, any API that would allow us to map these architectures directly to a dwfl/
> elf counterpart would simplify our code, or at least would make it easier to 
> understand, as we wouldn't have to wait for an Elf file we can open before 
> calling dwfl_attach_state.

So you map from simple architecture name like "x86" or "powerpc". But
what mechanism do you have to whether that is 32 or 64 bit, and big or
little endian?

Thanks,

Mark

Re: dwfl_attach_state alternative taking Ebl?

[Ulf Hermann]

> So you map from simple architecture name like "x86" or "powerpc". But
> what mechanism do you have to whether that is 32 or 64 bit, and big or
> little endian?

The code can be extended to provide that information. We know it in advance, but didn't need it so far.

Ulf

Re: dwfl_attach_state alternative taking Ebl?

[Milian Wolff]

On Wednesday, April 5, 2017 2:46:34 PM CEST Mark Wielaard wrote:
> On Thu, 2017-03-30 at 13:14 +0200, Milian Wolff wrote:
> > > OK. How do you know the Elf architecture in that case? How and by what
> > > is it given? Is that an EM constant or some architecture string?
> > 
> > In our case we either get it from perf, or the user specifies it directly
> > on the command line. In both cases it is a string like "x86_64", "arm" or
> > "aarch64". We map that to a list of architectures we know about, see
> > e.g.:
> > 
> > http://code.qt.io/cgit/qt-creator/perfparser.git/tree/app/
> > perfregisterinfo.h#n29
> > 
> > So, any API that would allow us to map these architectures directly to a
> > dwfl/ elf counterpart would simplify our code, or at least would make it
> > easier to understand, as we wouldn't have to wait for an Elf file we can
> > open before calling dwfl_attach_state.
> 
> So you map from simple architecture name like "x86" or "powerpc". But
> what mechanism do you have to whether that is 32 or 64 bit, and big or
> little endian?

As Ulf said, we can add the required mappings. So from my side, I guess your 
approach with the three machine/class/data constants would be a good 
improvement already.

Cheers

-- 
Milian Wolff
mail@milianw.de
http://milianw.de

Re: dwfl_attach_state alternative taking Ebl?

[Mark Wielaard]

On Wed, 2017-04-05 at 15:04 +0200, Milian Wolff wrote:
> On Wednesday, April 5, 2017 2:46:34 PM CEST Mark Wielaard wrote:
> > On Thu, 2017-03-30 at 13:14 +0200, Milian Wolff wrote:
> > > > OK. How do you know the Elf architecture in that case? How and by what
> > > > is it given? Is that an EM constant or some architecture string?
> > > 
> > > In our case we either get it from perf, or the user specifies it directly
> > > on the command line. In both cases it is a string like "x86_64", "arm" or
> > > "aarch64". We map that to a list of architectures we know about, see
> > > e.g.:
> > > 
> > > http://code.qt.io/cgit/qt-creator/perfparser.git/tree/app/
> > > perfregisterinfo.h#n29
> > > 
> > > So, any API that would allow us to map these architectures directly to a
> > > dwfl/ elf counterpart would simplify our code, or at least would make it
> > > easier to understand, as we wouldn't have to wait for an Elf file we can
> > > open before calling dwfl_attach_state.
> > 
> > So you map from simple architecture name like "x86" or "powerpc". But
> > what mechanism do you have to whether that is 32 or 64 bit, and big or
> > little endian?
> 
> As Ulf said, we can add the required mappings. So from my side, I guess your 
> approach with the three machine/class/data constants would be a good 
> improvement already.

OK, so we could add a dwfl_attach_arch_state which is almost like
dwfl_attach_state except instead of an ELF it would take machine, class,
data.

bool dwfl_attach_arch_state (Dwfl *dwfl, GElf_Half e_machine,
                             unsigned char ei_class,
                             unsigned char ei_data, pid_t pid,
                             const Dwfl_Thread_Callbacks *thread_callbacks,
                             void *dwfl_arg);

But before we do I like to understand your use case a little better, to
make sure it is really necessary (once we add a public interface we are
stuck with it forever).

First note that providing an Elf handle to dwfl_attach_arch_state isn't
necessary if the Dwfl already has Dwfl_Modules, then it can guess the
architecture from the Elf associated with the Dwfl_Modules. From the
documentation:

/* [...] Architecture of DWFL  
   modules is specified by ELF, ELF must remain valid during DWFL lifetime.     
   Use NULL ELF to detect architecture from DWFL, the function will then detect
   it from arbitrary Dwfl_Module of DWFL.  [...]  */

So would that be an alternative for you? How do you create the Dwfl? Do
you add modules to it (how?) and when do you need to call
dwfl_attach_state?

Thanks,

Mark

Re: dwfl_attach_state alternative taking Ebl?

[Ulf Hermann]

> /* [...] Architecture of DWFL
>    modules is specified by ELF, ELF must remain valid during DWFL lifetime.
>    Use NULL ELF to detect architecture from DWFL, the function will then detect
>    it from arbitrary Dwfl_Module of DWFL.  [...]  */
>
> So would that be an alternative for you? How do you create the Dwfl? Do
> you add modules to it (how?) and when do you need to call
> dwfl_attach_state?

This is what we do. We parse perf.data. perf.data may contain 
information about file mappings at any point, and it may take a while 
until the first valid one shows up. So we postpone dwfl_attach_state 
until we have a mapping we can resolve to a local elf file.

The code would be cleaner if we could attach_state before starting to 
parse. And there might be pathological cases where no valid elf file can 
ever be found but we can still unwind by frame pointer.

br,
Ulf

Re: dwfl_attach_state alternative taking Ebl?

[Mark Wielaard]

On Tue, 2017-04-11 at 14:27 +0200, Ulf Hermann wrote:
> The code would be cleaner if we could attach_state before starting to 
> parse. And there might be pathological cases where no valid elf file can 
> ever be found but we can still unwind by frame pointer.

Another reason to try and get the frame pointer based unwinder
upstream :)

Cheers,

Mark