[RFA] (riscv/ada) fix error when calling functions with range argument

  From: KONRAD Frederic <konrad@adacore.com>

Hello,

This is a patch that one of my coworkers wrote, which I have been
meaning to contribute for a long time, but haven't because we are not
set up to run the official testsuite on this platform (because of
the way our baremetal compiler is set up). But since the patch is
quite straightforward in my opinion, I thought I would propose it
anyway.

Using the gdb.ada/call_pn.exp testcase, and running it by hand on
riscv64-elf, we get the following error:

    (gdb) call pn(55)
    Could not compute alignment of type

The problem occurs because the parameter's type is a TYPE_CODE_RANGE,
and that type code is not handled by riscv_type_alignment. So this patch
fixes the issue by handling TYPE_CODE_RANGE the same way we handle other
integral types.

gdb/ChangeLog:

        * gdb/riscv-rdep.c (riscv_type_alignment): Handle TYPE_CODE_RANGE.

Tested on riscv64-elf using AdaCore's testsuite.
OK to apply?

Thanks,
  

* Joel Brobecker <brobecker@adacore.com> [2019-02-10 08:38:26 -0500]:

> From: KONRAD Frederic <konrad@adacore.com>
> 
> Hello,
> 
> This is a patch that one of my coworkers wrote, which I have been
> meaning to contribute for a long time, but haven't because we are not
> set up to run the official testsuite on this platform (because of
> the way our baremetal compiler is set up). But since the patch is
> quite straightforward in my opinion, I thought I would propose it
> anyway.
> 
> Using the gdb.ada/call_pn.exp testcase, and running it by hand on
> riscv64-elf, we get the following error:
> 
>     (gdb) call pn(55)
>     Could not compute alignment of type
> 
> The problem occurs because the parameter's type is a TYPE_CODE_RANGE,
> and that type code is not handled by riscv_type_alignment. So this patch
> fixes the issue by handling TYPE_CODE_RANGE the same way we handle other
> integral types.
> 
> gdb/ChangeLog:
> 
>         * gdb/riscv-rdep.c (riscv_type_alignment): Handle TYPE_CODE_RANGE.
> 
> Tested on riscv64-elf using AdaCore's testsuite.
> OK to apply?

I'm happy for this to go in.

Thanks,
Andrew



> 
> Thanks,
> -- 
> Joel
> 
> ---
>  gdb/riscv-tdep.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index fb5e2c5..3e8f564 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -1632,6 +1632,7 @@ riscv_type_alignment (struct type *t)
>      default:
>        error (_("Could not compute alignment of type"));
>  
> +    case TYPE_CODE_RANGE:
>      case TYPE_CODE_RVALUE_REF:
>      case TYPE_CODE_PTR:
>      case TYPE_CODE_ENUM:
> -- 
> 2.1.4
>
  

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> gdb/ChangeLog:
Joel>         * gdb/riscv-rdep.c (riscv_type_alignment): Handle TYPE_CODE_RANGE.
Joel> Tested on riscv64-elf using AdaCore's testsuite.
Joel> OK to apply?

I don't have any issue with this, but I do wonder if
riscv_type_alignment can be removed and/or simplified in favor
type_align and the gdbarch method.

I see several ports have this issue.  type_align should be preferred,
IMO, because it respects any additional alignment specified in the
DWARF.  I assume there are latent bugs in function calling on various
platforms caused by this, though I haven't checked.

Tom
  

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> I don't have any issue with this, but I do wonder if
Tom> riscv_type_alignment can be removed and/or simplified in favor
Tom> type_align and the gdbarch method.

Also now I wonder whether the TYPE_CODE_RANGE case in
gdbtypes.c:type_align ought to be changed.  Right now it just returns 0,
which means "cannot be determined".

Tom
  

* Tom Tromey <tom@tromey.com> [2019-02-12 09:54:13 -0700]:

> >>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
> 
> Joel> gdb/ChangeLog:
> Joel>         * gdb/riscv-rdep.c (riscv_type_alignment): Handle TYPE_CODE_RANGE.
> Joel> Tested on riscv64-elf using AdaCore's testsuite.
> Joel> OK to apply?
> 
> I don't have any issue with this, but I do wonder if
> riscv_type_alignment can be removed and/or simplified in favor
> type_align and the gdbarch method.
> 
> I see several ports have this issue.  type_align should be preferred,
> IMO, because it respects any additional alignment specified in the
> DWARF.  I assume there are latent bugs in function calling on various
> platforms caused by this, though I haven't checked.

I'll test changing RISC-V over to using gdbtypes.c:type_align.

Thanks,
Andrew
  

> Tom> I don't have any issue with this, but I do wonder if
> Tom> riscv_type_alignment can be removed and/or simplified in favor
> Tom> type_align and the gdbarch method.
> 
> Also now I wonder whether the TYPE_CODE_RANGE case in
> gdbtypes.c:type_align ought to be changed.  Right now it just returns 0,
> which means "cannot be determined".

Makes sense to me. I don't see this as being any problem at all, but
just in case, Let's make the change internally at AdaCore, and
have it be evaluated on all our platforms.
  

> > gdb/ChangeLog:
> > 
> >         * gdb/riscv-rdep.c (riscv_type_alignment): Handle TYPE_CODE_RANGE.
> > 
> > Tested on riscv64-elf using AdaCore's testsuite.
> > OK to apply?
> 
> I'm happy for this to go in.

Thanks Andrew. I know there is the option of switching over to using
type_align, but since this shouldn't cause any extra work to have
this one in, I pushed it for now.