commit 68b1afa36d2389c4f2fb526d0b134e5a3c68dedb

Author: Mark Wielaard <mjw@redhat.com>

Date:   Tue Jun 10 15:09:23 2014 +0200

    libdwfl: dwfl_standard_argp should not fail when not able to attach Dwfl.

    As pointed out in https://bugzilla.redhat.com/show_bug.cgi?id=1107654

    commit 191080 introduced a thinko that caused dwfl_standard_argp

    to fail if the Dwfl couldn't be attached. Instead of generating a warning

    as the comment intended, the failure would be fatal. But even warning

    about dwfl_core_file_attach () or dwfl_linux_proc_attach () failing

    would be a mistake. The caller/user might not be interested in such

    a non-fatal issue. So just ignore if the call failed for whatever reason.

    If the caller is interested in warning up front about this issue, then

    dwfl_pid () should be called to check the Dwfl is attached. Things should

    work just fine for anything that doesn't call any of the dwfl_state related

    functions.

    Signed-off-by: Mark Wielaard <mjw@redhat.com>

diff --git a/libdwfl/argp-std.c b/libdwfl/argp-std.c

index 8d2bc6a..42b7e78 100644

--- a/libdwfl/argp-std.c

+++ b/libdwfl/argp-std.c

@@ -171,10 +171,9 @@ parse_opt (int key, char *arg, struct argp_state *state)

 	    if (result != 0)

 	      return fail (dwfl, result, arg);

-	    result = INTUSE(dwfl_linux_proc_attach) (dwfl, atoi (arg), false);

-	    if (result != 0)

-	      /* Non-fatal to not be able to attach to process.  */

-	      failure (dwfl, result, _("cannot attach to process"));

+	    /* Non-fatal to not be able to attach to process, ignore error.  */

+	    INTUSE(dwfl_linux_proc_attach) (dwfl, atoi (arg), false);

+

 	    opt->dwfl = dwfl;

 	  }

 	else

@@ -301,10 +300,8 @@ parse_opt (int key, char *arg, struct argp_state *state)

 		return fail (dwfl, result, opt->core);

 	      }

-	    result = INTUSE(dwfl_core_file_attach) (dwfl, core);

-	    if (result < 0)

-	      /* Non-fatal to not be able to attach to core.  */

-	      failure (dwfl, result, _("cannot attach to core"));

+	    /* Non-fatal to not be able to attach to core, ignore error.  */

+	    INTUSE(dwfl_core_file_attach) (dwfl, core);

 	    /* From now we leak FD and CORE.  */

commit 14beac3b6f22b8d7a054980f74c4f8d33b969fc4

Author: Mark Wielaard <mjw@redhat.com>

Date:   Wed Jun 11 15:14:23 2014 +0200

    libdwfl: Record dwfl_attach_state error and return it on failure.

    When dwfl_attach_state fails functions that need the process state should

    return the error that caused the attach to fail. Use this in the backtrace

    test to signal any attach failure. This makes sure that architectures that

    don't provide unwinder support get properly detected (and the tests SKIPs)

    Also don't assert when trying to attach a non-core ELF file, but return an

    error to indicate failure.

    Signed-off-by: Mark Wielaard <mjw@redhat.com>

diff --git a/libdwfl/dwfl_frame.c b/libdwfl/dwfl_frame.c

index fd0b9ae..f6f86c0 100644

--- a/libdwfl/dwfl_frame.c

+++ b/libdwfl/dwfl_frame.c

@@ -117,6 +117,7 @@ __libdwfl_process_free (Dwfl_Process *process)

   if (process->ebl_close)

     ebl_closebackend (process->ebl);

   free (process);

+  dwfl->attacherr = DWFL_E_NOERROR;

 }

 /* Allocate new Dwfl_Process for DWFL.  */

