[RFC,4/4] gdbserver: Add RISC-V/Linux support

  Implement RISC-V/Linux support for both RV64 and RV32 systems, including 
XML target description handling based on features determined, GPR and 
FPR regset support including dynamic sizing of the latter, and software 
breakpoint handling.

Also handle a glibc bug where ELF_NFPREG is defined in terms of NFPREG,
however NFPREG is nowhere defined.

	gdb/
	* arch/riscv.h (riscv_create_target_description): Remove `const' 
	qualifier from the return type.
	* arch/riscv.c (riscv_create_target_description): Likewise.

	gdb/gdbserver/
	* linux-riscv-low.c: New file.
	* Makefile.in (SFILES): Add linux-riscv-low.c and arch/riscv.c.
	* configure.srv <riscv*-*-linux*> (srv_tgtobj)
	(srv_linux_regsets, srv_linux_usrregs, srv_linux_thread_db): 
	Define.
---
Hi,

 Posted as an RFC as this hits a major issue with GDB rejecting the XML 
description supplied, e.g.:

warning: while parsing target description (at line 4): Target description specified unknown architecture "riscv:rv64id"
warning: Could not load XML target description; ignoring

with HiFive Unleashed; clearly GDB is unhappy with the `riscv:rv64id' 
architecture name produced by its own shared code, and not surprisingly, 
as it only has `riscv', `riscv:rv64' and `riscv:rv32' available to choose 
from as far as RISC-V targets are concerned, as easily verified with:

(gdb) set architecture

As the architecture names come from BFD it will clearly have to be there 
that the problem is fixed, with the ISA suffixes driving the choice of 
instructions handled by the disassembler and possibly other functions.

 Also I think large parts of `riscv_linux_nat_target::read_description' in 
RISC-V/Linux/native support could be factored out to a piece of code 
shared with `riscv_arch_setup'.  This might be less important a problem, 
but nevertheless I think it should be done sooner than later so as not to 
make the two pieces diverge, and a preparatory change inserted ahead of 
this one for this purpose might actually be the best approach.  I have 
planned to do that once the XML target description acceptance problem has 
been fixed.

 I have run initial lp64d multilib remote testing with HiFive Unleashed as 
the target.  Results were as follows:

		=== gdb Summary ===

# of expected passes		58054
# of unexpected failures	621
# of unexpected successes	4
# of expected failures		49
# of unknown successes		5
# of known failures		72
# of unresolved testcases	110
# of untested testcases		100
# of unsupported tests		232

but no further analysis has been made and also they come from Nov 6th 
only, so they may not be up to date with current GDB.  I thought it would 
be more important to get the XML issue fixed first.  Also native gdbserver 
testing should also be done as well as should native reference testing.

 Questions, comments?

  Maciej
---
 gdb/arch/riscv.c                |    2 
 gdb/arch/riscv.h                |    4 
 gdb/gdbserver/Makefile.in       |    2 
 gdb/gdbserver/configure.srv     |    6 
 gdb/gdbserver/linux-riscv-low.c |  300 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 311 insertions(+), 3 deletions(-)

gdb-riscv-gdbserver-linux.diff
  

>>>>> "Maciej" == Maciej W Rozycki <macro@wdc.com> writes:

Maciej> Implement RISC-V/Linux support for both RV64 and RV32 systems, including 
Maciej> XML target description handling based on features determined, GPR and 
Maciej> FPR regset support including dynamic sizing of the latter, and software 
Maciej> breakpoint handling.

I saw a couple of small nits here.

Maciej> Also handle a glibc bug where ELF_NFPREG is defined in terms of NFPREG,
Maciej> however NFPREG is nowhere defined.

In case you haven't already, please report this to glibc.

Maciej> As the architecture names come from BFD it will clearly have to be there 
Maciej> that the problem is fixed, with the ISA suffixes driving the choice of 
Maciej> instructions handled by the disassembler and possibly other functions.

Seems reasonable enough.  I don't really know much about this area
though.

Maciej>  Also I think large parts of `riscv_linux_nat_target::read_description' in 
Maciej> RISC-V/Linux/native support could be factored out to a piece of code 
Maciej> shared with `riscv_arch_setup'.  This might be less important a problem, 
Maciej> but nevertheless I think it should be done sooner than later so as not to 
Maciej> make the two pieces diverge

Agreed.

Maciej> +/* Implementation of linux_target_ops method "regs_info".  */
Maciej> +
Maciej> +static const struct regs_info *
Maciej> +riscv_regs_info (void)

We stopped using "(void)" in new code, in favor of just "()".
This showed up in a few spots.

Tom
  

On Thu, Jan 23, 2020 at 11:42 AM Maciej W. Rozycki <macro@wdc.com> wrote:
> Also handle a glibc bug where ELF_NFPREG is defined in terms of NFPREG,
> however NFPREG is nowhere defined.

I remember that this was discussed on the glibc lists, but I don't
know if a formal bug report was filed.  We unfortunately don't have an
active glibc maintainer, and I'm helping to maintain so much stuff
that I'm not fixing a glibc bug unless it is blocking someone's
progress, and this does not appear to be a blocker for this patch.

