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

On Fri, Jun 26, 2026 at 03:30:13PM +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]>



> Changes in v10:
> - Rename offset_shift/reg_shift to offset_multiplier/reg_multiplier.
I think you misinterpreted my review - I wanted you to rename the local
variables in seh_aarch64_add_unwind_element:
divided_by -> offset_multipler
regn_divided_by -> reg_multiplier

The offset_shift and reg_shift field names were fine, and the new names are
wrong (because the values are shift amounts, not multipliers).  Please change
the field names back to offset_shift and reg_shift.


From previous review:
> > > Yes, it makes sense to stop compilation when function size cannot be properly calculated.
> > > It means the assembler has an internal issue and should be fixed.
> > 
> > I think you missed my point - I'm asking whether "the function size has not
> > been evaluated" only happens when func_size < 0, or can it happen with
> > func_size >= 0 as well?
> 
> Yes, something should be wrong if func_size < 0, no need to check if func_size >= 0.

Can you explain why there's no need to check when func_size >= 0?  It looks to
me like we could end up in that case without the function size being fully
evaluated, but I don't really understand what this is checking.


> > > The big endian host should be supported properly by md_number_to_chars that is used
> > > for emitting pdata/xdata records.
> > 
> > That only ensures that the conversion from valueT to bytes-on-disk is handled
> > correctly.  The issue is with the bit-field to valueT conversion - on a
> > big-endian host I think this would pack the bitfields in the wrong order. 
> 
> The description will be extended with text that big endian host has not been validated.
> It will be validated, and a separate patch will be submitted if needed. 

I think deliberately committing code that is broken for a big endian host, and
then testing and fixing it later, is the wrong approach when doing it correctly
is no more complex (and, in my opinion, might be a bit clearer).

> > It would be clearer to use separate variables here, and only combine them into
> > a single header value within seh_aarch64_emit_xdata_record.   We can't do the
> > type-punning anyway, so there's no benefit to putting the values into a struct.
> > This also avoids any ambiguity about whether the short or extended fields
> > should be used for computations in this function.
> 
> This can be done after validating big endian host if needed.

Or just do it now - see below.


