[COMMITTED PATCH] gdb: LoongArch: Implement the return_value gdbarch method

[COMMITTED PATCH] gdb: LoongArch: Implement the return_value gdbarch method

[Tiezhu Yang]

When execute the following command on LoongArch:

  make check-gdb TESTS="gdb.base/async.exp"

there exist the following failed testcases:

  FAIL: gdb.base/async.exp: finish& (timeout)
  FAIL: gdb.base/async.exp: jump& (timeout)
  FAIL: gdb.base/async.exp: until& (timeout)
  FAIL: gdb.base/async.exp: set exec-done-display off (GDB internal error)

we can see the following messages in gdb/testsuite/gdb.log:

  finish&
  Run till exit from #0  foo () at /home/loongson/gdb.git/gdb/testsuite/gdb.base/async.c:9
  (gdb) /home/loongson/gdb.git/gdb/gdbarch.c:2646: internal-error: gdbarch_return_value: Assertion `gdbarch->return_value != NULL' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.

In order to fix the above failed testcases, implement the return_value
gdbarch method on LoongArch.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 gdb/loongarch-tdep.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/gdb/loongarch-tdep.c b/gdb/loongarch-tdep.c
index e30ef8084da..28f04094f64 100644
--- a/gdb/loongarch-tdep.c
+++ b/gdb/loongarch-tdep.c
@@ -247,6 +247,36 @@ static const struct frame_unwind loongarch_frame_unwind = {
   /*.prev_arch	   =*/nullptr,
 };
 
+/* Implement the return_value gdbarch method for LoongArch.  */
+
+static enum return_value_convention
+loongarch_return_value (struct gdbarch *gdbarch, struct value *function,
+			struct type *type, struct regcache *regcache,
+			gdb_byte *readbuf, const gdb_byte *writebuf)
+{
+  loongarch_gdbarch_tdep *tdep = (loongarch_gdbarch_tdep *) gdbarch_tdep (gdbarch);
+  auto regs = tdep->regs;
+  int len = TYPE_LENGTH (type);
+  int regnum = -1;
+
+  /* See if our value is returned through a register.  If it is, then
+     store the associated register number in REGNUM.  */
+  switch (type->code ())
+    {
+      case TYPE_CODE_INT:
+	regnum = regs.r + 4;
+	break;
+    }
+
+  /* Extract the return value from the register where it was stored.  */
+  if (readbuf)
+    regcache->raw_read_part (regnum, 0, len, readbuf);
+  if (writebuf)
+    regcache->raw_write_part (regnum, 0, len, writebuf);
+
+  return RETURN_VALUE_REGISTER_CONVENTION;
+}
+
 /* Implement the "dwarf2_reg_to_regnum" gdbarch method.  */
 
 static int
@@ -418,6 +448,9 @@ loongarch_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Finalise the target description registers.  */
   tdesc_use_registers (gdbarch, tdesc, std::move (tdesc_data));
 
+  /* Return value info  */
+  set_gdbarch_return_value (gdbarch, loongarch_return_value);
+
   /* Advance PC across function entry code.  */
   set_gdbarch_skip_prologue (gdbarch, loongarch_skip_prologue);
 
-- 
2.27.0

Re: [COMMITTED PATCH] gdb: LoongArch: Implement the return_value gdbarch method

[Tom Tromey]

>>>>> Tiezhu Yang <yangtiezhu@loongson.cn> writes:

> +  /* See if our value is returned through a register.  If it is, then
> +     store the associated register number in REGNUM.  */
> +  switch (type->code ())
> +    {
> +      case TYPE_CODE_INT:
> +	regnum = regs.r + 4;

I don't know the ABI on this architecture, but you may want to consider
also including TYPE_CODE_ENUM and TYPE_CODE_FIXED_POINT here.  These are
both int-like.

Sometimes there are also size restrictions, for example gdb has a little
support for 128-bit integers.

Tom

Re: [COMMITTED PATCH] gdb: LoongArch: Implement the return_value gdbarch method

[Tiezhu Yang]

On 05/10/2022 01:26 AM, Tom Tromey wrote:
>>>>>> Tiezhu Yang <yangtiezhu@loongson.cn> writes:
>
>> +  /* See if our value is returned through a register.  If it is, then
>> +     store the associated register number in REGNUM.  */
>> +  switch (type->code ())
>> +    {
>> +      case TYPE_CODE_INT:
>> +	regnum = regs.r + 4;
>
> I don't know the ABI on this architecture, but you may want to consider
> also including TYPE_CODE_ENUM and TYPE_CODE_FIXED_POINT here.  These are
> both int-like.
>
> Sometimes there are also size restrictions, for example gdb has a little
> support for 128-bit integers.
>
> Tom
>

Hi Tom,

Thank you very much for your suggestions.

The initial aim of this commit is only to fix the failed test cases
in gdb.base/async.exp on LoongArch, I will add the other case later.

Thanks,
Tiezhu

Re: [COMMITTED PATCH] gdb: LoongArch: Implement the return_value gdbarch method

[Andrew Burgess]

Tiezhu Yang <yangtiezhu@loongson.cn> writes:

> When execute the following command on LoongArch:
>
>   make check-gdb TESTS="gdb.base/async.exp"
>
> there exist the following failed testcases:
>
>   FAIL: gdb.base/async.exp: finish& (timeout)
>   FAIL: gdb.base/async.exp: jump& (timeout)
>   FAIL: gdb.base/async.exp: until& (timeout)
>   FAIL: gdb.base/async.exp: set exec-done-display off (GDB internal error)
>
> we can see the following messages in gdb/testsuite/gdb.log:
>
>   finish&
>   Run till exit from #0  foo () at /home/loongson/gdb.git/gdb/testsuite/gdb.base/async.c:9
>   (gdb) /home/loongson/gdb.git/gdb/gdbarch.c:2646: internal-error: gdbarch_return_value: Assertion `gdbarch->return_value != NULL' failed.
>   A problem internal to GDB has been detected,
>   further debugging may prove unreliable.
>
> In order to fix the above failed testcases, implement the return_value
> gdbarch method on LoongArch.
>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  gdb/loongarch-tdep.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/gdb/loongarch-tdep.c b/gdb/loongarch-tdep.c
> index e30ef8084da..28f04094f64 100644
> --- a/gdb/loongarch-tdep.c
> +++ b/gdb/loongarch-tdep.c
> @@ -247,6 +247,36 @@ static const struct frame_unwind loongarch_frame_unwind = {
>    /*.prev_arch	   =*/nullptr,
>  };
>  
> +/* Implement the return_value gdbarch method for LoongArch.  */
> +
> +static enum return_value_convention
> +loongarch_return_value (struct gdbarch *gdbarch, struct value *function,
> +			struct type *type, struct regcache *regcache,
> +			gdb_byte *readbuf, const gdb_byte *writebuf)
> +{
> +  loongarch_gdbarch_tdep *tdep = (loongarch_gdbarch_tdep *) gdbarch_tdep (gdbarch);
> +  auto regs = tdep->regs;
> +  int len = TYPE_LENGTH (type);
> +  int regnum = -1;
> +
> +  /* See if our value is returned through a register.  If it is, then
> +     store the associated register number in REGNUM.  */
> +  switch (type->code ())
> +    {
> +      case TYPE_CODE_INT:
> +	regnum = regs.r + 4;
> +	break;
> +    }
> +
> +  /* Extract the return value from the register where it was stored.  */
> +  if (readbuf)
> +    regcache->raw_read_part (regnum, 0, len, readbuf);
> +  if (writebuf)
> +    regcache->raw_write_part (regnum, 0, len, writebuf);

The GDB style is to write 'if (readbuf != nullptr)', and the same for
writebuf.

Thanks,
Andrew


> +
> +  return RETURN_VALUE_REGISTER_CONVENTION;
> +}
> +
>  /* Implement the "dwarf2_reg_to_regnum" gdbarch method.  */
>  
>  static int
> @@ -418,6 +448,9 @@ loongarch_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>    /* Finalise the target description registers.  */
>    tdesc_use_registers (gdbarch, tdesc, std::move (tdesc_data));
>  
> +  /* Return value info  */
> +  set_gdbarch_return_value (gdbarch, loongarch_return_value);
> +
>    /* Advance PC across function entry code.  */
>    set_gdbarch_skip_prologue (gdbarch, loongarch_skip_prologue);
>  
> -- 
> 2.27.0

Re: [COMMITTED PATCH] gdb: LoongArch: Implement the return_value gdbarch method

[Tiezhu Yang]

On 05/10/2022 05:19 PM, Andrew Burgess wrote:
> Tiezhu Yang <yangtiezhu@loongson.cn> writes:
>
>> +  /* Extract the return value from the register where it was stored.  */
>> +  if (readbuf)
>> +    regcache->raw_read_part (regnum, 0, len, readbuf);
>> +  if (writebuf)
>> +    regcache->raw_write_part (regnum, 0, len, writebuf);
>
> The GDB style is to write 'if (readbuf != nullptr)', and the same for
> writebuf.
>
> Thanks,
> Andrew

Hi Andrew,

Thank you for your kindly reply, sorry for that, I will modify it
as soon as possible.

Thanks,
Tiezhu