[PATCH] stack: Add new '-n MAXFRAMES' option. Resolve addresses after unwind.

Mark Wielaard mjw at redhat.com
Thu Jan 2 22:52:05 UTC 2014


On Thu, Jan 02, 2014 at 08:45:47PM +0100, Jan Kratochvil wrote:
> On Mon, 23 Dec 2013 11:21:19 +0100, Mark Wielaard wrote:
> > Limit the number of frames printed per thread (defaults to 64)
> 
> The new option is sure OK but any non-infinite default is not right:

I don't agree with the infinite default, there should be a limit by
default, but I agree with everything else.
 
>  * Normal applications have deeper stack than 64.  When I broke into my
>    GTK running app it has 103 frames: http://people.redhat.com/jkratoch/app.bt

Yeah, 64 is silly small. Used 2048 now.

>  * Applications with deeper stack just take longer to unwind, that's normal.
>    If there is a special use case where target application should not be
>    stopped for a noticeable time then the '-n' option is perfect.
>    But this is not the default case.

It shouldn't be unlimited, but a large limit is fine.

>  * Such limit makes IMO sense only for runaway apps with "infinite" backtrace.
>    Such applications have probably already crashed (I am backtracing a core)
>    or the application is no longer responsive.  In such case:
>     * It does not matter if eu-stack takes slightly longer or not.
>     * I really need the full unwind as only the top of stack can disclose
>       why the infinite deadlock occured.
>    Primarily I still miss a use case for this default of non-infinite limit.

The backtrace could just be bad causing us to endlessly unwind.
It isn't reasonable to show infinite backtraces, but it should indeed
be possible to show a finite backtrace however large it is.

>  * It does not warn that it stopped the unwind due to such a limit:
>    #62 0x000000000040054e bt
>    #63 0x000000000040054e bt
>    $ _ (rc = 0)

Right, it returns 1 now like any other non-fatal error that caused the
code to possibly not show the whole stack.

>  * One cannot easily make the unwind unlimited:
>    $ stack -p $p -n 0
>    stack: -n MAXFRAMES should be 1 or higher.
>    Try `stack --help' or `stack --usage' for more information.
>    $ ./src/stack -p $p -n -1
>    Segmentation fault
>    Program received signal SIGSEGV, Segmentation fault.
>    0x0000000000402974 in main (argc=5, argv=0x7fffffffd978) at stack.c:530
>    528    struct frames *frames = malloc (sizeof (struct frames)
>    529                                    + sizeof (struct frame) * maxframes);
>    530 -> frames->frames = 0;

Clearly there was a NULL check missing. Also added support for -n 0 to do
make it unlimited (it will still hit a memory error if there really are
infinite frames).

Thanks,

Mark
-------------- next part --------------
>From 6034e2f4366c6ac1c02aa2c795db480e7fc9b655 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw at redhat.com>
Date: Thu, 2 Jan 2014 22:31:07 +0100
Subject: [PATCH] stack: Allow -n 0 for unlimited frames. Make default 2048
 frames.

Signed-off-by: Mark Wielaard <mjw at redhat.com>
---
 src/ChangeLog | 10 ++++++++++
 src/stack.c   | 46 +++++++++++++++++++++++++++++++---------------
 2 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index 811004d..6a915a1 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,13 @@
+2014-01-02  Mark Wielaard  <mjw at redhat.com>
+
+	* stack.c (maxframes): Increase to 2048.
+	(struct frames): Add allocated field.
+	(frame_callback): If frames used is frames allocated, realloc.
+	(print_frames): Show an error if maxframes has been reached.
+	(parse_opt): Allow -n 0 for unlimited frames.
+	(main): Document -n 0 and new default 2048 frames. Allocate initial
+	number of frames with malloc.
+
 2013-12-30  Mark Wielaard  <mjw at redhat.com>
 
 	* stack.c (parse_opt): Explicitly call dwfl_linux_proc_attach
diff --git a/src/stack.c b/src/stack.c
index 2c1d8ff..f9f964a 100644
--- a/src/stack.c
+++ b/src/stack.c
@@ -52,7 +52,7 @@ static bool show_raw = false;
 #endif
 static bool show_modules = false;
 
