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

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

[Mikael Szreder]

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:
-- 
2.48.1

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

[Richard Henderson]

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(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

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

[Richard Henderson]

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~

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

[Mikael Szreder]

This patch should be applicable to the stable releases as well, as the issue has existed since a while back.

Best regards
Mikael Szreder

On February 15, 2025 8:58:09 PM GMT+01:00, Richard Henderson <richard.henderson@linaro.org> wrote:
>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~