Skip to content

Commit 158807d

Browse files
nickdesaulniersKAGA-KOKO
authored andcommitted
x86/uaccess: Make __get_user_size() Clang compliant on 32-bit
Clang fails to compile __get_user_size() on 32-bit for the following code: long long val; __get_user(val, usrptr); with: error: invalid output size for constraint '=q' GCC compiles the same code without complaints. The reason is that GCC and Clang are architecturally different, which leads to subtle issues for code that's invalid but clearly dead, i.e. with code that emulates polymorphism with the preprocessor and sizeof. GCC will perform semantic analysis after early inlining and dead code elimination, so it will not warn on invalid code that's dead. Clang strictly performs optimizations after semantic analysis, so it will warn for dead code. Neither Clang nor GCC like this very much with -m32: long long ret; asm ("movb $5, %0" : "=q" (ret)); However, GCC can tolerate this variant: long long ret; switch (sizeof(ret)) { case 1: asm ("movb $5, %0" : "=q" (ret)); break; case 8:; } Clang, on the other hand, won't accept that because it validates the inline asm for the '1' case before the optimisation phase where it realises that it wouldn't have to emit it anyway. If LLVM (Clang's "back end") fails such as during instruction selection or register allocation, it cannot provide accurate diagnostics (warnings / errors) that contain line information, as the AST has been discarded from memory at that point. While there have been early discussions about having C/C++ specific language optimizations in Clang via the use of MLIR, which would enable such earlier optimizations, such work is not scoped and likely a multi-year endeavor. It was discussed to change the asm output constraint for the one byte case from "=q" to "=r". While it works for 64-bit, it fails on 32-bit. With '=r' the compiler could fail to chose a register accessible as high/low which is required for the byte operation. If that happens the assembly will fail. Use a local temporary variable of type 'unsigned char' as output for the byte copy inline asm and then assign it to the real output variable. This prevents Clang from failing the semantic analysis in the above case. The resulting code for the actual one byte copy is not affected as the temporary variable is optimized out. [ tglx: Amended changelog ] Reported-by: Arnd Bergmann <[email protected]> Reported-by: David Woodhouse <[email protected]> Reported-by: Dmitry Golovin <[email protected]> Reported-by: Linus Torvalds <[email protected]> Signed-off-by: Nick Desaulniers <[email protected]> Signed-off-by: Thomas Gleixner <[email protected]> Tested-by: Sedat Dilek <[email protected]> Acked-by: Linus Torvalds <[email protected]> Acked-by: Dennis Zhou <[email protected]> Link: https://bugs.llvm.org/show_bug.cgi?id=33587 Link: ClangBuiltLinux#3 Link: ClangBuiltLinux#194 Link: ClangBuiltLinux#781 Link: https://lore.kernel.org/lkml/[email protected]/ Link: https://lore.kernel.org/lkml/CAK8P3a1EBaWdbAEzirFDSgHVJMtWjuNt2HGG8z+vpXeNHwETFQ@mail.gmail.com/ Link: https://lkml.kernel.org/r/[email protected]
1 parent 4719ffe commit 158807d

File tree

1 file changed

+4
-1
lines changed

1 file changed

+4
-1
lines changed

arch/x86/include/asm/uaccess.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,11 +314,14 @@ do { \
314314

315315
#define __get_user_size(x, ptr, size, retval) \
316316
do { \
317+
unsigned char x_u8__; \
318+
\
317319
retval = 0; \
318320
__chk_user_ptr(ptr); \
319321
switch (size) { \
320322
case 1: \
321-
__get_user_asm(x, ptr, retval, "b", "=q"); \
323+
__get_user_asm(x_u8__, ptr, retval, "b", "=q"); \
324+
(x) = x_u8__; \
322325
break; \
323326
case 2: \
324327
__get_user_asm(x, ptr, retval, "w", "=r"); \

0 commit comments

Comments
 (0)