Re: perf test LLVM & clang 6 failing
From: Matthias Kaehlcke
Date: Mon Nov 27 2017 - 16:45:31 EST
El Mon, Nov 27, 2017 at 01:57:56PM -0600 Josh Poimboeuf ha dit:
> On Mon, Nov 27, 2017 at 04:34:25PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Nov 27, 2017 at 11:11:56AM -0800, Yonghong Song escreveu:
> > > On 11/27/17 9:04 AM, Arnaldo Carvalho de Melo wrote:
> > > > Em Fri, Nov 24, 2017 at 04:16:52PM +0100, Daniel Borkmann escreveu:
> > > > > [ +Yonghong ]
> > > >
> > > > + Josh
> > > > > On 11/24/2017 03:47 PM, Arnaldo Carvalho de Melo wrote:
> > > > > > FYI, just noticed, recently updated clang to version 6, from its
> > > > > > upstream git repo.
> > > > >
> > > > > Do you recall what was your LLVM version prior to this where it was
> > > > > working fine? (Wild guess from below would be the BPF inline asm
> > > > > support that was added recently to LLVM (2865ab6996) vs asm() used
> > > > > in headers included in the stdin header causing trouble due to arch
> > > > > mixup?)
> > > >
> > > > So, if I go to the cset just before:
> > > >
> > > > commit f5caf621ee357279e759c0911daf6d55c7d36f03
> > > > Author: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> > > > Date: Wed Sep 20 16:24:33 2017 -0500
> > > >
> > > > x86/asm: Fix inline asm call constraints for Clang
> > > > ---
> > > >
> > > > 'perf test LLVM' works again:
> > > >
> > > > [root@jouet ~]# perf test LLVM
> > > > 37: LLVM search and compile :
> > > > 37.1: Basic BPF llvm compile : Ok
> > > > 37.2: kbuild searching : Ok
> > > > 37.3: Compile source for BPF prologue generation : Ok
> > > > 37.4: Compile source for BPF relocation : Ok
> > > > [root@jouet ~]#
> > > >
> > > > I.e. 'perf test LLVM' built from what is in my acme/perf/urgent branch,
> > > > targetted to v4.15, uses kernel headers and if I go to just before
> > > > f5caf621ee, it works again, both with clang from fedora26 (4.0.1) and
> > > > with 6.0, built from sources.
> > >
> > > This patch introduced a module level inline assembly.
> > >
> > > ...
> > > --- a/arch/x86/include/asm/asm.h
> > > +++ b/arch/x86/include/asm/asm.h
> > > @@ -132,4 +132,15 @@
> > > /* For C file, we already have NOKPROBE_SYMBOL macro */
> > > #endif
> > >
> > > +#ifndef __ASSEMBLY__
> > > +/*
> > > + * This output constraint should be used for any inline asm which has a
> > > "call"
> > > + * instruction. Otherwise the asm may be inserted before the frame pointer
> > > + * gets set up by the containing function. If you forget to do this,
> > > objtool
> > > + * may print a "call without frame pointer save/setup" warning.
> > > + */
> > > +register unsigned int __asm_call_sp asm("esp");
> > > +#define ASM_CALL_CONSTRAINT "+r" (__asm_call_sp)
> > > +#endif
> > > ...
> > >
> > > This will cause "clang ... -target bpf ..." failure since 4.0 does not have
> > > bpf inline asm support and 6.0 does not recognize the register 'esp'.
> >
> > Ok, explains the problem then, Josh, ideas on how to proceed here?
>
> The original change to add the global inline asm:
>
> 5caf621ee35 ("x86/asm: Fix inline asm call constraints for Clang")
>
> was done to support clang in the first place. Before that change, a
> clang-built kernel didn't even boot. So I'm a bit perplexed by the fact
> that this change would be causing clang problems, since it was done to
> fix clang in the first place.
>
> Adding Andrey and Matthias, maybe they can help clarify things.
Indeed the change was needed to boot on x86.
I know next to nothing about BPF, if I understand correctly the error
is generated when compiling for the BPF "architecture" not for x86. In
this process x86 assembly headers are included, one of which contains
the declaration of the register variable, in an register that exists
on x86, but not BPS.
I guess the first questions is whether the x86 asm headers should/need
to be included when compiling for BPF. If this needed/can not be
easily avoided one option could be to put the declaration within an
ifdef __x86_64__ block.