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

On Tue, Jul 21, 2026 at 10:35:51AM +0200, Evgeny Karpov wrote:
> On Mon, 20 Jul 2026, Alice Carlotti wrote:
> > > +  /* Calculate how many unwind bytes will be emitted in .xdata record.  */
> > > +  unsigned unwind_bytes = seh_ctx->unwind_codes_byte_count;
> > > +
> > > +  /* Check if current fragment has a phantom prologue.  If yes, then
> > > +      the unwinding size should be adjusted.  */
> > > +  const bool has_phantom_prologue = fragment_offset != 0;
> > > +  if (has_phantom_prologue && unwind_bytes)
> > 
> > The list of unwind codes will always include at least one `end` unwind code for
> > the prologue (phantom or non-phantom), even after any future optimisations are
> > applied.  So any checks that unwind_bytes or code_words are nonzero can be
> > dropped (or replaced by a single assert that unwind_bytes is zero at the start
> > of the function).
> 
> Ok, in current implementation, code_words are nonzero, however it might be changed later.
> The check will be removed for now. 
> 
> 
> > > +    {
> > > +      /* One more epilogue scope and unwind code are emitted with phantom
> > > +	  prologue.  */
> > > +      unwind_bytes += 1;
> > > +      ++epilogue_count;
> > 
> > What is this epilogue_count increment doing?  You don't seem to emit any extra
> > epilogue scope to match the increased count, so I think this increment needs to
> > be removed.
> 
> There is a comment above to clarify this.

Do you mean the comment that says "One more epilogue scope ... are emitted"?
If so, then that doesn't clarify anything.

> It is required for fragments that have a phantom prologue in the current implementation.
> Otherwise, an exception in the second and later fragment will not be unwound.

Why?  I see no reason for this.

> This epilogue references these unwind codes.

Which unwind codes?

> This is tested in gas/testsuite/gas/pe/seh-aarch64-large-func.d.

That test case only uses the E=1 format, so doesn't address my question about
(not) emitting your extra epilogue scope when there are multiple epilogues.

> 
>  
> > > +  header |= seh_ctx->has_exception_data << has_exception_data_shift;
> > > +
> > > +  /* Check if short or extended header for a .xdata record should be
> > > +      used.  */
> > > +  unsigned header_size = 8;
> > > +  bool single_epilog = false;
> > > +  if ((code_words != 0 || epilogue_count != 0)
> > 
> > code_words is always nonzero (as explained above).
> 
> Ok, it will be removed for now.
> 
> 
> > > +      && code_words < 32
> > > +      && epilogue_count < 32)
> > > +    {
> > > +      header_size = 4;
> > > +      if (epilogue_count == 1)
> > 
> > This condition still needs more checks (epilogue start index < 32, and I think
> > also the epilogue needs to be at the end of the function).
>  
> Start index is not used in that place, it is not clear why it is requested to be checked.

Start index is recorded in the field that normally countains the epilogue
count.

> As it was mentioned earlier, the epilogue at the end of the function does not use unwind codes
> and it is not required to emit it as an epilogue scope.

As far as I can tell this is incorrect - can you provide any evidence to support your claim?


> > > +	{
> > > +	  single_epilog = true;
> > > +	  epilogue_count = 0;

Your above comment reminded me that this is the wrong value for that header
field when E=1 - the field should record the start index of the single
epilogue.

> > > +	}
> > > +      header |= epilogue_count << epilogue_count_shift;
> > > +      header |= code_words << code_words_shift;
> > > +    }


Alice