From 1aafe054e7d1efbf8e8482a9cdd4be5753b79e2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roger=20Pau=20Monn=C3=A9?= Date: Tue, 27 Feb 2024 14:11:04 +0100 Subject: [PATCH] x86/altcall: use a union as register type for function parameters on clang MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 with -O2 clang will generate incorrect code, 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 assembly 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. clang doesn't zero extend the parameters in the callee when required, as the psABI mandates. The above can be worked around by using a 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. Apply such workaround only when built with clang. Reported-by: Matthew Grooms Link: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=277200 Link: https://github.com/llvm/llvm-project/issues/12579 Link: https://github.com/llvm/llvm-project/issues/82598 Signed-off-by: Roger Pau Monné Acked-by: Jan Beulich master commit: 2ce562b2a413cbdb2e1128989ed1722290a27c4e master date: 2024-02-26 10:18:01 +0100 --- xen/arch/x86/include/asm/alternative.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/xen/arch/x86/include/asm/alternative.h b/xen/arch/x86/include/asm/alternative.h index a7a82c2c03..bcb1dc94f4 100644 --- a/xen/arch/x86/include/asm/alternative.h +++ b/xen/arch/x86/include/asm/alternative.h @@ -167,9 +167,34 @@ extern void alternative_branches(void); #define ALT_CALL_arg5 "r8" #define ALT_CALL_arg6 "r9" +#ifdef CONFIG_CC_IS_CLANG +/* + * 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, in turn covering + * for ABI-non-compliance in that the necessary clipping / extension of + * the value is supposed to be carried out in the callee. + * + * 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: + * 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; \ + unsigned long r; \ + } a ## n ## _ asm ( ALT_CALL_arg ## n ) = { \ + .e = ({ BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); (arg); }) \ + } +#else #define ALT_CALL_ARG(arg, n) \ register typeof(arg) a ## n ## _ asm ( ALT_CALL_arg ## n ) = \ ({ BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); (arg); }) +#endif #define ALT_CALL_NO_ARG(n) \ register unsigned long a ## n ## _ asm ( ALT_CALL_arg ## n )