On Tue, 29 Oct 2013 14:37:02 +0100, Mark Wielaard wrote:
> +/* libdwfl/argp-std.c */
> +#define OPT_COREFILE 0x101
No longer used in this file.
Removed.
> + error (0, 0, "%s", dwfl_errmsg (-1));
> + return 1;
nitpick. libdw.h defines DWARF_CB_ABORT == 1,
which is slightly nicer to use to indicate the callback is done.
Used.
I think you should loose the space between # and the frame number if
possible (left align it, instead of right aligning with width 2, which
is good, as is now). Make sure all addresses are printed with the same
number of digits.
I printed it intentionally differently but no problem with changing it,
changed it.
And just print the adjusted pc directly by default (or
pc if you want to match what p/gstack does). That IMHO gives the output
users would expect (and is easier to parse).
pc_adjusted really cannot be printed, just cannot, it points nowhere, it is
incompatible with everything etc.
So far I have kept the "- 1" output there. elfutils do not completely conform
character-by-character to any existing utility and neither does eu-stack in
other parts of the output.
I find that " - 1" part very useful for users in the output, when looking at
objdump -d etc. And it can be safely ignored if nobody understands that.
If you object to " - 1" one could print for example " (call)" there
instead.
Keep it simple by default.
elfutils should be useful, dropping useful information to make it more close
to existing utilities does not seem correct to me.
You could add an extra flag to print more information about a frame,
like whether or not it is an activation, but I don't think that should
be the default.
If there is no other choice then at least some -v flag would be needed.
// Try to find the address wide if possible.
static int wide = 0;
if (wide == 0)
if (mod)
{
Dwarf_Addr bias;
Elf *elf = dwfl_module_getelf (mod, &bias);
if (elf)
{
GElf_Ehdr ehdr_mem;
GElf_Ehdr *ehdr = gelf_getehdr (elf, &ehdr_mem);
if (ehdr)
wide = ehdr->e_ident[EI_CLASS] == ELFCLASS32 ? 8 : 16;
}
}
if (wide == 0)
wide = 16;
Used. Although s/wide/width/, that was IMO a typo, wasn't it? Plus:
- if (wide == 0)
- if (mod)
+ if (wide == 0 && mod)
+ {
+ .doc = N_("\
+Print a stack for each thread in a process or core file.\n\
+Only real user processes are supported, no kernel or process maps."),
+ .children = children
+ };
+
Used it.
Thanks,
Jan
diff --git a/src/stack.c b/src/stack.c
index 8eebadc..3b8a908 100644
--- a/src/stack.c
+++ b/src/stack.c
@@ -27,9 +27,6 @@
#include <fcntl.h>
#include ELFUTILS_HEADER(dwfl)
-/* libdwfl/argp-std.c */
-#define OPT_COREFILE 0x101
-
static int
frame_callback (Dwfl_Frame *state, void *arg)
{
@@ -39,7 +36,7 @@ frame_callback (Dwfl_Frame *state, void *arg)
if (! dwfl_frame_pc (state, &pc, &isactivation))
{
error (0, 0, "%s", dwfl_errmsg (-1));
- return 1;
+ return DWARF_CB_ABORT;
}
Dwarf_Addr pc_adjusted = pc - (isactivation ? 0 : 1);
@@ -50,7 +47,24 @@ frame_callback (Dwfl_Frame *state, void *arg)
if (mod)
symname = dwfl_module_addrname (mod, pc_adjusted);
- printf ("#%2u %#" PRIx64 "%4s\t%s\n", (*framenop)++, (uint64_t)
pc,
+ // Try to find the address wide if possible.
+ static int width = 0;
+ if (width == 0 && mod)
+ {
+ Dwarf_Addr bias;
+ Elf *elf = dwfl_module_getelf (mod, &bias);
+ if (elf)
+ {
+ GElf_Ehdr ehdr_mem;
+ GElf_Ehdr *ehdr = gelf_getehdr (elf, &ehdr_mem);
+ if (ehdr)
+ width = ehdr->e_ident[EI_CLASS] == ELFCLASS32 ? 8 : 16;
+ }
+ }
+ if (width == 0)
+ width = 16;
+
+ printf ("#%-2u 0x%0*" PRIx64 "%4s\t%s\n", (*framenop)++, width,
(uint64_t) pc,
! isactivation ? "- 1" : "", symname);
return DWARF_CB_OK;
}
@@ -62,8 +76,8 @@ thread_callback (Dwfl_Thread *thread, void *thread_arg __attribute__
((unused)))
unsigned frameno = 0;
switch (dwfl_thread_getframes (thread, frame_callback, &frameno))
{
- case 0:
- case 1:
+ case DWARF_CB_OK:
+ case DWARF_CB_ABORT:
break;
case -1:
error (0, 0, "dwfl_thread_getframes: %s", dwfl_errmsg (-1));
@@ -85,7 +99,20 @@ main (int argc, char **argv)
/* Set locale. */
(void) setlocale (LC_ALL, "");
- struct argp argp = *dwfl_standard_argp ();
+ const struct argp_child children[] =
+ {
+ { .argp = dwfl_standard_argp () },
+ { .argp = NULL },
+ };
+
+ const struct argp argp =
+ {
+ .doc = N_("\
+Print a stack for each thread in a process or core file.\n\
+Only real user processes are supported, no kernel or process maps."),
+ .children = children
+ };
+
int remaining;
Dwfl *dwfl = NULL;
argp_parse (&argp, argc, argv, 0, &remaining, &dwfl);
@@ -101,7 +128,7 @@ main (int argc, char **argv)
switch (dwfl_getthreads (dwfl, thread_callback, NULL))
{
- case 0:
+ case DWARF_CB_OK:
break;
case -1:
error (0, 0, "dwfl_getthreads: %s", dwfl_errmsg (-1));