On Fri, 20 Dec 2013 15:23:54 +0100, Mark Wielaard wrote:
While playing with the new unwinder interfaces I noticed that I was
writing the same extra wrapper callbacks every time.
Glad you agree the callbacks without lambda functions are a pain. :-)
[...]
--- a/libdwfl/libdwfl.h
+++ b/libdwfl/libdwfl.h
@@ -662,6 +662,17 @@ typedef struct
pid_t (*next_thread) (Dwfl *dwfl, void *dwfl_arg, void **thread_argp)
__nonnull_attribute__ (1);
+ /* Called to get a specific thread. Returns true if there is a
+ thread with the given thread id number, returns false if no such
+ thread exists and may set dwfl_errno in that case. THREAD_ARGP
+ is never NULL. *THREAD_ARGP will be passed to
+ set_initial_registers or thread_detach callbacks together with
+ Dwfl_Thread *thread. This method may be NULL and will then be
+ emulated using the next_thread callback. */
+ bool (*get_thread) (Dwfl *dwfl, pid_t tid, void *dwfl_arg,
+ void **thread_argp)
+ __nonnull_attribute__ (1);
I find this backend extension is not needed. One could just add to
Dwfl_Thread_Callbacks.next_thread
comment/requirement
Thread group leader (the thread with TID == PID) is always reported
first (current Linux kernel always keeps it existing at least as
a zombie).
Current libdwfl/linux-pid-attach.c sure needs an update for it.
It would then also change + simplify dwfl_getthread() implementation.
+
/* Called during unwinding to access memory (stack) state. Returns true for
successfully read *RESULT or false and sets dwfl_errno () on failure.
This method may be NULL - in such case dwfl_thread_getframes will return
@@ -748,6 +759,16 @@ int dwfl_getthreads (Dwfl *dwfl,
void *arg)
__nonnull_attribute__ (1, 2);
+/* Like dwfl_getthreads, but only for one specific thread with the
+ given thead id number if it exists. Returns zero on success,
+ 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 (Dwfl *dwfl, pid_t tid,
+ int (*callback) (Dwfl_Thread *thread, void *arg),
+ void *arg)
+ __nonnull_attribute__ (1, 3);
I am not sure even this function needs to be exported as public.
But this all belongs to your "callbacked" API.
+
/* Iterate through the frames for a thread. Returns zero if all frames
have been processed by the callback, returns -1 on error, or the value of
the callback when not DWARF_CB_OK. -1 returned on error will
@@ -762,6 +783,16 @@ int dwfl_thread_getframes (Dwfl_Thread *thread,
void *arg)
__nonnull_attribute__ (1, 2);
+/* 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.
+ int (*callback) (Dwfl_Frame *thread, void *arg),
+ void *arg)
+ __nonnull_attribute__ (1, 3);
+
/* Return *PC (program counter) for thread-specific frame STATE.
Set *ISACTIVATION according to DWARF frame "activation" definition.
Typically you need to substract 1 from *PC if *ACTIVATION is false to safely
[...]
--- a/libdwfl/linux-pid-attach.c
+++ b/libdwfl/linux-pid-attach.c
@@ -201,6 +201,17 @@ pid_next_thread (Dwfl *dwfl __attribute__ ((unused)), void
*dwfl_arg,
return tid;
}
+/* Just checks that the thread id status file (currently) exists. */
+static bool
+pid_getthread (Dwfl *dwfl __attribute__ ((unused)), pid_t tid,
+ void *dwfl_arg, void **thread_argp)
+{
+ char fbuf[36];
+ snprintf (fbuf, sizeof (fbuf), "/proc/%ld/status", (long) tid);
+ *thread_argp = dwfl_arg;
+ return access (fbuf, R_OK) == 0;
It should be enough to do:
return kill (tid, 0) == 0;
BTW:
* Zombie processes/threads return true (valid) in both cases.
* If a thread group leader (TID==PID) exits it remains a zombie:
gdb/testsuite/gdb.threads/leader-exit.c
+}
+
/* Implement the ebl_set_initial_registers_tid setfunc callback. */
static bool
@@ -260,6 +271,7 @@ pid_thread_detach (Dwfl_Thread *thread, void *thread_arg)
static const Dwfl_Thread_Callbacks pid_thread_callbacks =
{
pid_next_thread,
+ pid_getthread,
It should have been called pid_get_thread as the method is called
(*get_thread).
I see I missed some underscore inconsistencies in the current API, I would
call it
dwfl_getthreads -> dwfl_get_threads
dwfl_thread_getframes -> dwfl_thread_get_frames
dwfl_getthread_frames -> dwfl_get_thread_frames
I can post a patch for it if you agree.
pid_memory_read,
pid_set_initial_registers,
pid_detach,
[...]
Thanks,
Jan