Re: [PATCH -tip v3 1/2] kcov: Make runtime functions noinstr-compatible

Marco Elver
2020-06-17T15:20:09Z

On Wed, Jun 17, 2020 at 04:49PM +0200, Peter Zijlstra wrote:
> On Wed, Jun 17, 2020 at 04:32:08PM +0200, Marco Elver wrote:
> > On Mon, Jun 15, 2020 at 05:20PM +0200, Peter Zijlstra wrote:
> > > On Mon, Jun 15, 2020 at 05:03:27PM +0200, Peter Zijlstra wrote:
> > >
> > > > Yes, I think so. x86_64 needs lib/memcpy_64.S in .noinstr.text then. For
> > > > i386 it's an __always_inline inline-asm thing.
> > >
> > > Bah, I tried writing it without memcpy, but clang inserts memcpy anyway
> > > :/
> >
> > Hmm, __builtin_memcpy() won't help either.
> >
> > Turns out, Clang 11 got __builtin_memcpy_inline(): https://reviews.llvm.org/D73543
> >
> > The below works, no more crash on either KASAN or KCSAN with Clang. We
> > can test if we have it with __has_feature(__builtin_memcpy_inline)
> > (although that's currently not working as expected, trying to fix :-/).
> >
> > Would a memcpy_inline() be generally useful? It's not just Clang but
> > also GCC that isn't entirely upfront about which memcpy is inlined and
> > which isn't. If the compiler has __builtin_memcpy_inline(), we can use
> > it, otherwise the arch likely has to provide the implementation.
> >
> > Thoughts?
>
> I had the below, except of course that yields another objtool
> complaint, and I was still looking at that.
>
> Does GCC (8, as per the new KASAN thing) have that
> __builtin_memcpy_inline() ?

No, sadly it doesn't. Only Clang 11. :-/

But using a call to __memcpy() somehow breaks with Clang+KCSAN. Yet,
it's not the memcpy that BUGs, but once again check_preemption_disabled
(which is noinstr!). Just adding calls anywhere here seems to results in
unpredictable behaviour. Are we running out of stack space?

Thanks,
-- Marco