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

["Alejandro Vallejo" <alejandro.vallejo@xxxxxxxxx>]

On Mon Jul 29, 2024 at 11:47 AM BST, Jan Beulich wrote:
> On 29.07.2024 12:30, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/include/asm/alternative.h
> > +++ b/xen/arch/x86/include/asm/alternative.h
> > @@ -183,13 +183,13 @@ extern void alternative_branches(void);
> >   * https://github.com/llvm/llvm-project/issues/12579
> >   * 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;                                                \
> > -    } a ## n ## _ asm ( ALT_CALL_arg ## n ) = {                         \
> > -        .e[0] = ({ BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); (arg); })\
> > -    }
> > +#define ALT_CALL_ARG(arg, n)                                             \
> > +     register unsigned long a ## n ## _ asm ( ALT_CALL_arg ## n ) = ({   \
> > +         unsigned long tmp = 0;                                          \
> > +         *(typeof(arg) *)&tmp = (arg);                                   \
> > +         BUILD_BUG_ON(sizeof(arg) > sizeof(void *));                     \
>
> With this, even more so than before, I think the type of tmp would better
> be void * (or the BUILD_BUG_ON() be made use unsigned long, yet I consider
> that less desirable). As a nit, I also don't think the backslashes need
> moving out by one position. Finally I'm afraid you're leaving stale the
> comment ahead of this construct.
>
> Jan

I wouldn't be thrilled to create a temp variable of yet another type that is
potentially neither "typeof(arg)" nor "unsigned long". No need to create a 3
body problem, where 2 is enough. Adjusting BUILD_BUG_ON() to use the same temp
type seems sensible, but I don't mind very much.

With the stale comment adjusted:

  Reviewed-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>

Cheers,
Alejandro