Re: [PATCH] xen: Drop __text_section()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 17/03/2025 5:15 pm, Andrew Cooper wrote:
> On 17/03/2025 8:43 am, Jan Beulich wrote:
>> On 14.03.2025 18:59, Andrew Cooper wrote:
>>> This is an unnecessary indirection.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> So, this is the patch causing build chaos.  I've reproduced locally,
> even though I still can't explain what is going on.
>
> One example breakage is:
>
>> In file included from drivers/passthrough/amd/iommu_acpi.c:20:
>> In file included from ./include/xen/acpi.h:41:
>> In file included from ./include/xen/list.h:10:
>> ./include/xen/bug.h:107:5: error: invalid operand for inline asm
>> constraint 'i'
>>     BUG_FRAME(BUGFRAME_run_fn, 0, fn, 0, NULL);
>>     ^
>> ./include/xen/bug.h:94:20: note: expanded from macro 'BUG_FRAME'
>>     asm volatile (
>> _ASM_BUGFRAME_TEXT(second_frame)                          \
>>                    ^
>> ./include/xen/bug.h:68:5: note: expanded from macro '_ASM_BUGFRAME_TEXT'
>>    
>> ".Lbug%=:"BUG_INSTR"\n"                                                      
>>   
>> \
>>     ^
>> 1 error generated.
>> make[4]: *** [Rules.mk:249: drivers/passthrough/amd/iommu_acpi.o] Error 1
> although most translation units fail in the same way.
>
> This is run_in_exception_handler(), but there's nothing obviously wrong
> in the preprocessed source.  The parameters are respectively:
>
> : [bf_type] "i" (0),
>   [bf_ptr] "i" (fn),
>   [bf_msg] "i" (((void*)0)),
>   [bf_line_lo] "i" (((0) & ((1 << (31 - 24)) - 1)) << 24),
>   [bf_line_hi] "i" (((0) >> (31 - 24)) << 24)
>
> which is as expected.
>
>
> There is one difference I have noticed which is clearly relevant but I'm
> not sure how.
>
> __section() isn't exactly __attribute__((__section__(s))), because (just
> out of scope of the patch), __clang__ adds a __used attribute to
> __section().
>
> The logic doing this is ahead of __used being defined, and the TUs which
> break are -DINIT_SECTIONS_ONLY which includes init.h prior to __used
> having being defined.
>
> And yet, there's no un-expanded __used in the preprocessed source.
>
> Also, this only goes wrong in the Shim build, not in the main build.
>
> Also, building the object file in isolation seems to work fine, although
> this seems to remove -DINIT_SECTIONS_ONLY from the cmdline which is
> almost certainly masking what's going wrong.

It is the extra __used in the Clang case.  Taking that out causes
everything to compile fine.

Attempts to minimise the failing case aren't working.  cvise has
minimised to:

void run_in_exception_handler_fn() {
  asm("" ::"i"(run_in_exception_handler_fn));
}

but this is clearly not the same issue because it's not sensitive to the
presence/absence of __used.

I can't help but feeling that "error: invalid operand for inline asm
constraint 'i'" is a knock-on effect from something else, rather than
the original root cause.

~Andrew