On Wed, Dec 01, 2021 at 11:53AM +0000, Mark Rutland wrote:
[...]

* In the past clang did not have an attribute to suppress tsan instrumenation
  and would instrument noinstr regions. I'm not sure when clang gained the
  relevant attribute to supress this, but we will need to depend on this
  existing, either based on the clang version or with a test for the attribute.

  (If we're lucky, clang 12.0.0 is sufficient, and we solve BTI and this in one
  go).

  I *think* GCC always had an attribute, but I'm not certain.

  Marco, is there an existing dependency somewhere for this to work on x86? I
  thought there was an objtool pass to NOP this out, but I couldn't find it in
  mainline. If x86 is implicitly depending on a sufficiently recent version of
  clang, we add something to the common KCSAN Kconfig for ARCH_WANTS_NO_INSTR?

Apologies for the confusion w.r.t. attributes and which sanitizers on
which compilers respect which attributes. I think you may be confusing
things with KCOV (see 540540d06e9d9). I think the various 'select
ARCH_HAS_KCOV' may need adjusting, that is true.

But KCOV != KCSAN, and for KCSAN the story is different. Since the first
version of KCSAN in 5.8, we've had a working __no_kcsan (aka
__no_sanitize_thread) with all versions of Clang and GCC that support
KCSAN (see HAVE_KCSAN_COMPILER).

The recent discussion was for CONFIG_KCSAN_WEAK_MEMORY [1], because
Clang's no_sanitize("thread") would still instrument builtin atomics and
__tsan_func_{entry,exit}, which only that mode would start inserting
instrumentation for. That's not in mainline yet.

[1] https://lkml.kernel.org/r/20211130114433.2580590-1-elver@google.com

It is true that v1 and v2 of that series would have caused issues on
arm64, but after our discussion last week and tried a little harder and
v3 does the right thing for all architectures now and __no_kcsan will
disable all instrumentation (even for barriers).

So the attribute and noinstr story should not need anything else, and
the new KCSAN_WEAK_MEMORY has all right Kconfig dependencies in place
when it lands in mainline.

* There are some latent issues with some code (e.g. alternatives, patching, insn)
  code being instrumentable even though this is unsound, and depending on
  compiler choices this can happen to be fine or can result in boot-time
  failures (I saw lockups when I started trying to add KCSAN for arm64).

  While this isn't just a KCSAN problem, fixing that requires some fairly
  significant rework to a bunch of code, and until that's done we're on very
  shaky ground. So I'd like to make KCSAN depend on EXPERT for now.

I take it you mean arm64 should do 'select HAVE_ARCH_KCSAN if EXPERT'. I
certainly don't mind, but usually folks interested in running debug
tools won't be stopped by a dependency on EXPERT. You could do 'select
HAVE_ARCH_KCSAN if BROKEN' which is more likely to prevent usage while
things are still likely to be broken on arm64.

Thanks,
-- Marco

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel