[Bug 51996] New: aarch64: NtGetContextThread/NtSetContextThread on another thread ignores FPU state due to buggy SIGUSR1 handler

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51996

Bug ID: 51996
           Summary: aarch64: NtGetContextThread/NtSetContextThread on
                    another thread ignores FPU state due to buggy SIGUSR1
                    handler
           Product: Wine
           Version: 6.21
          Hardware: aarch64
                OS: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: ntdll
          Assignee: [e-mail]
          Reporter: [e-mail]
   Regression SHA1: 07f37f21377e930aefeb1fef3ab9c3cf9daee84f
      Distribution: ---

Created attachment 70987
  --> https://bugs.winehq.org/attachment.cgi?id=70987
Test program reproducing the behaviour

Compiled with -O2, the attached test program's output should be:

value = 2.0

When the actual output is:

value = -1.0

This is due to NtGetContextThread/NtSetContextThread not being able to
save/restore FP registers, since save_context()/restore_context() ignores them.

The bug can be traced back to commit 07f37f21377 (ntdll: Add support for
saving/restoring FPU state on ARM64., 2019-08-13), which implements
save_fpu()/restore_fpu() but does not make use of them in
save_context()/restore_context().

[Bug 51996] aarch64: NtGetContextThread/NtSetContextThread on another thread ignores FPU state due to buggy SIGUSR1 handler

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51996

--- Comment #1 from Jinoh Kang  ---
IMHO a dedicated signal handler on SIGUSR2 to restore full context (used in
NtContinue etc.) seems excessive to me. Is there any state on AArch64 that
cannot be restored in user mode and may only be modified kernel side?

[Bug 51996] aarch64: NtGetContextThread/NtSetContextThread on another thread ignores FPU state due to buggy SIGUSR1 handler

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51996

Zebediah Figura  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #2 from Zebediah Figura  ---
(In reply to Jinoh Kang from comment #1)
> IMHO a dedicated signal handler on SIGUSR2 to restore full context (used in
> NtContinue etc.) seems excessive to me. Is there any state on AArch64 that
> cannot be restored in user mode and may only be modified kernel side?

Yes. As far as we're aware, it's impossible to restore the *whole* context
simultaneously, without using privileged instructions like ERET. You need to
scratch a register to set the instruction pointer.

[Bug 51996] aarch64: NtGetContextThread/NtSetContextThread on another thread ignores FPU state due to buggy SIGUSR1 handler

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51996

--- Comment #3 from Jinoh Kang  ---
(In reply to Zebediah Figura from comment #2)
> (In reply to Jinoh Kang from comment #1)
> > IMHO a dedicated signal handler on SIGUSR2 to restore full context (used in
> > NtContinue etc.) seems excessive to me. Is there any state on AArch64 that
> > cannot be restored in user mode and may only be modified kernel side?
>
> Yes. As far as we're aware, it's impossible to restore the *whole* context
> simultaneously, without using privileged instructions like ERET. You need to
> scratch a register to set the instruction pointer.

Thank you for explaining.  Looks like it, and also the stack pointer.
Thankfully FPU has nothing to do with this.

Any chance we would be using sigreturn() directly, though?  On select known
platforms with stable syscall ABI, of course.

[Bug 51996] aarch64: NtGetContextThread/NtSetContextThread on another thread ignores FPU state due to buggy SIGUSR1 handler

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51996

--- Comment #4 from Jinoh Kang  ---
> Any chance we would be using sigreturn() directly, though?  On select known
> platforms with stable syscall ABI, of course.

Please disregard my previous comment. Dealing with some unknown sigcontext
extensions such as SVE/SVE2 would be a headache. CONTEXT_INTEGER In
syscall_frame->restore_flags would be a pretty rare case anyway.

[Bug 51996] aarch64: NtGetContextThread/NtSetContextThread on another thread ignores FPU state due to buggy SIGUSR1 handler

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51996

--- Comment #5 from Jinoh Kang  ---
Patch submitted: https://source.winehq.org/patches/data/220477

[Bug 51996] aarch64: NtGetContextThread/NtSetContextThread on another thread ignores FPU state due to buggy SIGUSR1 handler

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51996

Jinoh Kang  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|6.21                        |6.22

[Bug 51996] aarch64: NtGetContextThread/NtSetContextThread on another thread ignores FPU state due to buggy SIGUSR1 handler

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51996

Jinoh Kang  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #6 from Jinoh Kang  ---
Committed:
https://source.winehq.org/git/wine.git/commit/9f0df41a6cc594c2cbffe2205e3d96...

[Bug 51996] aarch64: NtGetContextThread/NtSetContextThread on another thread ignores FPU state due to buggy SIGUSR1 handler

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51996

Gijs Vermeulen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |9f0df41a6cc594c2cbffe2205e3
                   |                            |d969d62bc5b85

[Bug 51996] aarch64: NtGetContextThread/NtSetContextThread on another thread ignores FPU state due to buggy SIGUSR1 handler

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51996

Gijs Vermeulen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |regression

[Bug 51996] aarch64: NtGetContextThread/NtSetContextThread on another thread ignores FPU state due to buggy SIGUSR1 handler

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51996

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #7 from Alexandre Julliard  ---
Closing bugs fixed in 6.23.