From patch:
> +static void
> +obj_coff_seh_save_reg (const int type)
> +{
> +  gas_assert (type >= 0 && type <= unwind_last_type);
> +
> +  const struct aarch64_unwind_info *info
> +    = aarch64_unwind_code_data + type;
> +
> +  SKIP_WHITESPACE ();
> +
> +  char *symbol_name = NULL;
> +  unsigned reg = -1;
> +
> +  if (info->reg_bits)
> +    {
> +      char name_end = get_symbol_name (&symbol_name);
> +      if (info->reg_type != *symbol_name)
> +	as_bad ("unexpected register name");
> +
> +      reg = atoi (symbol_name + 1);
> +      (void) restore_line_pointer (name_end);
> +
> +      if (!skip_whitespace_and_comma (1))
> +	return;
> +
> +      /* Check that reference registers are not higher than x30.  */

Nit: s/reference/referenced/

> +      if (info->reg_type == 'x' && (reg + (info->reg_pair ? 1 : 0)) > 30)
> +	as_bad (_("unexpected register number"));
> +    }
> +

...
> +static void
> +seh_aarch64_emit_xdata_record (struct seh_aarch64_context *seh_ctx,
> +			       const bool is_fragmented_function,
> +			       const bool is_last_frag,
> +			       const uintptr_t frag_size,
> +			       const uintptr_t fragment_offset,
> +			       const unsigned prologue_size,
> +			       const unsigned prologue_insn_count,
> +			       const unsigned first_fragment_scope,
> +			       const unsigned last_fragment_scope)
> +{
> +  seh_aarch64_xdata_header *header = &seh_ctx->xdata_header;
> +  const seh_aarch64_epilogue_scope *scopes = seh_ctx->epilogue_scopes;
> +
> +  /* Initialize the .xdata record.  */
> +  const uint32_t func_length_encoded = frag_size >> 2;
> +  header->func_length = func_length_encoded;
> +  header->vers = 0;
> +  header->e = 0;
> +  header->code_words = 0;
> +  header->epilogue_count = 0;
> +
> +  header->ext_code_words = 0;
> +  header->ext_epilogue_count = last_fragment_scope
> +				- first_fragment_scope;
> +  header->reserved = 0;
> +
> +  /* Calculate epilogue indexes for the current fragment.  */
> +  unsigned first_epilogue_index = 0;
> +  unsigned last_epilogue_index = 0;
> +  if (!header->ext_epilogue_count)
> +    {
> +      first_epilogue_index = prologue_size;
> +      last_epilogue_index = prologue_size;
> +    }
> +  else
> +    {
> +      const seh_aarch64_epilogue_scope *scope;
> +      scope = scopes + first_fragment_scope;
> +      first_epilogue_index = scope->epilogue_start_index;
> +      last_epilogue_index = seh_ctx->unwind_codes_byte_count;
> +    }
> +
> +  /* Calculate how many unwind bytes will be emitted in .xdata record.  */
> +  unsigned unwind_bytes = prologue_size;
> +
> +  /* Check if current fragment has a phantom prologue.  If yes, then
> +      the unwinding size should be adjusted.  */
> +  const bool has_phantom_prologue = is_fragmented_function && is_last_frag;
This condition is still wrong.


> +  if (has_phantom_prologue && unwind_bytes)
> +    {
> +      /* One more epilogue scope and unwind code are emitted with phantom
> +	  prologue.  */
> +      unwind_bytes += 1;
> +      ++header->ext_epilogue_count;
> +    }
> +
> +  /* Calculate the number of code words with 4-byte alignment.  */
> +  header->ext_code_words = (unwind_bytes + 3) / 4;
> +
> +  /* Check if short or extended header for a .xdata record should be
> +      used.  */
> +  unsigned header_size = 8;
> +  if ((header->ext_code_words != 0 || header->ext_epilogue_count != 0)
> +      && header->ext_code_words < 32
> +      && header->ext_epilogue_count < 32)
> +    {
> +      header_size = 4;
> +      header->code_words = header->ext_code_words;
> +      header->epilogue_count = header->ext_epilogue_count;
> +      if (header->epilogue_count == 1)
> +	{
> +	  header->e = 1;
> +	  if (has_phantom_prologue)
> +	    header->ext_epilogue_count = 0;
This looks wrong - surely the start index is 1 in this case (with unwind code
index 0 being end_c, to indicate the phantom prologue).

> +	  else
> +	    {
> +	      const seh_aarch64_epilogue_scope *scope;
> +	      scope = scopes + first_fragment_scope;
> +	      header->ext_epilogue_count = scope->epilogue_start_index;
> +	    }
> +	}
> +    }
> +
> +  md_number_to_chars (frag_more (header_size), seh_ctx->xdata_header_value,
> +		      header_size);

I think we should just build up the headers manually - this avoids endianess or
other platform-specific issues, and simplifies the code a bit.  So something
like:

+  unsigned code_words = ((unwind_bytes + 3) >> 2);
+
+  /* Compute and emit the xdata record header.  */
+  uint32_t header = frag_size >> 2;
+  if (seh_ctx->has_exception_data)
+    header |= 1 << 20;
+  if (epilogue_count <= 31 && code_words <= 31)
+    {
+      header |= epilogue_count << 22;
+      header |= code_words << 27;
+      md_number_to_chars (frag_more (4), header, 4));
+    }
+  else
+    {
+      md_number_to_chars (frag_more (4), header, 4));
+      uint32_t header2 = epilogue_count | (code_words << 16)
+      md_number_to_chars (frag_more (4), header2, 4));
+    }

This doesn't use the E=1 format (which only saves 4 bytes) - if we wanted to
include that then we'd need to add an initial check for:
+  if (epilogue_count == 1 && code_words <= 31
+      && /* Check epilogue start index <= 31.  */
+      && /* Check epilogue is at the end of the function - this doesn't seem
to be documented, but it's the only sensible way I can see to specify the
offset in this case.  */)

