Attachment 248661 Details for Bug 277200 – Fix clang codegen

[patch] Fix clang codegen

0001-x86-altcall-use-an-union-as-register-type-for-functi.patch (text/plain), 3.83 KB, created by Roger Pau Monné on 2024-02-21 17:01:26 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Roger Pau Monné

Created: 2024-02-21 17:01:26 UTC

Size: 3.83 KB

patch

obsolete

>From 968c974203629a624234ca197db131b22b41efce Mon Sep 17 00:00:00 2001
>From: Roger Pau Monne <roger.pau@citrix.com>
>To: xen-devel@lists.xenproject.org
>Cc: Jan Beulich <jbeulich@suse.com>
>Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>Cc: "Roger Pau MonnÃ©" <roger.pau@citrix.com>
>Cc: Wei Liu <wl@xen.org>
>Date: Wed, 21 Feb 2024 16:55:12 +0100
>Subject: [PATCH] x86/altcall: use an union as register type for function
> parameters
>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 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.
>
>Signed-off-by: Roger Pau MonnÃ© <roger.pau@citrix.com>
>---
> xen/arch/x86/include/asm/alternative.h | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
>diff --git a/xen/arch/x86/include/asm/alternative.h b/xen/arch/x86/include/asm/alternative.h
>index a1cd6a9fe5b8..837dfd953d2f 100644
>--- a/xen/arch/x86/include/asm/alternative.h
>+++ b/xen/arch/x86/include/asm/alternative.h
>@@ -167,9 +167,18 @@ extern void alternative_branches(void);
> #define ALT_CALL_arg5 "r8"
> #define ALT_CALL_arg6 "r9"
> 
>-#define ALT_CALL_ARG(arg, n) \
>-    register typeof(arg) a ## n ## _ asm ( ALT_CALL_arg ## n ) = \
>-        ({ BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); (arg); })
>+/*
>+ * Use an union with an unsigned long in order to prevent clang >= 16.0.0 from
>+ * skipping a possible truncation of the value.  By using the union any
>+ * truncation is carried before the call instruction.
>+ */
>+#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); })   \
>+    }
> #define ALT_CALL_NO_ARG(n) \
>     register unsigned long a ## n ## _ asm ( ALT_CALL_arg ## n )
> 
>-- 
>2.43.0
>

Actions: View | Diff

Attachments on bug 277200: 248646 | 248661 | 252140