[PATCH v2 4/7] Add aarch64-specific SEH commands
Evgeny Karpov
Evgeny.Karpov@microsoft.com
Wed Jul 23 14:54:11 GMT 2025
Friday, May 16
Jan Beulich <jbeulich@suse.com> wrote:
>> --- a/gas/config/obj-coff-seh.c
>> +++ b/gas/config/obj-coff-seh.c
>> @@ -681,6 +681,86 @@ obj_coff_seh_endprologue (int what ATTRIBUTE_UNUSED)
>> }
>> }
>>
>> +#if defined (COFFAARCH64)
>
> With this, is ...
>
>> +static void
>> +obj_coff_seh_startepilogue (int what ATTRIBUTE_UNUSED)
>> +{
>> + symbolS *epilogue_start_addr;
>> + expressionS exp;
>> +
>> + if (!verify_context (".seh_startepilogue")
>> + || !seh_validate_seg (".seh_startepilogue"))
>> + return;
>> + demand_empty_rest_of_line ();
>> +
>> + if (seh_get_target_kind () != seh_kind_arm64)
>> + return;
>
> ... this possible at all?
The implementation has been refactored and no longer contains the condition seh_get_target_kind() != seh_kind_arm64.
>> + epilogue_start_addr = symbol_temp_new_now ();
>> + exp.X_op = O_subtract;
>> + exp.X_add_symbol = epilogue_start_addr;
>> + exp.X_op_symbol = seh_ctx_cur->start_addr;
>> + exp.X_add_number = 0;
>> +
>> + if (!resolve_expression (&exp) || exp.X_op != O_constant
>> + || exp.X_add_number < 0)
>> + as_bad (_(".seh_startepilog offset expression for %s "
>> + "does not evaluate to a non-negative constant"),
>> + S_GET_NAME (epilogue_start_addr));
>
> As indicated on an earlier patch, such wants indenting differently.
> Going over the file you add to, I find a number of as_bad() with
> enough arguments to require wrapping. All look well-formed to me, so
> any can serve as a reference. (As a rule of thumb, indentation wants
> to properly express the pending open parentheses, at any given point.)
The indentation has been fixed.
> Further, is it necessary for the expression to resolve to a (non-
> negative) constant right at parsing time? Wouldn't it be sufficient
> for it to resolve to a suitable constant once we're finalizing
> assembly?
Resolution is needed at this point because it creates an array of epilogue scopes, and the context changes during parsing.
>> + seh_arm64_epilogue_scope *epilogue_scope;
>> + epilogue_scope = seh_ctx_cur->arm64_ctx.epilogue_scopes
>> + + seh_ctx_cur->arm64_ctx.epilogue_scopes_count;
>> + epilogue_scope->epilogue_start_offset = exp.X_add_number / 4;
>> + epilogue_scope->reserved = 0;
>> + epilogue_scope->epilogue_start_index
>> + = seh_ctx_cur->arm64_ctx.unwind_codes_byte_count;
>> + seh_ctx_cur->arm64_ctx.epilogue_scopes_count++;
>
> What if there are too many of these directives? Won't you overrun
> some array?
Good catch. Validation has been added.
>> @@ -866,6 +946,64 @@ obj_coff_seh_save (int what)
>> }
>> #endif
>>
>> +#if defined (COFFAARCH64)
>> +static void
>> +obj_coff_seh_save_reg (int type)
>> +{
>> + if (type < 0 || type > unwind_last_type)
>> + {
>> + as_bad (_("invalid pseudo operation."));
>
> as_bad() tells the user that they have done something wrong. This is an
> internal error though, aiui, so this may want to be gas_assert() or
> know() or some such.
>
> Also: No full stops please at the end of diagnostics.
The validation has been replaced with gas_assert.
>> + return;
>> + }
>> +
>> + const struct unwind_code_pack_info *unwind_code_pack_info;
>> + unwind_code_pack_info = unwind_code_pack_infos + type;
>> +
>> + if (!unwind_code_pack_info->directive
>> + || !verify_context_and_target (unwind_code_pack_info->directive,
>> + seh_kind_arm64)
>
> Nit: Indentation again. (I guess I won't make further remarks in this
> regard.)
The implementation has been refactored and no longer contains the indentation issue.
>> + || !seh_validate_seg (unwind_code_pack_info->directive))
>> + return;
>> +
>> + SKIP_WHITESPACE ();
>> +
>> + char *symbol_name = NULL;
>> + int reg = -1;
>> +
>> + if (unwind_code_pack_info->reg_bits)
>> + {
>> + char name_end = get_symbol_name (&symbol_name);
>> + reg = atoi (symbol_name + 1);
>
> What about symbol_name[0]?
symbol_name[0] contains the prefix 'x' for a register and should be skipped when parsing
the register number.
>> + (void) restore_line_pointer (name_end);
>> +
>> + if (!skip_whitespace_and_comma (1))
>> + return;
>> +
>> + if (reg < 0)
>> + {
>> + as_bad (_("register is negative"));
>> + return;
>> + }
>> + }
>
> No upper bound on the register number?
Good point. It makes sense to add an upper bound and it has been added.
Regards,
Evgeny
[PATCH] Add aarch64-specific SEH commands
Implementation for aarch64 contains SEH commands that are not present for
x64 architecture, such as .seh_startepilogue/.seh_endepilogue,
.seh_save_reg*, .seh_save_freg*, and others.
gas/ChangeLog:
* config/obj-coff-seh.c (defined): Use COFFAARCH64 guard.
(obj_coff_seh_startepilogue): New.
(obj_coff_seh_endepilogue): New.
(obj_coff_seh_endfunclet): New.
(obj_coff_seh_save_reg): New.
---
gas/config/obj-coff-seh.c | 136 ++++++++++++++++++++++++++++++++++++++
1 file changed, 136 insertions(+)
diff --git a/gas/config/obj-coff-seh.c b/gas/config/obj-coff-seh.c
index 199203be1cc..67d213a4ca7 100644
--- a/gas/config/obj-coff-seh.c
+++ b/gas/config/obj-coff-seh.c
@@ -682,6 +682,90 @@ obj_coff_seh_endprologue (int what ATTRIBUTE_UNUSED)
#endif
}
+#if defined (COFFAARCH64)
+static void
+obj_coff_seh_startepilogue (int what ATTRIBUTE_UNUSED)
+{
+ symbolS *epilogue_start_addr;
+ expressionS exp;
+
+ if (!verify_context (".seh_startepilogue")
+ || !seh_validate_seg (".seh_startepilogue"))
+ return;
+ demand_empty_rest_of_line ();
+
+ const unsigned max_epilogue_scopes = AARCH64_MAX_EPILOGUE_SCOPES;
+ if (seh_ctx_cur->aarch64_ctx.epilogue_scopes_count >= max_epilogue_scopes)
+ {
+ as_bad (_("no epilogue scopes available."));
+ return;
+ }
+
+ epilogue_start_addr = symbol_temp_new_now ();
+ exp.X_op = O_subtract;
+ exp.X_add_symbol = epilogue_start_addr;
+ exp.X_op_symbol = seh_ctx_cur->start_addr;
+ exp.X_add_number = 0;
+
+ if (!resolve_expression (&exp) || exp.X_op != O_constant
+ || exp.X_add_number < 0)
+ as_bad (_(".seh_startepilog offset expression for %s "
+ "does not evaluate to a non-negative constant"),
+ S_GET_NAME (epilogue_start_addr));
+
+ seh_aarch64_epilogue_scope *epilogue_scope;
+ epilogue_scope = seh_ctx_cur->aarch64_ctx.epilogue_scopes
+ + seh_ctx_cur->aarch64_ctx.epilogue_scopes_count;
+ epilogue_scope->epilogue_start_offset = exp.X_add_number / 4;
+ epilogue_scope->reserved = 0;
+ epilogue_scope->epilogue_start_index
+ = seh_ctx_cur->aarch64_ctx.unwind_codes_byte_count;
+ seh_ctx_cur->aarch64_ctx.epilogue_scopes_count++;
+}
+
+static void
+obj_coff_seh_endepilogue (int what ATTRIBUTE_UNUSED)
+{
+ if (!verify_context (".seh_endepilogue")
+ || !seh_validate_seg (".seh_endepilogue"))
+ return;
+
+ demand_empty_rest_of_line ();
+
+ expressionS exp;
+ symbolS *epilogue_end_addr = symbol_temp_new_now ();
+ exp.X_op = O_subtract;
+ exp.X_add_symbol = epilogue_end_addr;
+ exp.X_op_symbol = seh_ctx_cur->start_addr;
+ exp.X_add_number = 0;
+
+ if (!resolve_expression (&exp) || exp.X_op != O_constant
+ || exp.X_add_number < 0)
+ as_bad (_(".seh_endepilogue offset expression for %s "
+ "does not evaluate to a non-negative constant"),
+ S_GET_NAME (epilogue_end_addr));
+
+ seh_aarch64_epilogue_scope *epilogue_scope;
+ epilogue_scope = seh_ctx_cur->aarch64_ctx.epilogue_scopes
+ + seh_ctx_cur->aarch64_ctx.epilogue_scopes_count - 1;
+
+ epilogue_scope->epilogue_end_offset = exp.X_add_number;
+
+ /* End code. */
+ seh_aarch64_add_unwind_element (end, 0, 0);
+}
+
+static void
+obj_coff_seh_endfunclet (int what ATTRIBUTE_UNUSED)
+{
+ if (!verify_context (".seh_endfunclet")
+ || !seh_validate_seg (".seh_endfunclet"))
+ return;
+
+ demand_empty_rest_of_line ();
+}
+#endif
+
/* End-of-file hook. */
void
@@ -867,6 +951,58 @@ obj_coff_seh_save (int what)
}
#endif
+#if defined (COFFAARCH64)
+static void
+obj_coff_seh_save_reg (int type)
+{
+ gas_assert (type >= 0 && type <= unwind_last_type);
+
+ const struct aarch64_unwind_code_pack_info *unwind_code_pack_info;
+ unwind_code_pack_info = aarch64_unwind_code_pack_data + type;
+
+ if (!unwind_code_pack_info->directive
+ || !seh_validate_seg (unwind_code_pack_info->directive))
+ return;
+
+ SKIP_WHITESPACE ();
+
+ char *symbol_name = NULL;
+ int reg = -1;
+
+ if (unwind_code_pack_info->reg_bits)
+ {
+ char name_end = get_symbol_name (&symbol_name);
+ reg = atoi (symbol_name + 1);
+ (void) restore_line_pointer (name_end);
+
+ if (!skip_whitespace_and_comma (1))
+ return;
+
+ if (reg < 0 || reg > 30)
+ {
+ as_bad (_("register number is out of range"));
+ return;
+ }
+ }
+
+ offsetT off = -1;
+ if (unwind_code_pack_info->offset_bits)
+ {
+ off = get_absolute_expression ();
+
+ if (off < 0)
+ {
+ as_bad (_("offset is negative"));
+ return;
+ }
+ }
+
+ demand_empty_rest_of_line ();
+
+ seh_aarch64_add_unwind_element (type, off, reg);
+}
+#endif
+
/* Add a stack-allocation token to current context. */
static void
--
2.34.1
More information about the Binutils
mailing list