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, 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. Apply such workaround only when built
with clang.
Reported-by: Matthew Grooms <[email protected]>
Link: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=277200
Link: https://github.com/llvm/llvm-project/issues/82598
Signed-off-by: Roger Pau Monné <[email protected]>
---
Changes since v1:
- Only apply the union workaround with clang.
Seems like all gitlab build tests are OK with this approach.
---
xen/arch/x86/include/asm/alternative.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/xen/arch/x86/include/asm/alternative.h
b/xen/arch/x86/include/asm/alternative.h
index a1cd6a9fe5b8..3fe27ea791bf 100644
--- 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
+ */
+#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 )
--
2.43.0
