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

[Jan Beulich]

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