Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <YjxTt3pFIcV3lt8I@zn.tnic>
In-Reply-To: <YjxTt3pFIcV3lt8I@zn.tnic>
From:   Nick Desaulniers <ndesaulniers@google.com>
Date:   Thu, 24 Mar 2022 11:43:46 -0700
Message-ID: <CAKwvOdkw0Bbm+=ZyViXQhBE1L6uSbvkstHJuHpQ21tzJRftgAw@mail.gmail.com>
Subject: Re: clang memcpy calls
To:     Borislav Petkov <bp@alien8.de>
Cc:     Nathan Chancellor <nathan@kernel.org>, x86-ml <x86@kernel.org>,
        lkml <linux-kernel@vger.kernel.org>, llvm@lists.linux.dev,
        Mark Rutland <mark.rutland@arm.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Josh Poimboeuf <jpoimboe@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Mar 24, 2022 at 4:19 AM Borislav Petkov <bp@alien8.de> wrote:
>
> Hi folks,
>
> so I've been looking at a recent objtool noinstr warning from clang
> builds:
>
> vmlinux.o: warning: objtool: sync_regs()+0x20: call to memcpy() leaves .noinstr.text section
>
> The issue is that clang generates a memcpy() call when a struct copy
> happens:
>
>         if (regs != eregs)
>                 *regs = *eregs;

Specifically, this is copying one struct pt_regs to another. It looks
like the sizeof struct pt_regs is just large enough to have clang emit
the libcall.
https://godbolt.org/z/scx6aa8jq
Otherwise clang will also use rep; movsq; when -mno-sse -O2 is set and
the structs are below ARBITRARY_THRESHOLD.  Should ARBITRARY_THRESHOLD
be raised so that we continue to inline the memcpy? *shrug*

Though, looking at the compiled memcpy (`llvm-objdump -D
--disassemble-symbols=memcpy vmlinux`), maybe we *should* try harder.
Filed
https://github.com/llvm/llvm-project/issues/54535.

>
> see below for asm output.
>
> While gcc does simply generate an actual "rep; movsq".
>
> So, how hard would it be to make clang do that too pls?

As Mark said in the sibling reply; I don't know of general ways to
inhibit libcall optimizations on the level you're looking for, short
of heavy handy methods of disabling optimizations entirely.  There's
games that can be played with -fno-builtin-*, but they're not super
portable, and I think there's a handful of *blessed* functions that
must exist in any env, freestanding or not: memcpy, memmove, memset,
and memcmp for which you cannot yet express "these do not exist."

>
> Oh, and another thing while we're comparing asm: I'd love for clang's
> -fverbose-asm to issue interleaved C source lines too, like gcc does.
>
> That's it - no pink pony - just "normal" wishes. :-)

Looks like someone had started work on this in 2017, but it stalled out:
https://github.com/llvm/llvm-project/issues/17839#issuecomment-980923994
Linus has asked for this in the past, too. So it's something we'll add
to the TODO list to revisit.  I highly suspect that clang has
discarded the AST by the time LLVM gets to asm emission, but I could
be wrong.  https://github.com/llvm/llvm-project/issues/17839#issuecomment-1077933948

>
> GCC:
> ====
>
> sync_regs:
> .LASANPC4246:
> # arch/x86/kernel/traps.c:770: {
>         movq    %rdi, %rsi      # tmp91, eregs
> # arch/x86/kernel/traps.c:771:  struct pt_regs *regs = (struct pt_regs *)this_cpu_read(cpu_current_top_of_stack) - 1;
> #APP
> # 771 "arch/x86/kernel/traps.c" 1
>         movq %gs:cpu_current_top_of_stack(%rip), %rax   # cpu_current_top_of_stack, pfo_val__
> # 0 "" 2
> # arch/x86/kernel/traps.c:771:  struct pt_regs *regs = (struct pt_regs *)this_cpu_read(cpu_current_top_of_stack) - 1;
> #NO_APP
>         subq    $168, %rax      #, <retval>
> # arch/x86/kernel/traps.c:772:  if (regs != eregs)
>         cmpq    %rdi, %rax      # eregs, <retval>
>         je      .L387   #,
> # arch/x86/kernel/traps.c:773:          *regs = *eregs;
>         movl    $21, %ecx       #, tmp89
>         movq    %rax, %rdi      # <retval>, <retval>
>         rep movsq
> .L387:
> # arch/x86/kernel/traps.c:775: }
>         ret
>
> CLANG:
> ======
>
>         .section        .noinstr.text,"ax",@progbits
>         .globl  sync_regs                       # -- Begin function sync_regs
>         .p2align        6, 0x90
>         .type   sync_regs,@function
> sync_regs:                              # @sync_regs
> # %bb.0:                                # %entry
>         pushq   %rbx
>         #APP
>         movq    %gs:cpu_current_top_of_stack(%rip), %rbx
>         #NO_APP
>         addq    $-168, %rbx
>         cmpq    %rdi, %rbx
>         je      .LBB19_2
> # %bb.1:                                # %if.then
>         movq    %rdi, %rsi
>         movl    $168, %edx
>         movq    %rbx, %rdi
>         callq   memcpy@PLT
> .LBB19_2:                               # %if.end
>         movq    %rbx, %rax
>         popq    %rbx
>         retq
>
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette


-- 
Thanks,
~Nick Desaulniers