Page MenuHomeFreeBSD

Fix compiler flags related to kernel coverage
ClosedPublic

Authored by tuexen on Feb 9 2019, 2:48 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Oct 15, 10:10 PM
Unknown Object (File)
Thu, Oct 2, 3:09 AM
Unknown Object (File)
Thu, Sep 25, 12:07 AM
Unknown Object (File)
Sun, Sep 21, 8:19 PM
Unknown Object (File)
Sun, Sep 21, 12:50 PM
Unknown Object (File)
Sat, Sep 20, 11:12 PM
Unknown Object (File)
Thu, Sep 18, 1:30 PM
Unknown Object (File)
Thu, Sep 18, 9:15 AM
Subscribers

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
Lint Not Applicable
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.