Wilco Dijkstra <wilco.dijks...@arm.com> writes:
> Hi,
>
>> @Wilco, can you please send the rebased patch for patch review? We would
>> need in out openSUSE package soon.
>
> Here is an updated and rebased version:
>
> Cheers,
> Wilco
>
> v4: rebase and add REG_UNSAVED_ARCHEXT.
>
> A recent change only initializes the regs.how[] during Dwarf unwinding
> which resulted in an uninitialized offset used in return address signing
> and random failures during unwinding. The fix is to encode the return
> address signing state in REG_UNSAVED and a new state REG_UNSAVED_ARCHEXT.
>
> Passes bootstrap & regress, OK for commit?
>
> libgcc/
> PR target/107678
> * unwind-dw2.h (REG_UNSAVED_ARCHEXT): Add new enum.
> * unwind-dw2.c (uw_update_context_1): Add REG_UNSAVED_ARCHEXT case.
> * unwind-dw2-execute_cfa.h: Use REG_UNSAVED_ARCHEXT/REG_UNSAVED to
> encode the return address signing state.
> * config/aarch64/aarch64-unwind.h (aarch64_demangle_return_addr)
> Check current return address signing state.
> (aarch64_frob_update_contex): Remove.
>
> ---
> diff --git a/libgcc/config/aarch64/aarch64-unwind.h
> b/libgcc/config/aarch64/aarch64-unwind.h
> index
> 874cf6c3e77fb72d999f51b636d74cb0b5728bbd..727c27ba5da983958b3134715d9d4d7c0af5c1e2
> 100644
> --- a/libgcc/config/aarch64/aarch64-unwind.h
> +++ b/libgcc/config/aarch64/aarch64-unwind.h
> @@ -29,8 +29,6 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
> If not, see
>
> #define MD_DEMANGLE_RETURN_ADDR(context, fs, addr) \
> aarch64_demangle_return_addr (context, fs, addr)
> -#define MD_FROB_UPDATE_CONTEXT(context, fs) \
> - aarch64_frob_update_context (context, fs)
>
> static inline int
> aarch64_cie_signed_with_b_key (struct _Unwind_Context *context)
> @@ -55,42 +53,27 @@ aarch64_cie_signed_with_b_key (struct _Unwind_Context
> *context)
>
> static inline void *
> aarch64_demangle_return_addr (struct _Unwind_Context *context,
> - _Unwind_FrameState *fs ATTRIBUTE_UNUSED,
> + _Unwind_FrameState *fs,
> _Unwind_Word addr_word)
> {
> void *addr = (void *)addr_word;
> - if (context->flags & RA_SIGNED_BIT)
> + const int reg = DWARF_REGNUM_AARCH64_RA_STATE;
> +
> + if (fs->regs.how[reg] == REG_UNSAVED)
> + return addr;
> +
> + /* Return-address signing state is toggled by DW_CFA_GNU_window_save (where
> + REG_UNDEFINED means enabled), or set by a DW_CFA_expression. */
Needs updating to REG_UNSAVED_ARCHEXT.
OK with that changes, thanks, and sorry for the delays & runaround.
Richard
> + if (fs->regs.how[reg] == REG_UNSAVED_ARCHEXT
> + || (_Unwind_GetGR (context, reg) & 0x1) != 0)
> {
> _Unwind_Word salt = (_Unwind_Word) context->cfa;
> if (aarch64_cie_signed_with_b_key (context) != 0)
> return __builtin_aarch64_autib1716 (addr, salt);
> return __builtin_aarch64_autia1716 (addr, salt);
> }
> - else
> - return addr;
> -}
> -
> -/* Do AArch64 private initialization on CONTEXT based on frame info FS. Mark
> - CONTEXT as return address signed if bit 0 of
> DWARF_REGNUM_AARCH64_RA_STATE is
> - set. */
> -
> -static inline void
> -aarch64_frob_update_context (struct _Unwind_Context *context,
> - _Unwind_FrameState *fs)
> -{
> - const int reg = DWARF_REGNUM_AARCH64_RA_STATE;
> - int ra_signed;
> - if (fs->regs.how[reg] == REG_UNSAVED)
> - ra_signed = fs->regs.reg[reg].loc.offset & 0x1;
> - else
> - ra_signed = _Unwind_GetGR (context, reg) & 0x1;
> - if (ra_signed)
> - /* The flag is used for re-authenticating EH handler's address. */
> - context->flags |= RA_SIGNED_BIT;
> - else
> - context->flags &= ~RA_SIGNED_BIT;
>
> - return;
> + return addr;
> }
>
> #endif /* defined AARCH64_UNWIND_H && defined __ILP32__ */
> diff --git a/libgcc/unwind-dw2-execute_cfa.h b/libgcc/unwind-dw2-execute_cfa.h
> index
> 264c11c03ec4a09cac2c19a241c5b110b1b6b602..aef377092ceede6bdda8532679f9b081c98fadce
> 100644
> --- a/libgcc/unwind-dw2-execute_cfa.h
> +++ b/libgcc/unwind-dw2-execute_cfa.h
> @@ -278,10 +278,15 @@
> case DW_CFA_GNU_window_save:
> #if defined (__aarch64__) && !defined (__ILP32__)
> /* This CFA is multiplexed with Sparc. On AArch64 it's used to
> toggle
> - return address signing status. */
> + return address signing status. REG_UNSAVED/REG_UNSAVED_ARCHEXT
> + mean RA signing is disabled/enabled. */
> reg = DWARF_REGNUM_AARCH64_RA_STATE;
> - gcc_assert (fs->regs.how[reg] == REG_UNSAVED);
> - fs->regs.reg[reg].loc.offset ^= 1;
> + gcc_assert (fs->regs.how[reg] == REG_UNSAVED
> + || fs->regs.how[reg] == REG_UNSAVED_ARCHEXT);
> + if (fs->regs.how[reg] == REG_UNSAVED)
> + fs->regs.how[reg] = REG_UNSAVED_ARCHEXT;
> + else
> + fs->regs.how[reg] = REG_UNSAVED;
> #else
> /* ??? Hardcoded for SPARC register window configuration. */
> if (__LIBGCC_DWARF_FRAME_REGISTERS__ >= 32)
> diff --git a/libgcc/unwind-dw2.h b/libgcc/unwind-dw2.h
> index
> e2f81983e1dcf3df6aebde2454630b7bee87d597..53e1b183c7d60112a14411d3356c49cb39cd0de7
> 100644
> --- a/libgcc/unwind-dw2.h
> +++ b/libgcc/unwind-dw2.h
> @@ -29,6 +29,7 @@ enum {
> REG_SAVED_EXP,
> REG_SAVED_VAL_OFFSET,
> REG_SAVED_VAL_EXP,
> + REG_UNSAVED_ARCHEXT, /* Target specific extension. */
> REG_UNDEFINED
> };
>
> diff --git a/libgcc/unwind-dw2.c b/libgcc/unwind-dw2.c
> index
> 9c5bf7821916d8ac8e10e25a7123cd03f848019a..d0afce7a9ea9f5b12a5a01ef1e940e1452b48cab
> 100644
> --- a/libgcc/unwind-dw2.c
> +++ b/libgcc/unwind-dw2.c
> @@ -137,9 +137,6 @@ struct _Unwind_Context
> #define SIGNAL_FRAME_BIT ((~(_Unwind_Word) 0 >> 1) + 1)
> /* Context which has version/args_size/by_value fields. */
> #define EXTENDED_CONTEXT_BIT ((~(_Unwind_Word) 0 >> 2) + 1)
> - /* Bit reserved on AArch64, return address has been signed with A or B
> - key. */
> -#define RA_SIGNED_BIT ((~(_Unwind_Word) 0 >> 3) + 1)
> _Unwind_Word flags;
> /* 0 for now, can be increased when further fields are added to
> struct _Unwind_Context. */
> @@ -1200,6 +1197,7 @@ uw_update_context_1 (struct _Unwind_Context *context,
> _Unwind_FrameState *fs)
> {
> case REG_UNSAVED:
> case REG_UNDEFINED:
> + case REG_UNSAVED_ARCHEXT:
> break;
>
> case REG_SAVED_OFFSET:
