On Sun, 2013-12-22 at 16:51 +0100, Jan Kratochvil wrote:
this whole discussion is difficult without seeing how you want to use
it.
I am considering only the eu-stack case as you supplied patch for it.
eu-stack is indeed a somewhat simplistic program that just uses some of
the standard callbacks. It is easiest to think of the libdwfl interfaces
as abstracting a process as a Dwfl with executable segments (which can
be reported in various ways) and state that represents a process as a
whole plus various threads of execution in it (also reported through
various providers).
> Yes, the new callback is not strictly needed, the implementation
allows
> it to be NULL (and then does a fallback search for the tid through the
> existing next_thread callback mechanism). But it might provide a more
> efficient way to access a specific tid of a process. I am not sure how
> it would help to return the thread group leader first. That would only
> help for TID == PID, but not in any other case.
If I have PID=12000 and I do 'eu-stack -p 12001' (its thread TID) then for
elfutils in fact PID is 12001. So if Dwfl_Thread_Callbacks.next_thread
returns 12001 first we can backtrace it and stop.
This just shows a bug in linux-pid-attach. It doesn't properly report
the PID (Tgid). It should only report 12001 as PID if it really is the
thread group leader of the given TID. I'll fix that.
> > I am not sure even this function needs to be exported as
public.
> > But this all belongs to your "callbacked" API.
>
> No, both functions are not strictly necessary. But while writing code
> using the current interface I noticed I always write some form of
> dwfl_getthread to inspect a specific thread and/or dwfl_getthread_frames
> to inspect the frames of a specific thread.
The question was whether you have any use case for dwfl_getthread.
eu-stack uses dwfl_getthread_frames() but not dwfl_getthread.
I cannot imagine when to use dwfl_getthread when there is
dwfl_getthread_frames() available.
Dwfl_Threads can have more state than just their frames. But indeed at
the moment we don't have support for querying most of the other state. I
am fine with postponing exporting it as public function till we do. Done
in updated patch below.
> > > +/* Like dwfl_thread_getframes, but specifying the
thread by its unique
> > > + identifier number. Returns zero if all frames have been processed
> > > + by the callback, returns -1 on error (and when no thread with
> > > + the given thread id number exists), or the value of the callback
> > > + when not DWARF_CB_OK. -1 returned on error will set dwfl_errno ().
*/
> > > +int dwfl_getthread_frames (Dwfl *dwfl, pid_t tid,
> >
> > I do not think TID is needed here. When one wants to backtrace single TID one
> > attaches to just that TID. With the proposed
> > Dwfl_Thread_Callbacks.next_thread change to report PID as first TID we just
> > need to prevent calling Dwfl_Thread_Callbacks.next_thread for the second time.
>
> So you propose to associate a new Dwfl or detach and attach a Dwfl for
> each thread in a process? That seems somewhat counter intuitive to me.
> And it seems unnecessary extra work on the user.
If I want to do something with 'each thread in a process' I am fine with the
current already checked-in API.
I am proposing to associate new Dwfl only with the specific one (possibly
non-leader) thread TID (in such case elfutils will consider that TID as PID
although in reality process group PID is different).
I think that is a weird abstraction to be honest. And it is hard to
support in some providers like the linux-core-attach one for example
that don't get initialized using a PID or TID in the first place. It
doesn't make sense to lie about the actual PID of the process or make
one thread ID special so that calling dwfl_getthread_frames () only
works for one particular TID in the Dwfl process, but not others.
Updated patch attached.
Cheers,
Mark