>         gdb/
>         * arch/riscv.h (riscv_create_target_description): Remove `const'
>         qualifier from the return type.
>         * arch/riscv.c (riscv_create_target_description): Likewise.
>
>         gdb/gdbserver/
>         * linux-riscv-low.c: New file.
>         * Makefile.in (SFILES): Add linux-riscv-low.c and arch/riscv.c.
>         * configure.srv <riscv*-*-linux*> (srv_tgtobj)
>         (srv_linux_regsets, srv_linux_usrregs, srv_linux_thread_db):
>         Define.

I didn't really review the patch since I've never done any gdbserver
work.  But I did build and test it and it worked for me.  My gdb
testsuite results look roughly the same with and without the patch
set.

I did have to add a missing build_gdbserver=yes line to the
riscv*-*-linux* block in configure.tgt.  I will submit that as a patch
if you don't.

>  Posted as an RFC as this hits a major issue with GDB rejecting the XML
> description supplied, e.g.:
>
> warning: while parsing target description (at line 4): Target description specified unknown architecture "riscv:rv64id"
> warning: Could not load XML target description; ignoring

The problem isn't only in bfd.  gdb/arch/riscv.c
riscv_create_target_description() creates the same kind of strings.
Anyways, this seems to be a simple issue with bfd/cpu-riscv.c using
bfd_default_scan.  It needs to be fixed to use a riscv specific scan
function that ignores trailing characters in the string when they
don't affect the bfd arch match.  I was able to reproduce the problem,
and wrote a bfd patch that is working for me.  This should not be a
blocking factor for the patch, as I can get the bfd patch in tomorrow
or so, unless maybe you want to try it first.

Jim
  

On Thu, 23 Jan 2020, Tom Tromey wrote:

> Maciej> Implement RISC-V/Linux support for both RV64 and RV32 systems, including 
> Maciej> XML target description handling based on features determined, GPR and 
> Maciej> FPR regset support including dynamic sizing of the latter, and software 
> Maciej> breakpoint handling.
> 
> I saw a couple of small nits here.

 Thanks for looking through my proposal!

> Maciej> Also handle a glibc bug where ELF_NFPREG is defined in terms of NFPREG,
> Maciej> however NFPREG is nowhere defined.
> 
> In case you haven't already, please report this to glibc.

 Yep, see: <https://sourceware.org/ml/libc-alpha/2019-10/msg00233.html>, 
and the discussion downthread.

> Maciej> +/* Implementation of linux_target_ops method "regs_info".  */
> Maciej> +
> Maciej> +static const struct regs_info *
> Maciej> +riscv_regs_info (void)
> 
> We stopped using "(void)" in new code, in favor of just "()".
> This showed up in a few spots.

 Right, I'm pretty much in the C world still; will fix that in the next 
iteration.

  Maciej
  

On Thu, 23 Jan 2020, Jim Wilson wrote:

> > Also handle a glibc bug where ELF_NFPREG is defined in terms of NFPREG,
> > however NFPREG is nowhere defined.
> 
> I remember that this was discussed on the glibc lists, but I don't
> know if a formal bug report was filed.  We unfortunately don't have an
> active glibc maintainer, and I'm helping to maintain so much stuff
> that I'm not fixing a glibc bug unless it is blocking someone's
> progress, and this does not appear to be a blocker for this patch.

 I actually volunteered to become a glibc RISC-V machine maintainer, but 
then I have to admit I haven't been pursuing it particularly actively. :(

> >         gdb/
> >         * arch/riscv.h (riscv_create_target_description): Remove `const'
> >         qualifier from the return type.
> >         * arch/riscv.c (riscv_create_target_description): Likewise.
> >
> >         gdb/gdbserver/
> >         * linux-riscv-low.c: New file.
> >         * Makefile.in (SFILES): Add linux-riscv-low.c and arch/riscv.c.
> >         * configure.srv <riscv*-*-linux*> (srv_tgtobj)
> >         (srv_linux_regsets, srv_linux_usrregs, srv_linux_thread_db):
> >         Define.
> 
> I didn't really review the patch since I've never done any gdbserver
> work.  But I did build and test it and it worked for me.  My gdb
> testsuite results look roughly the same with and without the patch
> set.
> 
> I did have to add a missing build_gdbserver=yes line to the
> riscv*-*-linux* block in configure.tgt.  I will submit that as a patch
> if you don't.

 Indeed, that's useful for a native configuration to build the gdbserver/ 
subdirectory automagically and I missed that, having not looked into it 
(for cross-compilation you need to build `gdbserver' explictly of course).  
Now fixed and included with an updated version, thanks for catching it!

> >  Posted as an RFC as this hits a major issue with GDB rejecting the XML
> > description supplied, e.g.:
> >
> > warning: while parsing target description (at line 4): Target description specified unknown architecture "riscv:rv64id"
> > warning: Could not load XML target description; ignoring
> 
> The problem isn't only in bfd.  gdb/arch/riscv.c
> riscv_create_target_description() creates the same kind of strings.

 Well, that's exactly where `gdbserver' gets its string from.

> Anyways, this seems to be a simple issue with bfd/cpu-riscv.c using
> bfd_default_scan.  It needs to be fixed to use a riscv specific scan
> function that ignores trailing characters in the string when they
> don't affect the bfd arch match.  I was able to reproduce the problem,
> and wrote a bfd patch that is working for me.  This should not be a
> blocking factor for the patch, as I can get the bfd patch in tomorrow
> or so, unless maybe you want to try it first.

 Native regression-testing of 1/4 has now completed successfully, so I'll 
run remote testing now with your patch preapplied and adjustments made to 
4/4.  We'll see how it goes.

  Maciej