Matthieu Longo <matthieu.lo...@arm.com> writes:
> This patch series is only a refactoring of the existing implementation of 
> PAuth and returned-address signing. The existing behavior is preserved.

The series LGTM, thanks.  OK for trunk if there are no objections
before Monday.

Richard

>
> 1. aarch64: store signing key and signing method in DWARF _Unwind_FrameState
>
> _Unwind_FrameState already contains several CIE and FDE information (see the 
> attributes below the comment "The information we care about from the CIE/FDE" 
> in libgcc/unwind-dw2.h).
> The patch aims at moving the information from DWARF CIE (signing key stored 
> in the augmentation string) and FDE (the used signing method) into 
> _Unwind_FrameState along the already-stored CIE and FDE information.
> Note: those information have to be saved in frame_state_reg_info instead of 
> _Unwind_FrameState as they need to be savable by DW_CFA_remember_state and 
> restorable by DW_CFA_restore_state, that both rely on the attribute "prev".
> Those new information in _Unwind_FrameState simplifies the look-up of the 
> signing key when the return address is demangled. It also allows future 
> signing methods to be easily added.
> _Unwind_FrameState is not a part of the public API of libunwind, so the 
> change is backward compatible.
>
> A new architecture-specific handler MD_ARCH_EXTENSION_FRAME_INIT allows to 
> reset values in the frame state and unwind context if needed by the 
> architecture extension before changing the frame state to the caller context.
> A new architecture-specific handler MD_ARCH_EXTENSION_CIE_AUG_HANDLER 
> isolates the architecture-specific augmentation strings in AArch64 backend, 
> and allows others architectures to reuse augmentation strings that would have 
> clashed with AArch64 DWARF extensions.
> aarch64_demangle_return_addr, DW_CFA_AARCH64_negate_ra_state and 
> DW_CFA_val_expression cases in libgcc/unwind-dw2-execute_cfa.h were 
> documented to clarify where the value of the RA state register is stored (FS 
> and CONTEXT respectively).
>
> 2. aarch64: skip copy of RA state register into target context
>
> The RA state register is local to a frame, so it should not be copied to the 
> target frame during the context installation.
> This patch adds a new backend handler that check whether a register needs to 
> be skipped or not before its installation.
>
> 3. libgcc: hide CIE and FDE data for DWARF architecture extensions behind a 
> handler.
>
> This patch provides a new handler MD_ARCH_FRAME_STATE_T to hide an 
> architecture-specific structure containing CIE and FDE data related to DWARF 
> architecture extensions.
> Hiding the architecture-specific attributes behind a handler has the 
> following benefits:
>     1. isolating those data from the generic ones in _Unwind_FrameState
>     2. avoiding casts to custom types.
>     3. preserving typing information when debugging with GDB, and so 
> facilitating their printing.
>
> This approach required to add a new header md-unwind-def.h included at the 
> top of libgcc/unwind-dw2.h, and redirecting to the corresponding architecture 
> header via a symbolic link.
> An obvious drawback is the increase in complexity with macros, and headers. 
> It also caused a split of architecture definitions between md-unwind-def.h 
> (types definitions used in unwind-dw2.h) and md-unwind.h (local types 
> definitions and handlers implementations).
> The naming of md-unwind.h with .h extension is a bit misleading as the file 
> is only included in the middle of unwind-dw2.c. Changing this naming would 
> require modification of others backends, which I prefered to abstain from.
> Overall the benefits are worth the added complexity from my perspective.
>
> 4. libgcc: update configure (regenerated by autoreconf)
>
> Regenerate the build files.
> Note: This patch should be squashed with the previous one before merging.
>
> ### Diff between revisions 1 & 2
>
> 1: code style formatting + remove resetting of the local frame register in 
> the context as recommended by Richard Sandiford.
> 2: add a new handler as recommended by Richard Sandiford to skip the local 
> frame registers before installation in the target context.
> 3: code style formatting.
> 4: no change.
>
> ## Testing
>
> Those changes were testing by covering the 3 following cases:
> - backtracing.
> - exception handling in a C++ program.
> - gcc/testsuite/gcc.target/aarch64/pr104689.c: pac-ret with unusual DWARF [1]
>
> Regression tested on aarch64-unknown-linux-gnu, and no regression found.
>
> [1]: https://gcc.gnu.org/pipermail/gcc-patches/2022-May/594414.html
>
> Ok for master? I don't have commit access so I need someone to commit on my 
> behalf.
>
> Regards,
> Matthieu.
>
> Matthieu Longo (4):
>   aarch64: store signing key and signing method in DWARF
>     _Unwind_FrameState
>   aarch64: skip copy of RA state register into target context
>   libgcc: hide CIE and FDE data for DWARF architecture extensions behind
>     a handler.
>   libgcc: update configure (regenerated by autoreconf)
>
>  libgcc/Makefile.in                         |   6 +-
>  libgcc/config.host                         |  13 +-
>  libgcc/config/aarch64/aarch64-unwind-def.h |  41 ++++++
>  libgcc/config/aarch64/aarch64-unwind.h     | 152 +++++++++++++++++----
>  libgcc/config/no-unwind.h                  |   3 +-
>  libgcc/configure                           |   2 +
>  libgcc/configure.ac                        |   1 +
>  libgcc/unwind-dw2-execute_cfa.h            |  26 ++--
>  libgcc/unwind-dw2.c                        |  24 +++-
>  libgcc/unwind-dw2.h                        |  19 ++-
>  10 files changed, 235 insertions(+), 52 deletions(-)
>  create mode 100644 libgcc/config/aarch64/aarch64-unwind-def.h

Reply via email to