Re: [PATCH] x86/altcall: further refine clang workaround

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.07.2024 09:31, Roger Pau Monné wrote:
> On Thu, Jul 25, 2024 at 05:00:22PM +0200, Jan Beulich wrote:
>> On 25.07.2024 16:54, Roger Pau Monné wrote:
>>> On Thu, Jul 25, 2024 at 03:18:29PM +0200, Jan Beulich wrote:
>>>> On 25.07.2024 12:56, Roger Pau Monne wrote:
>>>>> --- a/xen/arch/x86/include/asm/alternative.h
>>>>> +++ b/xen/arch/x86/include/asm/alternative.h
>>>>> @@ -184,11 +184,11 @@ extern void alternative_branches(void);
>>>>>   * https://github.com/llvm/llvm-project/issues/82598
>>>>>   */
>>>>>  #define ALT_CALL_ARG(arg, n)                                            \
>>>>> -    register union {                                                    \
>>>>> -        typeof(arg) e[sizeof(long) / sizeof(arg)];                      \
>>>>> -        unsigned long r;                                                \
>>>>> +    register struct {                                                   \
>>>>> +        typeof(arg) e;                                                  \
>>>>> +        char pad[sizeof(void *) - sizeof(arg)];                         \
>>>>
>>>> One thing that occurred to me only after our discussion, and I then forgot
>>>> to mention this before you would send a patch: What if sizeof(void *) ==
>>>> sizeof(arg)? Zero-sized arrays are explicitly something we're trying to
>>>> get rid of.
>>>
>>> I wondered about this, but I though it was only [] that we were trying
>>> to get rid of, not [0].
>>
>> Sadly (here) it's actually the other way around, aiui.
> 
> The only other option I have in mind is using an oversized array on
> the union, like:
> 
> #define ALT_CALL_ARG(arg, n)                                            \
>     union {                                                             \
>         typeof(arg) e[(sizeof(long) + sizeof(arg) - 1) / sizeof(arg)];  \
>         unsigned long r;                                                \
>     } a ## n ## __  = {                                                 \
>         .e[0] = ({ BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); (arg); })\
>     };                                                                  \
>     register unsigned long a ## n ## _ asm ( ALT_CALL_arg ## n ) =      \
>         a ## n ## __.r

Yet that's likely awful code-gen wise? For the time being, can we perhaps
just tighten the BUILD_BUG_ON(), as iirc Alejandro had suggested?

Jan