[v1,5/7] Write SEH records to pdata/xdata

Message ID VI2PR83MB0718AD9046AFA837257EBB35F8B42@VI2PR83MB0718.EURPRD83.prod.outlook.com
State Superseded
Headers
Series Structured Exception Handling (SEH) implementation for aarch64-w64-mingw32 |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Evgeny Karpov April 9, 2025, 2:11 p.m. UTC
  The patch emits the required records to the pdata/xdata sections that contain
unwinding information for SEH.

gas/ChangeLog:

	* config/obj-coff-seh.c (do_seh_endproc): Update.
	(seh_arm64_emit_epilog_scopes): New.
	(seh_arm64_emit_unwind_codes): New.
	(seh_arm64_write_function_xdata): New.
	(write_function_xdata): Update.
	(write_function_pdata): Update.
---
 gas/config/obj-coff-seh.c | 312 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 304 insertions(+), 8 deletions(-)
  

Comments

Jan Beulich April 9, 2025, 2:37 p.m. UTC | #1
On 09.04.2025 16:11, Evgeny Karpov wrote:
> --- a/gas/config/obj-coff-seh.c
> +++ b/gas/config/obj-coff-seh.c
> @@ -557,13 +557,23 @@ static void
>  do_seh_endproc (void)
>  {
>    seh_ctx_cur->end_addr = symbol_temp_new_now ();
> +  const seh_kind kind = seh_get_target_kind ();
>  
> -  write_function_xdata (seh_ctx_cur);
> -  write_function_pdata (seh_ctx_cur);
> -  free (seh_ctx_cur->elems);
> -  free (seh_ctx_cur->func_name);
> -  free (seh_ctx_cur);
> -  seh_ctx_cur = NULL;
> +  if (kind != seh_kind_arm64
> +       || seh_ctx_cur->arm64_ctx.unwind_codes_byte_count > 0)

Nit: indentation is off by one here.

> +    {
> +      write_function_xdata (seh_ctx_cur);
> +      write_function_pdata (seh_ctx_cur);
> +    }

Are the two functions incapable of dealing with "nothing to emit"? The
description is lacking details on why this and the below change to existing
code are necessary.

> +  while (seh_ctx_cur)
> +  {
> +    seh_context *ctx = seh_ctx_cur;
> +    seh_ctx_cur = seh_ctx_cur->next;
> +    free (ctx->elems);
> +    free (ctx->func_name);
> +    XDELETE (ctx);
> +  }

Nit: Both braces and what they contain need indenting by one more level.

Also, why XDELETE() when it was free() before?

>  }
>  
>  static void
> @@ -1169,6 +1179,74 @@ seh_x64_write_prologue_data (const seh_context *c)
>      }
>  }
>  
> +static void
> +seh_arm64_emit_epilog_scopes (const seh_context *c, uint64_t fragment_offset,
> +			      uint32_t prolog_size,
> +			      uint32_t first_fragment_scope,
> +			      uint32_t last_fragment_scope,
> +			      bool has_phantom_prolog)
> +{
> +  int32_t start_index_offset = 0;
> +  const
> +  seh_arm64_epilogue_scope* scopes = seh_ctx_cur->arm64_ctx.epilogue_scopes;

Nit: * and blank want to change places.

> +  if (first_fragment_scope < seh_ctx_cur->arm64_ctx.epilogue_scopes_count)
> +    start_index_offset = scopes[first_fragment_scope].epilogue_start_index
> +			 - prolog_size;
> +  if (has_phantom_prolog)
> +    start_index_offset -= 1;
> +  for (int i = first_fragment_scope; i < last_fragment_scope; ++i)

Why plain int when both bounds are of an unsigned type (where in turn it's
questionable whether they really need to be uint32_t, rather than unsigned
int)?

> +  {
> +    seh_arm64_epilogue_scope scope = seh_ctx_cur->arm64_ctx.epilogue_scopes[i];
> +    scope.epilogue_start_offset_reduced = (scope.epilogue_start_offset
> +					  - fragment_offset) >> 2;
> +    scope.epilogue_start_index -= start_index_offset;
> +    out_four (*(uint32_t*) &scope);
> +  }

Block indentation issue again. There's more of this below.

> +  switch (target_kind)
> +  {
> +    case seh_kind_x64:
> +      seh_x64_write_function_xdata (c);
> +      break;
> +    case seh_kind_arm64:
> +      seh_arm64_write_function_xdata (c);
> +      break;
> +    default:
> +      break;
> +  }

Here it's slightly different though: Only the braces want sliding in by two
blanks.

Jan
  
Evgeny Karpov April 10, 2025, 11:13 a.m. UTC | #2
Wednesday, April 9, 2025
Jan Beulich <jbeulich@suse.com> wrote:

> Are the two functions incapable of dealing with "nothing to emit"? The
> description is lacking details on why this and the below change to existing
> code are necessary.

The logic for "nothing to emit" will be moved into the functions, it is possible.

> Also, why XDELETE() when it was free() before?

The recent change fixed memory leaks by using
free(seh_ctx_cur);  
however seh_ctx_cur is allocated by XCNEW, which is connected with XDELETE  
to release memory. XDELETE uses the same free, however potentially,  
the implementation might be changed.

> Why plain int when both bounds are of an unsigned type (where in turn it's
> questionable whether they really need to be uint32_t, rather than unsigned
> int)?

Both variables are expected to be non-negative.
The variable in the loop will be changed to uint32_t.

> Here it's slightly different though: Only the braces want sliding in by two
> blanks.

All highlighted style corrections will be applied.

Regards,
Evgeny
  
Jan Beulich April 10, 2025, 11:27 a.m. UTC | #3
On 10.04.2025 13:13, Evgeny Karpov wrote:
> Wednesday, April 9, 2025
> Jan Beulich <jbeulich@suse.com> wrote:
> 
>> Also, why XDELETE() when it was free() before?
> 
> The recent change fixed memory leaks by using
> free(seh_ctx_cur);  
> however seh_ctx_cur is allocated by XCNEW, which is connected with XDELETE  
> to release memory. XDELETE uses the same free, however potentially,  
> the implementation might be changed.

That sounds like it wants to be a separate change then. Or at least such an
unrelated adjustment wants highlighting / justifying in the description.

>> Why plain int when both bounds are of an unsigned type (where in turn it's
>> questionable whether they really need to be uint32_t, rather than unsigned
>> int)?
> 
> Both variables are expected to be non-negative.
> The variable in the loop will be changed to uint32_t.

As indicated - please prefer unsigned int over uint32_t.

Jan
  

Patch

diff --git a/gas/config/obj-coff-seh.c b/gas/config/obj-coff-seh.c
index 96ddfd51a82..f9a8ea80c28 100644
--- a/gas/config/obj-coff-seh.c
+++ b/gas/config/obj-coff-seh.c
@@ -557,13 +557,23 @@  static void
 do_seh_endproc (void)
 {
   seh_ctx_cur->end_addr = symbol_temp_new_now ();
+  const seh_kind kind = seh_get_target_kind ();
 
-  write_function_xdata (seh_ctx_cur);
-  write_function_pdata (seh_ctx_cur);
-  free (seh_ctx_cur->elems);
-  free (seh_ctx_cur->func_name);
-  free (seh_ctx_cur);
-  seh_ctx_cur = NULL;
+  if (kind != seh_kind_arm64
+       || seh_ctx_cur->arm64_ctx.unwind_codes_byte_count > 0)
+    {
+      write_function_xdata (seh_ctx_cur);
+      write_function_pdata (seh_ctx_cur);
+    }
+
+  while (seh_ctx_cur)
+  {
+    seh_context *ctx = seh_ctx_cur;
+    seh_ctx_cur = seh_ctx_cur->next;
+    free (ctx->elems);
+    free (ctx->func_name);
+    XDELETE (ctx);
+  }
 }
 
 static void
@@ -1169,6 +1179,74 @@  seh_x64_write_prologue_data (const seh_context *c)
     }
 }
 
+static void
+seh_arm64_emit_epilog_scopes (const seh_context *c, uint64_t fragment_offset,
+			      uint32_t prolog_size,
+			      uint32_t first_fragment_scope,
+			      uint32_t last_fragment_scope,
+			      bool has_phantom_prolog)
+{
+  int32_t start_index_offset = 0;
+  const
+  seh_arm64_epilogue_scope* scopes = seh_ctx_cur->arm64_ctx.epilogue_scopes;
+  if (first_fragment_scope < seh_ctx_cur->arm64_ctx.epilogue_scopes_count)
+    start_index_offset = scopes[first_fragment_scope].epilogue_start_index
+			 - prolog_size;
+  if (has_phantom_prolog)
+    start_index_offset -= 1;
+  for (int i = first_fragment_scope; i < last_fragment_scope; ++i)
+  {
+    seh_arm64_epilogue_scope scope = seh_ctx_cur->arm64_ctx.epilogue_scopes[i];
+    scope.epilogue_start_offset_reduced = (scope.epilogue_start_offset
+					  - fragment_offset) >> 2;
+    scope.epilogue_start_index -= start_index_offset;
+    out_four (*(uint32_t*) &scope);
+  }
+}
+
+static void
+seh_arm64_emit_unwind_codes (const seh_context *c, uint32_t prolog_size,
+			     uint32_t first_epilog_index,
+			     uint32_t last_epilog_index,
+			     bool has_phantom_prolog)
+{
+  uint32_t total_byte_count = 0;
+
+  if (has_phantom_prolog)
+  {
+    ++total_byte_count;
+    md_number_to_chars (frag_more (1), ARM64_UNOP_ENDC, 1);
+  }
+
+  uint32_t unwind_bytes_offset = 0;
+  for (int i = 0; i < (int)c->arm64_ctx.unwind_codes_count; ++i)
+  {
+    const seh_arm64_unwind_code *code = c->arm64_ctx.unwind_codes + i;
+    const int byte_count = unwind_code_pack_infos[code->type].size;
+    unwind_bytes_offset += byte_count;
+
+    if (unwind_bytes_offset > last_epilog_index)
+      break;
+
+    if (unwind_bytes_offset > prolog_size
+	&& unwind_bytes_offset <= first_epilog_index)
+      continue;
+
+    /*  emit unwind code bytes in big endian.  */
+    number_to_chars_bigendian (frag_more (byte_count), code->value, byte_count);
+    total_byte_count += byte_count;
+  }
+
+  /* handle word alignment.  */
+  int required_padding = (4 - total_byte_count % 4) % 4;
+  if (required_padding)
+  {
+    const uint32_t nop_chain = 0xe3e3e3e3;
+    md_number_to_chars (frag_more (required_padding), nop_chain,
+			required_padding);
+  }
+}
+
 static int
 seh_x64_size_prologue_data (const seh_context *c)
 {
@@ -1263,6 +1341,195 @@  seh_x64_write_function_xdata (seh_context *c)
   /* Handler data will be tacked in here by subsections.  */
 }
 
+/* Write out the xdata information for one function (arm64).  */
+static void
+seh_arm64_write_function_xdata (seh_context *c)
+{
+  /* Set 4-byte alignment.  */
+  frag_align (2, 0, 0);
+
+  uintptr_t func_length = 0;
+  expressionS exp;
+  exp.X_op = O_subtract;
+  exp.X_add_symbol = c->end_addr;
+  exp.X_op_symbol = c->start_addr;
+  exp.X_add_number = 0;
+  if (!resolve_expression (&exp) || exp.X_op != O_constant
+      || exp.X_add_number < 0)
+    as_bad (_("the function size expression for %s "
+	    "does not evaluate to a non-negative constant"),
+	    S_GET_NAME (c->start_addr));
+
+  func_length = exp.X_add_number;
+
+  const uint32_t max_frag_size = ((1 << 18) - 1) << 2;
+  uintptr_t fragment_offset = 0;
+  bool is_fragmented_function = func_length > max_frag_size;
+
+  /* [first_fragment_scope, last_fragment_scope).  */
+  uint32_t first_fragment_scope = 0;
+  uint32_t last_fragment_scope = 0;
+  uint32_t prolog_size = 0;
+  uint32_t prolog_insruction_count = 0;
+  for (int i = 0; i < c->arm64_ctx.unwind_codes_count; ++i)
+  {
+    if (c->arm64_ctx.unwind_codes[i].type == end)
+    {
+      prolog_insruction_count = i + 1;
+      break;
+    }
+  }
+
+  if (c->arm64_ctx.epilogue_scopes_count)
+    prolog_size = c->arm64_ctx.epilogue_scopes[0].epilogue_start_index;
+  else
+    prolog_size = c->arm64_ctx.unwind_codes_byte_count;
+
+  while (true)
+  {
+    c->xdata_addr = symbol_temp_new_now ();
+    c->next = NULL;
+    c->arm64_ctx.fragment_offset = fragment_offset;
+
+    uintptr_t frag_size = func_length - fragment_offset;
+    if (frag_size > max_frag_size)
+      frag_size = max_frag_size;
+
+    bool is_first_frag = fragment_offset == 0;
+    bool is_last_frag = (fragment_offset + frag_size) == func_length;
+
+    if (!is_fragmented_function)
+      last_fragment_scope = c->arm64_ctx.epilogue_scopes_count;
+    else
+    {
+      first_fragment_scope = last_fragment_scope;
+      for (int i = first_fragment_scope; i < c->arm64_ctx.epilogue_scopes_count;
+	   ++i)
+      {
+	const seh_arm64_epilogue_scope *scope = c->arm64_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;
+	}
+
+	last_fragment_scope = i + 1;
+      }
+    }
+
+    seh_arm64_xdata_header* header = &c->arm64_ctx.xdata_header;
+    const
+    seh_arm64_epilogue_scope* scopes = seh_ctx_cur->arm64_ctx.epilogue_scopes;
+
+    header->func_length = frag_size >> 2;
+    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;
+
+    uint32_t first_epilog_index = 0;
+    uint32_t last_epilog_index = 0;
+    if (!header->ext_epilogue_count)
+    {
+      first_epilog_index = prolog_size;
+      last_epilog_index = prolog_size;
+    }
+    else
+    {
+      first_epilog_index = scopes[first_fragment_scope].epilogue_start_index;
+      if (last_fragment_scope == c->arm64_ctx.epilogue_scopes_count)
+	last_epilog_index = c->arm64_ctx.unwind_codes_byte_count;
+      else
+	last_epilog_index = scopes[last_fragment_scope].epilogue_start_index;
+    }
+
+    uint32_t unwind_bytes = 0;
+    if (is_first_frag || is_last_frag)
+      unwind_bytes += prolog_size;
+
+    if (header->ext_epilogue_count)
+      unwind_bytes += last_epilog_index - first_epilog_index;
+
+    if (is_fragmented_function && is_last_frag && unwind_bytes)
+    {
+      unwind_bytes += 1;
+      ++header->ext_epilogue_count;
+    }
+
+    header->ext_code_words = (unwind_bytes  + 3) / 4;
+
+    if (header->ext_code_words == 0 && header->ext_epilogue_count == 0
+	|| header->ext_code_words > 31
+	|| header->ext_epilogue_count > 31)
+	md_number_to_chars (frag_more (8), c->arm64_ctx.xdata_header_value, 8);
+    else
+    {
+      header->code_words = header->ext_code_words;
+      header->epilogue_count = header->ext_epilogue_count;
+      if (header->epilogue_count == 1)
+      {
+	header->e = 1;
+	if (is_fragmented_function && is_last_frag)
+	  header->ext_epilogue_count = 0;
+	else
+	{
+	  uint32_t start_index;
+	  start_index = scopes[first_fragment_scope].epilogue_start_index;
+	  header->ext_epilogue_count = start_index;
+	}
+      }
+      out_four (c->arm64_ctx.xdata_header_value);
+    }
+
+    bool has_phantom_prolog = is_fragmented_function && is_last_frag;
+    if (header->ext_epilogue_count && !header->e)
+    {
+      seh_arm64_emit_epilog_scopes (c, fragment_offset, prolog_size,
+				    first_fragment_scope, last_fragment_scope,
+				    has_phantom_prolog);
+      if (is_fragmented_function && is_last_frag)
+      {
+	uint32_t epilog_start_offset = frag_size - prolog_insruction_count * 4;
+	md_number_to_chars (frag_more (4),
+			    (1 << 22) | (epilog_start_offset >> 2), 4);
+      }
+    }
+
+    if (header->ext_code_words)
+      seh_arm64_emit_unwind_codes (c, prolog_size, first_epilog_index,
+				   last_epilog_index, has_phantom_prolog);
+
+    if (header->x == 1)
+    {
+      if (c->handler.X_op == O_symbol)
+	c->handler.X_op = O_symbol_rva;
+
+      emit_expr (&c->handler, 4);
+    }
+
+    fragment_offset += frag_size;
+    if (fragment_offset == func_length)
+      break;
+
+    seh_context *next = XCNEW (seh_context);
+    memcpy (next, c, sizeof (seh_context));
+    next->elems = NULL;
+    next->func_name = NULL;
+
+    c->next = next;
+    c = next;
+  }
+}
+
 /* Write out xdata for one function.  */
 
 static void
