Re: [patch 4/4 v2] unwinder: The unwinder (x86* only)

[Mark Wielaard <mjw at redhat dot com>]

On Thu, 2013-06-13 at 14:45 +0200, Jan Kratochvil wrote:
> > - GCC version check attributes, noclone was introduced with GCC 4.5
> 
> I would prefer "NORETURN", at least sourceware tree uses:
> 	src/include/ansidecl.h:#define ATTRIBUTE_NORETURN __attribute__ ((__noreturn__))

Ah, OK, I think that makes sense, but I might just be missing something
because...

> >   That is for the testcase. But I might have misunderstood. I never
> >   could make the selfdump tests work since it tries to find the
> >   "jmp" symbol (which seems to never be used?).
> 
> make check will not PASS for run-backtrace.sh?  It PASSes for me.
> 
> $ nm tests/backtrace-child|grep -w jmp
> 0000000000000acb t jmp
> 
> Tested on Fedora 18 x86_64 and wit mjw/unwind now also on CentOS 6.3 x86_64.

That is indeed strange, those are similar to the platforms (f18 and
rhel6.4 x86_64) that I also tested on. I assume I had something weird in
my tree. I'll try again with clean trees. But I don't fully understand
why/where jmp is actually used in the test case.

> > - tests/backtrace.c: Drop out of dwfl_getmodules when done.
> >   Just to show that you can return from a callback early and don't have
> >   to iterate over all modules once you found your match.
> 
> I do not agree with this part, one of secondary goals was to verify exactly
> one of the modules patches the filename.  Multiple modules of the same name
> will not be caught by your change.

s/patches/matches/ I assume. OK. I just included it because it looked
like it was doing too much work. But since it is a test case it makes
sense to be paranoid and double check everything.

> > Then I thought I could just write up the rest of the interface with
> > extra callbacks for dwfl_begin when you want state (threads, registers
> > and memory).
> 
> It should be enough to support registers + memory callback.  registers are
> fetched only initially for a new thread so they in fact define a thread.
> [...]
> Besides dwfl_end (which automatically detaches all attached TIDs) there could
> be also
> 	dwfl_detach_thread (Dwfl_Frame_State *state);
> although it would be thread-specific, not frame-specific, so you would like
> more:
> 	typedef struct Dwfl_Thread Dwfl_Thread;
> 	dwfl_detach_thread (Dwfl_Thread *thread);
> So with some iterator across threads one can detach each thread after
> unwinding it.

Yes, and one could inspect the thread (well just look at the tid I
guess) before deciding to "commit" to unwind it. That does mean there is
an extra step going from "process" to "thread" to "unwind", but then you
can delay freezing/attaching to threads you are not interested in. See a
suggested interface that has the above in my other email that crossed
this one.

Cheers,

Mark