Implement save_fpstate on AArch64

[fhahn]

It is not implemented at the moment. We should also add a test that requires save_fpstate, because it looks like there is non for AArch64 at the moment.

xref #1569

[egrimley]

But what should save_fpstate do on AArch64? The comment in AArch64's proc_save_fpstate says:

    /* All registers are saved by insert_push_all_registers so nothing extra
     * needs to be saved here.
     */

And there's the comment in proc.h:

/* On ARM/AArch64 proc_save_fpstate saves nothing, so use the smallest
 * legal size for an array.
 */
# define DR_FPSTATE_BUF_SIZE 1

Clearly save_fpstate is not the same thing as proc_save_fpstate, but perhaps the solution for save_fpstate is to replace ASSERT_NOT_IMPLEMENTED with ASSERT_NOT_REACHED: perhaps the function is required by an API but should not be called on ARM/AArch64?

[egrimley]

On ARM and AArch64 the only use of save_fpstate is in thread_set_self_context, which (on any architecture) is only used by check_wait_at_safe_spot, sometimes via thread_set_self_mcontext.

[abhinav92003]

On ARM and AArch64 the only use of save_fpstate is in thread_set_self_context, which (on any architecture) is only used by check_wait_at_safe_spot, sometimes via thread_set_self_mcontext.

@egrimley Is this just an observation of the existing code's state, or are you pointing out that we do need the save_fpstate in thread_set_self_context.

For context: I'm working on PR #4507 and we're trying to figure out whether we need this call or not for AArch64 (see comment thread on PR).

[derekbruening]

Both save_fpstate and proc_save_fpstate only exist because the x87 floating point registers and status words are not kept in the dr_mcontext_t. save_fpstate is used to update the x87 state when a signal is delayed and the state stored by the kernel needs updating in case the app ran x87 instructions in the delay window; proc_save_fpstate is for when DR or a client uses x87 instructions and keeps that use from clobbering the native x87 state which is what the app uses.

But, on arm and aarch64, the entire register state is in dr_mcontext_t, including all the SIMD registers which are used for floating-point operations. Right? So I don't think we need either save_fpstate nor proc_save_fpstate on any aarchxx architecture.

[AssadHashmi]

But, on arm and aarch64, the entire register state is in dr_mcontext_t, including all the SIMD registers which are used for floating-point operations. Right? So I don't think we need either save_fpstate nor proc_save_fpstate on any aarchxx architecture.

That sounds right. I've not done any work in this area and need to spend some time on it if I'm to be of any use for reviewing and patching.

[derekbruening]

So maybe the only action item here is to add clarifications to the API docs and internal code as to which platforms/situations they apply. Probably we should clarify what "floating point state" means for x86 as well: that it's only x87 state plus mmx registers (occupying the same silicon) and not xmm/ymm/zmm.

[egrimley-arm]

On ARM and AArch64 the only use of save_fpstate is in thread_set_self_context, which (on any architecture) is only used by check_wait_at_safe_spot, sometimes via thread_set_self_mcontext.

@egrimley Is this just an observation of the existing code's state

Yes.

Provided you're using an architecture variant that has the registers, the SIMD registers on AArch64 aren't usually treated much differently from any other register. I've seen a compiler spill an integer value to a SIMD register, for example.