Re: Difficulties while adding local init_regs support to libebl

[Mark Wielaard <mjw at redhat dot com>]

Hi Ben,

On Mon, 2015-09-07 at 15:24 +0200, Ben Gamari wrote:
> > There is dl_iterate_phdr which is a GNU extensions, but some other
> > systems have something similar.
> >
> Right, I actually went ahead and implemented your suggestion after
> stumbling upon this in the manpages.
> [...]
> > Although it wouldn't bad to have such an interface, I don't think it is
> > necessary. See below.
> >
> Too late; already implemented ;).

Nice. That seems an independently useful thing. So please do feel free
to post it to the list for review and inclusion if you like.

> > Attached is a quick patch to implement that for dwfl_linux_proc_attach.
> > I think you could use the same for dwfl_linux_local_attach to make the
> > above possible.
> >
> Ahh, I see. Indeed this seems to fix it. Will this patch be making it upstream?

It does seem useful on itself, so I'll clean up the patch and post it to
the list.

> Unfortunately, there is one more issue that took me far too long to
> realize. The trouble is the indirection that libebl calls experience
> interact poorly with collection of the initial registers.
> 
> Consider the call stack that you would see when collecting initial
> registers,
> 
>     #0  x86_64_set_initial_registers_local () at x86_64_initreg_local.s:18
>     #1  0x00007ffff7bca385 in ebl_set_initial_registers_local (ebl=0x6020a0, setfunc=0x7ffff7bc3e56 <local_thread_state_registers_cb>, arg=0x7fffffffdf50) at eblinitreg.c:55
>     #2  0x00007ffff7bc3f5c in local_set_initial_registers (thread=0x7fffffffdf50, thread_arg=0x0) at linux-local-attach.c:98
>     #3  0x00007ffff7bbff2d in dwfl_thread_getframes (thread=0x7fffffffdf50, callback=0x400c46 <frame_cb>, arg=0x602010) at dwfl_frame.c:428
>     #4  0x00007ffff7bbfe2c in get_one_thread_frames_cb (thread=0x7fffffffdf50, arg=0x7fffffffdfd0) at dwfl_frame.c:392
>     #5  0x00007ffff7bbfd40 in getthread (dwfl=0x602010, tid=27815, callback=0x7ffff7bbfdf6 <get_one_thread_frames_cb>, arg=0x7fffffffdfd0) at dwfl_frame.c:357
>     #6  0x00007ffff7bbfe72 in dwfl_getthread_frames (dwfl=0x602010, tid=27815, callback=0x400c46 <frame_cb>, arg=0x602010) at dwfl_frame.c:401
>     #7  0x0000000000400ece in dump_backtrace () at backtrace-local.c:106
>     #8  0x0000000000400f32 in main (argc=1, argv=0x7fffffffe108) at backtrace-local.c:122
> 
> Where x86_64_set_initial_registers_local is in libebl and is responsible for collecting
> the initial register values and invoking an ebl_tid_registers_t callback
> with them. More specifically, this function records precisely the
> register values that it was entered with.
> 
> Unfortunately, this is idea is dead on arrival (which I didn't realize
> until far too much head-scratching): the stack frames between
> x86_64_set_initial_registers_local and dwfl_thread_getframes are gone by
> the time we try to unwind. We return three times back to
> dwfl_thread_getframes, before calling into the unwinder.

O fun. I also didn't realize this till now... You want to unwind a bit
before starting to unwind so you start unwinding at the correct frame...

> This didn't affect me in GHC since there were no stack frames sitting
> between the initial register collection and the unwinder.
> 
> I see two ways to work around this,
> 
>   1. Implement a small amount of unwinding in the initial register collection
>      code to get back to a stack frame which won't disappear from
>      beneath our feet
> 
>   2. Ensure that there are no stack frames sitting between initial
>      register collection and the unwinder
> 
> Option 1 strikes me as quite complex and fragile.

Yeah. I will think a bit about whether we can have a callback interface
for ebl_set_initial_registers_local so it can reuse the libdwfl
unwinder. I hope it doesn't have to be complex and fragile, but I am not
100% sure yet. The current code isn't really setup for it. But I think
it should be possible. But...

>  Unfortunately the only
> way to implement Option 2 would be to implement a special
> `dwfl_local_getframes` interface, which would be specialized to the
> current architecture. Neither option is particularly appealing.

That might not be so bad. It doesn't really need to a real interface.
Unlike other ebl backend functions this one is very specifically only
for the local arch. There will always be only one implementation for
each libdw.so build. It doesn't have to, and cannot be, cross-arch. So,
although slightly ugly, I don't think I would mind one big ifdef by arch
and one init_regs.c file per arch e.g:

#if defined __i386__
#include init_regs_i386.c
#elif __x86_64__
#include init_regs_x86_64.c
#elif ...
#else
int dwarf_nregs = 0;
Dwarf_Word dwarf_regs[0];
#endif

where the init_regs_arch.c file is just a C/inline asm snippet that
defines the dwarf_nregs and dwarf_regs[]. If dwarf_reg_nr == 0 then we
just return DWFL_E_NO_UNWIND, otherwise we call
dwfl_thread_state_registers (nregs, regs).

Something like the above would be acceptable in this case I think, since
the local init_regs is special.

Cheers,

Mark