@@ -1271,13 +1538,25 @@  write_function_xdata (seh_context *c)
   segT save_seg = now_seg;
   int save_subseg = now_subseg;
 
+  seh_kind target_kind = seh_get_target_kind ();
+
   /* MIPS, SH, ARM don't have xdata.  */
-  if (seh_get_target_kind () != seh_kind_x64)
+  if ((target_kind != seh_kind_x64) && (target_kind != seh_kind_arm64))
     return;
 
   switch_xdata (c->subsection, c->code_seg);
 
-  seh_x64_write_function_xdata (c);
+  switch (target_kind)
+  {
+    case seh_kind_x64:
+      seh_x64_write_function_xdata (c);
+      break;
+    case seh_kind_arm64:
+      seh_arm64_write_function_xdata (c);
+      break;
+    default:
+      break;
+  }
 
   subseg_set (save_seg, save_subseg);
 }
@@ -1362,6 +1641,23 @@  write_function_pdata (seh_context *c)
       emit_expr (&exp, 4);
       break;
 
+    case seh_kind_arm64:
+      while (c)
+      {
+	exp.X_op = O_symbol_rva;
+	exp.X_add_number = c->arm64_ctx.fragment_offset;
+	exp.X_add_symbol = c->start_addr;
+	emit_expr (&exp, 4);
+
+	exp.X_op = O_symbol_rva;
+	/* TODO: Implementing packed unwind data.  */
+	exp.X_add_number = 0;
+	exp.X_add_symbol = c->xdata_addr;
+	emit_expr (&exp, 4);
+	c = c->next;
+      }
+      break;
+
     case seh_kind_mips:
       exp.X_op = O_symbol;
       exp.X_add_number = 0;