lkml.org 
[lkml]   [2020]   [Jul]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[tip: x86/asm] x86/uaccess: Make __get_user_size() Clang compliant on 32-bit
    The following commit has been merged into the x86/asm branch of tip:

    Commit-ID: 158807de5822d1079e162a3762956fd743dd483e
    Gitweb: https://git.kernel.org/tip/158807de5822d1079e162a3762956fd743dd483e
    Author: Nick Desaulniers <ndesaulniers@google.com>
    AuthorDate: Mon, 20 Jul 2020 13:49:25 -07:00
    Committer: Thomas Gleixner <tglx@linutronix.de>
    CommitterDate: Thu, 23 Jul 2020 12:38:31 +02:00

    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 <arnd@arndb.de>
    Reported-by: David Woodhouse <dwmw2@infradead.org>
    Reported-by: Dmitry Golovin <dima@golovin.in>
    Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
    Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
    Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
    Acked-by: Dennis Zhou <dennis@kernel.org>
    Link: https://bugs.llvm.org/show_bug.cgi?id=33587
    Link: https://github.com/ClangBuiltLinux/linux/issues/3
    Link: https://github.com/ClangBuiltLinux/linux/issues/194
    Link: https://github.com/ClangBuiltLinux/linux/issues/781
    Link: https://lore.kernel.org/lkml/20180209161833.4605-1-dwmw2@infradead.org/
    Link: https://lore.kernel.org/lkml/CAK8P3a1EBaWdbAEzirFDSgHVJMtWjuNt2HGG8z+vpXeNHwETFQ@mail.gmail.com/
    Link: https://lkml.kernel.org/r/20200720204925.3654302-12-ndesaulniers@google.com
    ---
    arch/x86/include/asm/uaccess.h | 5 ++++-
    1 file changed, 4 insertions(+), 1 deletion(-)

    diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
    index 18dfa07..2f3e8f2 100644
    --- a/arch/x86/include/asm/uaccess.h
    +++ b/arch/x86/include/asm/uaccess.h
    @@ -314,11 +314,14 @@ do { \

    #define __get_user_size(x, ptr, size, retval) \
    do { \
    + unsigned char x_u8__; \
    + \
    retval = 0; \
    __chk_user_ptr(ptr); \
    switch (size) { \
    case 1: \
    - __get_user_asm(x, ptr, retval, "b", "=q"); \
    + __get_user_asm(x_u8__, ptr, retval, "b", "=q"); \
    + (x) = x_u8__; \
    break; \
    case 2: \
    __get_user_asm(x, ptr, retval, "w", "=r"); \
    \
     
     \ /
      Last update: 2020-07-23 12:44    [W:2.682 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site