[PATCH 0/2] Add SCFI support for aarch64

[Richard Sandiford]

Hi,

I was having a look at the v3 series, but had a question about the
design & known limitations that I thought was better to ask here:

Indu Bhagat <indu.bhagat@oracle.com> writes:
> Known limitations 
> -----------------
> These are planned to be worked on in the near future:
>
>  - The current SCFI machinery does not currently synthesize the PAC-related
>    aarch64-specific CFI directives: .cfi_b_key_frame.  Other opcodes used when
>    pointer authentication is enabled also need to be handled (braa, brab,
>    retaa, etc.).
>
>  - Supporting the following pattern:
>    mov x16,4266
>    add sp, x16, sp
>    ...
>
>  - Not a limitation per se, but a note that ATM, that predicated insns are
>    skipped from ginsn translation.  IIUC, these instructions are not such that
>    can be used alongside stack management ops. To be double-checked.

AFAICT, the current code only handles GPRs.  It doesn't handle D8-D15,
which are also call-preserved under the base AAPCS64.  Is that right?
I think we should try to handle those as well.

D8-D15 are "interesting" because they are the low 64 bits of Q8-Q15,
and of Z8-Z15 if SVE is used.  However, a CFI save slot always represents
the low 64 bits, regardless of whether a save occurs on D, Q or Z registers.
This matters for big-endian code, because there are two additional
PCS variants:

* the "vector PCS", which preserves Q8-Q23
* the "SVE PCS", which preserves Z8-Z23 and P3-P15

So vector PCS functions might need to save and restore Q8 when returning
normally, but the CFI only describes the save of the D8 portion (since
that's the only portion that is preserved by exceptions).  This means
that, on big-endian:

	str	q8, [sp, #16]

should record D8 as being saved at sp+24 rather than sp+16.

A further complication is that STR Qn and STR Zn do not store in
the same byte order for big-endian: STR Qn stores as a 128-bit
integer (MSB first), whereas STR Zn stores as a stream of bytes
(LSB first).  This means that GCC-generated big-endian SVE PCS
functions use things like:

	st1d	z8.d, p2, [sp, #1, mul vl]

with the D8 save slot then being at sp + 2*VL - 64.

I think it's OK to punt on the big-endian SVE PCS case for now (provided
that there's a warning that the code isn't understood, which it looks
like there is).  But I think it's worth handling the Q register saves.

Other comments:

- I like the new approach of using a combination of the iclass and a
  "subclass" field of the flags.  How about making aarch64-gen.c enforce
  that:

  - if aarch64-ginsn.c looks at the subclass of a particular iclass,
    every instruction of that iclass has a nonzero subclass field

  - every other instruction has a zero subclass field

  This would help to ensure that the data stays up to date.
  The subclass enum could include a nonzero "other" value where
  necessary.

- I think we should only add things like F_LDST_LOAD and F_LDST_STORE
  to instructions that are semantically simple loads and stores
  (unless the iclass gives us the information needed to handle
  more complicated cases).  E.g. it looks like patch 2/7 adds
  F_LDST_LOAD to things like ld4, which are AoS->SoA loads.
  It would not be correct to interpret an LD4 on byte elements
  (say) as a register restore for CFI purposes.

  I realise the information could be useful for other things
  besides ginsns.  But while ginsns are the only things using the
  information, I think we should be careful to make sure that the
  information can't be misunderstood.

Thanks,
Richard