@@ -134,17 +135,24 @@ bool

 dwfl_attach_state (Dwfl *dwfl, Elf *elf, pid_t pid,

 		   const Dwfl_Thread_Callbacks *thread_callbacks, void *arg)

 {

-  if (thread_callbacks == NULL || thread_callbacks->next_thread == NULL

-      || thread_callbacks->set_initial_registers == NULL)

+  if (dwfl->process != NULL)

     {

-      __libdwfl_seterrno (DWFL_E_INVALID_ARGUMENT);

+      __libdwfl_seterrno (DWFL_E_ATTACH_STATE_CONFLICT);

       return false;

     }

-  if (dwfl->process != NULL)

+

+  /* Reset any previous error, we are just going to try again.  */

+  dwfl->attacherr = DWFL_E_NOERROR;

+  if (thread_callbacks == NULL || thread_callbacks->next_thread == NULL

+      || thread_callbacks->set_initial_registers == NULL)

     {

-      __libdwfl_seterrno (DWFL_E_ATTACH_STATE_CONFLICT);

+      dwfl->attacherr = DWFL_E_INVALID_ARGUMENT;

+    fail:

+      dwfl->attacherr = __libdwfl_canon_error (dwfl->attacherr);

+      __libdwfl_seterrno (dwfl->attacherr);

       return false;

     }

+

   Ebl *ebl;

   bool ebl_close;

   if (elf != NULL)

@@ -180,8 +188,8 @@ dwfl_attach_state (Dwfl *dwfl, Elf *elf, pid_t pid,

   if (ebl == NULL)

     {

       /* Not identified EBL from any of the modules.  */

-      __libdwfl_seterrno (DWFL_E_PROCESS_NO_ARCH);

-      return false;

+      dwfl->attacherr = DWFL_E_PROCESS_NO_ARCH;

+      goto fail;

     }

   process_alloc (dwfl);

   Dwfl_Process *process = dwfl->process;

@@ -189,8 +197,8 @@ dwfl_attach_state (Dwfl *dwfl, Elf *elf, pid_t pid,

     {

       if (ebl_close)

 	ebl_closebackend (ebl);

-      __libdwfl_seterrno (DWFL_E_NOMEM);

-      return false;

+      dwfl->attacherr = DWFL_E_NOMEM;

+      goto fail;

     }

   process->ebl = ebl;

   process->ebl_close = ebl_close;

@@ -204,6 +212,12 @@ INTDEF(dwfl_attach_state)

 pid_t

 dwfl_pid (Dwfl *dwfl)

 {

+  if (dwfl->attacherr != DWFL_E_NOERROR)

+    {

+      __libdwfl_seterrno (dwfl->attacherr);

+      return -1;

+    }

+

   if (dwfl->process == NULL)

     {

       __libdwfl_seterrno (DWFL_E_NO_ATTACH_STATE);

@@ -238,6 +252,12 @@ int

 dwfl_getthreads (Dwfl *dwfl, int (*callback) (Dwfl_Thread *thread, void *arg),

 		 void *arg)

 {

+  if (dwfl->attacherr != DWFL_E_NOERROR)

+    {

+      __libdwfl_seterrno (dwfl->attacherr);

+      return -1;

+    }

+

   Dwfl_Process *process = dwfl->process;

   if (process == NULL)

     {

@@ -309,6 +329,12 @@ getthread (Dwfl *dwfl, pid_t tid,

 	   int (*callback) (Dwfl_Thread *thread, void *arg),

 	   void *arg)

 {

+  if (dwfl->attacherr != DWFL_E_NOERROR)

+    {

+      __libdwfl_seterrno (dwfl->attacherr);

+      return -1;

+    }

+

   Dwfl_Process *process = dwfl->process;

   if (process == NULL)

     {

diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h

index 9b03d8a..30c0f8a 100644

--- a/libdwfl/libdwflP.h

+++ b/libdwfl/libdwflP.h

@@ -91,7 +91,8 @@ typedef struct Dwfl_Process Dwfl_Process;

   DWFL_ERROR (ATTACH_STATE_CONFLICT, N_("Dwfl already has attached state"))   \

   DWFL_ERROR (NO_ATTACH_STATE, N_("Dwfl has no attached state"))	      \

   DWFL_ERROR (NO_UNWIND, N_("Unwinding not supported for this architecture")) \

-  DWFL_ERROR (INVALID_ARGUMENT, N_("Invalid argument"))

+  DWFL_ERROR (INVALID_ARGUMENT, N_("Invalid argument"))			      \

+  DWFL_ERROR (NO_CORE_FILE, N_("Not an ET_CORE ELF file"))

 #define DWFL_ERROR(name, text) DWFL_E_##name,

 typedef enum { DWFL_ERRORS DWFL_E_NUM } Dwfl_Error;

@@ -110,6 +111,7 @@ struct Dwfl

   Dwfl_Module *modulelist;    /* List in order used by full traversals.  */

   Dwfl_Process *process;

+  Dwfl_Error attacherr;      /* Previous error attaching process.  */

   GElf_Addr offline_next_address;

diff --git a/libdwfl/linux-core-attach.c b/libdwfl/linux-core-attach.c

index 1002788..7ef3f25 100644

--- a/libdwfl/linux-core-attach.c

+++ b/libdwfl/linux-core-attach.c

@@ -309,33 +309,41 @@ static const Dwfl_Thread_Callbacks core_thread_callbacks =

 int

 dwfl_core_file_attach (Dwfl *dwfl, Elf *core)

 {

+  Dwfl_Error err = DWFL_E_NOERROR;

   Ebl *ebl = ebl_openbackend (core);

   if (ebl == NULL)

     {

-      __libdwfl_seterrno (DWFL_E_LIBEBL);

+      err = DWFL_E_LIBEBL;

+    fail_err:

+      if (dwfl->process == NULL && dwfl->attacherr == DWFL_E_NOERROR)

+	dwfl->attacherr = __libdwfl_canon_error (err);

+      __libdwfl_seterrno (err);

       return -1;

     }

   size_t nregs = ebl_frame_nregs (ebl);

   if (nregs == 0)

     {

-      __libdwfl_seterrno (DWFL_E_NO_UNWIND);

+      err = DWFL_E_NO_UNWIND;

+    fail:

       ebl_closebackend (ebl);

-      return -1;

+      goto fail_err;

     }

   GElf_Ehdr ehdr_mem, *ehdr = gelf_getehdr (core, &ehdr_mem);

   if (ehdr == NULL)

     {

-      __libdwfl_seterrno (DWFL_E_LIBELF);

-      ebl_closebackend (ebl);

-      return -1;

+      err = DWFL_E_LIBELF;

+      goto fail;

+    }

+  if (ehdr->e_type != ET_CORE)

+    {

+      err = DWFL_E_NO_CORE_FILE;

+      goto fail;

     }

-  assert (ehdr->e_type == ET_CORE);

   size_t phnum;

   if (elf_getphdrnum (core, &phnum) < 0)

     {

-      __libdwfl_seterrno (DWFL_E_LIBELF);

-      ebl_closebackend (ebl);

-      return -1;

+      err = DWFL_E_LIBELF;

+      goto fail;

     }

   pid_t pid = -1;

   Elf_Data *note_data = NULL;

@@ -351,8 +359,8 @@ dwfl_core_file_attach (Dwfl *dwfl, Elf *core)

     }

   if (note_data == NULL)

     {

-      ebl_closebackend (ebl);

-      return DWFL_E_LIBELF;

+      err = DWFL_E_LIBELF;

+      goto fail;

     }

   size_t offset = 0;

   GElf_Nhdr nhdr;

@@ -394,16 +402,14 @@ dwfl_core_file_attach (Dwfl *dwfl, Elf *core)

   if (pid == -1)

     {

       /* No valid NT_PRPSINFO recognized in this CORE.  */

-      __libdwfl_seterrno (DWFL_E_BADELF);

-      ebl_closebackend (ebl);

-      return -1;

+      err = DWFL_E_BADELF;

+      goto fail;

     }

   struct core_arg *core_arg = malloc (sizeof *core_arg);

   if (core_arg == NULL)

     {

-      __libdwfl_seterrno (DWFL_E_NOMEM);

-      ebl_closebackend (ebl);

-      return -1;

+      err = DWFL_E_NOMEM;

+      goto fail;

     }

   core_arg->core = core;

   core_arg->note_data = note_data;

diff --git a/libdwfl/linux-pid-attach.c b/libdwfl/linux-pid-attach.c

index 8aee721..d60955e 100644

--- a/libdwfl/linux-pid-attach.c

+++ b/libdwfl/linux-pid-attach.c

@@ -290,13 +290,23 @@ dwfl_linux_proc_attach (Dwfl *dwfl, pid_t pid, bool assume_ptrace_stopped)

 {

   char buffer[36];

   FILE *procfile;

+  int err = 0; /* The errno to return and set for dwfl->attcherr.  */

   /* Make sure to report the actual PID (thread group leader) to

      dwfl_attach_state.  */

   snprintf (buffer, sizeof (buffer), "/proc/%ld/status", (long) pid);

   procfile = fopen (buffer, "r");

   if (procfile == NULL)

-    return errno;

+    {

+      err = errno;

+    fail:

+      if (dwfl->process == NULL && dwfl->attacherr == DWFL_E_NOERROR)

+	{

+	  errno = err;

+	  dwfl->attacherr = __libdwfl_canon_error (DWFL_E_ERRNO);

+	}

+      return err;

+    }

   char *line = NULL;

   size_t linelen = 0;

@@ -317,19 +327,26 @@ dwfl_linux_proc_attach (Dwfl *dwfl, pid_t pid, bool assume_ptrace_stopped)

   fclose (procfile);

   if (pid == 0)

-    return ESRCH;

+    {

+      err = ESRCH;

+      goto fail;

+    }

   char dirname[64];

   int i = snprintf (dirname, sizeof (dirname), "/proc/%ld/task", (long) pid);

   assert (i > 0 && i < (ssize_t) sizeof (dirname) - 1);

   DIR *dir = opendir (dirname);

   if (dir == NULL)

-    return errno;

+    {

+      err = errno;

+      goto fail;

+    }

   struct __libdwfl_pid_arg *pid_arg = malloc (sizeof *pid_arg);

   if (pid_arg == NULL)

     {

       closedir (dir);

-      return ENOMEM;

+      err = ENOMEM;

+      goto fail;

     }

   pid_arg->dir = dir;

   pid_arg->tid_attached = 0;

diff --git a/tests/backtrace.c b/tests/backtrace.c

index 1a4709b..ce0bd17 100644

--- a/tests/backtrace.c

+++ b/tests/backtrace.c

@@ -1,5 +1,5 @@

 /* Test program for unwinding of frames.

-   Copyright (C) 2013 Red Hat, Inc.

+   Copyright (C) 2013, 2014 Red Hat, Inc.

    This file is part of elfutils.

    This file is free software; you can redistribute it and/or modify

@@ -459,6 +459,9 @@ main (int argc __attribute__ ((unused)), char **argv)

     };

   (void) argp_parse (&argp, argc, argv, 0, NULL, &dwfl);

   assert (dwfl != NULL);

+  /* We want to make sure the dwfl was properly attached.  */

+  if (dwfl_pid (dwfl) < 0)

+    error (2, 0, "dwfl_pid: %s", dwfl_errmsg (-1));

   dump (dwfl);

   dwfl_end (dwfl);

   return 0;