[PATCH v2 7/7] Fix the calculation of the function length

[Richard Earnshaw (lists)]

On 23/07/2025 16:55, Evgeny Karpov wrote:
> Wednesday, May 21
> Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote:
>>
>>> +/* Sections are assumed to start aligned.  In executable section, there is no
>>> +   MAP_DATA symbol pending.  So we only align the address during
>>> +   MAP_DATA --> MAP_INSN transition.
>>> +   For other sections, this is not guaranteed.  */
>>>  #define md_do_align(N, FILL, LEN, MAX, LABEL)					\
>>>    if (FILL == NULL && (N) != 0 && ! need_pass_2 && subseg_text_p (now_seg))	\
>>>      {										\
>>> -      frag_align_code (N, MAX);							\
>>> +      enum mstate mapstate = seg_info (now_seg)->tc_segment_info_data.mapstate;	\
>>> +      if (mapstate == MAP_DATA)							\
>>> +	frag_align_code (N, MAX);						\
>>>        goto LABEL;								\
>>>      }
>>>  
>>
>> Can you show a testcase for this please?  I'm trying to understand the issue you're trying to resolve.
> 
> Repro case:
> 
> .text
> .seh_proc	test
> .seh_endprologue
>     mov x0, xzr
>     .align 2 // Once this is removed, the expression for function size can be evaluated correctly
>     ret
> .seh_endproc
> 

I don't think it's that uncommon to see code with alignment of branch targets.  For example,

.text
     .p2align 4  // 16-byte alignment so that func is at start of cache line
func:
     // code
     b  Lsomewhere
     // no fall-through path
     .p2align 4  // 16-byte alignment so that Lsomewhere_else is at start of cache line
Lsomewhere_else:
     // more code

We certainly want to support that.  See, for example, -falign-labels and -falign-loops in the gcc manual.

Maybe your problem is that you're trying to calculate the function size for the seh data too early, before the frags have been consolidated: the intra-section alignment data should all be replaced with appropriate padding before the object file is emitted, so all offsets within a function become fixed.

R.

> The fix aligns the code section only when it is in the MAP_DATA state.
> In general, it is expected that this is not the case for executable sections.
> 
> The patch description will be extended with this repro case.
> 
> However, it looks like it breaks some tests on other platforms.
> Any ideas what might be the issue?
> https://patchwork.sourceware.org/project/binutils/patch/VI2PR83MB071806BA44D9B7485D1BEACEF88AA@VI2PR83MB0718.EURPRD83.prod.outlook.com/
> 
> Regards,
> Evgeny