From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 73A4D38618DF for ; Sat, 3 Oct 2020 03:57:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 73A4D38618DF Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark@simark.ca Received: from [10.0.0.11] (173-246-6-90.qc.cable.ebox.net [173.246.6.90]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id BD5EB1E58E; Fri, 2 Oct 2020 23:57:02 -0400 (EDT) Subject: Re: [PATCH] gdb: add support for handling core dumps on arm-none-eabi To: Paul Mathieu , Luis Machado Cc: gdb-patches@sourceware.org References: From: Simon Marchi Message-ID: <55f5bbd9-86ce-c9bc-61ac-5447f078d2ec@simark.ca> Date: Fri, 2 Oct 2020 23:57:02 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: fr Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-9.0 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, NICE_REPLY_A, SPF_HELO_PASS, 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: Sat, 03 Oct 2020 03:57:05 -0000 On 2020-10-02 5:54 p.m., Paul Mathieu via Gdb-patches wrote: > Woops, sorry about this. I'm submitting a new patch, hopefully it will be > treated well by the email gods. Sorry, I replied to this message but I completely missed the rest of your reply below. Note that I am unable to apply this patch either because long lines were again wrapped by your email client. Please use git-send-email. You can still reply in the thread by using --in-reply-to. >> Who is generating such core files? Is there some way I can access the >> tool to give it a try? How can I test the patch itself? >> > > I made one myself, with a python script that I wrote. > I have a live embedded target that I can use to dump memory regions and CPU > registers over a serial line. My python script then formats the memory > dumps into PT_LOAD segments, and the CPU registers into a NT_PRSTATUS note > segment, and stores it as an ELF32 core file. > I'm happy to share any material that would help testing this, including the > python script, an example of firmware and core file and some embedded > source code. Ok, so that answers the question in my other email, about which tool produces this format of core. The answer is this specific python script, and the format is "Paul's ARM core format" :). I don't know what Luis thinks about this, but I would be a bit hesitant to add support in GDB for a core format that's not standard nor the output of some "well-known" tool (which would be a de facto standard). Is there a format that already exists in the ARM ecosystem for core dumps of bare-metal systems, we could base our stuff on? Alternatively, do you think you could implement GDB's generate-core-file command for bare-metal ARM? First, it would make sense for GDB to be able to produce a core in some format and be able to ingest it again. And it would act as some kind of documentation / reference implementation of what GDB expects, and that could become some de facto standard. People who would like to have their core readable by GDB would produce it in this format. > I addressed the rest of your feedback through essentially re-writing the > patch to not use `deprecated_add_core_fns` > In the new patch, I had to use the GDB_OSABI_LINUX, otherwise my core > wouldn't be recognized. I'm not sure if this can potentially interfere with > the arm-linux implementation. Since it has its own entry in configure.tgt, > I imagine not? Aren't you able to use GDB_OSABI_NONE? But see comments below. > gdb/ChangeLog: > 2018-09-29 Robin Haberkorn > 2020-10-02 Paul Mathieu > * arm-none-tdep.c: Source file added. Provide CPU registers from a core > file > * floating point registers not yet supported (FIXME) > --- > gdb/Makefile.in | 2 + > gdb/arm-none-tdep.c | 107 ++++++++++++++++++++++++++++++++++++++++++++ > gdb/configure.tgt | 2 +- > 3 files changed, 110 insertions(+), 1 deletion(-) > create mode 100644 gdb/arm-none-tdep.c > > diff --git a/gdb/Makefile.in b/gdb/Makefile.in > index dbede7a9cf..7f0e3ea0b0 100644 > --- a/gdb/Makefile.in > +++ b/gdb/Makefile.in > @@ -720,6 +720,7 @@ ALL_TARGET_OBS = \ > arm-obsd-tdep.o \ > arm-pikeos-tdep.o \ > arm-symbian-tdep.o \ > + arm-none-tdep.o \ > arm-tdep.o \ > arm-wince-tdep.o \ > avr-tdep.o \ > @@ -2150,6 +2151,7 @@ ALLDEPFILES = \ > arm-nbsd-tdep.c \ > arm-obsd-tdep.c \ > arm-symbian-tdep.c \ > + arm-none-tdep.c \ > arm-tdep.c \ > avr-tdep.c \ > bfin-linux-tdep.c \ Please keep the lists sorted. Note that so far, everything related to bare-metal for an architecture was simply in -tdep.c. So perhaps it should just be there? I'll defer to Luis to decide what is the best organization for the ARM code. > +static void > +arm_none_supply_gregset (const struct regset *regset, > + struct regcache *regcache, int regnum, > + const void *gregs, size_t len) > +{ > + const arm_none_reg *gregset = static_cast (gregs); > + gdb_assert (len >= sizeof (arm_none_reg)); > + > + /* Integer registers. */ > + for (int i = ARM_A1_REGNUM; i < ARM_SP_REGNUM; i++) > + if (regnum == -1 || regnum == i) > + regcache->raw_supply (i, (char *)&gregset->reg[i]); > + > + if (regnum == -1 || regnum == ARM_SP_REGNUM) > + regcache->raw_supply (ARM_SP_REGNUM, (char *)&gregset->sp); > + > + if (regnum == -1 || regnum == ARM_LR_REGNUM) > + regcache->raw_supply (ARM_LR_REGNUM, (char *)&gregset->lr); > + > + if (regnum == -1 || regnum == ARM_PC_REGNUM) > + { > + CORE_ADDR r_pc > + = gdbarch_addr_bits_remove (regcache->arch (), gregset->pc); We'd typically use one ident (two spaces) before the equal sign. But otherwise that's spot on. > + regcache->raw_supply (ARM_PC_REGNUM, (char *)&r_pc); > + } > + > + if (regnum == -1 || regnum == ARM_PS_REGNUM) > + { > + if (arm_apcs_32) > + regcache->raw_supply (ARM_PS_REGNUM, (char *)&gregset->cpsr); > + else > + regcache->raw_supply (ARM_PS_REGNUM, (char *)&gregset->pc); > + } > +} > + > +static const struct regset arm_none_regset > + = { nullptr, arm_none_supply_gregset, > + /* We don't need a collect function because we only use this > reading > + registers (via iterate_over_regset_sections and > + fetch_regs/fetch_register). */ > + nullptr, 0 }; See above where `reading` is on a separate line, that's an example of the patch getting mangled by the email client. > +void _initialize_arm_none_tdep (void); > +void > +_initialize_arm_none_tdep (void) > +{ > + gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_LINUX, > + arm_none_elf_init_abi); I don't think this is what you want to do. This "declares" that combination of architecture ARM and osabi Linux exists, and that arm_none_elf_init_abi should be called when such a combo is detected to initialize the gdbarch object. However, such a combo is already registered somewhere else, as you can guess: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/arm-linux-tdep.c;h=f60cb51763f1c5b14811494b646acf54b62d1f10;hb=HEAD#l2010 So I presume that if you build a GDB configured with --enable-targets=all (which will include arm-linux-tdep.c), you will hit this assertion: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/osabi.c;h=e8a813b62f268417edf5f0ad164c9c942bd43b1e;hb=HEAD#l179 What I think you want is to register an osabi sniffer, whose job is to try to guess an osabi given a bfd. Apparently that bfd can sometimes represent the ELF core file, so you can look up sections and see if that file looks like a core file you can handle. And if so, return GDB_OSABI_NONE. And then, you'd need to register (with gdbarch_register_osabi) osabi GDB_OSABI_NONE with the arm architecture (same as you did, but with GDB_OSABI_NONE instead of GDB_OSABI_LINUX) with the callback to call when this combo is detected: gdbarch_register_osabi (bfd_arch_arm, 0, GDB_OSABI_NONE, arm_none_elf_init_abi); I'm not sure about that part, but I think it's the right direction. Now, the question is how to recognize one of these cores? As an example, to recognize Cygwin core dumps (which use ELF files as a container without a recognizable ABI tag, so it's kind of the same situation as you), we use the .reg section size to determine if the BFD is a Cygwin core: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/amd64-windows-tdep.c;h=e427c20538961bda2a1a0f033207eebce64c4729;hb=HEAD#l1387 So, you can do that, although I think we all agree that it's not ideal. If there was some way of identifying that the core is in the format we recognize (say, if there was some special section with a tag), it would make it much easier to identify it without having to guess. Simon