-static unsigned maxframes = 64;
+static int maxframes = 2048;
 
 struct frame
 {
@@ -62,8 +62,9 @@ struct frame
 
 struct frames
 {
-  unsigned frames;
-  struct frame frame[];
+  int frames;
+  int allocated;
+  struct frame *frame;
 };
 
 static Dwfl *dwfl = NULL;
@@ -172,7 +173,7 @@ static int
 frame_callback (Dwfl_Frame *state, void *arg)
 {
   struct frames *frames = (struct frames *) arg;
-  unsigned nr = frames->frames;
+  int nr = frames->frames;
   if (! dwfl_frame_pc (state, &frames->frame[nr].pc,
 		       &frames->frame[nr].isactivation))
     return -1;
@@ -181,6 +182,15 @@ frame_callback (Dwfl_Frame *state, void *arg)
   if (frames->frames == maxframes)
     return DWARF_CB_ABORT;
 
+  if (frames->frames == frames->allocated)
+    {
+      frames->allocated *= 2;
+      frames->frame = realloc (frames->frame,
+			       sizeof (struct frame) * frames->allocated);
+      if (frames->frame == NULL)
+	error (EXIT_BAD, errno, "realloc frames.frame");
+    }
+
   return DWARF_CB_OK;
 }
 
@@ -191,7 +201,7 @@ print_frames (struct frames *frames, pid_t tid, int dwflerr, const char *what)
     frames_shown = true;
 
   printf ("TID %d:\n", tid);
-  for (unsigned nr = 0; nr < frames->frames; nr++)
+  for (int nr = 0; nr < frames->frames; nr++)
     {
       Dwarf_Addr pc = frames->frame[nr].pc;
       bool isactivation = frames->frame[nr].isactivation;
@@ -297,6 +307,9 @@ print_frames (struct frames *frames, pid_t tid, int dwflerr, const char *what)
       else
 	error (0, 0, "%s tid %d: %s", what, tid, dwfl_errmsg (dwflerr));
     }
+  else if (frames->frames > 0 && frames->frames == maxframes)
+    error (0, 0, "tid %d: shown max number of frames "
+	   "(%d, use -n 0 for unlimited)", tid, maxframes);
 }
 
 static int
@@ -393,9 +406,9 @@ parse_opt (int key, char *arg __attribute__ ((unused)),
 
     case 'n':
       maxframes = atoi (arg);
-      if (maxframes == 0)
+      if (maxframes < 0)
 	{
-	  argp_error (state, N_("-n MAXFRAMES should be 1 or higher."));
+	  argp_error (state, N_("-n MAXFRAMES should be 0 or higher."));
 	  return EINVAL;
 	}
       break;
@@ -512,7 +525,7 @@ main (int argc, char **argv)
       { NULL, '1', NULL, 0,
 	N_("Show the backtrace of only one thread"), 0 },
       { NULL, 'n', "MAXFRAMES", 0,
-	N_("Show at most MAXFRAMES per thread (default 64)"), 0 },
+	N_("Show at most MAXFRAMES per thread (default 2048, use 0 for unlimited)"), 0 },
       { "list-modules", 'l', NULL, 0,
 	N_("Show module memory map with build-id, elf and debug files detected"), 0 },
       { NULL, 0, NULL, 0, NULL, 0 }
@@ -541,14 +554,17 @@ invoked with bad or missing arguments it will exit with return code 64.")
 	error (EXIT_BAD, 0, "dwfl_getmodules: %s", dwfl_errmsg (-1));
     }
 
-  struct frames *frames = malloc (sizeof (struct frames)
-				  + sizeof (struct frame) * maxframes);
-  frames->frames = 0;
+  struct frames frames;
+  frames.allocated = maxframes == 0 ? 2048 : maxframes;
+  frames.frames = 0;
+  frames.frame = malloc (sizeof (struct frame) * frames.allocated);
+  if (frames.frame == NULL)
+    error (EXIT_BAD, errno, "malloc frames.frame");
 
   if (show_one_tid)
     {
       int err = 0;
-      switch (dwfl_getthread_frames (dwfl, pid, frame_callback, frames))
+      switch (dwfl_getthread_frames (dwfl, pid, frame_callback, &frames))
 	{
 	case DWARF_CB_OK:
 	case DWARF_CB_ABORT:
@@ -559,12 +575,12 @@ invoked with bad or missing arguments it will exit with return code 64.")
 	default:
 	  abort ();
 	}
-      print_frames (frames, pid, err, "dwfl_getthread_frames");
+      print_frames (&frames, pid, err, "dwfl_getthread_frames");
     }
   else
     {
       printf ("PID %d - %s\n", dwfl_pid (dwfl), pid != 0 ? "process" : "core");
-      switch (dwfl_getthreads (dwfl, thread_callback, frames))
+      switch (dwfl_getthreads (dwfl, thread_callback, &frames))
 	{
 	case DWARF_CB_OK:
 	case DWARF_CB_ABORT:
@@ -576,7 +592,7 @@ invoked with bad or missing arguments it will exit with return code 64.")
 	  abort ();
 	}
     }
-  free (frames);
+  free (frames.frame);
   dwfl_end (dwfl);
 
   if (core != NULL)
-- 
1.8.4.2



More information about the elfutils-devel mailing list