Hi German,
On Mon, 2022-04-25 at 14:03 +0000, German Gomez via Elfutils-devel
wrote:
> Implement DW_CFA_AARCH64_negate_ra_state in accordance with the DWARF
> AARCH64 ABI [1].
>
> Followup commits will use the value of this register to remove the PAC
> from return addresses.
>
> [1]
> https://github.com/ARM-software/abi-aa/blob/main/aadwarf64/aadwarf64.rst#44call-frame-instructions
>
> Signed-off-by: German Gomez <[email protected]>
This looks good, but two comments below.
> ---
> libdw/cfi.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/libdw/cfi.c b/libdw/cfi.c
> index a73fb03f..f985b4d8 100644
> --- a/libdw/cfi.c
> +++ b/libdw/cfi.c
> @@ -125,6 +125,15 @@ execute_cfi (Dwarf_CFI *cache,
> fs->regs[regno].value = (r_value); \
> } while (0)
>
> + /* The AARCH64 DWARF ABI states that register 34 (ra_sign_state) must
> + be initialized to 0. So do it before executing the CFI. */
> + if (cache->e_machine == EM_AARCH64)
> + {
> + if (unlikely (! enough_registers (DW_AARCH64_RA_SIGN_STATE, &fs,
> &result)))
> + goto out;
> + fs->regs[DW_AARCH64_RA_SIGN_STATE].value = 0;
> + }
Right. I thought this would be better expressed as part of the abi_cfi
(see aarch64_abi_cfi in backends/aarch64_cfi.c). But that would require
a DW_CFA_val_expression which we don't allow for abi_cfi. So this is
probably the best way to do it.
> while (program < end)
> {
> uint8_t opcode = *program++;
> @@ -355,7 +364,10 @@ execute_cfi (Dwarf_CFI *cache,
> {
> /* Toggles the return address state, indicating whether
> the return address is encrypted or not on
> - aarch64. XXX not handled yet. */
> + aarch64. */
> + if (unlikely (! enough_registers (DW_AARCH64_RA_SIGN_STATE, &fs,
> &result)))
> + goto out;
> + fs->regs[DW_AARCH64_RA_SIGN_STATE].value ^= 0x1;
> }
> else
> {
Looks good.
Lets also move the DW_AARCH64_RA_SIGN_STATE definition into cfi.h (from
libdw.h in the previous patch).
Thanks,
Mark
