[4/5] RISC-V: Add native linux support.

  * Andreas Schwab <schwab@suse.de> [2018-10-25 12:49:09 +0200]:

> On Aug 08 2018, Jim Wilson <jimw@sifive.com> wrote:
> 
> > +  if ((regnum == RISCV_CSR_MISA_REGNUM)
> > +      || (regnum == -1))
> > +    {
> > +      /* TODO: Need to add a ptrace call for this.  */
> > +      regcache->raw_supply_zeroed (regnum);
> 
> ../../gdb/gdb/regcache.c:337: internal-error: void reg_buffer::assert_regnum(int) const: Assertion `regnum >= 0' failed.

Thanks for the report.

I pushed the patch below to fix this issue.

Thanks,
Andrew

---

[PATCH] gdb/riscv: Use correct regnum in riscv_linux_nat_target::fetch_registers

In riscv_linux_nat_target::fetch_registers, if we are asked to supply
all registers (regnum parameter is -1), then we currently end up
calling regcache::raw_supply_zeroed with the regnum -1, which is
invalid.  Instead we should be passing the regnum of the specific
register we wish to supply zeroed, in this case RISCV_CSR_MISA_REGNUM.

I removed the extra { ... } block in line with the coding standard
while editing this area.

gdb/ChangeLog:

	* riscv-linux-nat.c (riscv_linux_nat_target::fetch_registers):
	Pass correct regnum to raw_supply_zeroed.
---
 gdb/ChangeLog         | 5 +++++
 gdb/riscv-linux-nat.c | 6 ++----
 2 files changed, 7 insertions(+), 4 deletions(-)
  

On 10/25/2018 12:09 PM, Andrew Burgess wrote:
> 
> I removed the extra { ... } block in line with the coding standard
> while editing this area.

Actually, this applies here:

> Any two or more lines in code should be wrapped in braces, even if they are comments, as they look like separate statements: 
> if (i)
>   {
>     /* Return success.  */
>     return 0;
>   }
> 
> and not:
> 
> if (i)
>   /* Return success.  */
>   return 0;

From:

  https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards?highlight=%28coding%29%7C%28conventions%29#Whitespaces

Thanks,
Pedro Alves
  

On 10/25/18 4:09 AM, Andrew Burgess wrote:
> diff --git a/gdb/riscv-linux-nat.c b/gdb/riscv-linux-nat.c
> index 7dbfe651f2c..c09121d052b 100644
> --- a/gdb/riscv-linux-nat.c
> +++ b/gdb/riscv-linux-nat.c
> @@ -201,10 +201,8 @@ riscv_linux_nat_target::fetch_registers (struct regcache *regcache, int regnum)
>  
>    if ((regnum == RISCV_CSR_MISA_REGNUM)
>        || (regnum == -1))
> -    {
> -      /* TODO: Need to add a ptrace call for this.  */
> -      regcache->raw_supply_zeroed (regnum);
> -    }
> +    /* TODO: Need to add a ptrace call for this.  */
> +    regcache->raw_supply_zeroed (RISCV_CSR_MISA_REGNUM);
>  
>    /* Access to other CSRs has potential security issues, don't support them for
>       now.  */

Oops, I just replied to Andrew directly on the commit, but probably better to reply
on the list:

Now that the MISA defaults to 0 if not present, would it better to just remove
this and not set it to 0 explicitly?  The FreeBSD native target for RISC-V
doesn't set MISA to anything at all.
  

On Thu, Oct 25, 2018 at 10:55 AM John Baldwin <jhb@freebsd.org> wrote:
> Now that the MISA defaults to 0 if not present, would it better to just remove
> this and not set it to 0 explicitly?  The FreeBSD native target for RISC-V
> doesn't set MISA to anything at all.

There is still the issue of FP register size, which comes from MISA,
unless perhaps we can get it from auxvec/hw-cap info.  I was going to
look into that latter, and if the auxvec/hw-cap stuff works, then
remove the remaining MISA support in the riscv-linux-nat.c file.

Jim
  

On 10/25/18 11:17 AM, Jim Wilson wrote:
> On Thu, Oct 25, 2018 at 10:55 AM John Baldwin <jhb@freebsd.org> wrote:
>> Now that the MISA defaults to 0 if not present, would it better to just remove
>> this and not set it to 0 explicitly?  The FreeBSD native target for RISC-V
>> doesn't set MISA to anything at all.
> 
> There is still the issue of FP register size, which comes from MISA,
> unless perhaps we can get it from auxvec/hw-cap info.  I was going to
> look into that latter, and if the auxvec/hw-cap stuff works, then
> remove the remaining MISA support in the riscv-linux-nat.c file.

Ok.  I do agree that auxvec is probably the right way to handle this, as what
really matters is what format the kernel exports.  You can find existing uses
of auxvec for this on 32-bit arm support where AT_HWCAP flags are tested for
both Linux and FreeBSD in the respective tdep.c files to determine which
floating point registers are available.  You are free to use the same code
in a nat.c file as well of course.
  

On Thu, 25 Oct 2018 12:19:29 PDT (-0700), jhb@FreeBSD.org wrote:
> On 10/25/18 11:17 AM, Jim Wilson wrote:
>> On Thu, Oct 25, 2018 at 10:55 AM John Baldwin <jhb@freebsd.org> wrote:
>>> Now that the MISA defaults to 0 if not present, would it better to just remove
>>> this and not set it to 0 explicitly?  The FreeBSD native target for RISC-V
>>> doesn't set MISA to anything at all.
>>
>> There is still the issue of FP register size, which comes from MISA,
>> unless perhaps we can get it from auxvec/hw-cap info.  I was going to
>> look into that latter, and if the auxvec/hw-cap stuff works, then
>> remove the remaining MISA support in the riscv-linux-nat.c file.
>
> Ok.  I do agree that auxvec is probably the right way to handle this, as what
> really matters is what format the kernel exports.  You can find existing uses
> of auxvec for this on 32-bit arm support where AT_HWCAP flags are tested for
> both Linux and FreeBSD in the respective tdep.c files to determine which
> floating point registers are available.  You are free to use the same code
> in a nat.c file as well of course.

We have a very simple scheme here: there's a bit for every ISA extension that 
is set in HWCAP by the kernel when that extension is present as far as 
userspace is concerned.  The code is probably easier to understand

    https://git.kernel.org/pub/scm/linux/kernel/git/palmer/riscv-linux.git/tree/arch/riscv/kernel/cpufeature.c#n33

We should probably but this in an ABI document somewhere... :)
  

On Okt 25 2018, Jim Wilson <jimw@sifive.com> wrote:

> On Thu, Oct 25, 2018 at 10:55 AM John Baldwin <jhb@freebsd.org> wrote:
>> Now that the MISA defaults to 0 if not present, would it better to just remove
>> this and not set it to 0 explicitly?  The FreeBSD native target for RISC-V
>> doesn't set MISA to anything at all.
>
> There is still the issue of FP register size, which comes from MISA,
> unless perhaps we can get it from auxvec/hw-cap info.  I was going to
> look into that latter, and if the auxvec/hw-cap stuff works, then
> remove the remaining MISA support in the riscv-linux-nat.c file.

Note you need commit 0ddc77556c to get a correct AT_HWCAP.

Andreas.