Page MenuHomeFreeBSD

Fix compiler flags related to kernel coverage
ClosedPublic

Authored by tuexen on Feb 9 2019, 2:48 PM.

Details

Summary

r343746 ensured that the kernel compiles with gcc, but assumes that -fno-sanitize=all implies -fno-sanitize-coverage=trace-pc,trace-cmp, which is not true on clang 7 and gcc 8.2. This patch uses explicit flags and fixes PR 235611.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

I tested this patch on a kernel build with clang in base and with the amd64-xtoolchain-gcc cross toolchain.

sys/conf/kern.mk
28 ↗(On Diff #53729)

hyphens in the variable name is unusual

Use underscores instead of hyphens as suggested by Ed.

sys/conf/kern.mk
28 ↗(On Diff #53729)

Fixed. I should have figured out that by myself... Thanks for the comment!

sys/conf/files
3811 ↗(On Diff #53743)

This could be written ..."${NORMAL_C:N-fsanitize*}

sys/conf/kern.mk
28 ↗(On Diff #53729)

I'm a bit concerned we need to keep these flags in sync in multiple places. I think it would be safer to just filter out the flags we don't want.

sys/conf/files
3811 ↗(On Diff #53743)

Just using compile-with "${NORMAL_C:N-fsanitize*}" does not work. No -fno-sanitize-coverage... will be used during make. Do I need to implement something to get this pattern supported?

sys/conf/files
3811 ↗(On Diff #53743)

It should remove any argument from the compile command line that matches the glob. This will therefore remove the -fsanitize-coverage=... flag as that will match. It will also remove any other future sanitizers, however I think this will be needed to remove any risk of them recursing into each other.

tuexen added inline comments.
sys/conf/files
3811 ↗(On Diff #53743)

It actually does. I forgot the config step when building the kernel (the old way)...

This revision is now accepted and ready to land.Feb 11 2019, 3:06 PM
This revision was automatically updated to reflect the committed changes.
tuexen marked an inline comment as done.