Re: Linux Plumbers Conf 2021

[Marco Elver <elver@google.com>]

+Cc Martin, who helped us last time with no_sanitize issues in GCC.

Martin, as far as we're aware there's no
__attribute__((no_sanitize_coverage)) for GCC -- is it possible to
implement this? Some context below. Thank you.

On Wed, 19 May 2021 at 13:57, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, May 19, 2021 at 12:19:38PM +0200, Marco Elver wrote:
> > On Wed, 19 May 2021 at 11:32, Mark Rutland <mark.rutland@arm.com> wrote:
> > > On Tue, May 18, 2021 at 10:03:06PM +0200, Peter Zijlstra wrote:
> > > > On Tue, May 18, 2021 at 09:57:26AM -0700, Nick Desaulniers wrote:
> > > > > On Tue, May 18, 2021 at 6:23 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > > >
> > > > > >  - validates noinstr annotation; *HOWEVER* we rely on objtool to NOP
> > > > > >    all __sanitizer_cov_* calls in .noinstr/.entry text sections because
> > > > > >    __no_sanitize_cov is 'broken' in all known compilers.
> > > > >
> > > > > ^ Do you have more information on this?
> > > >
> > > > Random link in the middle of a relevant thread:
> > > >
> > > >   https://lkml.kernel.org/r/CANpmjNPNa2f=kAF6c199oYVJ0iSyirQRGxeOBLxa9PmakSXRbA@mail.gmail.com
> > > >
> > > > Notable quote:
> > > >
> > > > "It's everywhere. We cannot mark noinstr functions to not be
> > > > instrumented by KCOV/-fsanitize-coverage, because no compiler provides
> > > > an attribute to do so. GCC doesn't have
> > > > __attribute__((no_sanitize_coverage)) and Clang doesn't have
> > > > __attribute__((no_sanitize("coverage")), and therefore we can't have
> > > > __no_sanitize_coverage."
[...]
> > > That *really* needs an attribute, and going forward we shouldn't accept
> > > new compiler instrumentation that we can't opt-out of like this.
> >
> > Strangely enough, in Clang, there's -fsanitize-coverage-blocklist, but
> > that's pretty useless for the kernel. Maintaining a list of functions
> > doesn't scale, and we just need an attribute. I'll ping a few of the
> > people who know Clang's fsanitize-coverage a bit better.
>
> Great, thanks!

I did the simplest possible thing, so let's see if we can land this:
https://reviews.llvm.org/D102772

> I think that kernel-side we should have a policy going forward that for
> new instrumentation it must be possible to opt-out like this (and when
> support is added, `noinstr` gets upsated to opt-out). Correspondingly on
> the compiler-side, it'd be good if that was seen as a general
> requirement for new instrumentation.

No argument with that!

Thanks,
-- Marco