Re: [PULL 12/12] target/hppa: add 64 bit support to gdbstub

[Sven Schnelle <svens@stackframe.org>]

Sven Schnelle <svens@stackframe.org> writes:

> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
>> Hi,
>>
>> (now merged as commit b2c2d00f48cc5f4486cfba33b505ff86d79cb137)
>>
>> On 23/12/25 16:50, deller@kernel.org wrote:
>>> From: Sven Schnelle <svens@stackframe.org>
>>> Signed-off-by: Sven Schnelle <svens@stackframe.org>
>>> Reviewed-by: Helge Deller <deller@gmx.de>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>> ---
>>> target/hppa/gdbstub.c | 62
>> ++++++++++++++++++++++++++++---------------
>>>   1 file changed, 41 insertions(+), 21 deletions(-)
>>> diff --git a/target/hppa/gdbstub.c b/target/hppa/gdbstub.c
>>> index 0daa52f7af..777f4a48b9 100644
>>> --- a/target/hppa/gdbstub.c
>>> +++ b/target/hppa/gdbstub.c
>>> @@ -21,16 +21,25 @@
>>> @@ -133,24 +142,35 @@ int hppa_cpu_gdb_read_register(CPUState *cs,
>> GByteArray *mem_buf, int n)
>>>           val = env->cr[30];
>>>           break;
>>>       case 64 ... 127:
>>> - val = extract64(env->fr[(n - 64) / 2], (n & 1 ? 0 : 32), 32);
>>> -        break;
>>> -    default:
>>> -        if (n < 128) {
>>> -            val = 0;
>>> +        if (hppa_is_pa20(env)) {
>>> +            val = env->fr[n - 64];
>>
>> Coverity reports:
>>
>>>>>     CID 1645613:         Memory - illegal accesses  (OVERRUN)
>>>>>     Overrunning array "env->fr" of 32 8-byte elements at element
>>         index 32 (byte offset 263) using index "n - 64" (which
>>        evaluates to 32).
>
> Yes, there's indeed a check missing wether n is < 96. I'll submit a patch later.

Looking again there is

if (n >= hppa_num_regs(env)) {
        return 0;
}   

right at the beginning of both functions, which returns 96 for 64 bit
mode as the limit so n should have proper bounds already if I'm not mistaken.