On Thu, 14 May 2020 at 13:05, Will Deacon <w...@kernel.org> wrote: > > Hi Marco, > > On Thu, May 14, 2020 at 09:31:49AM +0200, Marco Elver wrote: > > Ouch. With the __{READ,WRITE}_ONCE requirement, we're going to need > > Clang 11 though. > > > > Because without the data_race() around __*_ONCE, > > arch_atomic_{read,set} will be broken for KCSAN, but we can't have > > data_race() because it would still add > > kcsan_{enable,disable}_current() calls to __no_sanitize functions (if > > compilation unit is instrumented). We can't make arch_atomic functions > > __no_sanitize_or_inline, because even in code that we want to > > sanitize, they should remain __always_inline (so they work properly in > > __no_sanitize functions). Therefore, Clang 11 with support for > > distinguishing volatiles will be the compiler that will satisfy all > > the constraints. > > > > If this is what we want, let me prepare a series on top of > > -tip/locking/kcsan with all the things I think we need. > > Stepping back a second, the locking/kcsan branch is at least functional at > the moment by virtue of KCSAN_SANITIZE := n being used liberally in > arch/x86/. However, I still think we want to do better than that because (a) > it would be good to get more x86 coverage and (b) enabling this for arm64, > where objtool is not yet available, will be fragile if we have to whitelist > object files. There's also a fair bit of arm64 low-level code spread around > drivers/, so it feels like we'd end up with a really bad case of whack-a-mole. > > Talking off-list, Clang >= 7 is pretty reasonable wrt inlining decisions > and the behaviour for __always_inline is: > > * An __always_inline function inlined into a __no_sanitize function is > not instrumented > * An __always_inline function inlined into an instrumented function is > instrumented > * You can't mark a function as both __always_inline __no_sanitize, because > __no_sanitize functions are never inlined > > GCC, on the other hand, may still inline __no_sanitize functions and then > subsequently instrument them. > > So if were willing to make KCSAN depend on Clang >= 7, then we could: > > - Remove the data_race() from __{READ,WRITE}_ONCE() > - Wrap arch_atomic*() in data_race() when called from the instrumented > atomic wrappers > > At which point, I *think* everything works as expected. READ_ONCE_NOCHECK() > won't generate any surprises, and Peter can happily use arch_atomic() > from non-instrumented code. > > Thoughts? I don't see the need to support buggy compilers when enabling > a new debug feature.
This is also a reply to https://lkml.kernel.org/r/20200514122038.gh3...@hirez.programming.kicks-ass.net -- the problem with __READ_ONCE would be solved with what Will proposed above. Let me try to spell out the requirements I see so far (this is for KCSAN only though -- other sanitizers might be similar): 1. __no_kcsan functions should not call anything, not even kcsan_{enable,disable}_current(), when using __{READ,WRITE}_ONCE. [Requires leaving data_race() off of these.] 2. __always_inline functions inlined into __no_sanitize function is not instrumented. [Has always been satisfied by GCC and Clang.] 3. __always_inline functions inlined into instrumented function is instrumented. [Has always been satisfied by GCC and Clang.] 4. __no_kcsan functions should never be spuriously inlined into instrumented functions, causing the accesses of the __no_kcsan function to be instrumented. [Satisfied by Clang >= 7. All GCC versions are broken.] 5. we should not break atomic_{read,set} for KCSAN. [Because of #1, we'd need to add data_race() around the arch-calls in atomic_{read,set}; or rely on Clang 11's -tsan-distinguish-volatile support (GCC 11 might get this as well).] 6. never emit __tsan_func_{entry,exit}. [Clang supports disabling this, GCC doesn't.] 7. kernel is supported by compiler. [Clang >= 9 seems to build -tip for me, anything below complains about lack of asm goto. GCC trivial.] So, because of #4 & #6 & #7 we're down to Clang >= 9. Because of #5 we'll have to make a choice between Clang >= 9 or Clang >= 11 (released in ~June). In an ideal world we might even fix GCC in future. That's not even considering the problems around UBSan and KASAN. But maybe one step at a time? Any preferences? Thanks, -- Marco