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

[PATCH for-4.19] x86/altcall: fix clang code-gen when using altcall in loop constructs



Yet another clang code generation issue when using altcalls.

The issue this time is with using loop constructs around alternative_{,v}call
instances using parameter types smaller than the register size.

Given the following example code:

static void bar(bool b)
{
    unsigned int i;

    for ( i = 0; i < 10; i++ )
    {
        int ret_;
        register union {
            bool e;
            unsigned long r;
        } di asm("rdi") = { .e = 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" );
    }
}

See: https://godbolt.org/z/qvxMGd84q

Clang will generate machine code that only resets the low 8 bits of %rdi
between loop calls, leaving the rest of the register possibly containing
garbage from the use of %rdi inside the called function.  Note also that clang
doesn't truncate the input parameters at the callee, thus breaking the psABI.

Fix this by turning the `e` element in the anonymous union into an array that
consumes the same space as an unsigned long, as this forces clang to reset the
whole %rdi register instead of just the low 8 bits.

Fixes: 2ce562b2a413 ('x86/altcall: use a union as register type for function 
parameters on clang')
Suggested-by: Jan Beulich <jbeulich@xxxxxxxx>
Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
Adding Oleksii as to whether this could be considered for 4.19: it's strictly
limited to clang builds, plus will need to be backported anyway.
---
 xen/arch/x86/include/asm/alternative.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/include/asm/alternative.h 
b/xen/arch/x86/include/asm/alternative.h
index 0d3697f1de49..e63b45927643 100644
--- a/xen/arch/x86/include/asm/alternative.h
+++ b/xen/arch/x86/include/asm/alternative.h
@@ -185,10 +185,10 @@ extern void alternative_branches(void);
  */
 #define ALT_CALL_ARG(arg, n)                                            \
     register union {                                                    \
-        typeof(arg) e;                                                  \
+        typeof(arg) e[sizeof(long) / sizeof(arg)];                      \
         unsigned long r;                                                \
     } a ## n ## _ asm ( ALT_CALL_arg ## n ) = {                         \
-        .e = ({ BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); (arg); })   \
+        .e[0] = ({ BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); (arg); })\
     }
 #else
 #define ALT_CALL_ARG(arg, n) \
-- 
2.45.2




 


Rackspace

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