Re: [PATCH 2/5] backends: Add set_initial_registers_tid callback for LoongArch

[Youling Tang <tangyouling@loongson.cn>]

Hi, Mark

On 05/09/2023 03:57 AM, Mark Wielaard wrote:
> Him
>
> On Fri, Apr 07, 2023 at 10:59:25AM +0800, Youling Tang wrote:
>> This patch implements the set_initial_registers_tid hook for LoongArch.
>
> Looks good, but one question:
>
>> +  /* Floating-point registers (only 64bits are used).  */
>> +  struct user_fp_struct fregs;
>> +  iovec.iov_base = &fregs;
>> +  iovec.iov_len = sizeof (fregs);
>> +  if (ptrace (PTRACE_GETREGSET, tid, NT_FPREGSET, &iovec) != 0)
>> +    return false;
>> +
>> +  Dwarf_Word dwarf_fregs[32];
>> +  for (int r = 0; r < 32; r++)
>> +    dwarf_fregs[r] = fregs.fpr[r] & 0xFFFFFFFF;
>
> The comment says 64bits, but the mask is for 32bits.
> I assume the comment is wrong and the masking is deliberate?

Thanks for pointing out that there is no need to mask the lower 32 bits
here.

It can be modified as follows,
--- a/backends/loongarch_initreg.c
+++ b/backends/loongarch_initreg.c
@@ -79,11 +79,8 @@ loongarch_set_initial_registers_tid (pid_t tid 
__attribute__ ((unused)),
    if (ptrace (PTRACE_GETREGSET, tid, NT_FPREGSET, &iovec) != 0)
      return false;

-  Dwarf_Word dwarf_fregs[32];
-  for (int r = 0; r < 32; r++)
-    dwarf_fregs[r] = fregs.fpr[r] & 0xFFFFFFFF;
-
-  if (! setfunc (32, 32, dwarf_fregs, arg))
+  /* $f0-$f31 */
+  if (! setfunc (32, 32, &fregs.fpr[0], arg))

Do I need to send the v2 patchset again?

Thanks,
Youling.

>
> Cheers,
>
> Mark
>