Re: [PATCH v12 1/1] aarch64: Implement Structured Exception Handling (SEH) on AArch64

On Thu, Jul 16, 2026 at 05:49:19PM +0200, Evgeny Karpov wrote:
> The patch reuses shared helpers for SEH and implements SEH on AArch64.
> The implementation is based on
> (https://learn.microsoft.com/en-us/cpp/build/arm64-exception-handling?view=msvc-170)
> and pdata/xdata SEH records are emitted from md_finish.
> 
> When .pdata/.xdata is emitted, the function size is required.
> Function sizes are calculated as late as possible, and the code segment needs
> to be relaxed to be able to calculate the function sizes.
> 
> Initially, obj_coff_generate_pdata was called in write_object_file.
> Before the change, obj_coff_generate_pdata was used only to validate
> syntax, which was sufficient for that purpose. However, that location
> seems incorrect, as it is too late to emit .pdata/.xdata records
> in the AArch64 case.
> 
> md_finish has been declared for AArch64 and extended with
> seh_aarch64_write_data to emit .pdata/.xdata records after all
> assembly has been completed.
> 
> Signed-off-by: Evgeny Karpov <[email protected]>
> 

Unless I find something else I've overlooked, I think the comments below cover
all the issues that need addressing before this can be merged.

There's definitely room for further optimisation, but that can be handled
separately in future.  I also haven't looked too closely at the tests, but I
think they're good enough for now, and they'd likely need adjusting anyway
if/when we implement better unwind data compression.

...
> +static void
> +seh_aarch64_emit_xdata_record (struct seh_aarch64_context *seh_ctx,
> +			       const uintptr_t frag_size,
> +			       const uintptr_t fragment_offset,
> +			       const unsigned first_fragment_scope,
> +			       const unsigned last_fragment_scope)
> +{
> +  unsigned epilogue_count = last_fragment_scope - first_fragment_scope;
> +
> +  /* Calculate how many unwind bytes will be emitted in .xdata record.  */
> +  unsigned unwind_bytes = seh_ctx->unwind_codes_byte_count;
> +
> +  /* Check if current fragment has a phantom prologue.  If yes, then
> +      the unwinding size should be adjusted.  */
> +  const bool has_phantom_prologue = fragment_offset != 0;
> +  if (has_phantom_prologue && unwind_bytes)

The list of unwind codes will always include at least one `end` unwind code for
the prologue (phantom or non-phantom), even after any future optimisations are
applied.  So any checks that unwind_bytes or code_words are nonzero can be
dropped (or replaced by a single assert that unwind_bytes is zero at the start
of the function).

> +    {
> +      /* One more epilogue scope and unwind code are emitted with phantom
> +	  prologue.  */
> +      unwind_bytes += 1;
> +      ++epilogue_count;

What is this epilogue_count increment doing?  You don't seem to emit any extra
epilogue scope to match the increased count, so I think this increment needs to
be removed.

> +    }
> +
> +  /* Calculate the number of code words with 4-byte alignment.  */
> +  unsigned code_words = (unwind_bytes + 3) / 4;
> +
> +  /* Initialize the .xdata header.  */
> +  const unsigned char has_exception_data_shift = 20;
> +  const unsigned char single_epilog_shift = 21;
> +  const unsigned char epilogue_count_shift = 22;
> +  const unsigned char code_words_shift = 27;
> +  const unsigned char ext_epilogue_count_shift = 32;
> +  const unsigned char ext_code_words_shift = 48;
> +  const uint32_t func_length_encoded = frag_size >> 2;
> +  uint64_t header = 0;
> +  header |= func_length_encoded;
> +  header |= seh_ctx->has_exception_data << has_exception_data_shift;
> +
> +  /* Check if short or extended header for a .xdata record should be
> +      used.  */
> +  unsigned header_size = 8;
> +  bool single_epilog = false;
> +  if ((code_words != 0 || epilogue_count != 0)

code_words is always nonzero (as explained above).

> +      && code_words < 32
> +      && epilogue_count < 32)
> +    {
> +      header_size = 4;
> +      if (epilogue_count == 1)

This condition still needs more checks (epilogue start index < 32, and I think
also the epilogue needs to be at the end of the function).

> +	{
> +	  single_epilog = true;
> +	  epilogue_count = 0;
> +	}
> +      header |= epilogue_count << epilogue_count_shift;
> +      header |= code_words << code_words_shift;
> +    }
> +  else
> +    {
> +      header |= (uint64_t) epilogue_count << ext_epilogue_count_shift;
> +      header |= (uint64_t) code_words << ext_code_words_shift;
> +    }
> +  header |= single_epilog << single_epilog_shift;
> +
> +  md_number_to_chars (frag_more (header_size), header, header_size);
> +
> +  if (epilogue_count && !single_epilog)
> +    {
> +      seh_aarch64_emit_epilogue_scopes (seh_ctx,
> +					fragment_offset,
> +					first_fragment_scope,
> +					last_fragment_scope);
> +    }
> +
> +  if (code_words)

code_words is always nonzero.

> +    seh_aarch64_emit_unwind_codes (seh_ctx, has_phantom_prologue);
> +
> +  if (seh_ctx->has_exception_data)
> +    {
> +      if (seh_ctx->handler.X_op == O_symbol)
> +	seh_ctx->handler.X_op = O_symbol_rva;
> +
> +      emit_expr (&seh_ctx->handler, 4);
> +
> +      /* Emit the fragment offset.  */
> +      md_number_to_chars (frag_more (4), fragment_offset, 4);
> +
> +      /* Use the same SEH handler data for all fragments.
> +	 The SEH handler data is emitted after the last fragment.  */
> +      expressionS exp;
> +      memset (&exp, 0, sizeof (expressionS));
> +      exp.X_op = O_symbol_rva;
> +      exp.X_add_symbol = seh_ctx->handler_data_xdata_addr;
> +      emit_expr (&exp, 4);
> +    }
> +}

...

> diff --git a/gas/config/obj-coff-seh-aarch64.h b/gas/config/obj-coff-seh-aarch64.h
> new file mode 100644
> index 00000000000..493c69f0527
> --- /dev/null
> +++ b/gas/config/obj-coff-seh-aarch64.h
...
> +typedef struct seh_aarch64_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_aarch64_packed_unwind_data;
> +
> +typedef struct seh_aarch64_except_info
> +{
> +  uint32_t flag : 2;
> +  uint32_t except_info_rva : 30;
> +} seh_aarch64_except_info;
> +
> +typedef union seh_aarch64_unwind_info
> +{
> +  seh_aarch64_except_info except_info;
> +  seh_aarch64_packed_unwind_data packed_unwind_data;
> +} seh_aarch64_unwind_info;

These three structs/unions are unused, so should be removed.

Thanks,
Alice