From: Andrew Burgess Date: Mon, 18 Jan 2021 16:00:38 +0000 (+0000) Subject: gdb: unify parts of the Linux and FreeBSD core dumping code X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=82a1fd3a4935;p=binutils-gdb.git gdb: unify parts of the Linux and FreeBSD core dumping code While reviewing the Linux and FreeBSD core dumping code within GDB for another patch series, I noticed that the code that collects the registers for each thread and writes these into ELF note format is basically identical between Linux and FreeBSD. This commit merges this code and moves it into the gcore.c file, which seemed like the right place for generic writing a core file code. The function find_signalled_thread is moved from linux-tdep.c despite not being shared. A later commit will make use of this function. There are a couple of minor changes to the FreeBSD target after this commit, but I believe that these are changes for the better: (1) For FreeBSD we always used to record the thread-id in the core file by using ptid_t.lwp (). In contrast the Linux code did this: /* For remote targets the LWP may not be available, so use the TID. */ long lwp = ptid.lwp (); if (lwp == 0) lwp = ptid.tid (); Both target now do this: /* The LWP is often not available for bare metal target, in which case use the tid instead. */ if (ptid.lwp_p ()) lwp = ptid.lwp (); else lwp = ptid.tid (); Which is equivalent for Linux, but is a change for FreeBSD. I think that all this means is that in some cases where GDB might have previously recorded a thread-id of 0 for each thread, we might now get something more useful. (2) When collecting the registers for Linux we collected into a zero initialised buffer. By contrast on FreeBSD the buffer is left uninitialised. In the new code the buffer is always zero initialised. I suspect once the registers are copied into the buffer there's probably no gaps left so this makes no difference, but if it does then using zeros rather than random bits of GDB's memory is probably a good thing. Otherwise, there should be no other user visible changes after this commit. Tested this on x86-64/GNU-Linux and x86-64/FreeBSD-12.2 with no regressions. gdb/ChangeLog: * Makefile.in (HFILES_NO_SRCDIR): Add corefile.h. * gcore.c (struct gcore_collect_regset_section_cb_data): Moved here from linux-tdep.c and given a new name. Minor cleanups. (gcore_collect_regset_section_cb): Likewise. (gcore_collect_thread_registers): Likewise. (gcore_build_thread_register_notes): Likewise. (gcore_find_signalled_thread): Likewise. * gcore.h (gcore_build_thread_register_notes): Declare. (gcore_find_signalled_thread): Declare. * fbsd-tdep.c: Add 'gcore.h' include. (struct fbsd_collect_regset_section_cb_data): Delete. (fbsd_collect_regset_section_cb): Delete. (fbsd_collect_thread_registers): Delete. (struct fbsd_corefile_thread_data): Delete. (fbsd_corefile_thread): Delete. (fbsd_make_corefile_notes): Call gcore_build_thread_register_notes instead of the now deleted FreeBSD code. * linux-tdep.c: Add 'gcore.h' include. (struct linux_collect_regset_section_cb_data): Delete. (linux_collect_regset_section_cb): Delete. (linux_collect_thread_registers): Delete. (linux_corefile_thread): Call gcore_build_thread_register_notes. (find_signalled_thread): Delete. (linux_make_corefile_notes): Call gcore_find_signalled_thread. --- diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 9d928b01eba..7323a13e0f2 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,32 @@ +2021-02-01 Andrew Burgess + + * Makefile.in (HFILES_NO_SRCDIR): Add corefile.h. + * gcore.c (struct gcore_collect_regset_section_cb_data): Moved + here from linux-tdep.c and given a new name. Minor cleanups. + (gcore_collect_regset_section_cb): Likewise. + (gcore_collect_thread_registers): Likewise. + (gcore_build_thread_register_notes): Likewise. + (gcore_find_signalled_thread): Likewise. + * gcore.h (gcore_build_thread_register_notes): Declare. + (gcore_find_signalled_thread): Declare. + * fbsd-tdep.c: Add 'gcore.h' include. + (struct fbsd_collect_regset_section_cb_data): Delete. + (fbsd_collect_regset_section_cb): Delete. + (fbsd_collect_thread_registers): Delete. + (struct fbsd_corefile_thread_data): Delete. + (fbsd_corefile_thread): Delete. + (fbsd_make_corefile_notes): Call + gcore_build_thread_register_notes instead of the now deleted + FreeBSD code. + * linux-tdep.c: Add 'gcore.h' include. + (struct linux_collect_regset_section_cb_data): Delete. + (linux_collect_regset_section_cb): Delete. + (linux_collect_thread_registers): Delete. + (linux_corefile_thread): Call + gcore_build_thread_register_notes. + (find_signalled_thread): Delete. + (linux_make_corefile_notes): Call gcore_find_signalled_thread. + 2021-01-29 Tom de Vries PR breakpoints/26063 diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c index cc51e921ae2..edd3edc4220 100644 --- a/gdb/fbsd-tdep.c +++ b/gdb/fbsd-tdep.c @@ -32,6 +32,7 @@ #include "elf-bfd.h" #include "fbsd-tdep.h" +#include "gcore.h" /* This enum is derived from FreeBSD's . */ @@ -583,129 +584,6 @@ find_signalled_thread (struct thread_info *info, void *data) return 0; } -/* Structure for passing information from - fbsd_collect_thread_registers via an iterator to - fbsd_collect_regset_section_cb. */ - -struct fbsd_collect_regset_section_cb_data -{ - fbsd_collect_regset_section_cb_data (const struct regcache *regcache, - bfd *obfd, - gdb::unique_xmalloc_ptr ¬e_data, - int *note_size, - unsigned long lwp, - gdb_signal stop_signal) - : regcache (regcache), - obfd (obfd), - note_data (note_data), - note_size (note_size), - lwp (lwp), - stop_signal (stop_signal) - {} - - const struct regcache *regcache; - bfd *obfd; - gdb::unique_xmalloc_ptr ¬e_data; - int *note_size; - unsigned long lwp; - enum gdb_signal stop_signal; - bool abort_iteration = false; -}; - -static void -fbsd_collect_regset_section_cb (const char *sect_name, int supply_size, - int collect_size, const struct regset *regset, - const char *human_name, void *cb_data) -{ - char *buf; - struct fbsd_collect_regset_section_cb_data *data - = (struct fbsd_collect_regset_section_cb_data *) cb_data; - - if (data->abort_iteration) - return; - - gdb_assert (regset->collect_regset); - - buf = (char *) xmalloc (collect_size); - regset->collect_regset (regset, data->regcache, -1, buf, collect_size); - - /* PRSTATUS still needs to be treated specially. */ - if (strcmp (sect_name, ".reg") == 0) - data->note_data.reset (elfcore_write_prstatus - (data->obfd, data->note_data.release (), - data->note_size, data->lwp, - gdb_signal_to_host (data->stop_signal), - buf)); - else - data->note_data.reset (elfcore_write_register_note - (data->obfd, data->note_data.release (), - data->note_size, sect_name, buf, - collect_size)); - xfree (buf); - - if (data->note_data == NULL) - data->abort_iteration = true; -} - -/* Records the thread's register state for the corefile note - section. */ - -static void -fbsd_collect_thread_registers (const struct regcache *regcache, - ptid_t ptid, bfd *obfd, - gdb::unique_xmalloc_ptr ¬e_data, - int *note_size, - enum gdb_signal stop_signal) -{ - fbsd_collect_regset_section_cb_data data (regcache, obfd, note_data, - note_size, ptid.lwp (), - stop_signal); - - gdbarch_iterate_over_regset_sections (regcache->arch (), - fbsd_collect_regset_section_cb, - &data, regcache); -} - -struct fbsd_corefile_thread_data -{ - fbsd_corefile_thread_data (struct gdbarch *gdbarch, - bfd *obfd, - gdb::unique_xmalloc_ptr ¬e_data, - int *note_size, - gdb_signal stop_signal) - : gdbarch (gdbarch), - obfd (obfd), - note_data (note_data), - note_size (note_size), - stop_signal (stop_signal) - {} - - struct gdbarch *gdbarch; - bfd *obfd; - gdb::unique_xmalloc_ptr ¬e_data; - int *note_size; - enum gdb_signal stop_signal; -}; - -/* Records the thread's register state for the corefile note - section. */ - -static void -fbsd_corefile_thread (struct thread_info *info, - struct fbsd_corefile_thread_data *args) -{ - struct regcache *regcache; - - regcache = get_thread_arch_regcache (info->inf->process_target (), - info->ptid, args->gdbarch); - - target_fetch_registers (regcache, -1); - - fbsd_collect_thread_registers (regcache, info->ptid, args->obfd, - args->note_data, args->note_size, - args->stop_signal); -} - /* Return a byte_vector containing the contents of a core dump note for the target object of type OBJECT. If STRUCTSIZE is non-zero, the data is prefixed with a 32-bit integer size to match the format @@ -782,16 +660,17 @@ fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size) signalled_thr = curr_thr; } - fbsd_corefile_thread_data thread_args (gdbarch, obfd, note_data, note_size, - signalled_thr->suspend.stop_signal); - - fbsd_corefile_thread (signalled_thr, &thread_args); + gcore_build_thread_register_notes (gdbarch, signalled_thr, + signalled_thr->suspend.stop_signal, + obfd, ¬e_data, note_size); for (thread_info *thr : current_inferior ()->non_exited_threads ()) { if (thr == signalled_thr) continue; - fbsd_corefile_thread (thr, &thread_args); + gcore_build_thread_register_notes (gdbarch, thr, + signalled_thr->suspend.stop_signal, + obfd, ¬e_data, note_size); } /* Auxiliary vector. */ diff --git a/gdb/gcore.c b/gdb/gcore.c index 73ac6b09c70..d62aa3a7109 100644 --- a/gdb/gcore.c +++ b/gdb/gcore.c @@ -579,6 +579,142 @@ gcore_memory_sections (bfd *obfd) return 1; } +/* Structure for passing information from GCORE_COLLECT_THREAD_REGISTERS + via an iterator to GCORE_COLLECT_REGSET_SECTION_CB. */ + +struct gcore_collect_regset_section_cb_data +{ + gcore_collect_regset_section_cb_data (struct gdbarch *gdbarch, + const struct regcache *regcache, + bfd *obfd, ptid_t ptid, + gdb_signal stop_signal, + gdb::unique_xmalloc_ptr *note_data, + int *note_size) + + : gdbarch (gdbarch), regcache (regcache), obfd (obfd), + note_data (note_data), note_size (note_size), + stop_signal (stop_signal) + { + /* The LWP is often not available for bare metal target, in which case + use the tid instead. */ + if (ptid.lwp_p ()) + lwp = ptid.lwp (); + else + lwp = ptid.tid (); + } + + struct gdbarch *gdbarch; + const struct regcache *regcache; + bfd *obfd; + gdb::unique_xmalloc_ptr *note_data; + int *note_size; + unsigned long lwp; + enum gdb_signal stop_signal; + bool abort_iteration = false; +}; + +/* Callback for ITERATE_OVER_REGSET_SECTIONS that records a single + regset in the core file note section. */ + +static void +gcore_collect_regset_section_cb (const char *sect_name, int supply_size, + int collect_size, + const struct regset *regset, + const char *human_name, void *cb_data) +{ + struct gcore_collect_regset_section_cb_data *data + = (struct gcore_collect_regset_section_cb_data *) cb_data; + bool variable_size_section = (regset != NULL + && regset->flags & REGSET_VARIABLE_SIZE); + + gdb_assert (variable_size_section || supply_size == collect_size); + + if (data->abort_iteration) + return; + + gdb_assert (regset != nullptr && regset->collect_regset != nullptr); + + /* This is intentionally zero-initialized by using std::vector, so + that any padding bytes in the core file will show as 0. */ + std::vector buf (collect_size); + + regset->collect_regset (regset, data->regcache, -1, buf.data (), + collect_size); + + /* PRSTATUS still needs to be treated specially. */ + if (strcmp (sect_name, ".reg") == 0) + data->note_data->reset (elfcore_write_prstatus + (data->obfd, data->note_data->release (), + data->note_size, data->lwp, + gdb_signal_to_host (data->stop_signal), + buf.data ())); + else + data->note_data->reset (elfcore_write_register_note + (data->obfd, data->note_data->release (), + data->note_size, sect_name, buf.data (), + collect_size)); + + if (data->note_data == nullptr) + data->abort_iteration = true; +} + +/* Records the register state of thread PTID out of REGCACHE into the note + buffer represented by *NOTE_DATA and NOTE_SIZE. OBFD is the bfd into + which the core file is being created, and STOP_SIGNAL is the signal that + cause thread PTID to stop. */ + +static void +gcore_collect_thread_registers (const struct regcache *regcache, + ptid_t ptid, bfd *obfd, + gdb::unique_xmalloc_ptr *note_data, + int *note_size, + enum gdb_signal stop_signal) +{ + struct gdbarch *gdbarch = regcache->arch (); + gcore_collect_regset_section_cb_data data (gdbarch, regcache, obfd, ptid, + stop_signal, note_data, + note_size); + gdbarch_iterate_over_regset_sections (gdbarch, + gcore_collect_regset_section_cb, + &data, regcache); +} + +/* See gcore.h. */ + +void +gcore_build_thread_register_notes + (struct gdbarch *gdbarch, struct thread_info *info, gdb_signal stop_signal, + bfd *obfd, gdb::unique_xmalloc_ptr *note_data, int *note_size) +{ + struct regcache *regcache + = get_thread_arch_regcache (info->inf->process_target (), + info->ptid, gdbarch); + target_fetch_registers (regcache, -1); + gcore_collect_thread_registers (regcache, info->ptid, obfd, note_data, + note_size, stop_signal); +} + +/* See gcore.h. */ + +thread_info * +gcore_find_signalled_thread () +{ + thread_info *curr_thr = inferior_thread (); + if (curr_thr->state != THREAD_EXITED + && curr_thr->suspend.stop_signal != GDB_SIGNAL_0) + return curr_thr; + + for (thread_info *thr : current_inferior ()->non_exited_threads ()) + if (thr->suspend.stop_signal != GDB_SIGNAL_0) + return thr; + + /* Default to the current thread, unless it has exited. */ + if (curr_thr->state != THREAD_EXITED) + return curr_thr; + + return nullptr; +} + void _initialize_gcore (); void _initialize_gcore () diff --git a/gdb/gcore.h b/gdb/gcore.h index af37ff39b41..ce60841c1a5 100644 --- a/gdb/gcore.h +++ b/gdb/gcore.h @@ -21,6 +21,10 @@ #define GCORE_H 1 #include "gdb_bfd.h" +#include "gdbsupport/gdb_signals.h" + +struct gdbarch; +struct thread_info; extern gdb_bfd_ref_ptr create_gcore_bfd (const char *filename); extern void write_gcore_file (bfd *obfd); @@ -28,4 +32,20 @@ extern int objfile_find_memory_regions (struct target_ops *self, find_memory_region_ftype func, void *obfd); +/* Add content to *NOTE_DATA (and update *NOTE_SIZE) to describe the + registers of thread INFO. Report the thread as having stopped with + STOP_SIGNAL. The core file is being written to OFD, and GDBARCH is the + architecture for which the core file is being generated. */ + +extern void gcore_build_thread_register_notes + (struct gdbarch *gdbarch, struct thread_info *info, gdb_signal stop_signal, + bfd *obfd, gdb::unique_xmalloc_ptr *note_data, int *note_size); + +/* Find the signalled thread. In case there's more than one signalled + thread, prefer the current thread, if it is signalled. If no thread was + signalled, default to the current thread, unless it has exited, in which + case return NULL. */ + +extern thread_info *gcore_find_signalled_thread (); + #endif /* GCORE_H */ diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c index e9f8e1b6133..3db4d31de37 100644 --- a/gdb/linux-tdep.c +++ b/gdb/linux-tdep.c @@ -39,6 +39,7 @@ #include "gdb_regex.h" #include "gdbsupport/enum-flags.h" #include "gdbsupport/gdb_optional.h" +#include "gcore.h" #include @@ -1597,104 +1598,6 @@ linux_make_mappings_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, } } -/* Structure for passing information from - linux_collect_thread_registers via an iterator to - linux_collect_regset_section_cb. */ - -struct linux_collect_regset_section_cb_data -{ - linux_collect_regset_section_cb_data (struct gdbarch *gdbarch, - const struct regcache *regcache, - bfd *obfd, - gdb::unique_xmalloc_ptr ¬e_data, - int *note_size, - unsigned long lwp, - gdb_signal stop_signal) - : gdbarch (gdbarch), regcache (regcache), obfd (obfd), - note_data (note_data), note_size (note_size), lwp (lwp), - stop_signal (stop_signal) - {} - - struct gdbarch *gdbarch; - const struct regcache *regcache; - bfd *obfd; - gdb::unique_xmalloc_ptr ¬e_data; - int *note_size; - unsigned long lwp; - enum gdb_signal stop_signal; - bool abort_iteration = false; -}; - -/* Callback for iterate_over_regset_sections that records a single - regset in the corefile note section. */ - -static void -linux_collect_regset_section_cb (const char *sect_name, int supply_size, - int collect_size, const struct regset *regset, - const char *human_name, void *cb_data) -{ - struct linux_collect_regset_section_cb_data *data - = (struct linux_collect_regset_section_cb_data *) cb_data; - bool variable_size_section = (regset != NULL - && regset->flags & REGSET_VARIABLE_SIZE); - - if (!variable_size_section) - gdb_assert (supply_size == collect_size); - - if (data->abort_iteration) - return; - - gdb_assert (regset && regset->collect_regset); - - /* This is intentionally zero-initialized by using std::vector, so - that any padding bytes in the core file will show as 0. */ - std::vector buf (collect_size); - - regset->collect_regset (regset, data->regcache, -1, buf.data (), - collect_size); - - /* PRSTATUS still needs to be treated specially. */ - if (strcmp (sect_name, ".reg") == 0) - data->note_data.reset (elfcore_write_prstatus - (data->obfd, data->note_data.release (), - data->note_size, data->lwp, - gdb_signal_to_host (data->stop_signal), - buf.data ())); - else - data->note_data.reset (elfcore_write_register_note - (data->obfd, data->note_data.release (), - data->note_size, sect_name, buf.data (), - collect_size)); - - if (data->note_data == NULL) - data->abort_iteration = true; -} - -/* Records the thread's register state for the corefile note - section. */ - -static void -linux_collect_thread_registers (const struct regcache *regcache, - ptid_t ptid, bfd *obfd, - gdb::unique_xmalloc_ptr ¬e_data, - int *note_size, - enum gdb_signal stop_signal) -{ - struct gdbarch *gdbarch = regcache->arch (); - - /* For remote targets the LWP may not be available, so use the TID. */ - long lwp = ptid.lwp (); - if (lwp == 0) - lwp = ptid.tid (); - - linux_collect_regset_section_cb_data data (gdbarch, regcache, obfd, note_data, - note_size, lwp, stop_signal); - - gdbarch_iterate_over_regset_sections (gdbarch, - linux_collect_regset_section_cb, - &data, regcache); -} - /* Fetch the siginfo data for the specified thread, if it exists. If there is no data, or we could not read it, return an empty buffer. */ @@ -1746,22 +1649,16 @@ static void linux_corefile_thread (struct thread_info *info, struct linux_corefile_thread_data *args) { - struct regcache *regcache; - - regcache = get_thread_arch_regcache (info->inf->process_target (), - info->ptid, args->gdbarch); - - target_fetch_registers (regcache, -1); - gdb::byte_vector siginfo_data = linux_get_siginfo_data (info, args->gdbarch); - - linux_collect_thread_registers (regcache, info->ptid, args->obfd, - args->note_data, args->note_size, - args->stop_signal); + gcore_build_thread_register_notes (args->gdbarch, info, args->stop_signal, + args->obfd, &args->note_data, + args->note_size); /* Don't return anything if we got no register information above, such a core file is useless. */ if (args->note_data != NULL) { + gdb::byte_vector siginfo_data + = linux_get_siginfo_data (info, args->gdbarch); if (!siginfo_data.empty ()) args->note_data.reset (elfcore_write_note (args->obfd, args->note_data.release (), @@ -1960,30 +1857,6 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p) return 1; } -/* Find the signalled thread. In case there's more than one signalled - thread, prefer the current thread, if it is signalled. If no - thread was signalled, default to the current thread, unless it has - exited, in which case return NULL. */ - -static thread_info * -find_signalled_thread () -{ - thread_info *curr_thr = inferior_thread (); - if (curr_thr->state != THREAD_EXITED - && curr_thr->suspend.stop_signal != GDB_SIGNAL_0) - return curr_thr; - - for (thread_info *thr : current_inferior ()->non_exited_threads ()) - if (thr->suspend.stop_signal != GDB_SIGNAL_0) - return thr; - - /* Default to the current thread, unless it has exited. */ - if (curr_thr->state != THREAD_EXITED) - return curr_thr; - - return nullptr; -} - /* Build the note section for a corefile, and return it in a malloc buffer. */ @@ -2021,7 +1894,7 @@ linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size) /* Like the kernel, prefer dumping the signalled thread first. "First thread" is what tools use to infer the signalled thread. */ - thread_info *signalled_thr = find_signalled_thread (); + thread_info *signalled_thr = gcore_find_signalled_thread (); gdb_signal stop_signal; if (signalled_thr != nullptr) stop_signal = signalled_thr->suspend.stop_signal;