public inbox for [email protected]
 help / color / mirror / Atom feed
From: Linus Torvalds <[email protected]>
To: Nick Desaulniers <[email protected]>
Cc: Kees Cook <[email protected]>,
	Sudip Mukherjee <[email protected]>,
	Nathan Chancellor <[email protected]>, Tom Rix <[email protected]>,
	Marco Elver <[email protected]>,
	Andrew Morton <[email protected]>,
	Josh Poimboeuf <[email protected]>,
	"Peter Zijlstra (Intel)" <[email protected]>,
	Linux Kernel Mailing List <[email protected]>,
	clang-built-linux <[email protected]>
Subject: Re: [PATCH] ubsan: disable UBSAN_DIV_ZERO for clang
Date: Thu, 14 Jul 2022 16:15:35 -0700	[thread overview]
Message-ID: <CAHk-=wgf=mrK7KzNh23dGtegSfJZx=423dCwF0hVTPOjfQ7jtQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>

On Thu, Jul 14, 2022 at 1:56 PM Nick Desaulniers
<[email protected]> wrote:
>
> Building with UBSAN_DIV_ZERO with clang produces numerous fallthrough
> warnings from objtool.

Ok, with this applied, things are better.

There are still the "__ubsan_handle_load_invalid_value() with UACCESS
enabled" messages, but those are misfeatures of the kvm cmpxchg
implementation.

I'm not entirely sure why the clang build warns but gcc doesn't, but I
*think* it's because clang is just being silly. It *looks* like it
checks that a "bool" has a value range of 0/1, and will complain if
not.

And the reason I say that's silly is that if I read it correctly, then
that value has literally been generated by clang itself, using "setz"
instruction.

It's the __try_cmpxchg_user_asm() macro, and with clang-14 I have it's
that CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT case, and the C code uses
inline asm and does

        asm_volatile_goto("\n"                                          \
                     "1: " LOCK_PREFIX "cmpxchg"itype" %[new], %[ptr]\n"\
                     _ASM_EXTABLE_UA(1b, %l[label])                     \
                     : CC_OUT(z) (success),                             \

where that CC_OUT() in this case turns into

   # define CC_OUT(c) "=@cc" #c

and clang generates this code for it:

   7d01e:       f0 48 0f b1 4d 00       lock cmpxchg %rcx,0x0(%rbp)
   7d024:       49 89 c5                mov    %rax,%r13
   7d027:       0f 94 c0                sete   %al
   7d02a:       41 88 c6                mov    %al,%r14b
   7d02d:       bf 02 00 00 00          mov    $0x2,%edi
   7d032:       44 89 f6                mov    %r14d,%esi
   7d035:       e8 00 00 00 00          call    __sanitizer_cov_trace_const_cmp1
   7d03a:       41 80 fe 01             cmp    $0x1,%r14b
   7d03e:       0f 87 af 01 00 00       ja     7d1f3
<emulator_cmpxchg_emulated+0x6b3>

where that last "ja     7d1f3" is the branch to the code that then
calls __ubsan_handle_load_invalid_value.

But look at that code: it's literally

    sete   %al
    mov    %al,%r14b
    cmp    $0x1,%r14b

where clang has generated that "sete itself, and then it verifies that
the result is "<= 1".

IOW, clang seems to be literally just checking that the "sete"
instruction works right.

That's silly.

Maybe I'm misreading this, but I think the reason the clang build
complains, but the gcc build does not, is simply because gcc isn't
doing crazy checks of how the CPU works.

Some mis-feature of the "asm with flag output" code, where clang
doesn't understand that it generated that code itself, and the "setcc"
instruction always returns 0/1?

The old issue with "memcpy/memset() leaves .noinstr.text section"
because clang has generated out-of-line functions for trivial copies
also remains, but whatever.

             Linus


      parent reply	other threads:[~2022-07-14 23:16 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-14 20:56 [PATCH] ubsan: disable UBSAN_DIV_ZERO for clang Nick Desaulniers
2022-07-14 21:24 ` Linus Torvalds
2022-07-14 21:38   ` Nick Desaulniers
2022-07-14 21:48     ` Linus Torvalds
2022-07-14 21:24 ` Nathan Chancellor
2022-07-14 23:15 ` Linus Torvalds [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=wgf=mrK7KzNh23dGtegSfJZx=423dCwF0hVTPOjfQ7jtQ@mail.gmail.com' \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox