Re: [PATCHv3 9/9] gdb/arm: add support for bare-metal core dumps

[Luis Machado via Gdb-patches <gdb-patches@sourceware.org>]

On 5/13/21 10:42 AM, Andrew Burgess wrote:
> When I merged the RISC-V bare-metal core file support I never merged
> this patch (ARM bare-metal corefile support).
> 
> I've been in discussion with the author of the original patch
> off-list, and this version of the patch does provide all of the
> functionality that was present in their original work, and the
> original author does have a copyright assignment in place.

It would be nice if the original author tested this on his end to make 
sure it works for his purpose.

> 
> I would like to propose merging this patch, even though I personally
> have not tested it.

I'm fine with that, but if the original author can test this, it would 
be better.

> 
> Thoughts?
> 
> Andrew
> 
> 
> 
> * Andrew Burgess <andrew.burgess@embecosm.com> [2021-02-15 17:29:12 +0000]:
> 
>>         **************** WARNING ********************
>>
>> This commit is based off a patch from the mailing list, I don't know
>> if the author of this patch has a copyright assignment in place.  As
>> such this commit MUST NOT be pushed to upstream master until this has
>> been cleared up.  A link to the original mailing list posting can be
>> found in the patch description below.
>>
>>         ************** END WARNING ******************
>>
>> This commit adds support for bare metal core dumps on the ARM target,
>> and is based off of this patch submitted to the mailing list:
>>
>>    https://sourceware.org/pipermail/gdb-patches/2020-October/172845.html
>>
>> Compared to the version linked above this version is updated to take
>> account of recent changes to the core dump infrastructure in GDB,
>> there is now more shared infrastructure for core dumping within GDB,
>> and also some common bare metal core dumping infrastructure.  As a
>> result this patch is smaller than the original proposed patch.
>>
>> Further, the original patch included some unrelated changes to the
>> simulator that have been removed from this version.
>>
>> I have written a ChangeLog entry as the original patch was missing
>> one.
>>
>> I have done absolutely no testing of this patch.  It is based on the
>> original submitted patch, which I assume was tested, but after my
>> modifications things might have been broken.
>>
>> The core dump format is based around generating an ELF containing
>> sections for the writable regions of memory that a user could be
>> using.  Which regions are dumped rely on GDB's existing common core
>> dumping code, GDB will attempt to figure out the stack and heap as
>> well as copying out writable data sections as identified by the
>> original ELF.
>>
>> Register information is added to the core dump using notes, just as it
>> is for Linux of FreeBSD core dumps.  The note types used consist of
>> the 2 basic types you would expect in a OS based core dump,
>> NT_PRPSINFO, NT_PRSTATUS, along with the architecture specific
>> NT_ARM_VFP note.
>>
>> The data layouts for each note type are described below, in all cases,
>> all padding fields should be set to zero.
>>
>> Note NT_PRPSINFO is optional.  Its data layout is:
>>
>>    struct prpsinfo_t
>>    {
>>      uint8_t padding[28];
>>      char fname[16];
>>      char psargs[80];
>>    }
>>
>> Field 'fname' - null terminated string consisting of the basename of
>>      (up to the fist 15 characters of) the executable.  Any additional
>>      space should be set to zero.  If there's no executable name then
>>      this field can be set to all zero.
>>
>> Field 'psargs' - a null terminated string up to 80 characters in
>>      length.  Any additional space should be filled with zero.  This
>>      field contains the full executable path and any arguments passed
>>      to the executable.  If there's nothing sensible to write in this
>>      field then fill it with zero.
>>
>> Note NT_PRSTATUS is required, its data layout is:
>>
>>    struct prstatus_t
>>    {
>>      uint8_t padding_1[12];
>>      uint16_t sig;
>>      uint8_t padding_2[10];
>>      uint32_t thread_id;
>>      uint8_t padding_3[44];
>>      uint32_t gregs[18];
>>    }
>>
>> Field 'sig' - the signal that stopped this thread.  It's implementation
>>      defined what this field actually means.  Within GDB this will be
>>      the signal number that the remote target reports as the stop
>>      reason for this thread.
>>
>> Field 'thread_is' - the thread id for this thread.  It's implementation
>>      defined what this field actually means.  Within GDB this will be
>>      thread thread-id that is assigned to each remote thread.
>>
>> Field 'gregs' - holds the general purpose registers $a1 through to $pc
>>      at indices 0 to 15.  At index 16 the program status register.
>>      Index 17 should be set to zero.
>>
>> Note NT_ARM_VFP is optional, its data layout is:
>>
>>    armvfp_t
>>    {
>>      uint64_t regs[32];
>>      uint32_t fpscr;
>>    }
>>
>> Field 'regs' - holds the 32 d-registers 0 to 31 in order.
>>
>> Field 'fpscr' - holds the fpscr register.
>>
>> The rules for ordering the notes is the same as for Linux.  The
>> NT_PRSTATUS note must come before any other notes about additional
>> register sets.  And for multi-threaded targets all registers for a
>> single thread should be grouped together.  This is because only
>> NT_PRSTATUS includes a thread-id, all additional register notes after
>> a NT_PRSTATUS are assumed to belong to the same thread until a
>> different NT_PRSTATUS is seen.
>>
>> gdb/ChangeLog:
>>
>> 	PR gdb/14383
>> 	* Makefile.in (ALL_TARGET_OBS): Add arm-none-tdep.o.
>> 	(ALLDEPFILES): Add arm-none-tdep.c
>> 	* arm-none-tdep.c: New file.
>> 	* configure.tgt (arm*-*-*): Add arm-none-tdep.o to cpu_obs.
>> ---
>>   gdb/ChangeLog       |   9 ++
>>   gdb/Makefile.in     |   2 +
>>   gdb/arm-none-tdep.c | 213 ++++++++++++++++++++++++++++++++++++++++++++
>>   gdb/configure.tgt   |   3 +-
>>   4 files changed, 226 insertions(+), 1 deletion(-)
>>   create mode 100644 gdb/arm-none-tdep.c
>>
>> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
>> index a6ca5a53655..77c2401e272 100644
>> --- a/gdb/Makefile.in
>> +++ b/gdb/Makefile.in
>> @@ -728,6 +728,7 @@ ALL_TARGET_OBS = \
>>   	arm-fbsd-tdep.o \
>>   	arm-linux-tdep.o \
>>   	arm-netbsd-tdep.o \
>> +	arm-none-tdep.o \
>>   	arm-obsd-tdep.o \
>>   	arm-pikeos-tdep.o \
>>   	arm-tdep.o \
>> @@ -2171,6 +2172,7 @@ ALLDEPFILES = \
>>   	arm-linux-tdep.c \
>>   	arm-netbsd-nat.c \
>>   	arm-netbsd-tdep.c \
>> +	arm-none-tdep.c \
>>   	arm-obsd-tdep.c \
>>   	arm-tdep.c \
>>   	avr-tdep.c \
>> diff --git a/gdb/arm-none-tdep.c b/gdb/arm-none-tdep.c
>> new file mode 100644
>> index 00000000000..2816c5954b3
>> --- /dev/null
>> +++ b/gdb/arm-none-tdep.c
>> @@ -0,0 +1,213 @@
>> +/* none on ARM target support.
>> +
>> +   Copyright (C) 2020-2021 Free Software Foundation, Inc.
>> +
>> +   This file is part of GDB.
>> +
>> +   This program is free software; you can redistribute it and/or modify
>> +   it under the terms of the GNU General Public License as published by
>> +   the Free Software Foundation; either version 3 of the License, or
>> +   (at your option) any later version.
>> +
>> +   This program is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>> +
>> +#include "defs.h"
>> +#include "arm-tdep.h"
>> +#include "arch-utils.h"
>> +#include "regcache.h"
>> +#include "elf-bfd.h"
>> +#include "regset.h"
>> +#include "user-regs.h"
>> +
>> +#ifdef HAVE_ELF
>> +#include "elf-none-tdep.h"
>> +#endif
>> +
>> +/* Core file and register set support.  */
>> +#define ARM_NONE_SIZEOF_GREGSET (18 * ARM_INT_REGISTER_SIZE)
>> +
>> +/* Support VFP register format.  */
>> +#define ARM_NONE_SIZEOF_VFP (32 * 8 + 4)
>> +
>> +/* The index to access CPSR in user_regs as defined in GLIBC.  */
>> +#define ARM_NONE_CPSR_GREGNUM 16
>> +
>> +/* Supply register REGNUM from buffer GREGS_BUF (length LEN bytes) into
>> +   REGCACHE.  If REGNUM is -1 then supply all registers.  The set of
>> +   registers that this function will supply is limited to the general
>> +   purpose registers.
>> +
>> +   The layout of the registers here is based on the ARM GNU/Linux
>> +   layout.  */
>> +
>> +static void
>> +arm_none_supply_gregset (const struct regset *regset,
>> +			 struct regcache *regcache,
>> +			 int regnum, const void *gregs_buf, size_t len)
>> +{
>> +  struct gdbarch *gdbarch = regcache->arch ();
>> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>> +  const gdb_byte *gregs = (const gdb_byte *) gregs_buf;
>> +
>> +  for (int regno = ARM_A1_REGNUM; regno < ARM_PC_REGNUM; regno++)
>> +    if (regnum == -1 || regnum == regno)
>> +      regcache->raw_supply (regno, gregs + ARM_INT_REGISTER_SIZE * regno);
>> +
>> +  if (regnum == ARM_PS_REGNUM || regnum == -1)
>> +    {
>> +      if (arm_apcs_32)
>> +	regcache->raw_supply (ARM_PS_REGNUM,
>> +			      gregs + ARM_INT_REGISTER_SIZE
>> +			      * ARM_NONE_CPSR_GREGNUM);
>> +      else
>> +	regcache->raw_supply (ARM_PS_REGNUM,
>> +			     gregs + ARM_INT_REGISTER_SIZE * ARM_PC_REGNUM);
>> +    }
>> +
>> +  if (regnum == ARM_PC_REGNUM || regnum == -1)
>> +    {
>> +      gdb_byte pc_buf[ARM_INT_REGISTER_SIZE];
>> +
>> +      CORE_ADDR reg_pc
>> +	= extract_unsigned_integer (gregs + ARM_INT_REGISTER_SIZE
>> +				    * ARM_PC_REGNUM,
>> +				    ARM_INT_REGISTER_SIZE, byte_order);
>> +      reg_pc = gdbarch_addr_bits_remove (gdbarch, reg_pc);
>> +      store_unsigned_integer (pc_buf, ARM_INT_REGISTER_SIZE, byte_order,
>> +			      reg_pc);
>> +      regcache->raw_supply (ARM_PC_REGNUM, pc_buf);
>> +    }
>> +}
>> +
>> +/* Collect register REGNUM from REGCACHE and place it into buffer GREGS_BUF
>> +   (length LEN bytes).  If REGNUM is -1 then collect all registers.  The
>> +   set of registers that this function will collect is limited to the
>> +   general purpose registers.
>> +
>> +   The layout of the registers here is based on the ARM GNU/Linux
>> +   layout.  */
>> +
>> +static void
>> +arm_none_collect_gregset (const struct regset *regset,
>> +			  const struct regcache *regcache,
>> +			  int regnum, void *gregs_buf, size_t len)
>> +{
>> +  gdb_byte *gregs = (gdb_byte *) gregs_buf;
>> +
>> +  for (int regno = ARM_A1_REGNUM; regno < ARM_PC_REGNUM; regno++)
>> +    if (regnum == -1 || regnum == regno)
>> +      regcache->raw_collect (regno,
>> +			     gregs + ARM_INT_REGISTER_SIZE * regno);
>> +
>> +  if (regnum == ARM_PS_REGNUM || regnum == -1)
>> +    {
>> +      if (arm_apcs_32)
>> +	regcache->raw_collect (ARM_PS_REGNUM,
>> +			       gregs + ARM_INT_REGISTER_SIZE
>> +			       * ARM_NONE_CPSR_GREGNUM);
>> +      else
>> +	regcache->raw_collect (ARM_PS_REGNUM,
>> +			       gregs + ARM_INT_REGISTER_SIZE * ARM_PC_REGNUM);
>> +    }
>> +
>> +  if (regnum == ARM_PC_REGNUM || regnum == -1)
>> +    regcache->raw_collect (ARM_PC_REGNUM,
>> +			   gregs + ARM_INT_REGISTER_SIZE * ARM_PC_REGNUM);
>> +}
>> +
>> +/* Supply VFP registers from REGS_BUF into REGCACHE.  */
>> +
>> +static void
>> +arm_none_supply_vfp (const struct regset *regset,
>> +		     struct regcache *regcache,
>> +		     int regnum, const void *regs_buf, size_t len)
>> +{
>> +  const gdb_byte *regs = (const gdb_byte *) regs_buf;
>> +
>> +  if (regnum == ARM_FPSCR_REGNUM || regnum == -1)
>> +    regcache->raw_supply (ARM_FPSCR_REGNUM, regs + 32 * 8);
>> +
>> +  for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
>> +    if (regnum == -1 || regnum == regno)
>> +      regcache->raw_supply (regno, regs + (regno - ARM_D0_REGNUM) * 8);
>> +}
>> +
>> +/* Collect VFP registers from REGCACHE into REGS_BUF.  */
>> +
>> +static void
>> +arm_none_collect_vfp (const struct regset *regset,
>> +		      const struct regcache *regcache,
>> +		      int regnum, void *regs_buf, size_t len)
>> +{
>> +  gdb_byte *regs = (gdb_byte *) regs_buf;
>> +
>> +  if (regnum == ARM_FPSCR_REGNUM || regnum == -1)
>> +    regcache->raw_collect (ARM_FPSCR_REGNUM, regs + 32 * 8);
>> +
>> +  for (int regno = ARM_D0_REGNUM; regno <= ARM_D31_REGNUM; regno++)
>> +    if (regnum == -1 || regnum == regno)
>> +      regcache->raw_collect (regno, regs + (regno - ARM_D0_REGNUM) * 8);
>> +}
>> +
>> +/* The general purpose register set.  */
>> +
>> +static const struct regset arm_none_gregset =
>> +  {
>> +    nullptr, arm_none_supply_gregset, arm_none_collect_gregset
>> +  };
>> +
>> +/* The VFP register set.  */
>> +
>> +static const struct regset arm_none_vfpregset =
>> +  {
>> +    nullptr, arm_none_supply_vfp, arm_none_collect_vfp
>> +  };
>> +
>> +/* Iterate over core file register note sections.  */
>> +
>> +static void
>> +arm_none_iterate_over_regset_sections (struct gdbarch *gdbarch,
>> +				       iterate_over_regset_sections_cb *cb,
>> +				       void *cb_data,
>> +				       const struct regcache *regcache)
>> +{
>> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>> +
>> +  cb (".reg", ARM_NONE_SIZEOF_GREGSET, ARM_NONE_SIZEOF_GREGSET,
>> +      &arm_none_gregset, nullptr, cb_data);
>> +
>> +  if (tdep->vfp_register_count > 0)
>> +    cb (".reg-arm-vfp", ARM_NONE_SIZEOF_VFP, ARM_NONE_SIZEOF_VFP,
>> +	&arm_none_vfpregset, "VFP floating-point", cb_data);
>> +}
>> +
>> +/* Initialize ARM bare-metal ABI info.  */
>> +
>> +static void
>> +arm_none_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>> +{
>> +#ifdef HAVE_ELF
>> +  elf_none_init_abi (gdbarch);
>> +#endif
>> +
>> +  /* Iterate over registers for reading and writing bare metal ARM core
>> +     files.  */
>> +  set_gdbarch_iterate_over_regset_sections
>> +    (gdbarch, arm_none_iterate_over_regset_sections);
>> +}
>> +
>> +/* Initialize ARM bare-metal target support.  */
>> +
>> +void _initialize_arm_none_tdep ();
>> +void
>> +_initialize_arm_none_tdep ()
>> +{
>> +  gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_NONE,
>> +			  arm_none_init_abi);
>> +}
>> diff --git a/gdb/configure.tgt b/gdb/configure.tgt
>> index 91020678bf0..f81f03b96c9 100644
>> --- a/gdb/configure.tgt
>> +++ b/gdb/configure.tgt
>> @@ -65,7 +65,8 @@ arc*-*-*)
>>   
>>   arm*-*-*)
>>   	cpu_obs="aarch32-tdep.o arch/aarch32.o arch/arm.o \
>> -		 arch/arm-get-next-pcs.o arm-tdep.o";;
>> +		 arch/arm-get-next-pcs.o arm-tdep.o arm-none-tdep.o"
>> +	;;
>>   
>>   hppa*-*-*)
>>   	# Target: HP PA-RISC
>> -- 
>> 2.25.4
>>