This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
Re: [PATCH] libdwfl: Add dwfl_getthread and dwfl_getthread_frames.
- From: Mark Wielaard <mjw at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Mon, 23 Dec 2013 13:54:19 +0100
- Subject: Re: [PATCH] libdwfl: Add dwfl_getthread and dwfl_getthread_frames.
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
>From 84b558329d6e840a28dda58a05cc3ced2ccfa14f Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Fri, 20 Dec 2013 10:09:12 +0100
Subject: [PATCH] libdwfl: Add dwfl_getthread_frames.
dwfl_getthread_frames is a convenience function for when the user is only
interested in one specific thread id of a process. It can be implemented by
a simple wrapper function that removes an extra callback layer just to
filter on thread id. But it also provides an optimized path to getting
access to just one particular Dwfl_Thread of the Dwfl process by providing
and (optional) new callback for the state provider. The pid_thread_callbacks
now provide an (optional) pid_getthread that doesn't need to travers all
threads anymore. Which is implemented for the linux-pid-attach provider.
stack now uses this to implement a new '-1' option that shows just one
specific thread of a process.
Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
ChangeLog | 5 ++
NEWS | 5 ++-
libdw/ChangeLog | 4 ++
libdw/libdw.map | 1 +
libdwfl/ChangeLog | 16 ++++++++
libdwfl/dwfl_frame.c | 87 +++++++++++++++++++++++++++++++++++++++++++
libdwfl/libdwfl.h | 21 ++++++++++
libdwfl/libdwflP.h | 1 +
libdwfl/linux-core-attach.c | 1 +
libdwfl/linux-pid-attach.c | 10 +++++
src/ChangeLog | 7 +++
src/stack.c | 46 ++++++++++++++++++-----
tests/backtrace-data.c | 1 +
13 files changed, 194 insertions(+), 11 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index fe109e1..a0ce570 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2013-12-20 Mark Wielaard <mjw@redhat.com>
+
+ * NEWS (libdwfl): Add dwfl_getthread_frames.
+ (stack): New entry.
+
2013-12-18 Mark Wielaard <mjw@redhat.com>
* NEWS (libdwfl): Add dwfl_module_getsym_info and
diff --git a/NEWS b/NEWS
index 49510f7..46b7249 100644
--- a/NEWS
+++ b/NEWS
@@ -7,10 +7,13 @@ libdwfl: dwfl_core_file_report has new parameter executable.
Dwfl_Thread and Dwfl_Frame and functions dwfl_attach_state,
dwfl_pid, dwfl_thread_dwfl, dwfl_thread_tid, dwfl_frame_thread,
dwfl_thread_state_registers, dwfl_thread_state_register_pc,
- dwfl_getthreads, dwfl_thread_getframes and dwfl_frame_pc.
+ dwfl_getthread_frames, dwfl_getthreads, dwfl_thread_getframes
+ and dwfl_frame_pc.
addr2line: New option -x to show the section an address was found in.
+stack: New utility that uses the new unwinder for processes and cores.
+
Version 0.157
libdw: Add new functions dwarf_getlocations, dwarf_getlocation_attr
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index ef3ed57..2b67759 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,7 @@
+2013-12-20 Mark Wielaard <mjw@redhat.com>
+
+ * libdw.map (ELFUTILS_0.158): Add dwfl_getthread_frames.
+
2013-12-18 Mark Wielaard <mjw@redhat.com>
* libdw.map (ELFUTILS_0.158): Remove dwfl_module_addrsym_elf and
diff --git a/libdw/libdw.map b/libdw/libdw.map
index 92392bc..08c4ddc 100644
--- a/libdw/libdw.map
+++ b/libdw/libdw.map
@@ -280,6 +280,7 @@ ELFUTILS_0.158 {
dwfl_frame_thread;
dwfl_thread_state_registers;
dwfl_thread_state_register_pc;
+ dwfl_getthread_frames;
dwfl_getthreads;
dwfl_thread_getframes;
dwfl_frame_pc;
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index fa605bd..d8ca9eb 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,5 +1,21 @@
2013-12-20 Mark Wielaard <mjw@redhat.com>
+ * dwfl_frame.c (one_arg): New struct.
+ (get_one_thread_cb): New function.
+ (dwfl_getthread): Likewise.
+ (one_thread): New struct.
+ (get_one_thread_frames_cb): New function.
+ (dwfl_getthread_frames): Likewise.
+ * libdwfl.h (Dwfl_Thread_Callbacks): Add get_thread function.
+ (dwfl_getthread_frames): Likewise.
+ * libdwflP.h (dwfl_getthread_frames): New internal function declaration.
+ * linux-core-attach.c (core_thread_callbacks): Initialize get_thread
+ to NULL.
+ * linux-pid-attach.c (pid_getthread): New function.
+ (pid_thread_callbacks): Initialize get_thread to pid_getthread.
+
+2013-12-20 Mark Wielaard <mjw@redhat.com>
+
* linux-kernel-modules.c (report_kernel_archive): Correct nested
asprintf result check for debug.a.
diff --git a/libdwfl/dwfl_frame.c b/libdwfl/dwfl_frame.c
index 4a7b3cd..538f4b0 100644
--- a/libdwfl/dwfl_frame.c
+++ b/libdwfl/dwfl_frame.c
@@ -273,6 +273,93 @@ dwfl_getthreads (Dwfl *dwfl, int (*callback) (Dwfl_Thread *thread, void *arg),
}
INTDEF(dwfl_getthreads)
+struct one_arg
+{
+ pid_t tid;
+ bool seen;
+ int (*callback) (Dwfl_Thread *thread, void *arg);
+ void *arg;
+};
+
+static int
+get_one_thread_cb (Dwfl_Thread *thread, void *arg)
+{
+ struct one_arg *oa = (struct one_arg *) arg;
+ if (! oa->seen && INTUSE(dwfl_thread_tid) (thread) == oa->tid)
+ {
+ oa->seen = true;
+ return oa->callback (thread, oa->arg);
+ }
+
+ return DWARF_CB_OK;
+}
+
+/* Note not currently exported, will be when there are more Dwfl_Thread
+ properties to query. Use dwfl_getthread_frames for now directly. */
+static int
+dwfl_getthread (Dwfl *dwfl, pid_t tid,
+ int (*callback) (Dwfl_Thread *thread, void *arg),
+ void *arg)
+{
+ Dwfl_Process *process = dwfl->process;
+ if (process == NULL)
+ {
+ __libdwfl_seterrno (dwfl->process_attach_error);
+ return -1;
+ }
+
+ if (process->callbacks->get_thread != NULL)
+ {
+ Dwfl_Thread thread;
+ thread.process = process;
+ thread.unwound = NULL;
+ thread.callbacks_arg = NULL;
+
+ if (process->callbacks->get_thread (dwfl, tid, process->callbacks_arg,
+ &thread.callbacks_arg))
+ {
+ int err;
+ thread.tid = tid;
+ err = callback (&thread, arg);
+ thread_free_all_states (&thread);
+ return err;
+ }
+
+ return -1;
+ }
+
+ struct one_arg oa = { .tid = tid, .callback = callback,
+ .arg = arg, .seen = false };
+ int err = INTUSE(dwfl_getthreads) (dwfl, get_one_thread_cb, &oa);
+ if (err == DWARF_CB_OK && ! oa.seen)
+ return -1;
+
+ return err;
+}
+
+struct one_thread
+{
+ int (*callback) (Dwfl_Frame *frame, void *arg);
+ void *arg;
+};
+
+static int
+get_one_thread_frames_cb (Dwfl_Thread *thread, void *arg)
+{
+ struct one_thread *ot = (struct one_thread *) arg;
+ return INTUSE(dwfl_thread_getframes) (thread, ot->callback, ot->arg);
+}
+
+int
+dwfl_getthread_frames (Dwfl *dwfl, pid_t tid,
+ int (*callback) (Dwfl_Frame *frame, void *arg),
+ void *arg)
+{
+ struct one_thread ot = { .callback = callback, .arg = arg };
+ return dwfl_getthread (dwfl, tid, get_one_thread_frames_cb, &ot);
+}
+INTDEF(dwfl_getthread_frames)
+
int
dwfl_thread_getframes (Dwfl_Thread *thread,
int (*callback) (Dwfl_Frame *state, void *arg),
diff --git a/libdwfl/libdwfl.h b/libdwfl/libdwfl.h
index 007089b..f1c0052 100644
--- 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);
+
/* 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
@@ -762,6 +773,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,
+ 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
diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
index 267b021..63615a8 100644
--- a/libdwfl/libdwflP.h
+++ b/libdwfl/libdwflP.h
@@ -702,6 +702,7 @@ INTDECL (dwfl_thread_tid)
INTDECL (dwfl_frame_thread)
INTDECL (dwfl_thread_state_registers)
INTDECL (dwfl_thread_state_register_pc)
+INTDECL (dwfl_getthread_frames)
INTDECL (dwfl_getthreads)
INTDECL (dwfl_thread_getframes)
INTDECL (dwfl_frame_pc)
diff --git a/libdwfl/linux-core-attach.c b/libdwfl/linux-core-attach.c
index f55faf7..f259b2c 100644
--- a/libdwfl/linux-core-attach.c
+++ b/libdwfl/linux-core-attach.c
@@ -288,6 +288,7 @@ core_detach (Dwfl *dwfl __attribute__ ((unused)), void *dwfl_arg)
static const Dwfl_Thread_Callbacks core_thread_callbacks =
{
core_next_thread,
+ NULL, /* get_thread */
core_memory_read,
core_set_initial_registers,
core_detach,
diff --git a/libdwfl/linux-pid-attach.c b/libdwfl/linux-pid-attach.c
index 3d0716a..4932ef7 100644
--- a/libdwfl/linux-pid-attach.c
+++ b/libdwfl/linux-pid-attach.c
@@ -201,6 +201,15 @@ pid_next_thread (Dwfl *dwfl __attribute__ ((unused)), void *dwfl_arg,
return tid;
}
+/* Just checks that the thread id exists. */
+static bool
+pid_getthread (Dwfl *dwfl __attribute__ ((unused)), pid_t tid,
+ void *dwfl_arg, void **thread_argp)
+{
+ *thread_argp = dwfl_arg;
+ return kill (tid, 0) == 0;
+}
+
/* Implement the ebl_set_initial_registers_tid setfunc callback. */
static bool
@@ -260,6 +269,7 @@ pid_thread_detach (Dwfl_Thread *thread, void *thread_arg)
static const Dwfl_Thread_Callbacks pid_thread_callbacks =
{
pid_next_thread,
+ pid_getthread,
pid_memory_read,
pid_set_initial_registers,
pid_detach,
diff --git a/src/ChangeLog b/src/ChangeLog
index 5722a50..0e6d2dc 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,10 @@
+2013-12-20 Mark Wielaard <mjw@redhat.com>
+
+ * stack.c (show_one_tid): New static boolean.
+ (parse_opt): Handle '-1'.
+ (main): Add -1 to options. Call dwfl_getthread_frames when
+ show_one_tid is true.
+
2013-12-18 Mark Wielaard <mjw@redhat.com>
* addr2line.c (options): Add symbol-sections, 'x'.
diff --git a/src/stack.c b/src/stack.c
index 242d393..512c85b 100644
--- a/src/stack.c
+++ b/src/stack.c
@@ -39,6 +39,7 @@ ARGP_PROGRAM_BUG_ADDRESS_DEF = PACKAGE_BUGREPORT;
static bool show_activation = false;
static bool show_module = false;
static bool show_source = false;
+static bool show_one_tid = false;
static int
frame_callback (Dwfl_Frame *state, void *arg)
@@ -170,6 +171,10 @@ parse_opt (int key, char *arg __attribute__ ((unused)),
show_activation = show_source = show_module = true;
break;
+ case '1':
+ show_one_tid = true;
+ break;
+
default:
return ARGP_ERR_UNKNOWN;
}
@@ -197,6 +202,8 @@ main (int argc, char **argv)
N_("Additionally show source file information"), 0 },
{ "verbose", 'v', NULL, 0,
N_("Show all additional information (activation, module and source)"), 0 },
+ { NULL, '1', NULL, 0,
+ N_("Show the backtrace of only one thread"), 0 },
{ NULL, 0, NULL, 0, NULL, 0 }
};
@@ -221,23 +228,42 @@ Only real user processes are supported, no kernel or process maps."),
argp_parse (&argp, argc, argv, 0, &remaining, &dwfl);
assert (dwfl != NULL);
if (remaining != argc)
- error (2, 0, "eu-stack [-a] [-m] [-s] [-v] [--debuginfo-path=<path>] {-p <process id>|"
- "--core=<file> [--executable=<file>]|--help}");
+ error (2, 0, "eu-stack [-a] [-m] [-s] [-v] [-1] [--debuginfo-path=<path>]"
+ " {-p <process id>|--core=<file> [--executable=<file>]|--help}");
/* dwfl_linux_proc_report has been already called from dwfl_standard_argp's
parse_opt function. */
if (dwfl_report_end (dwfl, NULL, NULL) != 0)
error (2, 0, "dwfl_report_end: %s", dwfl_errmsg (-1));
- switch (dwfl_getthreads (dwfl, thread_callback, NULL))
+ if (show_one_tid)
{
- case DWARF_CB_OK:
- break;
- case -1:
- error (0, 0, "dwfl_getthreads: %s", dwfl_errmsg (-1));
- break;
- default:
- abort ();
+ pid_t tid = dwfl_pid (dwfl);
+ unsigned frameno = 0;
+ switch (dwfl_getthread_frames (dwfl, tid, frame_callback, &frameno))
+ {
+ case DWARF_CB_OK:
+ break;
+ case -1:
+ error (0, 0, "dwfl_getthread_frames (%d): %s", tid,
+ dwfl_errmsg (-1));
+ break;
+ default:
+ abort ();
+ }
+ }
+ else
+ {
+ switch (dwfl_getthreads (dwfl, thread_callback, NULL))
+ {
+ case DWARF_CB_OK:
+ break;
+ case -1:
+ error (0, 0, "dwfl_getthreads: %s", dwfl_errmsg (-1));
+ break;
+ default:
+ abort ();
+ }
}
dwfl_end (dwfl);
diff --git a/tests/backtrace-data.c b/tests/backtrace-data.c
index 6d21345..9fa3c4a 100644
--- a/tests/backtrace-data.c
+++ b/tests/backtrace-data.c
@@ -205,6 +205,7 @@ set_initial_registers (Dwfl_Thread *thread,
static const Dwfl_Thread_Callbacks callbacks =
{
next_thread,
+ NULL, /* get_thread */
memory_read,
set_initial_registers,
NULL, /* detach */
--
1.7.1