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. Will