[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[xen staging] x86/altcall: use a union as register type for function parameters on clang



commit 2ce562b2a413cbdb2e1128989ed1722290a27c4e
Author:     Roger Pau Monné <roger.pau@xxxxxxxxxx>
AuthorDate: Mon Feb 26 10:18:01 2024 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Mon Feb 26 10:18:01 2024 +0100

    x86/altcall: use a union as register type for function parameters on clang
    
    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 <mgrooms@xxxxxxxxx>
    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é <roger.pau@xxxxxxxxxx>
    Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 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 a1cd6a9fe5..3c14db5078 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 )
 
--
generated by git-patchbot for /home/xen/git/xen.git#staging



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.