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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.02.2024 18:03, Roger Pau Monne wrote:
> The current code for alternative calls uses the caller parameter types as the
> types for the register variables that serve as function parameters:
> 
> uint8_t foo;
> [...]
> alternative_call(myfunc, foo);
> 
> Would expand roughly into:
> 
> register unint8_t a1_ asm("rdi") = foo;
> register unsigned long a2_ asm("rsi");
> [...]
> asm volatile ("call *%c[addr](%%rip)"...);
> 
> However under certain circumstances clang >= 16.0.0 with -O2 can generate
> incorrect code,

Considering that the related (wider) ABI issue looks to also be present on
Clang5, is the more specific issue here really limited to >= 16?

> given the following example:
> 
> unsigned int func(uint8_t t)
> {
>     return t;
> }
> 
> static void bar(uint8_t b)
> {
>     int ret_;
>     register uint8_t di asm("rdi") = b;
>     register unsigned long si asm("rsi");
>     register unsigned long dx asm("rdx");
>     register unsigned long cx asm("rcx");
>     register unsigned long r8 asm("r8");
>     register unsigned long r9 asm("r9");
>     register unsigned long r10 asm("r10");
>     register unsigned long r11 asm("r11");
> 
>     asm volatile ( "call %c[addr]"
>                    : "+r" (di), "=r" (si), "=r" (dx),
>                      "=r" (cx), "=r" (r8), "=r" (r9),
>                      "=r" (r10), "=r" (r11), "=a" (ret_)
>                    : [addr] "i" (&(func)), "g" (func)
>                    : "memory" );
> }
> 
> void foo(unsigned int a)
> {
>     bar(a);
> }
> 
> Clang generates the following code:
> 
> func:                                   # @func
>         movl    %edi, %eax
>         retq
> foo:                                    # @foo
>         callq   func
>         retq
> 
> Note the truncation of the unsigned int parameter 'a' of foo() to uint8_t when
> passed into bar() is lost.
> 
> The above can be worked around by using an union when defining the register
> variables, so that `di` becomes:
> 
> register union {
>     uint8_t e;
>     unsigned long r;
> } di asm("rdi") = { .e = b };
> 
> Which results in following code generated for `foo()`:
> 
> foo:                                    # @foo
>         movzbl  %dil, %edi
>         callq   func
>         retq
> 
> So the truncation is not longer lost.

But how do you explain this behavior? I see absolutely no reason why filling
the one union field should lead to zero-extension. If I'm not mistaken the
language allows the rest of the union to retain undefined contents. So to me
this looks like you're converting something that failed to build due to a
(presumed) bug in Clang to something that any compiler would be permitted to
translate to other than what we want.

In this context note in particular that the spec distinguishes aggregates
from unions, and the clause regarding filling unmentioned fields is limited
to aggregates:

"If there are fewer initializers in a brace-enclosed list than there are
 elements or members of an aggregate, or fewer characters in a string
 literal used to initialize an array of known size than there are elements
 in the array, the remainder of the aggregate shall be initialized
 implicitly the same as objects that have static storage duration."

Which makes sense, as a union initializer shall mention a single of the
members only anyway.

All of this of course doesn't invalidate your approach as a possible
workaround, but it then needs limiting to Clang versions where we are sure
the more strict behavior than demanded by the standard actually applies.

Jan