Bumping for comment+review.
On Mon, May 4, 2020 at 4:03 PM Nick Desaulniers <ndesaulni...@google.com> wrote:
>
> GCC and Clang are architecturally different, which leads to subtle
> issues for code that's invalid but clearly dead. This can happen 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.
>
> We also don't want to swap the use of "=q" with "=r". For 64b, it
> doesn't matter. For 32b, it's possible that a 32b register without a 8b
> lower alias (i.e. ESI, EDI, EBP) is selected which the assembler will
> then reject.
>
> With this, Clang can finally build an i386 defconfig.
>
> Reported-by: Arnd Bergmann <a...@arndb.de>
> Reported-by: David Woodhouse <dw...@infradead.org>
> Reported-by: Dmitry Golovin <d...@golovin.in>
> Reported-by: Linus Torvalds <torva...@linux-foundation.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-dw...@infradead.org/
> Link:
> https://lore.kernel.org/lkml/cak8p3a1ebawdbaezirfdsghvjmtwjunt2hgg8z+vpxenhwe...@mail.gmail.com/
> Signed-off-by: Nick Desaulniers <ndesaulni...@google.com>
> ---
> Note: this is a resend of:
> https://lore.kernel.org/lkml/20180209161833.4605-1-dw...@infradead.org/
> rebased on today's Linux next, and with the additional change to
> uaccess.h.
>
> I'm happy to resend with authorship and reported by tags changed to
> suggested by's or whatever, just let me know.
>
> Part of the commit message is stolen from David, and part from Linus.
> Shall I resend with David's authorship and
> [Nick: reworded]
> ???
>
> I don't really care, I just don't really want to carry this out of tree
> for our CI, which is green for i386 otherwise.
>
>
> arch/x86/include/asm/percpu.h | 12 ++++++++----
> arch/x86/include/asm/uaccess.h | 4 +++-
> 2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
> index 2278797c769d..826d086f71c9 100644
> --- a/arch/x86/include/asm/percpu.h
> +++ b/arch/x86/include/asm/percpu.h
> @@ -99,7 +99,7 @@ do { \
> case 1: \
> asm qual (op "b %1,"__percpu_arg(0) \
> : "+m" (var) \
> - : "qi" ((pto_T__)(val))); \
> + : "qi" ((unsigned char)(unsigned long)(val))); \
> break; \
> case 2: \
> asm qual (op "w %1,"__percpu_arg(0) \
> @@ -144,7 +144,7 @@ do {
> \
> else \
> asm qual ("addb %1, "__percpu_arg(0) \
> : "+m" (var) \
> - : "qi" ((pao_T__)(val))); \
> + : "qi" ((unsigned char)(unsigned long)(val))); \
> break; \
> case 2: \
> if (pao_ID__ == 1) \
> @@ -182,12 +182,14 @@ do {
> \
>
> #define percpu_from_op(qual, op, var) \
> ({ \
> + unsigned char pfo_u8__; \
> typeof(var) pfo_ret__; \
> switch (sizeof(var)) { \
> case 1: \
> asm qual (op "b "__percpu_arg(1)",%0" \
> - : "=q" (pfo_ret__) \
> + : "=q" (pfo_u8__) \
> : "m" (var)); \
> + pfo_ret__ = (typeof(var))(unsigned long)pfo_u8__; \
> break; \
> case 2: \
> asm qual (op "w "__percpu_arg(1)",%0" \
> @@ -211,12 +213,14 @@ do {
> \
>
> #define percpu_stable_op(op, var) \
> ({ \
> + unsigned char pfo_u8__; \
> typeof(var) pfo_ret__; \
> switch (sizeof(var)) { \
> case 1: \
> asm(op "b "__percpu_arg(P1)",%0" \
> - : "=q" (pfo_ret__) \
> + : "=q" (pfo_u8__) \
> : "p" (&(var))); \
> + pfo_ret__ = (typeof(var))(unsigned long)pfo_u8__; \
> break; \
> case 2: \
> asm(op "w "__percpu_arg(P1)",%0" \
> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> index d8f283b9a569..cf8483cd80e1 100644
> --- a/arch/x86/include/asm/uaccess.h
> +++ b/arch/x86/include/asm/uaccess.h
> @@ -314,11 +314,13 @@ 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"); \
> --
> 2.26.2.526.g744177e7f7-goog
>
--
Thanks,
~Nick Desaulniers
