[PATCH v9 1/1] aarch64: Implement Structured Exception Handling (SEH) on AArch64
Evgeny Karpov
evgeny.karpov@arm.com
Thu Jun 25 14:39:12 GMT 2026
On Mon, 22 Jun 2026, Alice Carlotti wrote:
> On Wed, Jun 17, 2026 at 10:39:47AM +0200, Evgeny Karpov wrote:
> ...
> > +/* Write out the xdata information for one function. */
> > +static void
> > +seh_aarch64_write_function_xdata (struct seh_aarch64_context *seh_ctx)
>
> Oops, I forgot to write my comments on this functions.
>
> In general, I think this function ought handle function fragmention, but the
> encoding details (including choosing between compact or extended header fields)
> should be handled in seh_aarch64_emit_xdata_record. This separation of
> concerns should help with readability.
>
> I think there are also several corner cases that are mishandled in this code,
> but I've been struggling to work out what's going on well enough to say that
> particular pieces are right or wrong. I'll try to point any specific issues I
> see, but I'd appreciate if you could also see whether you can do anything more
> to improve the structure of this code.
Some checks will be moved to seh_aarch64_emit_xdata_record when it makes sense.
> > + 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. */
>
> See previous review comments about how this implementation doesn't currently
> support (or need to handle) fragmenting due to too many unwind codes or too
> many epilogue scopes.
The description with limitations will be extended mentioning this.
> > + 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;
>
> 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.
> > +
> > + /* 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;
> The is_last_frag part looks wrong - for a typical function layout, I'd expect
> only the first fragment to have a real prologue, and the rest to have a phantom
> prologue.
Ok, it will be validated and addressed if needed in v10.
> > + {
> > + const seh_aarch64_epilogue_scope *scope;
> > + scope = scopes + first_fragment_scope;
> > + header->ext_epilogue_count = scope->epilogue_start_index;
> > + }
> > + }
> > + }
> This code is mostly selecting between different header formats, so I think it
> should be part of seh_aarch64_emit_xdata_record.
It will be moved to seh_aarch64_emit_xdata_record.
Regards,
Evgeny
More information about the Binutils
mailing list