Re: [PATCH v2 3/6] Display per-thread information for threads in FreeBSD cores.

Pedro Alves
2016-01-14T15:03:56Z

On 01/13/2016 09:45 PM, John Baldwin wrote:
> Display the LWP ID of each thread in a FreeBSD core. Extract thread names
> from the per-thread THRMISC note.
>
> gdb/ChangeLog:
>
> * fbsd_tdep.c (fbsd_core_pid_to_str): New function.
> (fbsd_init_abi): Add "core_pid_to_str" gdbarch method.
> ---
> gdb/ChangeLog | 5 +++++
> gdb/fbsd-tdep.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 46 insertions(+)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 471d02b..93760ba 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2016-01-09 John Baldwin <jhb@FreeBSD.org>
> +
> + * fbsd_tdep.c (fbsd_core_pid_to_str): New function.
> + (fbsd_init_abi): Add "core_pid_to_str" gdbarch method.
> +
> 2016-01-08 Yao Qi <yao.qi@linaro.org>
>
> * extension.c: Include target.h.
> diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
> index 0ef94d6..6851cc1 100644
> --- a/gdb/fbsd-tdep.c
> +++ b/gdb/fbsd-tdep.c
> @@ -28,6 +28,46 @@
> #include "fbsd-tdep.h"
>
>
> +/* This is how we want PTIDs from core files to be printed. */
> +
> +static char *
> +fbsd_core_pid_to_str (struct gdbarch *gdbarch, ptid_t ptid)
> +{
> + static char buf[80], name[64];

This static "name" buffer appears unused, and then masked
by the "name" pointer below.

> + struct bfd_section *section;
> + bfd_size_type size;
> + char sectionstr[32];
> +
> + if (ptid_get_lwp (ptid) != 0)
> + {
> + snprintf (sectionstr, sizeof sectionstr, ".thrmisc/%ld",
> + ptid_get_lwp (ptid));

xsnprintf.

> + section = bfd_get_section_by_name (core_bfd, sectionstr);
> + if (section != NULL)
> + {
> + char *name;
> +
> + size = bfd_section_size (core_bfd, section);
> + name = alloca (size + 1);
> + if (bfd_get_section_contents (core_bfd, section, name, (file_ptr) 0,
> + size) && name[0] != '\0')

This indentation / line break reads unusual to me. I think breaking
before the && would be clearer:

if (bfd_get_section_contents (core_bfd, section, name,
(file_ptr) 0, size)
&& name[0] != '\0')

Guess this should check size > 0 as well, otherwise name[0] contains
garbage.

> + {
> + name[size] = '\0';
> + if (strcmp(name, elf_tdata (core_bfd)->core->program) != 0)

Missing space after strcmp.

Is this ".thrmisc" section documented somewhere? Could you add a
small comment on what this entry you're skipping means?

> + {
> + snprintf (buf, sizeof buf, "LWP %ld \"%s\"",
> + ptid_get_lwp (ptid), name);

xsnprintf.

> + return buf;
> + }
> + }
> + }
> + snprintf (buf, sizeof buf, "LWP %ld", ptid_get_lwp (ptid));

xsnprintf.

> + return buf;
> + }
> +
> + return normal_pid_to_str (ptid);
> +}
> +
> static int
> find_signalled_thread (struct thread_info *info, void *data)
> {
> @@ -132,5 +172,6 @@ fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
> void
> fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> {
> + set_gdbarch_core_pid_to_str (gdbarch, fbsd_core_pid_to_str);
> set_gdbarch_make_corefile_notes (gdbarch, fbsd_make_corefile_notes);
> }
>

Thanks,
Pedro Alves