Messages in this thread | | | Date | Thu, 16 Jun 2016 11:28:10 +0100 | From | Mark Rutland <> | Subject | Re: [PATCHv2] kcov: reject open when kernel not instrumented |
| |
On Thu, Jun 16, 2016 at 11:09:37AM +0100, James Morse wrote: > Hi Mark, > > On 15/06/16 18:04, Mark Rutland wrote: > > If the toolchain does not support -fsanitize-coverage=trace-pc, we blat > > this option from CFLAGS_KCOV, and build the kernel without > > instrumentation, even if CONFIG_KCOV was selected. However, we still > > build the rest of the kcov infrastructure, and expose a kcov file under > > debugfs. This can be confusing, as the kernel will appear to support > > kcov, yet will never manage to sample any trace PC values. While we do > > note this fact at build time, this may be missed, and a user may not > > have access to build logs. > > > > This patch ensures that CC_HAVE_SANCOV_TRACE_PC is defined when the > > toolchain supports -fsanitize-coverage=trace-pc, and is not defined > > otherwise. When CC_HAVE_SANCOV_TRACE_PC is not defined, the kernel will > > return -ENOTSUPP if userspace attempts to open the kcov debugfs file, > > indicating that kcov functionality is unavailable. > > > diff --git a/Makefile b/Makefile > > index 0f70de6..3785a63 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -369,7 +369,7 @@ LDFLAGS_MODULE = > > CFLAGS_KERNEL = > > AFLAGS_KERNEL = > > CFLAGS_GCOV = -fprofile-arcs -ftest-coverage -fno-tree-loop-im -Wno-maybe-uninitialized > > -CFLAGS_KCOV = -fsanitize-coverage=trace-pc > > +CFLAGS_KCOV = -fsanitize-coverage=trace-pc -DCC_HAVE_SANCOV_TRACE_PC > > > > > > # Use USERINCLUDE when you must reference the UAPI directories only. > > diff --git a/kernel/kcov.c b/kernel/kcov.c > > index a02f2dd..0a0b164 100644 > > --- a/kernel/kcov.c > > +++ b/kernel/kcov.c > > @@ -3,6 +3,7 @@ > > #define DISABLE_BRANCH_PROFILING > > #include <linux/compiler.h> > > #include <linux/types.h> > > +#include <linux/errno.h> > > #include <linux/file.h> > > #include <linux/fs.h> > > #include <linux/mm.h> > > @@ -160,6 +161,14 @@ static int kcov_open(struct inode *inode, struct file *filep) > > { > > struct kcov *kcov; > > > > +#ifndef CC_HAVE_SANCOV_TRACE_PC > > I don't think this will work as kernel/kcov.c is listed in the Makefile as: > > # Don't self-instrument. > > KCOV_INSTRUMENT_kcov.o := n > > > This will cause the build machinery in scripts/Makefile.lib to not give > kernel/kcov.c the CFLAGS_KCOV contents: > > ifeq ($(CONFIG_KCOV),y) > > _c_flags += $(if $(patsubst n%,, \ > > $(KCOV_INSTRUMENT_$(basetarget).o)$(KCOV_INSTRUMENT)y), \ > > $(CFLAGS_KCOV)) > > endif > > ... so kernel/kcov.c will never see anything in CFLAGS_KCOV ...
Good spot!
Evidently I was trying to be overly clever here; my bad.
> An alternative would be to add the flag to the compiler test that generates the > 'not supported' warning, but it needs to go in another CFLAGS variable. > Something like: > > -------------------%<------------------- > diff --git a/Makefile b/Makefile > @@ -687,6 +687,8 @@ ifdef CONFIG_KCOV > $(warning Cannot use CONFIG_KCOV: \ > -fsanitize-coverage=trace-pc is not supported by compiler) > CFLAGS_KCOV = > + else > + KBUILD_CFLAGS += -DCC_HAVE_SANCOV_TRACE_PC > endif > endif > -------------------%<-------------------
I'll fold the above in for v3.
Thanks, Mark.
| |