However, I can't see clear documentation for how this works, so it might be
better to ignore the E=1 format for now, and seek clarification before
implementing it.

> +
> +  if (header->ext_epilogue_count && !header->e)
> +    {
> +      seh_aarch64_emit_epilogue_scopes (seh_ctx,
> +					fragment_offset, prologue_size,
> +					first_fragment_scope,
> +					last_fragment_scope,
> +					has_phantom_prologue);
> +      if (has_phantom_prologue)
> +	{
> +	  const uint32_t epilogue_start_index_encoded = 1 << 22;
> +	  const uint32_t epilogue_start_offset_encoded
> +	    = (frag_size - prologue_insn_count * 4) >> 2;
> +	  md_number_to_chars (frag_more (4),
> +			      epilogue_start_index_encoded
> +			      | epilogue_start_offset_encoded, 4);
> +	}
> +    }
> +
> +  if (header->ext_code_words)
> +    seh_aarch64_emit_unwind_codes (seh_ctx, prologue_size, first_epilogue_index,
> +				   last_epilogue_index, has_phantom_prologue);
> +
> +  if (header->x == 1)
> +    {
> +      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);
> +    }
> +}
> +
> +static bool
> +seh_function_size (const struct seh_aarch64_context *seh_ctx,
> +		  uintptr_t *size)
> +{
> +  fragS *start_frag, *end_frag;
> +  addressT start_offset, end_offset;
> +  start_frag = symbol_get_frag_and_value (seh_ctx->start_addr, &start_offset);
> +  end_frag = symbol_get_frag_and_value (seh_ctx->end_addr, &end_offset);
> +
> +  intptr_t func_size = end_frag->fr_address + end_offset
> +		       - start_frag->fr_address - start_offset;
> +  if (func_size < 0)
> +    return false;
> +
> +  *size = func_size;
> +  return true;
> +}
> +
> +/* Write out the xdata information for one function.  */
> +static void
> +seh_aarch64_write_function_xdata (struct seh_aarch64_context *seh_ctx)
> +{
> +  if (!seh_ctx->unwind_codes_byte_count)
> +    return;
> +
> +  const segT save_seg = now_seg;
> +  const subsegT save_subseg = now_subseg;
> +
> +  switch_xdata (seh_ctx->subsection, seh_ctx->code_seg);
> +
> +  /* Set 4-byte alignment.  */
> +  frag_align (2, 0, 0);
> +
> +  uintptr_t func_size = 0;
> +  if (!seh_function_size (seh_ctx, &func_size))
> +    {
> +      as_bad (_("the function size for %s has not been evaluated"),
> +	      seh_ctx->func_name);
> +      return;
> +    }
> +
> +  /* The large functions should be split into fragments smaller than 1MB with
> +     4 bytes alignment, based on
> +     "Microsoft ARM64 exception handling, large functions documentation".  */
> +  const unsigned max_frag_size = (1 << 20) - 4;
> +  const bool is_fragmented_function = func_size > max_frag_size;
> +
> +  unsigned prologue_insn_count = 0;
> +  for (unsigned i = 0; i < seh_ctx->unwind_codes_count; ++i)
> +    {
> +      if (seh_ctx->unwind_codes[i].type == unwind_end)
> +	{
> +	  prologue_insn_count = i + 1;
> +	  break;
> +	}
> +    }
> +
> +  unsigned prologue_size = seh_ctx->unwind_codes_byte_count;
> +
> +  seh_aarch64_func_fragment *fragment;
> +  fragment = &seh_ctx->func_fragment;
> +  uintptr_t fragment_offset = 0;
> +  unsigned first_fragment_scope = 0;
> +  unsigned last_fragment_scope = 0;
> +
> +  /* Large functions (>= 1MB) will be split into multiple fragments.
> +     However, it is expected the most of the functions will have only one
> +     fragment.  This loop iterates fragments and emit them.  */
> +  while (true)
> +    {
> +      fragment->xdata_addr = symbol_temp_new_now ();
> +      fragment->offset = fragment_offset;
> +      fragment->next = NULL;
> +
> +      /* Calculate current fragment size.  */
> +      uintptr_t frag_size = func_size - fragment_offset;
> +      if (frag_size > max_frag_size)
> +	frag_size = max_frag_size;
> +
> +      const bool is_last_frag = (fragment_offset + frag_size) == func_size;
This value shouldn't be needed in the function call, so we can just inline this
condition into the loop termination check.

> +
> +      /* If it is a fragmented function, the epilogue range should be calculated
> +	 and will be emitted for the current fragment, otherwise all epilogues
> +	 will be emitted.  */
> +      if (!is_fragmented_function)
> +	last_fragment_scope = seh_ctx->epilogue_scopes_count;
> +      else
> +	{
> +	  first_fragment_scope = last_fragment_scope;
> +	  for (unsigned i = first_fragment_scope;
> +	       i < seh_ctx->epilogue_scopes_count; ++i)
> +	    {
> +	      const seh_aarch64_epilogue_scope *scope
> +		= seh_ctx->epilogue_scopes;
> +	      scope += i;
> +	      if (scope->epilogue_start_offset >= (fragment_offset + frag_size))
> +		break;
> +
> +	      if (scope->epilogue_end_offset >= (fragment_offset + frag_size))
> +		{
> +		  frag_size = scope->epilogue_start_offset - fragment_offset;
> +		  break;
> +		}
> +
> +	      if (scope->epilogue_start_offset >= fragment_offset)
> +		last_fragment_scope = i + 1;
> +	    }
> +	}
> +
> +
> +      /* Emit a .xdata record for the current fragment.  */
> +      seh_aarch64_emit_xdata_record (seh_ctx, is_fragmented_function,
> +				     is_last_frag, frag_size, fragment_offset,
> +				     prologue_size, prologue_insn_count,
> +				     first_fragment_scope, last_fragment_scope);
> +
> +      fragment_offset += frag_size;
> +      /* Exit the loop if it is the latest fragment.  */
> +      if (is_last_frag)
> +	break;
> +
> +      /* Allocate a new fragment that will be used also for emitting a .pdata
> +	 record.  */
> +      fragment->next = XCNEW (seh_aarch64_func_fragment);
> +      fragment = fragment->next;
> +    }
> +
> +  subseg_set (save_seg, save_subseg);
> +}

