On 05/09/2023 09:32 AM, Youling Tang wrote:
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))
Need to add `(Dwarf_Word *)`,
if (! setfunc (32, 32, (Dwarf_Word *) &fregs. fpr[0], arg))
Do I need to send the v2 patchset again?
Thanks,
Youling.
Cheers,
Mark