Port the NetBSD KCSAN runtime to FreeBSD
ClosedPublic
Actions

Authored by andrew on Nov 11 2019, 2:30 PM.

Details

Reviewers

emaste
manu

Commits

rS354942: Port the NetBSD KCSAN runtime to FreeBSD.

Summary

This imports the NetBSD KCSan runtime into FreeBSD. KCSAN is the Kernel
Concurrency Sanitizer. It uses the thread sanitizer hooks to check
kernel memory accesses are not racy.

Every 1024 memory accesses a cell is created and the runtime performs a
short wait. If any other memory accesses touch the same memory in a
non-atomic way the runtime warns about a data race.

Test Plan

Booted on amd64 and arm64. Other architectures will need minimal work
in machine/atomic.h and machine/bus.h to port to them.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 27635
Build 25842: arc lint + arc unit

Event Timeline

andrew created this revision.Nov 11 2019, 2:30 PM

Herald added a reviewer: manu. · View Herald TranscriptNov 11 2019, 2:30 PM

Harbormaster completed remote builds in B27445: Diff 64175.Nov 11 2019, 2:30 PM

There is a GitHub repo with this change in https://github.com/freebsd-sanitizers/freebsd/tree/kcsan.

sys/amd64/conf/GENERIC
109	I won't enable this by default, but will leave it on for now for testing
sys/arm64/conf/GENERIC
28	This is disabled to allow testing on a ThunderX in Sentex.

emaste added inline comments.Nov 11 2019, 2:46 PM

contrib/llvm/tools/clang/lib/Driver/ToolChains/FreeBSD.cpp
424 ↗	(On Diff #64175)	Can this be upstreamed?
sys/arm64/conf/GENERIC
28	NUMA doesn't work on ThunderX?

lwhsu added a subscriber: lwhsu.Nov 11 2019, 3:08 PM

• hselasky added a subscriber: • hselasky.Nov 11 2019, 3:46 PM

• hselasky added inline comments.

sys/amd64/include/csan.h
60	If the delay is small this is fine, else look at pause() how to properly use DELAY().

andrew added inline comments.Nov 11 2019, 4:37 PM

sys/amd64/include/csan.h
60	It's 10us. It needs to be long enough for the other threads to perform memory accesses on the same location without slowing the execution too much.
sys/arm64/conf/GENERIC
28	Not when booting with fdt.

• hselasky added inline comments.Nov 11 2019, 7:31 PM

sys/amd64/include/csan.h
60	Why is the delay argument 64-bit. Maybe make it just 16-bits ?

andrew added inline comments.Nov 11 2019, 8:51 PM

sys/amd64/include/csan.h
60	I haven't changed that code from NetBSD.

Update to version I expect to commit

Harbormaster completed remote builds in B27632: Diff 64564.Nov 19 2019, 8:54 AM

• hselasky added inline comments.Nov 19 2019, 9:13 AM

sys/arm64/include/bus.h
328	Should this change be committed separately?
sys/sys/systm.h
375	How about: copystr(...) kcsan_copystr(__VA_ARGS__) Or you can skip the ()'s around the arguments, because they are already separated by comma.