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

[PATCH v3] x86/altcall: further refine clang workaround



The current code in ALT_CALL_ARG() won't successfully workaround the clang
code-generation issue if the arg parameter has a size that's not a power of 2.
While there are no such sized parameters at the moment, improve the workaround
to also be effective when such sizes are used.

Instead of using a union with a long use an unsigned long that's first
initialized to 0 and afterwards set to the argument value.

Reported-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
Suggested-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
Changes since v2:
 - Fix indentation and style issues.
 - Adjust comment to match the new workaround.
---
 xen/arch/x86/include/asm/alternative.h | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/include/asm/alternative.h 
b/xen/arch/x86/include/asm/alternative.h
index e63b45927643..c5fa242e76b3 100644
--- a/xen/arch/x86/include/asm/alternative.h
+++ b/xen/arch/x86/include/asm/alternative.h
@@ -169,27 +169,25 @@ extern void alternative_branches(void);
 
 #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.
+ * Clang doesn't follow the psABI and doesn't truncate parameter values at the
+ * callee.  This can lead to bad code being generated when using alternative
+ * calls.
  *
- * 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.
+ * Workaround it by using a temporary intermediate variable that's zeroed
+ * before being assigned the parameter value, as that forces clang to zero the
+ * register at the caller.
  *
  * 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[sizeof(long) / sizeof(arg)];                      \
-        unsigned long r;                                                \
-    } a ## n ## _ asm ( ALT_CALL_arg ## n ) = {                         \
-        .e[0] = ({ BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); (arg); })\
-    }
+    register unsigned long a ## n ## _ asm ( ALT_CALL_arg ## n ) = ({   \
+        unsigned long tmp = 0;                                          \
+        *(typeof(arg) *)&tmp = (arg);                                   \
+        BUILD_BUG_ON(sizeof(arg) > sizeof(unsigned long));              \
+        tmp;                                                            \
+    })
 #else
 #define ALT_CALL_ARG(arg, n) \
     register typeof(arg) a ## n ## _ asm ( ALT_CALL_arg ## n ) = \
-- 
2.45.2




 


Rackspace

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