Re: [PATCH v2] target/sparc: Fix gdbstub incorrectly handling registers f32-f62

[Richard Henderson <richard.henderson@linaro.org>]

On 2/13/25 23:03, Mikael Szreder wrote:
> The gdbstub implementation for the Sparc architecture would
> incorrectly calculate the the floating point register offset.
> This resulted in, for example, registers f32 and f34 to point to
> the same value.
> 
> The issue was caused by the confusion between even register numbers
> and even register indexes. For example, the register index of f32 is 64
> and f34 is 65.
> 
> Fixes: 30038fd81808 ("target-sparc: Change fpr representation to doubles.")
> Signed-off-by: Mikael Szreder <git@miszr.win>
> ---
>   target/sparc/gdbstub.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/target/sparc/gdbstub.c b/target/sparc/gdbstub.c
> index ec0036e9ef..134617fb23 100644
> --- a/target/sparc/gdbstub.c
> +++ b/target/sparc/gdbstub.c
> @@ -79,8 +79,13 @@ int sparc_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
>           }
>       }
>       if (n < 80) {
> -        /* f32-f62 (double width, even numbers only) */
> -        return gdb_get_reg64(mem_buf, env->fpr[(n - 32) / 2].ll);
> +        /* f32-f62 (16 double width registers, even register numbers only)
> +         * n == 64: f32 : env->fpr[16]
> +         * n == 65: f34 : env->fpr[17]
> +         * etc...
> +         * n == 79: f62 : env->fpr[31]
> +         */
> +        return gdb_get_reg64(mem_buf, env->fpr[(n - 64) + 16].ll);
>       }
>       switch (n) {
>       case 80:
> @@ -173,8 +178,13 @@ int sparc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>           }
>           return 4;
>       } else if (n < 80) {
> -        /* f32-f62 (double width, even numbers only) */
> -        env->fpr[(n - 32) / 2].ll = tmp;
> +        /* f32-f62 (16 double width registers, even register numbers only)
> +         * n == 64: f32 : env->fpr[16]
> +         * n == 65: f34 : env->fpr[17]
> +         * etc...
> +         * n == 79: f62 : env->fpr[31]
> +         */
> +        env->fpr[(n - 64) + 16].ll = tmp;
>       } else {
>           switch (n) {
>           case 80:

Queued, thanks.


r~