Re: [PATCH v2] x86/altcall: use an union as register type for function parameters

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Feb 22, 2024 at 05:55:00PM +0100, Jan Beulich wrote:
> On 22.02.2024 17:44, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/include/asm/alternative.h
> > +++ b/xen/arch/x86/include/asm/alternative.h
> > @@ -167,9 +167,25 @@ extern void alternative_branches(void);
> >  #define ALT_CALL_arg5 "r8"
> >  #define ALT_CALL_arg6 "r9"
> >  
> > +#ifdef CONFIG_CC_IS_CLANG
> > +/*
> > + * Use an union with an unsigned long in order to prevent clang from 
> > skipping a
> > + * possible truncation of the value.  By using the union any truncation is
> > + * carried before the call instruction.
> > + * https://github.com/llvm/llvm-project/issues/82598
> > + */
> 
> I think it needs saying that this is relying on compiler behavior not
> mandated by the standard, thus explaining why it's restricted to
> Clang (down the road we may even want to restrict to old versions,
> assuming they fix the issue at some point). Plus also giving future
> readers a clear understanding that if something breaks with this, it's
> not really a surprise.

What about:

Use a union with an unsigned long in order to prevent clang from
skipping a possible truncation of the value.  By using the union any
truncation is carried before the call instruction.  Note this
behavior is not mandated by the standard, and hence could stop being
a viable workaround, or worse, could cause a different set of
code-generation issues in future clang versions.

This has been reported upstream at:
https://github.com/llvm/llvm-project/issues/82598

> Aiui this bug is only a special case of the other, much older one, so
> referencing that one here too would seem advisable.

My report has been resolved as a duplicate of:

https://github.com/llvm/llvm-project/issues/43573

FWIW, I think for the context the link is used in (altcall) my bug
report is more representative, and readers can always follow the trail
into the other inter-related bugs.

> As a nit: I think it is "a union" (spelling according to pronunciation),
> and I guess the title could now do with saying "optionally" or
> mentioning Clang or some such.

OK.

Thanks, Roger