This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 3/3] gdbserver: Add RISC-V/Linux support
- From: Jim Wilson <jimw at sifive dot com>
- To: "Maciej W. Rozycki" <macro at wdc dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>, Andrew Burgess <andrew dot burgess at embecosm dot com>, Palmer Dabbelt <palmer at dabbelt dot com>, Tom Tromey <tom at tromey dot com>, "guoren at kernel dot org" <guoren at kernel dot org>, "lifang_xia at c-sky dot com" <lifang_xia at c-sky dot com>, "yunhai_shang at c-sky dot com" <yunhai_shang at c-sky dot com>, "jiangshuai_li at c-sky dot com" <jiangshuai_li at c-sky dot com>
- Date: Wed, 29 Jan 2020 15:39:32 -0800
- Subject: Re: [PATCH v2 3/3] gdbserver: Add RISC-V/Linux support
- References: <alpine.LFD.2.21.2001291207310.14118@redsun52.ssa.fujisawa.hgst.com> <alpine.LFD.2.21.2001291412560.14118@redsun52.ssa.fujisawa.hgst.com>
On Wed, Jan 29, 2020 at 10:14 AM Maciej W. Rozycki <macro@wdc.com> wrote:
> gdb/
> * arch/riscv.h (riscv_create_target_description): Remove `const'
> qualifier from the return type.
> * arch/riscv.c (riscv_create_target_description): Likewise.
> * nat/riscv-linux-tdesc.h (riscv_linux_read_description):
> Likewise.
> * nat/riscv-linux-tdesc.c (riscv_linux_read_description):
> Likewise.
> * configure.tgt <riscv*-*-linux*>: Set build_gdbserver=yes.
>
> gdb/gdbserver/
> * linux-riscv-low.c: New file.
> * Makefile.in (SFILES): Add linux-riscv-low.c, arch/riscv.c, and
> nat/riscv-linux-tdesc.c.
> * configure.srv <riscv*-*-linux*> (srv_tgtobj)
> (srv_linux_regsets, srv_linux_usrregs, srv_linux_thread_db):
> Define.
Looks good to me, though I'm not an official review.
I noticed on the gdbserver console I'm getting a lot of ptrace warnings.
ptrace(regsets_fetch_inferior_registers) PID=1678103: Invalid argument
Warning: ptrace(regsets_store_inferior_registers): Invalid argument
This looks like a side effect of having two FP regsets defined, it
tries the first one, fails, and then tries the second one which is
correct. If you mark them as OPTIONAL_REGS we would only get the
warning once which would be OK, except that they can't be both FP_REGS
and OPTIONAL_REGS at the same time. I don't know what if anything
would break if they aren't marked as FP_REGS. I only see explicit
checks for GENERAL_REGS and OPTIONAL_REGS; I don't see any checks for
FP_REGS. Anyways, I would suggest as a future improvement that the
linux gdbserver regset support be extended so that a regset can be
marked as both FP_REGS and OPTIONAL_REGS.
There are some new functions and structures that don't have
explanatory comments before them, but this is a minor issue.
Jim