From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf2a.google.com (mail-qv1-xf2a.google.com [IPv6:2607:f8b0:4864:20::f2a]) by sourceware.org (Postfix) with ESMTPS id 9D2603894C27 for ; Thu, 13 May 2021 13:51:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9D2603894C27 Received: by mail-qv1-xf2a.google.com with SMTP id z1so13791933qvo.4 for ; Thu, 13 May 2021 06:51:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=HqMDNOipsqgixSgq6/SmhRPO1ZdJkGjQAron/OLytE0=; b=ObEtWgS6MGmmj+5lYw2vgsTQl2JcVbr8JVAQuNF6dFltW+g32epiNy9NaH5MtRp3FR Speuni0biKLGsx9hLiOrHVcpDqGpngDNQY1n5yysWTXtI/bIkGZWYGPt4m3hyPlpg7QG uutasT4Q8U9zVxMKrRbFjv9dt9R6MCiWsgAV85T+RuzJEaOQLkf8GKQY/Udoe4x2EtEw fIgQWkgHX25GYhtGNaHqD2KhB/QeD3wElZJDFJz/ziImYx86Fh8Uq4WZWljaBRW2JVY7 gcUD124O+F3k9Yj384ichS0dRtDSVqzNd+0YLrPJ83AIDLwYhAvQZp8aVQu9EYOHgm0R e4fg== X-Gm-Message-State: AOAM532NBofpfqMxIn3olPQEBrMdUzwHHTJnDA5RP3K9SVl0Ur1b6kTJ ay0iqNims4zhD9ns2aJj+0VB2djFOuevZA== X-Google-Smtp-Source: ABdhPJyuzE+HMG69Bz8esQeYHhlJLGkp2KTZyRe5Eh/jxh5sZ0jUMdeSqwI6NiqVgRftCu14L6hTTw== X-Received: by 2002:ad4:5f48:: with SMTP id p8mr37994101qvg.55.1620913911020; Thu, 13 May 2021 06:51:51 -0700 (PDT) Received: from ?IPv6:2804:7f0:4841:40ad:71ff:69e2:5b42:ca57? ([2804:7f0:4841:40ad:71ff:69e2:5b42:ca57]) by smtp.gmail.com with ESMTPSA id n13sm2350320qtl.48.2021.05.13.06.51.49 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 13 May 2021 06:51:50 -0700 (PDT) Subject: Re: [PATCHv3 9/9] gdb/arm: add support for bare-metal core dumps To: Andrew Burgess , gdb-patches@sourceware.org References: <9019d61b8786631421728da83d077fd029fc45d2.1613410057.git.andrew.burgess@embecosm.com> <20210513134223.GG3067949@embecosm.com> From: Luis Machado Message-ID: <8bb2175b-979d-7420-4813-6a9720f12fab@linaro.org> Date: Thu, 13 May 2021 10:51:47 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210513134223.GG3067949@embecosm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 May 2021 13:51:54 -0000 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 [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 . */ >> + >> +#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 >>