[PATCH] stack: Improve error checking and exit code handling.

[Mark Wielaard <mjw at redhat dot com>]

Check up front whether we attached correctly, if not error out. Make sure
callbacks report -1 only on real errors and DWARF_CB_ABORT if exiting early
(but not in error). Handle all errors from frame callback in print_frames
after printing of good frames. Print as much information as possible like
tid, address and module name if known with error messages. Only exit with
exit code zero if everything went fine. Exit with error code one if there
were any non-fatal errors. Exit with error code two if no frames could be
printed or a fatal error occurred.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 src/ChangeLog |  17 ++++++++++
 src/stack.c   | 102 +++++++++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 97 insertions(+), 22 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index 142a1c3..7911236 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,20 @@
+2013-12-27  Mark Wielaard  <mjw@redhat.com>
+
+	* stack.c (frames_shown): New static boolean.
+	(EXIT_OK,EXIT_ERROR,EXIT_BAD,EXIT_USAGES): New defines.
+	(frame_callback): Return -1 on error. Don't print error.
+	(print_frames): Add arguments, tid, dwflerr and what. Print tid.
+	If there was an error report it with address and module if possible.
+	Record whether any frames were actually printed.
+	(thread_callback): Collect tid and err, pass it to print_frames.
+	(parse_opt): Use EXIT_BAD for errors. On ARGP_KEY_END print errno
+	if dwfl_linux_proc_report returned it. Check whether we are properly
+	attached with dwfl_pid.
+	(main): Document exit status. Don't report DWARF_CB_ABORT from
+	callbacks as error. Pass real errors to print_frames. Return
+	EXIT_BAD if no frames could be shown. Return EXIT_ERROR if there
+	were any non-fatal errors.
+
 2013-12-23  Mark Wielaard  <mjw@redhat.com>
 
 	* Makefile.am (stack_LDADD): Add demanglelib.
diff --git a/src/stack.c b/src/stack.c
index 3568298..d71d669 100644
--- a/src/stack.c
+++ b/src/stack.c
@@ -90,6 +90,18 @@ static size_t demangle_buffer_len = 0;
 static char *demangle_buffer = NULL;
 #endif
 
+/* Whether any frames have been shown at all.  Determines exit status.  */
+static bool frames_shown = false;
+
+/* Program exit codes. All frames shown without any errors is GOOD.
+   Some frames shown with some non-fatal errors is an ERROR.  A fatal
+   error or no frames shown at all is BAD.  A command line USAGE exit
+   is generated by argp_error.  */
+#define EXIT_OK     0
+#define EXIT_ERROR  1
+#define EXIT_BAD    2
+#define EXIT_USAGE 64
+
 static int
 frame_callback (Dwfl_Frame *state, void *arg)
 {
@@ -97,10 +109,7 @@ frame_callback (Dwfl_Frame *state, void *arg)
   unsigned nr = frames->frames;
   if (! dwfl_frame_pc (state, &frames->frame[nr].pc,
 		       &frames->frame[nr].isactivation))
-    {
-      error (0, 0, "%s", dwfl_errmsg (-1));
-      return DWARF_CB_ABORT;
-    }
+    return -1;
 
   frames->frames++;
   if (frames->frames == maxframes)
@@ -110,8 +119,12 @@ frame_callback (Dwfl_Frame *state, void *arg)
 }
 
 static void
-print_frames (struct frames *frames)
+print_frames (struct frames *frames, pid_t tid, int dwflerr, const char *what)
 {
+  if (frames->frames > 0)
+    frames_shown = true;
+
+  printf ("TID %d:\n", tid);
   for (unsigned nr = 0; nr < frames->frames; nr++)
     {
       Dwarf_Addr pc = frames->frame[nr].pc;
@@ -209,13 +222,39 @@ print_frames (struct frames *frames)
 	}
       printf ("\n");
     }
+  if (dwflerr != 0)
+    {
+      if (frames->frames > 0)
+	{
+	  unsigned nr = frames->frames - 1;
+	  Dwarf_Addr pc = frames->frame[nr].pc;
+	  bool isactivation = frames->frame[nr].isactivation;
+	  Dwarf_Addr pc_adjusted = pc - (isactivation ? 0 : 1);
+	  Dwfl_Module *mod = dwfl_addrmodule (dwfl, pc_adjusted);
+	  const char *mainfile = NULL;
+	  const char *modname = dwfl_module_info (mod, NULL, NULL, NULL, NULL,
+						  NULL, &mainfile, NULL);
+	  if (modname == NULL || modname[0] == '\0')
+	    {
+	      if (mainfile != NULL)
+		modname = mainfile;
+	      else
+		modname = "<unknown>";
+	    }
+	  error (0, 0, "%s tid %d at 0x%" PRIx64 " in %s: %s", what, tid,
+		 pc_adjusted, modname, dwfl_errmsg (dwflerr));
+	}
+      else
+	error (0, 0, "%s tid %d: %s", what, tid, dwfl_errmsg (dwflerr));
+    }
 }
 
 static int
 thread_callback (Dwfl_Thread *thread, void *thread_arg)
 {
-  printf ("TID %ld:\n", (long) dwfl_thread_tid (thread));
   struct frames *frames = (struct frames *) thread_arg;
+  pid_t tid = dwfl_thread_tid (thread);
+  int err = 0;
   frames->frames = 0;
   switch (dwfl_thread_getframes (thread, frame_callback, thread_arg))
     {
@@ -223,12 +262,12 @@ thread_callback (Dwfl_Thread *thread, void *thread_arg)
     case DWARF_CB_ABORT:
       break;
     case -1:
-      error (0, 0, "dwfl_thread_getframes: %s", dwfl_errmsg (-1));
+      err = dwfl_errno ();
       break;
     default:
       abort ();
     }
-  print_frames (frames);
+  print_frames (frames, tid, err, "dwfl_thread_getframes");
   return DWARF_CB_OK;
 }
 
