[PATCH v2 2/7] Define unwinding and SEH data structures for aarch64
Richard Earnshaw (lists)
Richard.Earnshaw@arm.com
Wed May 21 12:16:34 GMT 2025
On 16/05/2025 13:16, Jan Beulich wrote:
> On 09.05.2025 12:57, Evgeny Karpov wrote:
>> This patch adds unwinding codes and records that are needed to emit
>> SEH data to pdata and xdata sections, and an arm64 context to store
>> data required for emitting.
>>
>> gas/ChangeLog:
>>
>> * config/obj-coff-seh.h (ARM64_MAX_UNWIND_CODES): New.
>> (ARM64_MAX_EPILOGUE_SCOPES): New.
>> (ARM64_UNOP_ALLOCS): New.
>> (ARM64_UNOP_SAVER19R20X): New.
>> (ARM64_UNOP_SAVEFPLR): New.
>> (ARM64_UNOP_SAVEFPLRX): New.
>> (ARM64_UNOP_ALLOCM): New.
>> (ARM64_UNOP_SAVEREGP): New.
>> (ARM64_UNOP_SAVEREGPX): New.
>> (ARM64_UNOP_SAVEREG): New.
>> (ARM64_UNOP_SAVEREGX): New.
>> (ARM64_UNOP_SAVELRPAIR): New.
>> (ARM64_UNOP_SAVEFREGP): New.
>> (ARM64_UNOP_SAVEFREGPX): New.
>> (ARM64_UNOP_SAVEFREG): New.
>> (ARM64_UNOP_SAVEFREGX): New.
>> (ARM64_UNOP_ALLOCL): New.
>> (ARM64_UNOP_SETFP): New.
>> (ARM64_UNOP_ADDFP): New.
>> (ARM64_UNOP_NOP): New.
>> (ARM64_UNOP_END): New.
>> (ARM64_UNOP_ENDC): New.
>> (ARM64_UNOP_SAVENEXT): New.
>> (ARM64_UNOP_PACSIGNLR): New.
>> ---
>> gas/config/obj-coff-seh.h | 154 +++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 152 insertions(+), 2 deletions(-)
>>
>> diff --git a/gas/config/obj-coff-seh.h b/gas/config/obj-coff-seh.h
>> index 598870ae4e0..f926fc8622c 100644
>> --- a/gas/config/obj-coff-seh.h
>> +++ b/gas/config/obj-coff-seh.h
>> @@ -19,12 +19,13 @@
>> 02110-1301, USA. */
>>
>> /* Short overview:
>> - There are at the moment three different function entry formats preset.
>> + There are at the moment four different function entry formats preset.
>> The first is the MIPS one. The second version
>> is for ARM, PPC, SH3, and SH4 mainly for Windows CE.
>> The third is the IA64 and x64 version. Note, the IA64 isn't implemented yet,
>> but to find information about it, please see specification about IA64 on
>> http://download.intel.com/design/Itanium/Downloads/245358.pdf file.
>> + The fourth is the Arm64 version.
>>
>> The first version has just entries in the pdata section: BeginAddress,
>> EndAddress, ExceptionHandler, HandlerData, and PrologueEndAddress. Each
>> @@ -57,8 +58,43 @@
>> .seh_savexmm
>> .seh_pushframe
>> .seh_code
>> +
>> + The fourth version for Arm64 partially intersects with the x64
>> + version, however it has a different extension to the unwind codes.
>> + It emits SEH data to pdata and xdata sections. In some cases SEH
>> + data could be emitted to a packed record in the pdata section
>> + without the need for data in the xdata section. However, the packed
>> + pdata record is not implemented yet.
>> +
>> */
>
> Please omit the stray blank line at the end of the comment.
>
> Also, perhaps both here and in identifiers below: In the past Richard had
> asked me to use AArch64 / aarch64 in favor of Arm64 / arm64.
Please. Being able to 'git grep -i aarch64' and find all the related code (and conversely 'git grep -i arm' not be polluted with unrelated changes) is a useful property of the sources that I'd like to preserve. The only exception is when printing information out when the strings printed are defined by a public ABI document.
>
>> @@ -87,6 +123,91 @@ typedef struct seh_prologue_element
>> symbolS *pc_addr;
>> } seh_prologue_element;
>>
>> +typedef struct seh_arm64_unwind_code
>> +{
>> + uint32_t value;
>> + seh_arm64_unwind_types type;
>> +} seh_arm64_unwind_code;
>> +
>> +typedef struct seh_arm64_packed_unwind_data
>> +{
>> + uint32_t flag : 2;
>> + uint32_t func_length : 11;
>> + uint32_t frame_size : 9;
>> + uint32_t cr : 2;
>> + uint32_t h : 1;
>> + uint32_t regI : 4;
>> + uint32_t regF : 3;
>> +} seh_arm64_packed_unwind_data;
>
> I think at least the more opaquely named the fields here could do with at
> least some comments. Or at the very least reproduce a link to the spec
> somewhere around here (the one added in patch 1 is in a different file,
> and hence not directly relevant here). Same for e.g. struct
> seh_arm64_xdata_header further down.
>
>> +typedef struct seh_arm64_except_info
>> +{
>> + uint32_t flag : 2;
>> + uint32_t except_info_rva : 30;
>> +} seh_arm64_except_info;
>> +
>> +typedef union seh_arm64_unwind_info
>> +{
>> + seh_arm64_except_info except_info;
>> + seh_arm64_packed_unwind_data packed_unwind_data;
>> +} seh_arm64_unwind_info;
>> +
>> +typedef struct seh_arm64_pdata
>> +{
>> + unsigned int func_start_rva;
>> + seh_arm64_unwind_info except_info_unwind;
>> +} seh_arm64_pdata;
>
> The long field names are going to make for overly long field access expressions.
> Could you consider using shorter, yet still sufficiently descriptive names?
>
>> +typedef struct seh_arm64_xdata_header
>> +{
>> + uint32_t func_length : 18;
>> + uint32_t vers : 2;
>> + uint32_t x : 1;
>> + uint32_t e : 1;
>> + uint32_t epilogue_count : 5;
>> + uint32_t code_words : 5;
>> + uint32_t ext_epilogue_count : 16;
>> + uint32_t ext_code_words : 8;
>> + uint32_t reserved : 8;
>> +} seh_arm64_xdata_header;
>> +
Is this supposed to be some form of 'in-file' data structure? If so, this is probably not going to work when cross-building from a big-endian host, unless you've written some code to do the read/write transforms. But if you have, why do you need the various bits of padding here? The same applies to the other structs you've defined in this file.
>> +typedef struct seh_arm64_epilogue_scope
>> +{
>> + uint32_t epilogue_start_offset_reduced : 18;
>> + uint32_t reserved : 4;
>> + uint32_t epilogue_start_index : 10;
>> + uintptr_t epilogue_start_offset;
>> + uintptr_t epilogue_end_offset;
>> +} seh_arm64_epilogue_scope;
>> +
>> +typedef struct seh_arm64_func_fragment
>> +{
>> + uintptr_t offset;
>> + symbolS *xdata_addr;
>
> Without seeing uses it's hard to tell whether this couldn't possibly be
> pointer-to-const. Please in general try to use const for pointer targets
> wherever possible.
>
>> + struct seh_arm64_func_fragment *next;
>> +} seh_arm64_func_fragment;
>> +
>> +#define ARM64_MAX_UNWIND_CODES 286
>
> Where's this (magic) number coming from?
>
> Jan
More information about the Binutils
mailing list