mbox series

[v1,0/3,libgcc] store signing key and signing method in DWARF _Unwind_FrameState

Message ID 20240719145404.683815-1-matthieu.longo@arm.com
Headers show
Series store signing key and signing method in DWARF _Unwind_FrameState | expand

Message

Matthieu Longo July 19, 2024, 2:54 p.m. UTC 
This patch series is only a refactoring of the existing implementation of PAuth and returned-address signing. The existing behavior is preserved.

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. 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.

3. libgcc: update configure (regenerated by autoreconf)

Regenerate the build files.


## 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 (3):
  aarch64: store signing key and signing method in DWARF
    _Unwind_FrameState
  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     | 150 +++++++++++++++++----
 libgcc/configure                           |   2 +
 libgcc/configure.ac                        |   1 +
 libgcc/unwind-dw2-execute_cfa.h            |  34 +++--
 libgcc/unwind-dw2.c                        |  19 ++-
 libgcc/unwind-dw2.h                        |  19 ++-
 9 files changed, 233 insertions(+), 52 deletions(-)
 create mode 100644 libgcc/config/aarch64/aarch64-unwind-def.h

Comments

Matthieu Longo July 29, 2024, 10:25 a.m. UTC | #1 
On 2024-07-19 15:54, Matthieu Longo wrote:
> This patch series is only a refactoring of the existing implementation of PAuth and returned-address signing. The existing behavior is preserved.
> 
> 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. 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.
> 
> 3. libgcc: update configure (regenerated by autoreconf)
> 
> Regenerate the build files.
> 
> 
> ## 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 (3):
>    aarch64: store signing key and signing method in DWARF
>      _Unwind_FrameState
>    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     | 150 +++++++++++++++++----
>   libgcc/configure                           |   2 +
>   libgcc/configure.ac                        |   1 +
>   libgcc/unwind-dw2-execute_cfa.h            |  34 +++--
>   libgcc/unwind-dw2.c                        |  19 ++-
>   libgcc/unwind-dw2.h                        |  19 ++-
>   9 files changed, 233 insertions(+), 52 deletions(-)
>   create mode 100644 libgcc/config/aarch64/aarch64-unwind-def.h
> 

Adding Ian in CC as he is listed as the maintainer of libgcc in 
MAINTAINERS file.