@@ -253,11 +292,11 @@ parse_opt (int key, char *arg __attribute__ ((unused)),
     case OPT_COREFILE:
       core_fd = open (arg, O_RDONLY);
       if (core_fd < 0)
-	error (2, errno, N_("Cannot open core file '%s'."), arg);
+	error (EXIT_BAD, errno, N_("Cannot open core file '%s'"), arg);
       elf_version (EV_CURRENT);
       core = elf_begin (core_fd, ELF_C_READ_MMAP, NULL);
       if (core == NULL)
-	error (2, 0, "core '%s' elf_begin: %s", arg, elf_errmsg(-1));
+	error (EXIT_BAD, 0, "core '%s' elf_begin: %s", arg, elf_errmsg(-1));
       break;
 
     case 'e':
@@ -328,22 +367,31 @@ parse_opt (int key, char *arg __attribute__ ((unused)),
 	{
 	  dwfl = dwfl_begin (&proc_callbacks);
 	  if (dwfl == NULL)
-	    error (2, 0, "dwfl_begin: %s", dwfl_errmsg (-1));
-	  if (dwfl_linux_proc_report (dwfl, pid) != 0)
-	    error (2, 0, "dwfl_linux_proc_report: %s", dwfl_errmsg (-1));
+	    error (EXIT_BAD, 0, "dwfl_begin: %s", dwfl_errmsg (-1));
+
+	  int err = dwfl_linux_proc_report (dwfl, pid);
+	  if (err < 0)
+	    error (EXIT_BAD, 0, "dwfl_linux_proc_report pid %d: %s", pid,
+		   dwfl_errmsg (-1));
+	  else if (err > 0)
+	    error (EXIT_BAD, err, "dwfl_linux_proc_report pid %d", pid);
 	}
 
       if (core != NULL)
 	{
 	  dwfl = dwfl_begin (&core_callbacks);
 	  if (dwfl == NULL)
-	    error (2, 0, "dwfl_begin: %s", dwfl_errmsg (-1));
+	    error (EXIT_BAD, 0, "dwfl_begin: %s", dwfl_errmsg (-1));
 	  if (dwfl_core_file_report (dwfl, core, exec) < 0)
-	    error (2, 0, "dwfl_core_file_report: %s", dwfl_errmsg (-1));
+	    error (EXIT_BAD, 0, "dwfl_core_file_report: %s", dwfl_errmsg (-1));
 	}
 
       if (dwfl_report_end (dwfl, NULL, NULL) != 0)
-	error (2, 0, "dwfl_report_end: %s", dwfl_errmsg (-1));
+	error (EXIT_BAD, 0, "dwfl_report_end: %s", dwfl_errmsg (-1));
+
+      /* Makes sure we are properly attached.  */
+      if (dwfl_pid (dwfl) < 0)
+	error (EXIT_BAD, 0, "dwfl_pid: %s\n", dwfl_errmsg (-1));
       break;
 
     default:
@@ -402,7 +450,13 @@ main (int argc, char **argv)
     {
       .options = options,
       .parser = parse_opt,
-      .doc = N_("Print a stack for each thread in a process or core file."),
+      .doc = N_("Print a stack for each thread in a process or core file.\v\
+Program exits with return code 0 if all frames were shown without \
+any errors.  If some frames were shown, but there were some non-fatal \
+errors, possibly causing an incomplete backtrace, the program exits \
+with return code 1.  If no frames could be shown, or a fatal error \
+occured the program exits with return code 2.  If the program was \
+invoked with bad or missing arguments it will exit with return code 64.")
     };
 
   argp_parse (&argp, argc, argv, 0, NULL, NULL);
@@ -413,19 +467,19 @@ main (int argc, char **argv)
 
   if (show_one_tid)
     {
-      printf ("TID %d:\n", pid);
+      int err = 0;
       switch (dwfl_getthread_frames (dwfl, pid, frame_callback, frames))
 	{
 	case DWARF_CB_OK:
+	case DWARF_CB_ABORT:
 	  break;
 	case -1:
-	  error (0, 0, "dwfl_getthread_frames (%d): %s", pid,
-		 dwfl_errmsg (-1));
+	  err = dwfl_errno ();
 	  break;
 	default:
 	  abort ();
 	}
-      print_frames (frames);
+      print_frames (frames, pid, err, "dwfl_getthread_frames");
     }
   else
     {
@@ -433,6 +487,7 @@ main (int argc, char **argv)
       switch (dwfl_getthreads (dwfl, thread_callback, frames))
 	{
 	case DWARF_CB_OK:
+	case DWARF_CB_ABORT:
 	  break;
 	case -1:
 	  error (0, 0, "dwfl_getthreads: %s", dwfl_errmsg (-1));
@@ -454,5 +509,8 @@ main (int argc, char **argv)
   free (demangle_buffer);
 #endif
 
-  return 0;
+  if (! frames_shown)
+    error (EXIT_BAD, 0, N_("Couldn't show any frames."));
+
+  return error_message_count != 0 ? EXIT_ERROR : EXIT_OK;
 }
-- 
1.8.4.2