This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
Re: [tests patchv2] FYI unwinder: tests/ update
- From: Mark Wielaard <mjw at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Thu, 28 Nov 2013 16:57:13 +0100
- Subject: Re: [tests patchv2] FYI unwinder: tests/ update
On Sat, 2013-11-23 at 21:57 +0100, Jan Kratochvil wrote:
> Besides changes described below I had to make some small libdwfl/ extension to
> keep "Unwinding not supported for this architecture" error from
> __libdwfl_attach_state_for_core() call in dwfl_core_file_report()
> till the unwinding request later. One can for example comment out from
> backends/i386_init.c
> eh->frame_nregs = 9;
> HOOK (eh, set_initial_registers_tid);
> to test such behavior.
>
> __libdwfl_attach_state_for_core() should not report error even if
> dwfl_core_file_report() fails, as the caller may not want to unwind the core
> at all. Still the error has to be kept somewhere.
That seems sensible. We use something similar for the Dwfl_Module when
the main or debug file cannot be loaded.
> libdwfl/
> 2013-11-22 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * dwfl_frame.c (dwfl_pid, dwfl_getthreads): Use PROCESS_ATTACH_ERROR if
> PROCESS is NULL.
> * libdwflP.h (struct Dwfl): New field process_attach_error.
> * linux-core-attach.c (__libdwfl_attach_state_for_core): Rename to ...
> (attach_state_for_core): ... here, make it static, change return type,
> no longer use __libdwfl_seterrno.
> (__libdwfl_attach_state_for_core): New wrapper for it.
This seems independent of the rest and almost correct (see one question
below). Lets get this in (short patches/commits are good IMHO).
> diff --git a/libdwfl/dwfl_frame.c b/libdwfl/dwfl_frame.c
> index f286350..4a7b3cd 100644
> --- a/libdwfl/dwfl_frame.c
> +++ b/libdwfl/dwfl_frame.c
> @@ -200,7 +200,7 @@ dwfl_pid (Dwfl *dwfl)
> {
> if (dwfl->process == NULL)
> {
> - __libdwfl_seterrno (DWFL_E_NO_ATTACH_STATE);
> + __libdwfl_seterrno (dwfl->process_attach_error);
> return -1;
> }
> return dwfl->process->pid;
> @@ -235,7 +235,7 @@ dwfl_getthreads (Dwfl *dwfl, int (*callback) (Dwfl_Thread *thread, void *arg),
> Dwfl_Process *process = dwfl->process;
> if (process == NULL)
> {
> - __libdwfl_seterrno (DWFL_E_NO_ATTACH_STATE);
> + __libdwfl_seterrno (dwfl->process_attach_error);
> return -1;
> }
Won't this return -1 and set the error to DWFL_E_NO_ERROR (0) in case no
attach has ever been attempted?
Maybe check that dwfl->process_attach_error != DWFL_E_NO_ERROR first.
And if it is then seterrno to DWFL_E_NO_ATTACH_STATE.
Or initialize dwfl->process_attach_error to DWFL_E_NO_ATTACH_STATE in
dwfl_begin?
> diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
> index b8a64d8..7e73e9e 100644
> --- a/libdwfl/libdwflP.h
> +++ b/libdwfl/libdwflP.h
> @@ -108,6 +108,7 @@ struct Dwfl
> Dwfl_Module *modulelist; /* List in order used by full traversals. */
>
> Dwfl_Process *process;
> + Dwfl_Error process_attach_error;
>
> GElf_Addr offline_next_address;
OK.
> diff --git a/libdwfl/linux-core-attach.c b/libdwfl/linux-core-attach.c
> index 971d495..b2d703f 100644
> --- a/libdwfl/linux-core-attach.c
> +++ b/libdwfl/linux-core-attach.c
> @@ -264,37 +264,30 @@ static const Dwfl_Thread_Callbacks core_thread_callbacks =
> NULL, /* core_thread_detach */
> };
>
> -bool
> -internal_function
> -__libdwfl_attach_state_for_core (Dwfl *dwfl, Elf *core)
> +static Dwfl_Error
> +attach_state_for_core (Dwfl *dwfl, Elf *core)
> {
> Ebl *ebl = ebl_openbackend (core);
> if (ebl == NULL)
> - {
> - __libdwfl_seterrno (DWFL_E_LIBEBL);
> - return false;
> - }
> + return DWFL_E_LIBEBL;
> size_t nregs = ebl_frame_nregs (ebl);
> if (nregs == 0)
> {
> ebl_closebackend (ebl);
> - __libdwfl_seterrno (DWFL_E_LIBEBL);
> - return false;
> + return DWFL_E_NO_UNWIND;
> }
> GElf_Ehdr ehdr_mem, *ehdr = gelf_getehdr (core, &ehdr_mem);
> if (ehdr == NULL)
> {
> ebl_closebackend (ebl);
> - __libdwfl_seterrno (DWFL_E_LIBELF);
> - return false;
> + return DWFL_E_LIBELF;
> }
> assert (ehdr->e_type == ET_CORE);
> size_t phnum;
> if (elf_getphdrnum (core, &phnum) < 0)
> {
> ebl_closebackend (ebl);
> - __libdwfl_seterrno (DWFL_E_LIBELF);
> - return false;
> + return DWFL_E_LIBELF;
> }
> pid_t pid = -1;
> Elf_Data *note_data = NULL;
> @@ -311,8 +304,7 @@ __libdwfl_attach_state_for_core (Dwfl *dwfl, Elf *core)
> if (note_data == NULL)
> {
> ebl_closebackend (ebl);
> - __libdwfl_seterrno (DWFL_E_LIBELF);
> - return NULL;
> + return DWFL_E_LIBELF;
> }
> size_t offset = 0;
> GElf_Nhdr nhdr;
> @@ -355,15 +347,13 @@ __libdwfl_attach_state_for_core (Dwfl *dwfl, Elf *core)
> {
> /* No valid NT_PRPSINFO recognized in this CORE. */
> ebl_closebackend (ebl);
> - __libdwfl_seterrno (DWFL_E_BADELF);
> - return false;
> + return DWFL_E_BADELF;
> }
> struct core_arg *core_arg = malloc (sizeof *core_arg);
> if (core_arg == NULL)
> {
> ebl_closebackend (ebl);
> - __libdwfl_seterrno (DWFL_E_NOMEM);
> - return false;
> + return DWFL_E_NOMEM;
> }
OK.
> @@ -372,8 +362,30 @@ __libdwfl_attach_state_for_core (Dwfl *dwfl, Elf *core)
> if (! INTUSE(dwfl_attach_state) (dwfl, core, pid, &core_thread_callbacks,
> core_arg))
> {
> + Dwfl_Error error = dwfl_errno ();
> + assert (error != DWFL_E_NOERROR);
> free (core_arg);
> ebl_closebackend (ebl);
> + return error;
> + }
> + return DWFL_E_NOERROR;
> +}
OK.
> +bool
> +internal_function
> +__libdwfl_attach_state_for_core (Dwfl *dwfl, Elf *core)
> +{
> + if (dwfl->process != NULL)
> + {
> + __libdwfl_seterrno (DWFL_E_ATTACH_STATE_CONFLICT);
> + return false;
> + }
> + Dwfl_Error error = attach_state_for_core (dwfl, core);
> + assert ((dwfl->process != NULL) == (error == DWFL_E_NOERROR));
> + if (error != DWFL_E_NOERROR)
> + {
> + dwfl->process_attach_error = error;
> + __libdwfl_seterrno (error);
> return false;
> }
> return true;
And depending on how to resolve the question above about dwfl_pid and
dwfl_getthreads set dwfl->process_attach_error to DWFL_E_NOERROR?
Thanks,
Mark