Re: [PATCH] gdb: LoongArch: Change LOONGARCH_LINUX_NUM_GREGSET to 35

[John Baldwin <jhb@FreeBSD.org>]

On 2/26/24 5:25 PM, Hui Li wrote:
> Hi,
> 
> On 2024/2/23 上午8:33, John Baldwin wrote:
>> On 2/20/24 6:14 PM, Hui Li wrote:
>>> There is an assertion error "gdb_assert (n < tdesc->reg_defs.size ())"
>>> in find_register_by_number() when gdb connects to gdbserver, this is
>>> because the value of LOONGARCH_LINUX_NUM_GREGSET (45, which contains 10
>>> reserved regs) is different with the number of regs (35) in the feature
>>> file gdb/features/loongarch/base64.xml. Change
>>> LOONGARCH_LINUX_NUM_GREGSET
>>> to 35 to keep consistent with the xml file.
>>>
>>> without this patch:
>>>
>>> Execute on the target machine:
>>>
>>>     $ gdbserver 192.168.1.123:5678 ./test
>>>
>>> Execute on host machine:
>>>
>>>     $ gdb ./test
>>>     (gdb) target remote 192.168.1.123:5678
>>>
>>> Output on the target machine:
>>>
>>>     Process ./test created; pid = 67683
>>>     Listening on port 5678
>>>     Remote debugging from host 192.168.1.136, port 6789
>>>     gdbserver/regcache.cc:205: A problem internal to GDBserver has been
>>> detected.
>>>     find_register_by_number: Assertion 'n < tdesc->reg_defs.size ()'
>>> failed.
>>>
>>> Output on host machine:
>>>
>>>     Remote debugging using 192.168.1.123:5678
>>>     Remote connection closed
>>>
>>> Signed-off-by: Hui Li <lihui@loongson.cn>
>>> ---
>>>    gdb/arch/loongarch.h | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/gdb/arch/loongarch.h b/gdb/arch/loongarch.h
>>> index 4b7ab054ea0..6e51e1d097b 100644
>>> --- a/gdb/arch/loongarch.h
>>> +++ b/gdb/arch/loongarch.h
>>> @@ -33,7 +33,7 @@ enum loongarch_regnum
>>>      LOONGARCH_ORIG_A0_REGNUM = 32,    /* Syscall's original arg0.  */
>>>      LOONGARCH_PC_REGNUM = 33,        /* Program Counter.  */
>>>      LOONGARCH_BADV_REGNUM = 34,        /* Bad Vaddr for Addressing
>>> Exception.  */
>>> -  LOONGARCH_LINUX_NUM_GREGSET = 45,    /* 32 GPR, ORIG_A0, PC, BADV,
>>> RESERVED 10.  */
>>> +  LOONGARCH_LINUX_NUM_GREGSET = 35,    /* 32 GPR, ORIG_A0, PC, BADV.  */
>>>      LOONGARCH_ARG_REGNUM = 8,            /* r4-r11: general-purpose
>>> argument registers.
>>>                          f0-f7: floating-point argument registers.  */
>>>      LOONGARCH_FIRST_FP_REGNUM = LOONGARCH_LINUX_NUM_GREGSET,
>>
>> Does anything depend on the gap being present here for FIRST_FP_REGNUM?
>> That is,
>> should you perhaps be moving the +10 down to LOONGARCH_FIRST_FP_REGNUM
>> to keep the
>> first FP register number the same?
>>
> 
> Thanks for your review.
> 
> The purpose of this modification is to make LOONGARCH_FIRST_FP_REGNUM
> equal to 35. In gdb and gdbserver code, tdesc reg number is defined
> according to the feature file gdb/features/loongarch/*.xml(GPR: 0 ~ 34,
> and FPR starts at 35). But first FPR reg number in regcache is
> LOONGARCH_FIRST_FP_REGNUM(45), the total number of registers will not be
> consistent with tdesc reg numbers.
> 
> With this patch, LOONGARCH_FIRST_FP_REGNUM is changed to 35,
> the 10 reserved registers are not included in total number of registers,
> then all the reg numbers in regcache are consistent with tdesc reg numbers.
> 
> I send v2 version with modified code and commit message to make this
> change clearer.

Ok.  Mostly I was curious if there were existing stubs/servers that do not
use the XML descriptions and assume that the first FP register is at
register 45.

That said, GDB should also be able to handle a different numbering from the
remote target that doesn't match the regcache numbers and instead translates
remote register numbers to regcache register numbers.

-- 
John Baldwin