...

> +typedef struct seh_aarch64_context
> +{
> +  struct seh_aarch64_context *next;
> +
> +  /* Initial code-segment.  */
> +  segT code_seg;
> +  /* Function name.  */
> +  char *func_name;
> +  /* BeginAddress.  */
> +  symbolS *start_addr;
> +  /* EndAddress.  */
> +  symbolS *end_addr;
> +  /* PrologueEnd.  */
> +  symbolS *endprologue_addr;
> +
> +  symbolS *handler_data_xdata_addr;
> +  /* ExceptionHandler.  */
> +  expressionS handler;
> +  /* ExceptionHandlerData.  */
> +  expressionS handler_data;
> +
> +  subsegT subsection;
> +
> +  union {
> +    seh_aarch64_xdata_header xdata_header;
> +    valueT xdata_header_value;
> +  };
The only header field that is now used outside of seh_aarch64_emit_xdata_record
is xdata_header->x.  Please replace this union with a single boolean value
seh_ctx_cur->has_exception_data (or some similar name).  We can keep details
of the header layout contained to the single function that writes those other
fields.

> +  unsigned unwind_codes_count;
> +  unsigned unwind_codes_byte_count;
> +  seh_aarch64_unwind_code unwind_codes[AARCH64_MAX_UNWIND_CODES];
> +  unsigned epilogue_scopes_count;
> +  unsigned epilogue_scopes_capacity;
> +  seh_aarch64_epilogue_scope *epilogue_scopes;
> +  /* The function fragments.  */
> +  seh_aarch64_func_fragment func_fragment;
> +} seh_context;