[PATCH v2 00/11] Coverage deduplication for KCOV

[PATCH v2 00/11] Coverage deduplication for KCOV

[Alexander Potapenko]

As mentioned by Joey Jiao in [1], the current kcov implementation may
suffer from certain syscalls overflowing the userspace coverage buffer.

According to our measurements, among 24 syzkaller instances running
upstream Linux, 5 had a coverage overflow in at least 50% of executed
programs. The median percentage of programs with overflows across those 24
instances was 8.8%.

One way to mitigate this problem is to increase the size of the kcov buffer
in the userspace application using kcov. But right now syzkaller already
uses 4Mb per each of up to 32 threads to store the coverage, and increasing
it further would result in reduction in the number of executors on a single
machine.  Replaying the same program with an increased buffer size in the
case of overflow would also lead to fewer executions being possible.

When executing a single system call, excessive coverage usually stems from
loops, which write the same PCs into the output buffer repeatedly. Although
collecting precise traces may give us some insights into e.g. the number of
loop iterations and the branches being taken, the fuzzing engine does not
take advantage of these signals, and recording only unique PCs should be
just as practical.

In [1] Joey Jiao suggested using a hash table to deduplicate the coverage
signal on the kernel side. While being universally applicable to all types
of data collected by kcov, this approach adds another layer of complexity,
requiring dynamically growing the map. Another problem is potential hash
collisions, which can as well lead to lost coverage. Hash maps are also
unavoidably sparse, which potentially requires more memory.

The approach proposed in this patch series is to assign a unique (and
almost) sequential ID to each of the coverage callbacks in the kernel. Then
we carve out a fixed-sized bitmap from the userspace trace buffer, and on
every callback invocation we:

- obtain the callback_ID;
- if bitmap[callback_ID] is set, append the PC to the trace buffer;
- set bitmap[callback_ID] to true.

LLVM's -fsanitize-coverage=trace-pc-guard replaces every coverage callback
in the kernel with a call to
__sanitizer_cov_trace_pc_guard(&guard_variable) , where guard_variable is a
4-byte global that is unique for the callsite.

This allows us to lazily allocate sequential numbers just for the callbacks
that have actually been executed, using a lock-free algorithm.

This patch series implements a new config, CONFIG_KCOV_ENABLE_GUARDS, which
utilizes the mentioned LLVM flag for coverage instrumentation. In addition
to the existing coverage collection modes, it introduces
ioctl(KCOV_UNIQUE_ENABLE), which splits the existing kcov buffer into the
bitmap and the trace part for a particular fuzzing session, and collects
only unique coverage in the trace buffer.

To reset the coverage between runs, it is now necessary to set trace[0] to
0 AND clear the entire bitmap. This is still considered feasible, based on
the experimental results below.

Alternatively, users can call ioctl(KCOV_RESET_TRACE) to reset the coverage.
This makes it possible to make the coverage buffer read-only, so that it
is harder to corrupt.

The current design does not address the deduplication of KCOV_TRACE_CMP
comparisons; however, the number of kcov overflows during the hints
collection process is insignificant compared to the overflows of
KCOV_TRACE_PC.

In addition to the mentioned changes, this patch series implements
a selftest in tools/testing/selftests/kcov/kcov_test. This will help
check the variety of different coverage collection modes.

Experimental results.

We've conducted an experiment running syz-testbed [3] on 10 syzkaller
instances for 24 hours.  Out of those 10 instances, 5 were enabling the
kcov_deduplicate flag from [4], which makes use of the KCOV_UNIQUE_ENABLE
ioctl, reserving 4096 words (262144 bits) for the bitmap and leaving 520192
words for the trace collection.

Below are the average stats from the runs.

kcov_deduplicate=false:
  corpus: 52176
  coverage: 302658
  cover overflows: 225288
  comps overflows: 491
  exec total: 1417829
  max signal: 318894

kcov_deduplicate=true:
  corpus: 52581
  coverage: 304344
  cover overflows: 986
  comps overflows: 626
  exec total: 1484841
  max signal: 322455

[1] https://lore.kernel.org/linux-arm-kernel/20250114-kcov-v1-5-004294b931a2@quicinc.com/T/
[2] https://clang.llvm.org/docs/SanitizerCoverage.html
[3] https://github.com/google/syzkaller/tree/master/tools/syz-testbed
[4] https://github.com/ramosian-glider/syzkaller/tree/kcov_dedup-new

v2:
 - assorted cleanups (enum kcov_mode, docs)
 - address reviewers' comments
 - drop R_X86_64_REX_GOTPCRELX support
 - implement ioctl(KCOV_RESET_TRACE)
 - add a userspace selftest

Alexander Potapenko (11):
  x86: kcov: disable instrumentation of arch/x86/kernel/tsc.c
  kcov: apply clang-format to kcov code
  kcov: elaborate on using the shared buffer
  kcov: factor out struct kcov_state
  mm/kasan: define __asan_before_dynamic_init, __asan_after_dynamic_init
  kcov: x86: introduce CONFIG_KCOV_UNIQUE
  kcov: add trace and trace_size to struct kcov_state
  kcov: add ioctl(KCOV_UNIQUE_ENABLE)
  kcov: add ioctl(KCOV_RESET_TRACE)
  kcov: selftests: add kcov_test
  kcov: use enum kcov_mode in kcov_mode_enabled()

 Documentation/dev-tools/kcov.rst         | 124 ++++++
 MAINTAINERS                              |   3 +
 arch/x86/Kconfig                         |   1 +
 arch/x86/kernel/Makefile                 |   2 +
 arch/x86/kernel/vmlinux.lds.S            |   1 +
 include/asm-generic/vmlinux.lds.h        |  14 +-
 include/linux/kcov.h                     |  60 ++-
 include/linux/kcov_types.h               |  37 ++
 include/linux/sched.h                    |  13 +-
 include/uapi/linux/kcov.h                |   2 +
 kernel/kcov.c                            | 480 +++++++++++++++--------
 lib/Kconfig.debug                        |  24 ++
 mm/kasan/generic.c                       |  18 +
 mm/kasan/kasan.h                         |   2 +
 scripts/Makefile.kcov                    |   4 +
 scripts/module.lds.S                     |  23 ++
 tools/objtool/check.c                    |   1 +
 tools/testing/selftests/kcov/Makefile    |   6 +
 tools/testing/selftests/kcov/kcov_test.c | 364 +++++++++++++++++
 19 files changed, 981 insertions(+), 198 deletions(-)
 create mode 100644 include/linux/kcov_types.h
 create mode 100644 tools/testing/selftests/kcov/Makefile
 create mode 100644 tools/testing/selftests/kcov/kcov_test.c

-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v2 01/11] x86: kcov: disable instrumentation of arch/x86/kernel/tsc.c

[Alexander Potapenko]

sched_clock() appears to be called from interrupts, producing spurious
coverage, as reported by CONFIG_KCOV_SELFTEST:

  RIP: 0010:__sanitizer_cov_trace_pc_guard+0x66/0xe0 kernel/kcov.c:288
  ...
   fault_in_kernel_space+0x17/0x70 arch/x86/mm/fault.c:1119
   handle_page_fault arch/x86/mm/fault.c:1477
   exc_page_fault+0x56/0x110 arch/x86/mm/fault.c:1538
   asm_exc_page_fault+0x26/0x30 ./arch/x86/include/asm/idtentry.h:623
  RIP: 0010:__sanitizer_cov_trace_pc_guard+0x66/0xe0 kernel/kcov.c:288
  ...
   sched_clock+0x12/0x70 arch/x86/kernel/tsc.c:284
   __lock_pin_lock kernel/locking/lockdep.c:5628
   lock_pin_lock+0xd7/0x180 kernel/locking/lockdep.c:5959
   rq_pin_lock kernel/sched/sched.h:1761
   rq_lock kernel/sched/sched.h:1838
   __schedule+0x3a8/0x4b70 kernel/sched/core.c:6691
   preempt_schedule_irq+0xbf/0x160 kernel/sched/core.c:7090
   irqentry_exit+0x6f/0x90 kernel/entry/common.c:354
   asm_sysvec_reschedule_ipi+0x1a/0x20 ./arch/x86/include/asm/idtentry.h:707
  RIP: 0010:selftest+0x26/0x60 kernel/kcov.c:1223
  ...
   kcov_init+0x81/0xa0 kernel/kcov.c:1252
   do_one_initcall+0x2e1/0x910
   do_initcall_level+0xff/0x160 init/main.c:1319
   do_initcalls+0x4a/0xa0 init/main.c:1335
   kernel_init_freeable+0x448/0x610 init/main.c:1567
   kernel_init+0x24/0x230 init/main.c:1457
   ret_from_fork+0x60/0x90 arch/x86/kernel/process.c:153
   ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
   </TASK>

Signed-off-by: Alexander Potapenko <glider@google.com>
---
 arch/x86/kernel/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 84cfa179802c3..c08626d348c85 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -43,6 +43,8 @@ KCOV_INSTRUMENT_dumpstack_$(BITS).o			:= n
 KCOV_INSTRUMENT_unwind_orc.o				:= n
 KCOV_INSTRUMENT_unwind_frame.o				:= n
 KCOV_INSTRUMENT_unwind_guess.o				:= n
+# Avoid instrumenting code that produces spurious coverage in interrupts.
+KCOV_INSTRUMENT_tsc.o					:= n
 
 CFLAGS_head32.o := -fno-stack-protector
 CFLAGS_head64.o := -fno-stack-protector
-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v2 02/11] kcov: apply clang-format to kcov code

[Alexander Potapenko]

kcov used to obey clang-format style, but somehow diverged over time.
This patch applies clang-format to kernel/kcov.c and
include/linux/kcov.h, no functional change.

Signed-off-by: Alexander Potapenko <glider@google.com>
---
Change-Id: I49c21babad831e5d134ad8a4d4adffd1f5abc1dd

v2:
 - factor out handles_off in kcov_ioctl() for prettier formatting
---
 include/linux/kcov.h |  54 +++++++++++------
 kernel/kcov.c        | 134 ++++++++++++++++++++++---------------------
 2 files changed, 105 insertions(+), 83 deletions(-)

diff --git a/include/linux/kcov.h b/include/linux/kcov.h
index 75a2fb8b16c32..932b4face1005 100644
--- a/include/linux/kcov.h
+++ b/include/linux/kcov.h
@@ -25,20 +25,20 @@ enum kcov_mode {
 	KCOV_MODE_REMOTE = 4,
 };
 
-#define KCOV_IN_CTXSW	(1 << 30)
+#define KCOV_IN_CTXSW (1 << 30)
 
 void kcov_task_init(struct task_struct *t);
 void kcov_task_exit(struct task_struct *t);
 
-#define kcov_prepare_switch(t)			\
-do {						\
-	(t)->kcov_mode |= KCOV_IN_CTXSW;	\
-} while (0)
+#define kcov_prepare_switch(t)                   \
+	do {                                     \
+		(t)->kcov_mode |= KCOV_IN_CTXSW; \
+	} while (0)
 
-#define kcov_finish_switch(t)			\
-do {						\
-	(t)->kcov_mode &= ~KCOV_IN_CTXSW;	\
-} while (0)
+#define kcov_finish_switch(t)                     \
+	do {                                      \
+		(t)->kcov_mode &= ~KCOV_IN_CTXSW; \
+	} while (0)
 
 /* See Documentation/dev-tools/kcov.rst for usage details. */
 void kcov_remote_start(u64 handle);
@@ -119,23 +119,41 @@ void __sanitizer_cov_trace_switch(kcov_u64 val, void *cases);
 
 #else
 
-static inline void kcov_task_init(struct task_struct *t) {}
-static inline void kcov_task_exit(struct task_struct *t) {}
-static inline void kcov_prepare_switch(struct task_struct *t) {}
-static inline void kcov_finish_switch(struct task_struct *t) {}
-static inline void kcov_remote_start(u64 handle) {}
-static inline void kcov_remote_stop(void) {}
+static inline void kcov_task_init(struct task_struct *t)
+{
+}
+static inline void kcov_task_exit(struct task_struct *t)
+{
+}
+static inline void kcov_prepare_switch(struct task_struct *t)
+{
+}
+static inline void kcov_finish_switch(struct task_struct *t)
+{
+}
+static inline void kcov_remote_start(u64 handle)
+{
+}
+static inline void kcov_remote_stop(void)
+{
+}
 static inline u64 kcov_common_handle(void)
 {
 	return 0;
 }
-static inline void kcov_remote_start_common(u64 id) {}
-static inline void kcov_remote_start_usb(u64 id) {}
+static inline void kcov_remote_start_common(u64 id)
+{
+}
+static inline void kcov_remote_start_usb(u64 id)
+{
+}
 static inline unsigned long kcov_remote_start_usb_softirq(u64 id)
 {
 	return 0;
 }
-static inline void kcov_remote_stop_softirq(unsigned long flags) {}
+static inline void kcov_remote_stop_softirq(unsigned long flags)
+{
+}
 
 #endif /* CONFIG_KCOV */
 #endif /* _LINUX_KCOV_H */
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 187ba1b80bda1..0dd42b78694c9 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -4,27 +4,28 @@
 #define DISABLE_BRANCH_PROFILING
 #include <linux/atomic.h>
 #include <linux/compiler.h>
+#include <linux/debugfs.h>
 #include <linux/errno.h>
 #include <linux/export.h>
-#include <linux/types.h>
 #include <linux/file.h>
 #include <linux/fs.h>
 #include <linux/hashtable.h>
 #include <linux/init.h>
 #include <linux/jiffies.h>
+#include <linux/kcov.h>
 #include <linux/kmsan-checks.h>
+#include <linux/log2.h>
 #include <linux/mm.h>
 #include <linux/preempt.h>
 #include <linux/printk.h>
+#include <linux/refcount.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
-#include <linux/vmalloc.h>
-#include <linux/debugfs.h>
+#include <linux/types.h>
 #include <linux/uaccess.h>
-#include <linux/kcov.h>
-#include <linux/refcount.h>
-#include <linux/log2.h>
+#include <linux/vmalloc.h>
+
 #include <asm/setup.h>
 
 #define kcov_debug(fmt, ...) pr_debug("%s: " fmt, __func__, ##__VA_ARGS__)
@@ -52,36 +53,36 @@ struct kcov {
 	 *  - task with enabled coverage (we can't unwire it from another task)
 	 *  - each code section for remote coverage collection
 	 */
-	refcount_t		refcount;
+	refcount_t refcount;
 	/* The lock protects mode, size, area and t. */
-	spinlock_t		lock;
-	enum kcov_mode		mode;
+	spinlock_t lock;
+	enum kcov_mode mode;
 	/* Size of arena (in long's). */
-	unsigned int		size;
+	unsigned int size;
 	/* Coverage buffer shared with user space. */
-	void			*area;
+	void *area;
 	/* Task for which we collect coverage, or NULL. */
-	struct task_struct	*t;
+	struct task_struct *t;
 	/* Collecting coverage from remote (background) threads. */
-	bool			remote;
+	bool remote;
 	/* Size of remote area (in long's). */
-	unsigned int		remote_size;
+	unsigned int remote_size;
 	/*
 	 * Sequence is incremented each time kcov is reenabled, used by
 	 * kcov_remote_stop(), see the comment there.
 	 */
-	int			sequence;
+	int sequence;
 };
 
 struct kcov_remote_area {
-	struct list_head	list;
-	unsigned int		size;
+	struct list_head list;
+	unsigned int size;
 };
 
 struct kcov_remote {
-	u64			handle;
-	struct kcov		*kcov;
-	struct hlist_node	hnode;
+	u64 handle;
+	struct kcov *kcov;
+	struct hlist_node hnode;
 };
 
 static DEFINE_SPINLOCK(kcov_remote_lock);
@@ -89,14 +90,14 @@ static DEFINE_HASHTABLE(kcov_remote_map, 4);
 static struct list_head kcov_remote_areas = LIST_HEAD_INIT(kcov_remote_areas);
 
 struct kcov_percpu_data {
-	void			*irq_area;
-	local_lock_t		lock;
-
-	unsigned int		saved_mode;
-	unsigned int		saved_size;
-	void			*saved_area;
-	struct kcov		*saved_kcov;
-	int			saved_sequence;
+	void *irq_area;
+	local_lock_t lock;
+
+	unsigned int saved_mode;
+	unsigned int saved_size;
+	void *saved_area;
+	struct kcov *saved_kcov;
+	int saved_sequence;
 };
 
 static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {
@@ -149,7 +150,7 @@ static struct kcov_remote_area *kcov_remote_area_get(unsigned int size)
 
 /* Must be called with kcov_remote_lock locked. */
 static void kcov_remote_area_put(struct kcov_remote_area *area,
-					unsigned int size)
+				 unsigned int size)
 {
 	INIT_LIST_HEAD(&area->list);
 	area->size = size;
@@ -171,7 +172,8 @@ static __always_inline bool in_softirq_really(void)
 	return in_serving_softirq() && !in_hardirq() && !in_nmi();
 }
 
-static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_struct *t)
+static notrace bool check_kcov_mode(enum kcov_mode needed_mode,
+				    struct task_struct *t)
 {
 	unsigned int mode;
 
@@ -354,8 +356,8 @@ EXPORT_SYMBOL(__sanitizer_cov_trace_switch);
 #endif /* ifdef CONFIG_KCOV_ENABLE_COMPARISONS */
 
 static void kcov_start(struct task_struct *t, struct kcov *kcov,
-			unsigned int size, void *area, enum kcov_mode mode,
-			int sequence)
+		       unsigned int size, void *area, enum kcov_mode mode,
+		       int sequence)
 {
 	kcov_debug("t = %px, size = %u, area = %px\n", t, size, area);
 	t->kcov = kcov;
@@ -566,14 +568,14 @@ static void kcov_fault_in_area(struct kcov *kcov)
 }
 
 static inline bool kcov_check_handle(u64 handle, bool common_valid,
-				bool uncommon_valid, bool zero_valid)
+				     bool uncommon_valid, bool zero_valid)
 {
 	if (handle & ~(KCOV_SUBSYSTEM_MASK | KCOV_INSTANCE_MASK))
 		return false;
 	switch (handle & KCOV_SUBSYSTEM_MASK) {
 	case KCOV_SUBSYSTEM_COMMON:
-		return (handle & KCOV_INSTANCE_MASK) ?
-			common_valid : zero_valid;
+		return (handle & KCOV_INSTANCE_MASK) ? common_valid :
+						       zero_valid;
 	case KCOV_SUBSYSTEM_USB:
 		return uncommon_valid;
 	default:
@@ -611,7 +613,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
 		kcov_fault_in_area(kcov);
 		kcov->mode = mode;
 		kcov_start(t, kcov, kcov->size, kcov->area, kcov->mode,
-				kcov->sequence);
+			   kcov->sequence);
 		kcov->t = t;
 		/* Put either in kcov_task_exit() or in KCOV_DISABLE. */
 		kcov_get(kcov);
@@ -642,40 +644,40 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
 			return -EINVAL;
 		kcov->mode = mode;
 		t->kcov = kcov;
-	        t->kcov_mode = KCOV_MODE_REMOTE;
+		t->kcov_mode = KCOV_MODE_REMOTE;
 		kcov->t = t;
 		kcov->remote = true;
 		kcov->remote_size = remote_arg->area_size;
 		spin_lock_irqsave(&kcov_remote_lock, flags);
 		for (i = 0; i < remote_arg->num_handles; i++) {
-			if (!kcov_check_handle(remote_arg->handles[i],
-						false, true, false)) {
+			if (!kcov_check_handle(remote_arg->handles[i], false,
+					       true, false)) {
 				spin_unlock_irqrestore(&kcov_remote_lock,
-							flags);
+						       flags);
 				kcov_disable(t, kcov);
 				return -EINVAL;
 			}
 			remote = kcov_remote_add(kcov, remote_arg->handles[i]);
 			if (IS_ERR(remote)) {
 				spin_unlock_irqrestore(&kcov_remote_lock,
-							flags);
+						       flags);
 				kcov_disable(t, kcov);
 				return PTR_ERR(remote);
 			}
 		}
 		if (remote_arg->common_handle) {
-			if (!kcov_check_handle(remote_arg->common_handle,
-						true, false, false)) {
+			if (!kcov_check_handle(remote_arg->common_handle, true,
+					       false, false)) {
 				spin_unlock_irqrestore(&kcov_remote_lock,
-							flags);
+						       flags);
 				kcov_disable(t, kcov);
 				return -EINVAL;
 			}
 			remote = kcov_remote_add(kcov,
-					remote_arg->common_handle);
+						 remote_arg->common_handle);
 			if (IS_ERR(remote)) {
 				spin_unlock_irqrestore(&kcov_remote_lock,
-							flags);
+						       flags);
 				kcov_disable(t, kcov);
 				return PTR_ERR(remote);
 			}
@@ -698,6 +700,7 @@ static long kcov_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
 	unsigned int remote_num_handles;
 	unsigned long remote_arg_size;
 	unsigned long size, flags;
+	size_t handles_off;
 	void *area;
 
 	kcov = filep->private_data;
@@ -728,13 +731,14 @@ static long kcov_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
 		spin_unlock_irqrestore(&kcov->lock, flags);
 		return 0;
 	case KCOV_REMOTE_ENABLE:
-		if (get_user(remote_num_handles, (unsigned __user *)(arg +
-				offsetof(struct kcov_remote_arg, num_handles))))
+		handles_off = offsetof(struct kcov_remote_arg, num_handles);
+		if (get_user(remote_num_handles,
+			     (unsigned __user *)(arg + handles_off)))
 			return -EFAULT;
 		if (remote_num_handles > KCOV_REMOTE_MAX_HANDLES)
 			return -EINVAL;
-		remote_arg_size = struct_size(remote_arg, handles,
-					remote_num_handles);
+		remote_arg_size =
+			struct_size(remote_arg, handles, remote_num_handles);
 		remote_arg = memdup_user((void __user *)arg, remote_arg_size);
 		if (IS_ERR(remote_arg))
 			return PTR_ERR(remote_arg);
@@ -758,11 +762,11 @@ static long kcov_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
 }
 
 static const struct file_operations kcov_fops = {
-	.open		= kcov_open,
-	.unlocked_ioctl	= kcov_ioctl,
-	.compat_ioctl	= kcov_ioctl,
-	.mmap		= kcov_mmap,
-	.release        = kcov_close,
+	.open = kcov_open,
+	.unlocked_ioctl = kcov_ioctl,
+	.compat_ioctl = kcov_ioctl,
+	.mmap = kcov_mmap,
+	.release = kcov_close,
 };
 
 /*
@@ -836,8 +840,8 @@ static void kcov_remote_softirq_stop(struct task_struct *t)
 
 	if (data->saved_kcov) {
 		kcov_start(t, data->saved_kcov, data->saved_size,
-				data->saved_area, data->saved_mode,
-				data->saved_sequence);
+			   data->saved_area, data->saved_mode,
+			   data->saved_sequence);
 		data->saved_mode = 0;
 		data->saved_size = 0;
 		data->saved_area = NULL;
@@ -891,7 +895,7 @@ void kcov_remote_start(u64 handle)
 		return;
 	}
 	kcov_debug("handle = %llx, context: %s\n", handle,
-			in_task() ? "task" : "softirq");
+		   in_task() ? "task" : "softirq");
 	kcov = remote->kcov;
 	/* Put in kcov_remote_stop(). */
 	kcov_get(kcov);
@@ -931,12 +935,11 @@ void kcov_remote_start(u64 handle)
 	kcov_start(t, kcov, size, area, mode, sequence);
 
 	local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
-
 }
 EXPORT_SYMBOL(kcov_remote_start);
 
 static void kcov_move_area(enum kcov_mode mode, void *dst_area,
-				unsigned int dst_area_size, void *src_area)
+			   unsigned int dst_area_size, void *src_area)
 {
 	u64 word_size = sizeof(unsigned long);
 	u64 count_size, entry_size_log;
@@ -944,8 +947,8 @@ static void kcov_move_area(enum kcov_mode mode, void *dst_area,
 	void *dst_entries, *src_entries;
 	u64 dst_occupied, dst_free, bytes_to_move, entries_moved;
 
-	kcov_debug("%px %u <= %px %lu\n",
-		dst_area, dst_area_size, src_area, *(unsigned long *)src_area);
+	kcov_debug("%px %u <= %px %lu\n", dst_area, dst_area_size, src_area,
+		   *(unsigned long *)src_area);
 
 	switch (mode) {
 	case KCOV_MODE_TRACE_PC:
@@ -967,8 +970,8 @@ static void kcov_move_area(enum kcov_mode mode, void *dst_area,
 	}
 
 	/* As arm can't divide u64 integers use log of entry size. */
-	if (dst_len > ((dst_area_size * word_size - count_size) >>
-				entry_size_log))
+	if (dst_len >
+	    ((dst_area_size * word_size - count_size) >> entry_size_log))
 		return;
 	dst_occupied = count_size + (dst_len << entry_size_log);
 	dst_free = dst_area_size * word_size - dst_occupied;
@@ -1100,7 +1103,8 @@ static int __init kcov_init(void)
 
 	for_each_possible_cpu(cpu) {
 		void *area = vmalloc_node(CONFIG_KCOV_IRQ_AREA_SIZE *
-				sizeof(unsigned long), cpu_to_node(cpu));
+						  sizeof(unsigned long),
+					  cpu_to_node(cpu));
 		if (!area)
 			return -ENOMEM;
 		per_cpu_ptr(&kcov_percpu_data, cpu)->irq_area = area;
-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v2 03/11] kcov: elaborate on using the shared buffer

[Alexander Potapenko]

Add a paragraph about the shared buffer usage to kcov.rst.

Signed-off-by: Alexander Potapenko <glider@google.com>
---
Change-Id: Ia47ef7c3fcc74789fe57a6e1d93e29a42dbc0a97
---
 Documentation/dev-tools/kcov.rst | 55 ++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/Documentation/dev-tools/kcov.rst b/Documentation/dev-tools/kcov.rst
index 6611434e2dd24..abf3ad2e784e8 100644
--- a/Documentation/dev-tools/kcov.rst
+++ b/Documentation/dev-tools/kcov.rst
@@ -137,6 +137,61 @@ mmaps coverage buffer, and then forks child processes in a loop. The child
 processes only need to enable coverage (it gets disabled automatically when
 a thread exits).
 
+Shared buffer for coverage collection
+-------------------------------------
+KCOV employs a shared memory buffer as a central mechanism for efficient and
+direct transfer of code coverage information between the kernel and userspace
+applications.
+
+Calling ``ioctl(fd, KCOV_INIT_TRACE, size)`` initializes coverage collection for
+the current thread associated with the file descriptor ``fd``. The buffer
+allocated will hold ``size`` unsigned long values, as interpreted by the kernel.
+Notably, even in a 32-bit userspace program on a 64-bit kernel, each entry will
+occupy 64 bits.
+
+Following initialization, the actual shared memory buffer is created using::
+
+    mmap(NULL, size * sizeof(unsigned long), PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0)
+
+The size of this memory mapping, calculated as ``size * sizeof(unsigned long)``,
+must be a multiple of ``PAGE_SIZE``.
+
+This buffer is then shared between the kernel and the userspace. The first
+element of the buffer contains the number of PCs stored in it.
+Both the userspace and the kernel may write to the shared buffer, so to avoid
+race conditions each userspace thread should only update its own buffer.
+
+Normally the shared buffer is used as follows::
+
+              Userspace                                         Kernel
+    -----------------------------------------+-------------------------------------------
+    ioctl(fd, KCOV_INIT_TRACE, size)         |
+                                             |    Initialize coverage for current thread
+    mmap(..., MAP_SHARED, fd, 0)             |
+                                             |    Allocate the buffer, initialize it
+                                             |    with zeroes
+    ioctl(fd, KCOV_ENABLE, KCOV_TRACE_PC)    |
+                                             |    Enable PC collection for current thread
+                                             |    starting at buffer[1] (KCOV_ENABLE will
+                                             |    already write some coverage)
+    Atomically write 0 to buffer[0] to       |
+    reset the coverage                       |
+                                             |
+    Execute some syscall(s)                  |
+                                             |    Write new coverage starting at
+                                             |    buffer[1]
+    Atomically read buffer[0] to get the     |
+    total coverage size at this point in     |
+    time                                     |
+                                             |
+    ioctl(fd, KCOV_DISABLE, 0)               |
+                                             |    Write some more coverage for ioctl(),
+                                             |    then disable PC collection for current
+                                             |    thread
+    Safely read and process the coverage     |
+    up to the buffer[0] value saved above    |
+
+
 Comparison operands collection
 ------------------------------
 
-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v2 04/11] kcov: factor out struct kcov_state

[Alexander Potapenko]

Group several kcov-related fields (area, size, sequence) that are
stored in various structures, into `struct kcov_state`, so that
these fields can be easily passed around and manipulated.
Note that now the spinlock in struct kcov applies to every member
of struct kcov_state, including the sequence number.

This prepares us for the upcoming change that will introduce more
kcov state.

Also update the MAINTAINERS entry: add include/linux/kcov_types..h,
add myself as kcov reviewer.

Signed-off-by: Alexander Potapenko <glider@google.com>
---
Change-Id: If225682ea2f6e91245381b3270de16e7ea40df39

v2:
 - add myself to kcov MAINTAINERS
 - rename kcov-state.h to kcov_types.h
 - update the description
 - do not move mode into struct kcov_state
 - use '{ }' instead of '{ 0 }'
---
 MAINTAINERS                |   2 +
 include/linux/kcov.h       |   2 +-
 include/linux/kcov_types.h |  22 +++++++
 include/linux/sched.h      |  13 +----
 kernel/kcov.c              | 115 ++++++++++++++++---------------------
 5 files changed, 78 insertions(+), 76 deletions(-)
 create mode 100644 include/linux/kcov_types.h

diff --git a/MAINTAINERS b/MAINTAINERS
index dd844ac8d9107..5bbc78b0fa6ed 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12823,11 +12823,13 @@ F:	include/linux/kcore.h
 KCOV
 R:	Dmitry Vyukov <dvyukov@google.com>
 R:	Andrey Konovalov <andreyknvl@gmail.com>
+R:	Alexander Potapenko <glider@google.com>
 L:	kasan-dev@googlegroups.com
 S:	Maintained
 B:	https://bugzilla.kernel.org/buglist.cgi?component=Sanitizers&product=Memory%20Management
 F:	Documentation/dev-tools/kcov.rst
 F:	include/linux/kcov.h
+F:	include/linux/kcov_types.h
 F:	include/uapi/linux/kcov.h
 F:	kernel/kcov.c
 F:	scripts/Makefile.kcov
diff --git a/include/linux/kcov.h b/include/linux/kcov.h
index 932b4face1005..0e425c3524b86 100644
--- a/include/linux/kcov.h
+++ b/include/linux/kcov.h
@@ -2,7 +2,7 @@
 #ifndef _LINUX_KCOV_H
 #define _LINUX_KCOV_H
 
-#include <linux/sched.h>
+#include <linux/kcov_types.h>
 #include <uapi/linux/kcov.h>
 
 struct task_struct;
diff --git a/include/linux/kcov_types.h b/include/linux/kcov_types.h
new file mode 100644
index 0000000000000..53b25b6f0addd
--- /dev/null
+++ b/include/linux/kcov_types.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_KCOV_STATE_H
+#define _LINUX_KCOV_STATE_H
+
+#ifdef CONFIG_KCOV
+/* See kernel/kcov.c for more details. */
+struct kcov_state {
+	/* Size of the area (in long's). */
+	unsigned int size;
+
+	/* Buffer for coverage collection, shared with the userspace. */
+	void *area;
+
+	/*
+	 * KCOV sequence number: incremented each time kcov is reenabled, used
+	 * by kcov_remote_stop(), see the comment there.
+	 */
+	int sequence;
+};
+#endif /* CONFIG_KCOV */
+
+#endif /* _LINUX_KCOV_STATE_H */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index f96ac19828934..68af8d6eaee3a 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -42,6 +42,7 @@
 #include <linux/restart_block.h>
 #include <uapi/linux/rseq.h>
 #include <linux/seqlock_types.h>
+#include <linux/kcov_types.h>
 #include <linux/kcsan.h>
 #include <linux/rv.h>
 #include <linux/livepatch_sched.h>
@@ -1512,16 +1513,11 @@ struct task_struct {
 #endif /* CONFIG_TRACING */
 
 #ifdef CONFIG_KCOV
-	/* See kernel/kcov.c for more details. */
-
 	/* Coverage collection mode enabled for this task (0 if disabled): */
 	unsigned int			kcov_mode;
 
-	/* Size of the kcov_area: */
-	unsigned int			kcov_size;
-
-	/* Buffer for coverage collection: */
-	void				*kcov_area;
+	/* kcov buffer state for this task. */
+	struct kcov_state		kcov_state;
 
 	/* KCOV descriptor wired with this task or NULL: */
 	struct kcov			*kcov;
@@ -1529,9 +1525,6 @@ struct task_struct {
 	/* KCOV common handle for remote coverage collection: */
 	u64				kcov_handle;
 
-	/* KCOV sequence number: */
-	int				kcov_sequence;
-
 	/* Collect coverage from softirq context: */
 	unsigned int			kcov_softirq;
 #endif
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 0dd42b78694c9..ff7f118644f49 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -13,6 +13,7 @@
 #include <linux/init.h>
 #include <linux/jiffies.h>
 #include <linux/kcov.h>
+#include <linux/kcov_types.h>
 #include <linux/kmsan-checks.h>
 #include <linux/log2.h>
 #include <linux/mm.h>
@@ -54,24 +55,17 @@ struct kcov {
 	 *  - each code section for remote coverage collection
 	 */
 	refcount_t refcount;
-	/* The lock protects mode, size, area and t. */
+	/* The lock protects state and t. */
 	spinlock_t lock;
 	enum kcov_mode mode;
-	/* Size of arena (in long's). */
-	unsigned int size;
-	/* Coverage buffer shared with user space. */
-	void *area;
+	struct kcov_state state;
+
 	/* Task for which we collect coverage, or NULL. */
 	struct task_struct *t;
 	/* Collecting coverage from remote (background) threads. */
 	bool remote;
 	/* Size of remote area (in long's). */
 	unsigned int remote_size;
-	/*
-	 * Sequence is incremented each time kcov is reenabled, used by
-	 * kcov_remote_stop(), see the comment there.
-	 */
-	int sequence;
 };
 
 struct kcov_remote_area {
@@ -92,12 +86,9 @@ static struct list_head kcov_remote_areas = LIST_HEAD_INIT(kcov_remote_areas);
 struct kcov_percpu_data {
 	void *irq_area;
 	local_lock_t lock;
-
-	unsigned int saved_mode;
-	unsigned int saved_size;
-	void *saved_area;
+	enum kcov_mode saved_mode;
 	struct kcov *saved_kcov;
-	int saved_sequence;
+	struct kcov_state saved_state;
 };
 
 static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {
@@ -219,10 +210,10 @@ void notrace __sanitizer_cov_trace_pc(void)
 	if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t))
 		return;
 
-	area = t->kcov_area;
+	area = t->kcov_state.area;
 	/* The first 64-bit word is the number of subsequent PCs. */
 	pos = READ_ONCE(area[0]) + 1;
-	if (likely(pos < t->kcov_size)) {
+	if (likely(pos < t->kcov_state.size)) {
 		/* Previously we write pc before updating pos. However, some
 		 * early interrupt code could bypass check_kcov_mode() check
 		 * and invoke __sanitizer_cov_trace_pc(). If such interrupt is
@@ -252,10 +243,10 @@ static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip)
 
 	/*
 	 * We write all comparison arguments and types as u64.
-	 * The buffer was allocated for t->kcov_size unsigned longs.
+	 * The buffer was allocated for t->kcov_state.size unsigned longs.
 	 */
-	area = (u64 *)t->kcov_area;
-	max_pos = t->kcov_size * sizeof(unsigned long);
+	area = (u64 *)t->kcov_state.area;
+	max_pos = t->kcov_state.size * sizeof(unsigned long);
 
 	count = READ_ONCE(area[0]);
 
@@ -356,17 +347,15 @@ EXPORT_SYMBOL(__sanitizer_cov_trace_switch);
 #endif /* ifdef CONFIG_KCOV_ENABLE_COMPARISONS */
 
 static void kcov_start(struct task_struct *t, struct kcov *kcov,
-		       unsigned int size, void *area, enum kcov_mode mode,
-		       int sequence)
+		       enum kcov_mode mode, struct kcov_state *state)
 {
-	kcov_debug("t = %px, size = %u, area = %px\n", t, size, area);
+	kcov_debug("t = %px, size = %u, area = %px\n", t, state->size,
+		   state->area);
 	t->kcov = kcov;
 	/* Cache in task struct for performance. */
-	t->kcov_size = size;
-	t->kcov_area = area;
-	t->kcov_sequence = sequence;
-	/* See comment in check_kcov_mode(). */
+	t->kcov_state = *state;
 	barrier();
+	/* See comment in check_kcov_mode(). */
 	WRITE_ONCE(t->kcov_mode, mode);
 }
 
@@ -375,14 +364,14 @@ static void kcov_stop(struct task_struct *t)
 	WRITE_ONCE(t->kcov_mode, KCOV_MODE_DISABLED);
 	barrier();
 	t->kcov = NULL;
-	t->kcov_size = 0;
-	t->kcov_area = NULL;
+	t->kcov_state.size = 0;
+	t->kcov_state.area = NULL;
 }
 
 static void kcov_task_reset(struct task_struct *t)
 {
 	kcov_stop(t);
-	t->kcov_sequence = 0;
+	t->kcov_state.sequence = 0;
 	t->kcov_handle = 0;
 }
 
@@ -398,7 +387,7 @@ static void kcov_reset(struct kcov *kcov)
 	kcov->mode = KCOV_MODE_INIT;
 	kcov->remote = false;
 	kcov->remote_size = 0;
-	kcov->sequence++;
+	kcov->state.sequence++;
 }
 
 static void kcov_remote_reset(struct kcov *kcov)
@@ -438,7 +427,7 @@ static void kcov_put(struct kcov *kcov)
 {
 	if (refcount_dec_and_test(&kcov->refcount)) {
 		kcov_remote_reset(kcov);
-		vfree(kcov->area);
+		vfree(kcov->state.area);
 		kfree(kcov);
 	}
 }
@@ -495,8 +484,8 @@ static int kcov_mmap(struct file *filep, struct vm_area_struct *vma)
 	unsigned long flags;
 
 	spin_lock_irqsave(&kcov->lock, flags);
-	size = kcov->size * sizeof(unsigned long);
-	if (kcov->area == NULL || vma->vm_pgoff != 0 ||
+	size = kcov->state.size * sizeof(unsigned long);
+	if (kcov->state.area == NULL || vma->vm_pgoff != 0 ||
 	    vma->vm_end - vma->vm_start != size) {
 		res = -EINVAL;
 		goto exit;
@@ -504,7 +493,7 @@ static int kcov_mmap(struct file *filep, struct vm_area_struct *vma)
 	spin_unlock_irqrestore(&kcov->lock, flags);
 	vm_flags_set(vma, VM_DONTEXPAND);
 	for (off = 0; off < size; off += PAGE_SIZE) {
-		page = vmalloc_to_page(kcov->area + off);
+		page = vmalloc_to_page(kcov->state.area + off);
 		res = vm_insert_page(vma, vma->vm_start + off, page);
 		if (res) {
 			pr_warn_once("kcov: vm_insert_page() failed\n");
@@ -525,7 +514,7 @@ static int kcov_open(struct inode *inode, struct file *filep)
 	if (!kcov)
 		return -ENOMEM;
 	kcov->mode = KCOV_MODE_DISABLED;
-	kcov->sequence = 1;
+	kcov->state.sequence = 1;
 	refcount_set(&kcov->refcount, 1);
 	spin_lock_init(&kcov->lock);
 	filep->private_data = kcov;
@@ -560,10 +549,10 @@ static int kcov_get_mode(unsigned long arg)
 static void kcov_fault_in_area(struct kcov *kcov)
 {
 	unsigned long stride = PAGE_SIZE / sizeof(unsigned long);
-	unsigned long *area = kcov->area;
+	unsigned long *area = kcov->state.area;
 	unsigned long offset;
 
-	for (offset = 0; offset < kcov->size; offset += stride)
+	for (offset = 0; offset < kcov->state.size; offset += stride)
 		READ_ONCE(area[offset]);
 }
 
@@ -602,7 +591,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
 		 * at task exit or voluntary by KCOV_DISABLE. After that it can
 		 * be enabled for another task.
 		 */
-		if (kcov->mode != KCOV_MODE_INIT || !kcov->area)
+		if (kcov->mode != KCOV_MODE_INIT || !kcov->state.area)
 			return -EINVAL;
 		t = current;
 		if (kcov->t != NULL || t->kcov != NULL)
@@ -612,8 +601,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
 			return mode;
 		kcov_fault_in_area(kcov);
 		kcov->mode = mode;
-		kcov_start(t, kcov, kcov->size, kcov->area, kcov->mode,
-			   kcov->sequence);
+		kcov_start(t, kcov, mode, &kcov->state);
 		kcov->t = t;
 		/* Put either in kcov_task_exit() or in KCOV_DISABLE. */
 		kcov_get(kcov);
@@ -630,7 +618,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
 		kcov_put(kcov);
 		return 0;
 	case KCOV_REMOTE_ENABLE:
-		if (kcov->mode != KCOV_MODE_INIT || !kcov->area)
+		if (kcov->mode != KCOV_MODE_INIT || !kcov->state.area)
 			return -EINVAL;
 		t = current;
 		if (kcov->t != NULL || t->kcov != NULL)
@@ -725,8 +713,8 @@ static long kcov_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
 			vfree(area);
 			return -EBUSY;
 		}
-		kcov->area = area;
-		kcov->size = size;
+		kcov->state.area = area;
+		kcov->state.size = size;
 		kcov->mode = KCOV_MODE_INIT;
 		spin_unlock_irqrestore(&kcov->lock, flags);
 		return 0;
@@ -825,10 +813,8 @@ static void kcov_remote_softirq_start(struct task_struct *t)
 	mode = READ_ONCE(t->kcov_mode);
 	barrier();
 	if (kcov_mode_enabled(mode)) {
+		data->saved_state = t->kcov_state;
 		data->saved_mode = mode;
-		data->saved_size = t->kcov_size;
-		data->saved_area = t->kcov_area;
-		data->saved_sequence = t->kcov_sequence;
 		data->saved_kcov = t->kcov;
 		kcov_stop(t);
 	}
@@ -839,13 +825,9 @@ static void kcov_remote_softirq_stop(struct task_struct *t)
 	struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
 
 	if (data->saved_kcov) {
-		kcov_start(t, data->saved_kcov, data->saved_size,
-			   data->saved_area, data->saved_mode,
-			   data->saved_sequence);
-		data->saved_mode = 0;
-		data->saved_size = 0;
-		data->saved_area = NULL;
-		data->saved_sequence = 0;
+		kcov_start(t, data->saved_kcov, t->kcov_mode,
+			   &data->saved_state);
+		data->saved_state = (struct kcov_state){};
 		data->saved_kcov = NULL;
 	}
 }
@@ -854,12 +836,12 @@ void kcov_remote_start(u64 handle)
 {
 	struct task_struct *t = current;
 	struct kcov_remote *remote;
+	struct kcov_state state;
+	enum kcov_mode mode;
+	unsigned long flags;
+	unsigned int size;
 	struct kcov *kcov;
-	unsigned int mode;
 	void *area;
-	unsigned int size;
-	int sequence;
-	unsigned long flags;
 
 	if (WARN_ON(!kcov_check_handle(handle, true, true, true)))
 		return;
@@ -904,7 +886,7 @@ void kcov_remote_start(u64 handle)
 	 * KCOV_DISABLE / kcov_remote_reset().
 	 */
 	mode = kcov->mode;
-	sequence = kcov->sequence;
+	state.sequence = kcov->state.sequence;
 	if (in_task()) {
 		size = kcov->remote_size;
 		area = kcov_remote_area_get(size);
@@ -927,12 +909,14 @@ void kcov_remote_start(u64 handle)
 
 	/* Reset coverage size. */
 	*(u64 *)area = 0;
+	state.area = area;
+	state.size = size;
 
 	if (in_serving_softirq()) {
 		kcov_remote_softirq_start(t);
 		t->kcov_softirq = 1;
 	}
-	kcov_start(t, kcov, size, area, mode, sequence);
+	kcov_start(t, kcov, t->kcov_mode, &state);
 
 	local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
 }
@@ -1030,9 +1014,9 @@ void kcov_remote_stop(void)
 	}
 
 	kcov = t->kcov;
-	area = t->kcov_area;
-	size = t->kcov_size;
-	sequence = t->kcov_sequence;
+	area = t->kcov_state.area;
+	size = t->kcov_state.size;
+	sequence = t->kcov_state.sequence;
 
 	kcov_stop(t);
 	if (in_serving_softirq()) {
@@ -1045,8 +1029,9 @@ void kcov_remote_stop(void)
 	 * KCOV_DISABLE could have been called between kcov_remote_start()
 	 * and kcov_remote_stop(), hence the sequence check.
 	 */
-	if (sequence == kcov->sequence && kcov->remote)
-		kcov_move_area(kcov->mode, kcov->area, kcov->size, area);
+	if (sequence == kcov->state.sequence && kcov->remote)
+		kcov_move_area(kcov->mode, kcov->state.area, kcov->state.size,
+			       area);
 	spin_unlock(&kcov->lock);
 
 	if (in_task()) {
-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v2 05/11] mm/kasan: define __asan_before_dynamic_init, __asan_after_dynamic_init

[Alexander Potapenko]

Calls to __asan_before_dynamic_init() and __asan_after_dynamic_init()
are inserted by Clang when building with coverage guards.
These functions can be used to detect initialization order fiasco bugs
in the userspace, but it is fine for them to be no-ops in the kernel.

Signed-off-by: Alexander Potapenko <glider@google.com>

---
Change-Id: I7f8eb690a3d96f7d122205e8f1cba8039f6a68eb

v2:
 - Address comments by Dmitry Vyukov:
   - rename CONFIG_KCOV_ENABLE_GUARDS to CONFIG_KCOV_UNIQUE
 - Move this patch before the one introducing CONFIG_KCOV_UNIQUE,
   per Marco Elver's request.
---
 mm/kasan/generic.c | 18 ++++++++++++++++++
 mm/kasan/kasan.h   |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
index d54e89f8c3e76..b0b7781524348 100644
--- a/mm/kasan/generic.c
+++ b/mm/kasan/generic.c
@@ -238,6 +238,24 @@ void __asan_unregister_globals(void *ptr, ssize_t size)
 }
 EXPORT_SYMBOL(__asan_unregister_globals);
 
+#if defined(CONFIG_KCOV_UNIQUE)
+/*
+ * __asan_before_dynamic_init() and __asan_after_dynamic_init() are inserted
+ * when the user requests building with coverage guards. In the userspace, these
+ * two functions can be used to detect initialization order fiasco bugs, but in
+ * the kernel they can be no-ops.
+ */
+void __asan_before_dynamic_init(const char *module_name)
+{
+}
+EXPORT_SYMBOL(__asan_before_dynamic_init);
+
+void __asan_after_dynamic_init(void)
+{
+}
+EXPORT_SYMBOL(__asan_after_dynamic_init);
+#endif
+
 #define DEFINE_ASAN_LOAD_STORE(size)					\
 	void __asan_load##size(void *addr)				\
 	{								\
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 129178be5e649..c817c46b4fcd2 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -582,6 +582,8 @@ void kasan_restore_multi_shot(bool enabled);
 
 void __asan_register_globals(void *globals, ssize_t size);
 void __asan_unregister_globals(void *globals, ssize_t size);
+void __asan_before_dynamic_init(const char *module_name);
+void __asan_after_dynamic_init(void);
 void __asan_handle_no_return(void);
 void __asan_alloca_poison(void *, ssize_t size);
 void __asan_allocas_unpoison(void *stack_top, ssize_t stack_bottom);
-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v2 06/11] kcov: x86: introduce CONFIG_KCOV_UNIQUE

[Alexander Potapenko]

The new config switches coverage instrumentation to using
  __sanitizer_cov_trace_pc_guard(u32 *guard)
instead of
  __sanitizer_cov_trace_pc(void)

This relies on Clang's -fsanitize-coverage=trace-pc-guard flag [1].

Each callback receives a unique 32-bit guard variable residing in the
__sancov_guards section. Those guards can be used by kcov to deduplicate
the coverage on the fly.

As a first step, we make the new instrumentation mode 1:1 compatible
with the old one.

[1] https://clang.llvm.org/docs/SanitizerCoverage.html#tracing-pcs-with-guards

Cc: x86@kernel.org
Signed-off-by: Alexander Potapenko <glider@google.com>

---
Change-Id: Iacb1e71fd061a82c2acadf2347bba4863b9aec39

v2:
 - Address comments by Dmitry Vyukov
   - rename CONFIG_KCOV_ENABLE_GUARDS to CONFIG_KCOV_UNIQUE
   - update commit description and config description
 - Address comments by Marco Elver
   - rename sanitizer_cov_write_subsequent() to kcov_append_to_buffer()
   - make config depend on X86_64 (via ARCH_HAS_KCOV_UNIQUE)
   - swap #ifdef branches
   - tweak config description
   - remove redundant check for CONFIG_CC_HAS_SANCOV_TRACE_PC_GUARD
---
 arch/x86/Kconfig                  |  1 +
 arch/x86/kernel/vmlinux.lds.S     |  1 +
 include/asm-generic/vmlinux.lds.h | 14 ++++++-
 include/linux/kcov.h              |  2 +
 kernel/kcov.c                     | 61 +++++++++++++++++++++----------
 lib/Kconfig.debug                 | 24 ++++++++++++
 scripts/Makefile.kcov             |  4 ++
 scripts/module.lds.S              | 23 ++++++++++++
 tools/objtool/check.c             |  1 +
 9 files changed, 110 insertions(+), 21 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index e21cca404943e..d104c5a193bdf 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -93,6 +93,7 @@ config X86
 	select ARCH_HAS_FORTIFY_SOURCE
 	select ARCH_HAS_GCOV_PROFILE_ALL
 	select ARCH_HAS_KCOV			if X86_64
+	select ARCH_HAS_KCOV_UNIQUE		if X86_64
 	select ARCH_HAS_KERNEL_FPU_SUPPORT
 	select ARCH_HAS_MEM_ENCRYPT
 	select ARCH_HAS_MEMBARRIER_SYNC_CORE
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index cda5f8362e9da..8076e8953fddc 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -372,6 +372,7 @@ SECTIONS
 		. = ALIGN(PAGE_SIZE);
 		__bss_stop = .;
 	}
+	SANCOV_GUARDS_BSS
 
 	/*
 	 * The memory occupied from _text to here, __end_of_kernel_reserve, is
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 58a635a6d5bdf..875c4deb66208 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -102,7 +102,8 @@
  * sections to be brought in with rodata.
  */
 #if defined(CONFIG_LD_DEAD_CODE_DATA_ELIMINATION) || defined(CONFIG_LTO_CLANG) || \
-defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
+	defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG) || \
+	defined(CONFIG_KCOV_UNIQUE)
 #define TEXT_MAIN .text .text.[0-9a-zA-Z_]*
 #else
 #define TEXT_MAIN .text
@@ -121,6 +122,17 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
 #define SBSS_MAIN .sbss
 #endif
 
+#if defined(CONFIG_KCOV_UNIQUE)
+#define SANCOV_GUARDS_BSS			\
+	__sancov_guards(NOLOAD) : {		\
+		__start___sancov_guards = .;	\
+		*(__sancov_guards);		\
+		__stop___sancov_guards = .;	\
+	}
+#else
+#define SANCOV_GUARDS_BSS
+#endif
+
 /*
  * GCC 4.5 and later have a 32 bytes section alignment for structures.
  * Except GCC 4.9, that feels the need to align on 64 bytes.
diff --git a/include/linux/kcov.h b/include/linux/kcov.h
index 0e425c3524b86..dd8bbee6fe274 100644
--- a/include/linux/kcov.h
+++ b/include/linux/kcov.h
@@ -107,6 +107,8 @@ typedef unsigned long long kcov_u64;
 #endif
 
 void __sanitizer_cov_trace_pc(void);
+void __sanitizer_cov_trace_pc_guard(u32 *guard);
+void __sanitizer_cov_trace_pc_guard_init(uint32_t *start, uint32_t *stop);
 void __sanitizer_cov_trace_cmp1(u8 arg1, u8 arg2);
 void __sanitizer_cov_trace_cmp2(u16 arg1, u16 arg2);
 void __sanitizer_cov_trace_cmp4(u32 arg1, u32 arg2);
diff --git a/kernel/kcov.c b/kernel/kcov.c
index ff7f118644f49..8e98ca8d52743 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -195,27 +195,15 @@ static notrace unsigned long canonicalize_ip(unsigned long ip)
 	return ip;
 }
 
-/*
- * Entry point from instrumented code.
- * This is called once per basic-block/edge.
- */
-void notrace __sanitizer_cov_trace_pc(void)
+static notrace void kcov_append_to_buffer(unsigned long *area, int size,
+					  unsigned long ip)
 {
-	struct task_struct *t;
-	unsigned long *area;
-	unsigned long ip = canonicalize_ip(_RET_IP_);
-	unsigned long pos;
-
-	t = current;
-	if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t))
-		return;
-
-	area = t->kcov_state.area;
 	/* The first 64-bit word is the number of subsequent PCs. */
-	pos = READ_ONCE(area[0]) + 1;
-	if (likely(pos < t->kcov_state.size)) {
-		/* Previously we write pc before updating pos. However, some
-		 * early interrupt code could bypass check_kcov_mode() check
+	unsigned long pos = READ_ONCE(area[0]) + 1;
+
+	if (likely(pos < size)) {
+		/*
+		 * Some early interrupt code could bypass check_kcov_mode() check
 		 * and invoke __sanitizer_cov_trace_pc(). If such interrupt is
 		 * raised between writing pc and updating pos, the pc could be
 		 * overitten by the recursive __sanitizer_cov_trace_pc().
@@ -226,7 +214,40 @@ void notrace __sanitizer_cov_trace_pc(void)
 		area[pos] = ip;
 	}
 }
+
+/*
+ * Entry point from instrumented code.
+ * This is called once per basic-block/edge.
+ */
+#ifdef CONFIG_KCOV_UNIQUE
+void notrace __sanitizer_cov_trace_pc_guard(u32 *guard)
+{
+	if (!check_kcov_mode(KCOV_MODE_TRACE_PC, current))
+		return;
+
+	kcov_append_to_buffer(current->kcov_state.area,
+			      current->kcov_state.size,
+			      canonicalize_ip(_RET_IP_));
+}
+EXPORT_SYMBOL(__sanitizer_cov_trace_pc_guard);
+
+void notrace __sanitizer_cov_trace_pc_guard_init(uint32_t *start,
+						 uint32_t *stop)
+{
+}
+EXPORT_SYMBOL(__sanitizer_cov_trace_pc_guard_init);
+#else /* !CONFIG_KCOV_UNIQUE */
+void notrace __sanitizer_cov_trace_pc(void)
+{
+	if (!check_kcov_mode(KCOV_MODE_TRACE_PC, current))
+		return;
+
+	kcov_append_to_buffer(current->kcov_state.area,
+			      current->kcov_state.size,
+			      canonicalize_ip(_RET_IP_));
+}
 EXPORT_SYMBOL(__sanitizer_cov_trace_pc);
+#endif
 
 #ifdef CONFIG_KCOV_ENABLE_COMPARISONS
 static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip)
@@ -254,7 +275,7 @@ static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip)
 	start_index = 1 + count * KCOV_WORDS_PER_CMP;
 	end_pos = (start_index + KCOV_WORDS_PER_CMP) * sizeof(u64);
 	if (likely(end_pos <= max_pos)) {
-		/* See comment in __sanitizer_cov_trace_pc(). */
+		/* See comment in kcov_append_to_buffer(). */
 		WRITE_ONCE(area[0], count + 1);
 		barrier();
 		area[start_index] = type;
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index f9051ab610d54..24dcb721dbb0b 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2156,6 +2156,8 @@ config ARCH_HAS_KCOV
 config CC_HAS_SANCOV_TRACE_PC
 	def_bool $(cc-option,-fsanitize-coverage=trace-pc)
 
+config CC_HAS_SANCOV_TRACE_PC_GUARD
+	def_bool $(cc-option,-fsanitize-coverage=trace-pc-guard)
 
 config KCOV
 	bool "Code coverage for fuzzing"
@@ -2172,6 +2174,28 @@ config KCOV
 
 	  For more details, see Documentation/dev-tools/kcov.rst.
 
+config ARCH_HAS_KCOV_UNIQUE
+	bool
+	help
+	  An architecture should select this when it can successfully
+	  build and run with CONFIG_KCOV_UNIQUE.
+
+config KCOV_UNIQUE
+	depends on KCOV
+	depends on CC_HAS_SANCOV_TRACE_PC_GUARD && ARCH_HAS_KCOV_UNIQUE
+	bool "Use coverage guards for KCOV"
+	help
+	  Use coverage guards instrumentation for KCOV, passing
+	  -fsanitize-coverage=trace-pc-guard to the compiler.
+
+	  Every coverage callback is associated with a global variable that
+	  allows to efficiently deduplicate coverage at collection time.
+	  This drastically reduces the buffer size required for coverage
+	  collection.
+
+	  This config comes at a cost of increased binary size (4 bytes of .bss
+	  plus 1-2 instructions to pass an extra parameter, per basic block).
+
 config KCOV_ENABLE_COMPARISONS
 	bool "Enable comparison operands collection by KCOV"
 	depends on KCOV
diff --git a/scripts/Makefile.kcov b/scripts/Makefile.kcov
index 67e8cfe3474b7..0b17533ef35f6 100644
--- a/scripts/Makefile.kcov
+++ b/scripts/Makefile.kcov
@@ -1,5 +1,9 @@
 # SPDX-License-Identifier: GPL-2.0-only
+ifeq ($(CONFIG_KCOV_UNIQUE),y)
+kcov-flags-y					+= -fsanitize-coverage=trace-pc-guard
+else
 kcov-flags-$(CONFIG_CC_HAS_SANCOV_TRACE_PC)	+= -fsanitize-coverage=trace-pc
+endif
 kcov-flags-$(CONFIG_KCOV_ENABLE_COMPARISONS)	+= -fsanitize-coverage=trace-cmp
 kcov-flags-$(CONFIG_GCC_PLUGIN_SANCOV)		+= -fplugin=$(objtree)/scripts/gcc-plugins/sancov_plugin.so
 
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index 450f1088d5fd3..314b56680ea1a 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -64,6 +64,29 @@ SECTIONS {
 		MOD_CODETAG_SECTIONS()
 	}
 #endif
+
+#ifdef CONFIG_KCOV_UNIQUE
+	__sancov_guards(NOLOAD) : {
+		__start___sancov_guards = .;
+		*(__sancov_guards);
+		__stop___sancov_guards = .;
+	}
+
+	.text : {
+		*(.text .text.[0-9a-zA-Z_]*)
+		*(.text..L*)
+	}
+
+	.init.text : {
+		*(.init.text .init.text.[0-9a-zA-Z_]*)
+		*(.init.text..L*)
+	}
+	.exit.text : {
+		*(.exit.text .exit.text.[0-9a-zA-Z_]*)
+		*(.exit.text..L*)
+	}
+#endif
+
 	MOD_SEPARATE_CODETAG_SECTIONS()
 }
 
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index b21b12ec88d96..62fbe9b2aa077 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1154,6 +1154,7 @@ static const char *uaccess_safe_builtin[] = {
 	"write_comp_data",
 	"check_kcov_mode",
 	"__sanitizer_cov_trace_pc",
+	"__sanitizer_cov_trace_pc_guard",
 	"__sanitizer_cov_trace_const_cmp1",
 	"__sanitizer_cov_trace_const_cmp2",
 	"__sanitizer_cov_trace_const_cmp4",
-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v2 07/11] kcov: add trace and trace_size to struct kcov_state

[Alexander Potapenko]

Keep kcov_state.area as the pointer to the memory buffer used by
kcov and shared with the userspace. Store the pointer to the trace
(part of the buffer holding sequential events) separately, as we will
be splitting that buffer in multiple parts.
No functional changes so far.

Signed-off-by: Alexander Potapenko <glider@google.com>

---
Change-Id: I50b5589ef0e0b6726aa0579334093c648f76790a

v2:
 - Address comments by Dmitry Vyukov:
   - tweak commit description
 - Address comments by Marco Elver:
   - rename sanitizer_cov_write_subsequent() to kcov_append_to_buffer()
 - Update code to match the new description of struct kcov_state
---
 include/linux/kcov_types.h |  9 ++++++-
 kernel/kcov.c              | 54 ++++++++++++++++++++++----------------
 2 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/include/linux/kcov_types.h b/include/linux/kcov_types.h
index 53b25b6f0addd..233e7a682654b 100644
--- a/include/linux/kcov_types.h
+++ b/include/linux/kcov_types.h
@@ -7,9 +7,16 @@
 struct kcov_state {
 	/* Size of the area (in long's). */
 	unsigned int size;
+	/*
+	 * Pointer to user-provided memory used by kcov. This memory may
+	 * contain multiple buffers.
+	 */
+	void *area;
 
+	/* Size of the trace (in long's). */
+	unsigned int trace_size;
 	/* Buffer for coverage collection, shared with the userspace. */
-	void *area;
+	unsigned long *trace;
 
 	/*
 	 * KCOV sequence number: incremented each time kcov is reenabled, used
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 8e98ca8d52743..038261145cf93 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -195,11 +195,11 @@ static notrace unsigned long canonicalize_ip(unsigned long ip)
 	return ip;
 }
 
-static notrace void kcov_append_to_buffer(unsigned long *area, int size,
+static notrace void kcov_append_to_buffer(unsigned long *trace, int size,
 					  unsigned long ip)
 {
 	/* The first 64-bit word is the number of subsequent PCs. */
-	unsigned long pos = READ_ONCE(area[0]) + 1;
+	unsigned long pos = READ_ONCE(trace[0]) + 1;
 
 	if (likely(pos < size)) {
 		/*
@@ -209,9 +209,9 @@ static notrace void kcov_append_to_buffer(unsigned long *area, int size,
 		 * overitten by the recursive __sanitizer_cov_trace_pc().
 		 * Update pos before writing pc to avoid such interleaving.
 		 */
-		WRITE_ONCE(area[0], pos);
+		WRITE_ONCE(trace[0], pos);
 		barrier();
-		area[pos] = ip;
+		trace[pos] = ip;
 	}
 }
 
@@ -225,8 +225,8 @@ void notrace __sanitizer_cov_trace_pc_guard(u32 *guard)
 	if (!check_kcov_mode(KCOV_MODE_TRACE_PC, current))
 		return;
 
-	kcov_append_to_buffer(current->kcov_state.area,
-			      current->kcov_state.size,
+	kcov_append_to_buffer(current->kcov_state.trace,
+			      current->kcov_state.trace_size,
 			      canonicalize_ip(_RET_IP_));
 }
 EXPORT_SYMBOL(__sanitizer_cov_trace_pc_guard);
@@ -242,8 +242,8 @@ void notrace __sanitizer_cov_trace_pc(void)
 	if (!check_kcov_mode(KCOV_MODE_TRACE_PC, current))
 		return;
 
-	kcov_append_to_buffer(current->kcov_state.area,
-			      current->kcov_state.size,
+	kcov_append_to_buffer(current->kcov_state.trace,
+			      current->kcov_state.trace_size,
 			      canonicalize_ip(_RET_IP_));
 }
 EXPORT_SYMBOL(__sanitizer_cov_trace_pc);
@@ -252,9 +252,9 @@ EXPORT_SYMBOL(__sanitizer_cov_trace_pc);
 #ifdef CONFIG_KCOV_ENABLE_COMPARISONS
 static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip)
 {
-	struct task_struct *t;
-	u64 *area;
 	u64 count, start_index, end_pos, max_pos;
+	struct task_struct *t;
+	u64 *trace;
 
 	t = current;
 	if (!check_kcov_mode(KCOV_MODE_TRACE_CMP, t))
@@ -266,22 +266,22 @@ static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip)
 	 * We write all comparison arguments and types as u64.
 	 * The buffer was allocated for t->kcov_state.size unsigned longs.
 	 */
-	area = (u64 *)t->kcov_state.area;
+	trace = (u64 *)t->kcov_state.trace;
 	max_pos = t->kcov_state.size * sizeof(unsigned long);
 
-	count = READ_ONCE(area[0]);
+	count = READ_ONCE(trace[0]);
 
 	/* Every record is KCOV_WORDS_PER_CMP 64-bit words. */
 	start_index = 1 + count * KCOV_WORDS_PER_CMP;
 	end_pos = (start_index + KCOV_WORDS_PER_CMP) * sizeof(u64);
 	if (likely(end_pos <= max_pos)) {
 		/* See comment in kcov_append_to_buffer(). */
-		WRITE_ONCE(area[0], count + 1);
+		WRITE_ONCE(trace[0], count + 1);
 		barrier();
-		area[start_index] = type;
-		area[start_index + 1] = arg1;
-		area[start_index + 2] = arg2;
-		area[start_index + 3] = ip;
+		trace[start_index] = type;
+		trace[start_index + 1] = arg1;
+		trace[start_index + 2] = arg2;
+		trace[start_index + 3] = ip;
 	}
 }
 
@@ -382,11 +382,13 @@ static void kcov_start(struct task_struct *t, struct kcov *kcov,
 
 static void kcov_stop(struct task_struct *t)
 {
+	int saved_sequence = t->kcov_state.sequence;
+
 	WRITE_ONCE(t->kcov_mode, KCOV_MODE_DISABLED);
 	barrier();
 	t->kcov = NULL;
-	t->kcov_state.size = 0;
-	t->kcov_state.area = NULL;
+	t->kcov_state = (typeof(t->kcov_state)){ 0 };
+	t->kcov_state.sequence = saved_sequence;
 }
 
 static void kcov_task_reset(struct task_struct *t)
@@ -736,6 +738,8 @@ static long kcov_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
 		}
 		kcov->state.area = area;
 		kcov->state.size = size;
+		kcov->state.trace = area;
+		kcov->state.trace_size = size;
 		kcov->mode = KCOV_MODE_INIT;
 		spin_unlock_irqrestore(&kcov->lock, flags);
 		return 0;
@@ -928,10 +932,12 @@ void kcov_remote_start(u64 handle)
 		local_lock_irqsave(&kcov_percpu_data.lock, flags);
 	}
 
-	/* Reset coverage size. */
-	*(u64 *)area = 0;
 	state.area = area;
 	state.size = size;
+	state.trace = area;
+	state.trace_size = size;
+	/* Reset coverage size. */
+	state.trace[0] = 0;
 
 	if (in_serving_softirq()) {
 		kcov_remote_softirq_start(t);
@@ -1004,8 +1010,8 @@ void kcov_remote_stop(void)
 	struct task_struct *t = current;
 	struct kcov *kcov;
 	unsigned int mode;
-	void *area;
-	unsigned int size;
+	void *area, *trace;
+	unsigned int size, trace_size;
 	int sequence;
 	unsigned long flags;
 
@@ -1037,6 +1043,8 @@ void kcov_remote_stop(void)
 	kcov = t->kcov;
 	area = t->kcov_state.area;
 	size = t->kcov_state.size;
+	trace = t->kcov_state.trace;
+	trace_size = t->kcov_state.trace_size;
 	sequence = t->kcov_state.sequence;
 
 	kcov_stop(t);
-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v2 08/11] kcov: add ioctl(KCOV_UNIQUE_ENABLE)

[Alexander Potapenko]

ioctl(KCOV_UNIQUE_ENABLE) enables collection of deduplicated coverage
in the presence of CONFIG_KCOV_ENABLE_GUARDS.

The buffer shared with the userspace is divided in two parts, one holding
a bitmap, and the other one being the trace. The single parameter of
ioctl(KCOV_UNIQUE_ENABLE) determines the number of words used for the
bitmap.

Each __sanitizer_cov_trace_pc_guard() instrumentation hook receives a
pointer to a unique guard variable. Upon the first call of each hook,
the guard variable is initialized with a unique integer, which is used to
map those hooks to bits in the bitmap. In the new coverage collection mode,
the kernel first checks whether the bit corresponding to a particular hook
is set, and then, if it is not, the PC is written into the trace buffer,
and the bit is set.

Note: when CONFIG_KCOV_ENABLE_GUARDS is disabled, ioctl(KCOV_UNIQUE_ENABLE)
returns -ENOTSUPP, which is consistent with the existing kcov code.

Measuring the exact performance impact of this mode directly can be
challenging. However, based on fuzzing experiments (50 instances x 24h
with and without deduplication), we observe the following:
 - When normalized by pure fuzzing time, total executions decreased
   by 2.1% (p=0.01).
 - When normalized by fuzzer uptime, the reduction in total executions
   was statistically insignificant (-1.0% with p=0.20).
Despite a potential slight slowdown in execution count, the new mode
positively impacts fuzzing effectiveness:
 - Statistically significant increase in corpus size (+0.6%, p<0.01).
 - Statistically significant increase in coverage (+0.6%, p<0.01).
 - A 99.8% reduction in coverage overflows.

Also update the documentation.

Signed-off-by: Alexander Potapenko <glider@google.com>
---
Change-Id: I9805e7b22619a50e05cc7c7d794dacf6f7de2f03

v2:
 - Address comments by Dmitry Vyukov:
   - rename CONFIG_KCOV_ENABLE_GUARDS to CONFIG_KCOV_UNIQUE
   - rename KCOV_MODE_TRACE_UNIQUE_PC to KCOV_MODE_UNIQUE_PC
   - simplify index allocation
   - update documentation and comments
 - Address comments by Marco Elver:
   - change _IOR to _IOW in KCOV_UNIQUE_ENABLE definition
   - rename sanitizer_cov_write_subsequent() to kcov_append_to_buffer()
 - Use __test_and_set_bit() to avoid the lock prefix on the bit operation
 - Update code to match the new description of struct kcov_state
 - Rename kcov_get_mode() to kcov_arg_to_mode() to avoid confusion with
   get_kcov_mode(). Also make it use `enum kcov_mode`.
---
 Documentation/dev-tools/kcov.rst |  43 ++++++++
 include/linux/kcov.h             |   2 +
 include/linux/kcov_types.h       |   8 ++
 include/uapi/linux/kcov.h        |   1 +
 kernel/kcov.c                    | 165 ++++++++++++++++++++++++++-----
 5 files changed, 192 insertions(+), 27 deletions(-)

diff --git a/Documentation/dev-tools/kcov.rst b/Documentation/dev-tools/kcov.rst
index abf3ad2e784e8..6446887cd1c92 100644
--- a/Documentation/dev-tools/kcov.rst
+++ b/Documentation/dev-tools/kcov.rst
@@ -192,6 +192,49 @@ Normally the shared buffer is used as follows::
     up to the buffer[0] value saved above    |
 
 
+Unique coverage collection
+---------------------------
+
+Instead of collecting a trace of PCs, KCOV can deduplicate them on the fly.
+This mode is enabled by the ``KCOV_UNIQUE_ENABLE`` ioctl (only available if
+``CONFIG_KCOV_UNIQUE`` is on).
+
+.. code-block:: c
+
+	/* Same includes and defines as above. */
+	#define KCOV_UNIQUE_ENABLE		_IOW('c', 103, unsigned long)
+	#define BITMAP_SIZE			(4<<10)
+
+	/* Instead of KCOV_ENABLE, enable unique coverage collection. */
+	if (ioctl(fd, KCOV_UNIQUE_ENABLE, BITMAP_SIZE))
+		perror("ioctl"), exit(1);
+	/* Reset the coverage from the tail of the ioctl() call. */
+	__atomic_store_n(&cover[BITMAP_SIZE], 0, __ATOMIC_RELAXED);
+	memset(cover, 0, BITMAP_SIZE * sizeof(unsigned long));
+
+	/* Call the target syscall call. */
+	/* ... */
+
+	/* Read the number of collected PCs. */
+	n = __atomic_load_n(&cover[BITMAP_SIZE], __ATOMIC_RELAXED);
+	/* Disable the coverage collection. */
+	if (ioctl(fd, KCOV_DISABLE, 0))
+		perror("ioctl"), exit(1);
+
+Calling ``ioctl(fd, KCOV_UNIQUE_ENABLE, bitmap_size)`` carves out ``bitmap_size``
+unsigned long's from those allocated by ``KCOV_INIT_TRACE`` to keep an opaque
+bitmap that prevents the kernel from storing the same PC twice. The remaining
+part of the buffer is used to collect PCs, like in other modes (this part must
+contain at least two unsigned long's, like when collecting non-unique PCs).
+
+The mapping between a PC and its position in the bitmap is persistent during the
+kernel lifetime, so it is possible for the callers to directly use the bitmap
+contents as a coverage signal (like when fuzzing userspace with AFL).
+
+In order to reset the coverage between the runs, the user needs to rewind the
+trace (by writing 0 into the first buffer element past ``bitmap_size``) and zero
+the whole bitmap.
+
 Comparison operands collection
 ------------------------------
 
diff --git a/include/linux/kcov.h b/include/linux/kcov.h
index dd8bbee6fe274..13c120a894107 100644
--- a/include/linux/kcov.h
+++ b/include/linux/kcov.h
@@ -10,6 +10,7 @@ struct task_struct;
 #ifdef CONFIG_KCOV
 
 enum kcov_mode {
+	KCOV_MODE_INVALID = -1,
 	/* Coverage collection is not enabled yet. */
 	KCOV_MODE_DISABLED = 0,
 	/* KCOV was initialized, but tracing mode hasn't been chosen yet. */
@@ -23,6 +24,7 @@ enum kcov_mode {
 	KCOV_MODE_TRACE_CMP = 3,
 	/* The process owns a KCOV remote reference. */
 	KCOV_MODE_REMOTE = 4,
+	KCOV_MODE_UNIQUE_PC = 5,
 };
 
 #define KCOV_IN_CTXSW (1 << 30)
diff --git a/include/linux/kcov_types.h b/include/linux/kcov_types.h
index 233e7a682654b..5ab6e13a9e725 100644
--- a/include/linux/kcov_types.h
+++ b/include/linux/kcov_types.h
@@ -18,6 +18,14 @@ struct kcov_state {
 	/* Buffer for coverage collection, shared with the userspace. */
 	unsigned long *trace;
 
+	/* Size of the bitmap (in bits). */
+	unsigned int bitmap_size;
+	/*
+	 * Bitmap for coverage deduplication, shared with the
+	 * userspace.
+	 */
+	unsigned long *bitmap;
+
 	/*
 	 * KCOV sequence number: incremented each time kcov is reenabled, used
 	 * by kcov_remote_stop(), see the comment there.
diff --git a/include/uapi/linux/kcov.h b/include/uapi/linux/kcov.h
index ed95dba9fa37e..e743ee011eeca 100644
--- a/include/uapi/linux/kcov.h
+++ b/include/uapi/linux/kcov.h
@@ -22,6 +22,7 @@ struct kcov_remote_arg {
 #define KCOV_ENABLE			_IO('c', 100)
 #define KCOV_DISABLE			_IO('c', 101)
 #define KCOV_REMOTE_ENABLE		_IOW('c', 102, struct kcov_remote_arg)
+#define KCOV_UNIQUE_ENABLE		_IOW('c', 103, unsigned long)
 
 enum {
 	/*
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 038261145cf93..2a4edbaad50d0 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -29,6 +29,10 @@
 
 #include <asm/setup.h>
 
+#ifdef CONFIG_KCOV_UNIQUE
+atomic_t kcov_guard_max_index = ATOMIC_INIT(0);
+#endif
+
 #define kcov_debug(fmt, ...) pr_debug("%s: " fmt, __func__, ##__VA_ARGS__)
 
 /* Number of 64-bit words written per one comparison: */
@@ -163,10 +167,9 @@ static __always_inline bool in_softirq_really(void)
 	return in_serving_softirq() && !in_hardirq() && !in_nmi();
 }
 
-static notrace bool check_kcov_mode(enum kcov_mode needed_mode,
-				    struct task_struct *t)
+static notrace enum kcov_mode get_kcov_mode(struct task_struct *t)
 {
-	unsigned int mode;
+	enum kcov_mode mode;
 
 	/*
 	 * We are interested in code coverage as a function of a syscall inputs,
@@ -174,7 +177,7 @@ static notrace bool check_kcov_mode(enum kcov_mode needed_mode,
 	 * coverage collection section in a softirq.
 	 */
 	if (!in_task() && !(in_softirq_really() && t->kcov_softirq))
-		return false;
+		return KCOV_MODE_INVALID;
 	mode = READ_ONCE(t->kcov_mode);
 	/*
 	 * There is some code that runs in interrupts but for which
@@ -184,7 +187,7 @@ static notrace bool check_kcov_mode(enum kcov_mode needed_mode,
 	 * kcov_start().
 	 */
 	barrier();
-	return mode == needed_mode;
+	return mode;
 }
 
 static notrace unsigned long canonicalize_ip(unsigned long ip)
@@ -203,7 +206,7 @@ static notrace void kcov_append_to_buffer(unsigned long *trace, int size,
 
 	if (likely(pos < size)) {
 		/*
-		 * Some early interrupt code could bypass check_kcov_mode() check
+		 * Some early interrupt code could bypass get_kcov_mode() check
 		 * and invoke __sanitizer_cov_trace_pc(). If such interrupt is
 		 * raised between writing pc and updating pos, the pc could be
 		 * overitten by the recursive __sanitizer_cov_trace_pc().
@@ -220,14 +223,76 @@ static notrace void kcov_append_to_buffer(unsigned long *trace, int size,
  * This is called once per basic-block/edge.
  */
 #ifdef CONFIG_KCOV_UNIQUE
+DEFINE_PER_CPU(u32, saved_index);
+/*
+ * Assign an index to a guard variable that does not have one yet.
+ * For an unlikely case of a race with another task executing the same basic
+ * block for the first time with kcov enabled, we store the unused index in a
+ * per-cpu variable.
+ * In an even less likely case of the current task losing the race and getting
+ * rescheduled onto a CPU that already has a saved index, the index is
+ * discarded. This will result in an unused hole in the bitmap, but such events
+ * should have minor impact on the overall memory consumption.
+ */
+static __always_inline u32 init_pc_guard(u32 *guard)
+{
+	/* If the current CPU has a saved free index, use it. */
+	u32 index = this_cpu_xchg(saved_index, 0);
+	u32 old_guard;
+
+	if (likely(!index))
+		/*
+		 * Allocate a new index. No overflow is possible, because 2**32
+		 * unique basic blocks will take more space than the max size
+		 * of the kernel text segment.
+		 */
+		index = atomic_inc_return(&kcov_guard_max_index);
+
+	/*
+	 * Make sure another task is not initializing the same guard
+	 * concurrently.
+	 */
+	old_guard = cmpxchg(guard, 0, index);
+	if (unlikely(old_guard)) {
+		/* We lost the race, save the index for future use. */
+		this_cpu_write(saved_index, index);
+		return old_guard;
+	}
+	return index;
+}
+
 void notrace __sanitizer_cov_trace_pc_guard(u32 *guard)
 {
-	if (!check_kcov_mode(KCOV_MODE_TRACE_PC, current))
-		return;
+	enum kcov_mode mode = get_kcov_mode(current);
+	u32 pc_index;
 
-	kcov_append_to_buffer(current->kcov_state.trace,
-			      current->kcov_state.trace_size,
-			      canonicalize_ip(_RET_IP_));
+	switch (mode) {
+	case KCOV_MODE_UNIQUE_PC:
+		pc_index = READ_ONCE(*guard);
+		if (unlikely(!pc_index))
+			pc_index = init_pc_guard(guard);
+
+		/*
+		 * Use the bitmap for coverage deduplication. We assume both
+		 * s.bitmap and s.trace are non-NULL.
+		 */
+		if (likely(pc_index < current->kcov_state.bitmap_size))
+			if (__test_and_set_bit(pc_index,
+					       current->kcov_state.bitmap))
+				return;
+		/*
+		 * If the PC is new, or the bitmap is too small, write PC to the
+		 * trace.
+		 */
+		fallthrough;
+	case KCOV_MODE_TRACE_PC:
+		kcov_append_to_buffer(current->kcov_state.trace,
+				      current->kcov_state.trace_size,
+				      canonicalize_ip(_RET_IP_));
+		break;
+	default:
+		return;
+	}
 }
 EXPORT_SYMBOL(__sanitizer_cov_trace_pc_guard);
 
@@ -239,7 +304,7 @@ EXPORT_SYMBOL(__sanitizer_cov_trace_pc_guard_init);
 #else /* !CONFIG_KCOV_UNIQUE */
 void notrace __sanitizer_cov_trace_pc(void)
 {
-	if (!check_kcov_mode(KCOV_MODE_TRACE_PC, current))
+	if (get_kcov_mode(current) != KCOV_MODE_TRACE_PC)
 		return;
 
 	kcov_append_to_buffer(current->kcov_state.trace,
@@ -257,7 +322,7 @@ static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip)
 	u64 *trace;
 
 	t = current;
-	if (!check_kcov_mode(KCOV_MODE_TRACE_CMP, t))
+	if (get_kcov_mode(t) != KCOV_MODE_TRACE_CMP)
 		return;
 
 	ip = canonicalize_ip(ip);
@@ -376,7 +441,7 @@ static void kcov_start(struct task_struct *t, struct kcov *kcov,
 	/* Cache in task struct for performance. */
 	t->kcov_state = *state;
 	barrier();
-	/* See comment in check_kcov_mode(). */
+	/* See comment in get_kcov_mode(). */
 	WRITE_ONCE(t->kcov_mode, mode);
 }
 
@@ -410,6 +475,10 @@ static void kcov_reset(struct kcov *kcov)
 	kcov->mode = KCOV_MODE_INIT;
 	kcov->remote = false;
 	kcov->remote_size = 0;
+	kcov->state.trace = kcov->state.area;
+	kcov->state.trace_size = kcov->state.size;
+	kcov->state.bitmap = NULL;
+	kcov->state.bitmap_size = 0;
 	kcov->state.sequence++;
 }
 
@@ -550,18 +619,23 @@ static int kcov_close(struct inode *inode, struct file *filep)
 	return 0;
 }
 
-static int kcov_get_mode(unsigned long arg)
+static enum kcov_mode kcov_arg_to_mode(unsigned long arg, int *error)
 {
-	if (arg == KCOV_TRACE_PC)
+	if (arg == KCOV_TRACE_PC) {
 		return KCOV_MODE_TRACE_PC;
-	else if (arg == KCOV_TRACE_CMP)
+	} else if (arg == KCOV_TRACE_CMP) {
 #ifdef CONFIG_KCOV_ENABLE_COMPARISONS
 		return KCOV_MODE_TRACE_CMP;
 #else
-		return -ENOTSUPP;
+		if (error)
+			*error = -ENOTSUPP;
+		return KCOV_MODE_INVALID;
 #endif
-	else
-		return -EINVAL;
+	} else {
+		if (error)
+			*error = -EINVAL;
+		return KCOV_MODE_INVALID;
+	}
 }
 
 /*
@@ -596,12 +670,47 @@ static inline bool kcov_check_handle(u64 handle, bool common_valid,
 	return false;
 }
 
+static long kcov_handle_unique_enable(struct kcov *kcov,
+				      unsigned long bitmap_words)
+{
+	struct task_struct *t = current;
+
+	if (!IS_ENABLED(CONFIG_KCOV_UNIQUE))
+		return -ENOTSUPP;
+	if (kcov->mode != KCOV_MODE_INIT || !kcov->state.area)
+		return -EINVAL;
+	if (kcov->t != NULL || t->kcov != NULL)
+		return -EBUSY;
+
+	/*
+	 * Cannot use zero-sized bitmap, also the bitmap must leave at least two
+	 * words for the trace.
+	 */
+	if ((!bitmap_words) || (bitmap_words >= (kcov->state.size - 1)))
+		return -EINVAL;
+
+	kcov->state.bitmap_size = bitmap_words * sizeof(unsigned long) * 8;
+	kcov->state.bitmap = kcov->state.area;
+	kcov->state.trace_size = kcov->state.size - bitmap_words;
+	kcov->state.trace = ((unsigned long *)kcov->state.area + bitmap_words);
+
+	kcov_fault_in_area(kcov);
+	kcov->mode = KCOV_MODE_UNIQUE_PC;
+	kcov_start(t, kcov, kcov->mode, &kcov->state);
+	kcov->t = t;
+	/* Put either in kcov_task_exit() or in KCOV_DISABLE. */
+	kcov_get(kcov);
+
+	return 0;
+}
+
 static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
 			     unsigned long arg)
 {
 	struct task_struct *t;
 	unsigned long flags, unused;
-	int mode, i;
+	enum kcov_mode mode;
+	int error = 0, i;
 	struct kcov_remote_arg *remote_arg;
 	struct kcov_remote *remote;
 
@@ -619,9 +728,9 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
 		t = current;
 		if (kcov->t != NULL || t->kcov != NULL)
 			return -EBUSY;
-		mode = kcov_get_mode(arg);
-		if (mode < 0)
-			return mode;
+		mode = kcov_arg_to_mode(arg, &error);
+		if (mode == KCOV_MODE_INVALID)
+			return error;
 		kcov_fault_in_area(kcov);
 		kcov->mode = mode;
 		kcov_start(t, kcov, mode, &kcov->state);
@@ -629,6 +738,8 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
 		/* Put either in kcov_task_exit() or in KCOV_DISABLE. */
 		kcov_get(kcov);
 		return 0;
+	case KCOV_UNIQUE_ENABLE:
+		return kcov_handle_unique_enable(kcov, arg);
 	case KCOV_DISABLE:
 		/* Disable coverage for the current task. */
 		unused = arg;
@@ -647,9 +758,9 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
 		if (kcov->t != NULL || t->kcov != NULL)
 			return -EBUSY;
 		remote_arg = (struct kcov_remote_arg *)arg;
-		mode = kcov_get_mode(remote_arg->trace_mode);
-		if (mode < 0)
-			return mode;
+		mode = kcov_arg_to_mode(remote_arg->trace_mode, &error);
+		if (mode == KCOV_MODE_INVALID)
+			return error;
 		if ((unsigned long)remote_arg->area_size >
 		    LONG_MAX / sizeof(unsigned long))
 			return -EINVAL;
-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v2 09/11] kcov: add ioctl(KCOV_RESET_TRACE)

[Alexander Potapenko]

Provide a mechanism to reset the coverage for the current task
without writing directly to the coverage buffer.
This is slower, but allows the fuzzers to map the coverage buffer
as read-only, making it harder to corrupt.

Signed-off-by: Alexander Potapenko <glider@google.com>

---
v2:
 - Update code to match the new description of struct kcov_state
---
 Documentation/dev-tools/kcov.rst | 26 ++++++++++++++++++++++++++
 include/uapi/linux/kcov.h        |  1 +
 kernel/kcov.c                    | 15 +++++++++++++++
 3 files changed, 42 insertions(+)

diff --git a/Documentation/dev-tools/kcov.rst b/Documentation/dev-tools/kcov.rst
index 6446887cd1c92..e215c0651e16d 100644
--- a/Documentation/dev-tools/kcov.rst
+++ b/Documentation/dev-tools/kcov.rst
@@ -470,3 +470,29 @@ local tasks spawned by the process and the global task that handles USB bus #1:
 		perror("close"), exit(1);
 	return 0;
     }
+
+
+Resetting coverage with an KCOV_RESET_TRACE
+-------------------------------------------
+
+The ``KCOV_RESET_TRACE`` ioctl provides a mechanism to clear collected coverage
+data for the current task. It resets the program counter (PC) trace and, if
+``KCOV_UNIQUE_ENABLE`` mode is active, also zeroes the associated bitmap.
+
+The primary use case for this ioctl is to enhance safety during fuzzing.
+Normally, a user could map the kcov buffer with ``PROT_READ | PROT_WRITE`` and
+reset the trace from the user-space program. However, when fuzzing system calls,
+the kernel itself might inadvertently write to this shared buffer, corrupting
+the coverage data.
+
+To prevent this, a fuzzer can map the buffer with ``PROT_READ`` and use
+``ioctl(fd, KCOV_RESET_TRACE, 0)`` to safely clear the buffer from the kernel
+side before each fuzzing iteration.
+
+Note that:
+
+* This ioctl is safer but slower than directly writing to the shared memory
+  buffer due to the overhead of a system call.
+* ``KCOV_RESET_TRACE`` is itself a system call, and its execution will be traced
+  by kcov. Consequently, immediately after the ioctl returns, cover[0] will be
+  greater than 0.
diff --git a/include/uapi/linux/kcov.h b/include/uapi/linux/kcov.h
index e743ee011eeca..8ab77cc3afa76 100644
--- a/include/uapi/linux/kcov.h
+++ b/include/uapi/linux/kcov.h
@@ -23,6 +23,7 @@ struct kcov_remote_arg {
 #define KCOV_DISABLE			_IO('c', 101)
 #define KCOV_REMOTE_ENABLE		_IOW('c', 102, struct kcov_remote_arg)
 #define KCOV_UNIQUE_ENABLE		_IOW('c', 103, unsigned long)
+#define KCOV_RESET_TRACE		_IO('c', 104)
 
 enum {
 	/*
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 2a4edbaad50d0..1693004d89764 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -740,6 +740,21 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
 		return 0;
 	case KCOV_UNIQUE_ENABLE:
 		return kcov_handle_unique_enable(kcov, arg);
+	case KCOV_RESET_TRACE:
+		unused = arg;
+		if (unused != 0 || current->kcov != kcov)
+			return -EINVAL;
+		t = current;
+		if (WARN_ON(kcov->t != t))
+			return -EINVAL;
+		mode = kcov->mode;
+		if (mode < KCOV_MODE_TRACE_PC)
+			return -EINVAL;
+		if (kcov->state.bitmap)
+			bitmap_zero(kcov->state.bitmap,
+				    kcov->state.bitmap_size);
+		WRITE_ONCE(kcov->state.trace[0], 0);
+		return 0;
 	case KCOV_DISABLE:
 		/* Disable coverage for the current task. */
 		unused = arg;
-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v2 10/11] kcov: selftests: add kcov_test

[Alexander Potapenko]

Implement test fixtures for testing different combinations of coverage
collection modes:
 - unique and non-unique coverage;
 - collecting PCs and comparison arguments;
 - mapping the buffer as RO and RW.

To build:
 $ make -C tools/testing/selftests/kcov kcov_test

Signed-off-by: Alexander Potapenko <glider@google.com>
---
 MAINTAINERS                              |   1 +
 tools/testing/selftests/kcov/Makefile    |   6 +
 tools/testing/selftests/kcov/kcov_test.c | 364 +++++++++++++++++++++++
 3 files changed, 371 insertions(+)
 create mode 100644 tools/testing/selftests/kcov/Makefile
 create mode 100644 tools/testing/selftests/kcov/kcov_test.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 5bbc78b0fa6ed..0ec909e085077 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12833,6 +12833,7 @@ F:	include/linux/kcov_types.h
 F:	include/uapi/linux/kcov.h
 F:	kernel/kcov.c
 F:	scripts/Makefile.kcov
+F:	tools/testing/selftests/kcov/
 
 KCSAN
 M:	Marco Elver <elver@google.com>
diff --git a/tools/testing/selftests/kcov/Makefile b/tools/testing/selftests/kcov/Makefile
new file mode 100644
index 0000000000000..08abf8b60bcf9
--- /dev/null
+++ b/tools/testing/selftests/kcov/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-only
+LDFLAGS += -static
+
+TEST_GEN_PROGS := kcov_test
+
+include ../lib.mk
diff --git a/tools/testing/selftests/kcov/kcov_test.c b/tools/testing/selftests/kcov/kcov_test.c
new file mode 100644
index 0000000000000..4d3ca41f28af4
--- /dev/null
+++ b/tools/testing/selftests/kcov/kcov_test.c
@@ -0,0 +1,364 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test the kernel coverage (/sys/kernel/debug/kcov).
+ *
+ * Copyright 2025 Google LLC.
+ */
+#include <fcntl.h>
+#include <linux/kcov.h>
+#include <stdint.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "../kselftest_harness.h"
+
+/* Normally these defines should be provided by linux/kcov.h, but they aren't there yet. */
+#define KCOV_UNIQUE_ENABLE _IOW('c', 103, unsigned long)
+#define KCOV_RESET_TRACE _IO('c', 104)
+
+#define COVER_SIZE (64 << 10)
+#define BITMAP_SIZE (4 << 10)
+
+#define DEBUG_COVER_PCS 0
+
+FIXTURE(kcov)
+{
+	int fd;
+	unsigned long *mapping;
+	size_t mapping_size;
+};
+
+FIXTURE_VARIANT(kcov)
+{
+	int mode;
+	bool fast_reset;
+	bool map_readonly;
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(kcov, mode_trace_pc)
+{
+	/* clang-format on */
+	.mode = KCOV_TRACE_PC,
+	.fast_reset = true,
+	.map_readonly = false,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(kcov, mode_trace_cmp)
+{
+	/* clang-format on */
+	.mode = KCOV_TRACE_CMP,
+	.fast_reset = true,
+	.map_readonly = false,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(kcov, reset_ioctl_rw)
+{
+	/* clang-format on */
+	.mode = KCOV_TRACE_PC,
+	.fast_reset = false,
+	.map_readonly = false,
+};
+
+FIXTURE_VARIANT_ADD(kcov, reset_ioctl_ro)
+/* clang-format off */
+{
+	/* clang-format on */
+	.mode = KCOV_TRACE_PC,
+	.fast_reset = false,
+	.map_readonly = true,
+};
+
+int kcov_open_init(struct __test_metadata *_metadata, unsigned long size,
+		   int prot, unsigned long **out_mapping)
+{
+	unsigned long *mapping;
+
+	/* A single fd descriptor allows coverage collection on a single thread. */
+	int fd = open("/sys/kernel/debug/kcov", O_RDWR);
+
+	ASSERT_NE(fd, -1)
+	{
+		perror("open");
+	}
+
+	EXPECT_EQ(ioctl(fd, KCOV_INIT_TRACE, COVER_SIZE), 0)
+	{
+		perror("ioctl KCOV_INIT_TRACE");
+		close(fd);
+	}
+
+	/* Mmap buffer shared between kernel- and user-space. */
+	mapping = (unsigned long *)mmap(NULL, size * sizeof(unsigned long),
+					prot, MAP_SHARED, fd, 0);
+	ASSERT_NE((void *)mapping, MAP_FAILED)
+	{
+		perror("mmap");
+		close(fd);
+	}
+	*out_mapping = mapping;
+
+	return fd;
+}
+
+FIXTURE_SETUP(kcov)
+{
+	int prot = variant->map_readonly ? PROT_READ : (PROT_READ | PROT_WRITE);
+
+	/* Read-only mapping is incompatible with fast reset. */
+	ASSERT_FALSE(variant->map_readonly && variant->fast_reset);
+
+	self->mapping_size = COVER_SIZE;
+	self->fd = kcov_open_init(_metadata, self->mapping_size, prot,
+				  &(self->mapping));
+
+	/* Enable coverage collection on the current thread. */
+	EXPECT_EQ(ioctl(self->fd, KCOV_ENABLE, variant->mode), 0)
+	{
+		perror("ioctl KCOV_ENABLE");
+		munmap(self->mapping, COVER_SIZE * sizeof(unsigned long));
+		close(self->fd);
+	}
+}
+
+void kcov_uninit_close(struct __test_metadata *_metadata, int fd,
+		       unsigned long *mapping, size_t size)
+{
+	/* Disable coverage collection for the current thread. */
+	EXPECT_EQ(ioctl(fd, KCOV_DISABLE, 0), 0)
+	{
+		perror("ioctl KCOV_DISABLE");
+	}
+
+	/* Free resources. */
+	EXPECT_EQ(munmap(mapping, size * sizeof(unsigned long)), 0)
+	{
+		perror("munmap");
+	}
+
+	EXPECT_EQ(close(fd), 0)
+	{
+		perror("close");
+	}
+}
+
+FIXTURE_TEARDOWN(kcov)
+{
+	kcov_uninit_close(_metadata, self->fd, self->mapping,
+			  self->mapping_size);
+}
+
+void dump_collected_pcs(struct __test_metadata *_metadata, unsigned long *cover,
+			size_t start, size_t end)
+{
+	int i = 0;
+
+	TH_LOG("Collected %lu PCs", end - start);
+#if DEBUG_COVER_PCS
+	for (i = start; i < end; i++)
+		TH_LOG("0x%lx", cover[i + 1]);
+#endif
+}
+
+/* Coverage collection helper without assertions. */
+unsigned long collect_coverage_unchecked(struct __test_metadata *_metadata,
+					 unsigned long *cover, bool dump)
+{
+	unsigned long before, after;
+
+	before = __atomic_load_n(&cover[0], __ATOMIC_RELAXED);
+	/*
+	 * Call the target syscall call. Here we use read(-1, NULL, 0) as an example.
+	 * This will likely return an error (-EFAULT or -EBADF), but the goal is to
+	 * collect coverage for the syscall's entry/exit paths.
+	 */
+	read(-1, NULL, 0);
+
+	after = __atomic_load_n(&cover[0], __ATOMIC_RELAXED);
+
+	if (dump)
+		dump_collected_pcs(_metadata, cover, before, after);
+	return after - before;
+}
+
+unsigned long collect_coverage_once(struct __test_metadata *_metadata,
+				    unsigned long *cover)
+{
+	unsigned long collected =
+		collect_coverage_unchecked(_metadata, cover, /*dump*/ true);
+
+	/* Coverage must be non-zero. */
+	EXPECT_GT(collected, 0);
+	return collected;
+}
+
+void reset_coverage(struct __test_metadata *_metadata, bool fast, int fd,
+		    unsigned long *mapping)
+{
+	unsigned long count;
+
+	if (fast) {
+		__atomic_store_n(&mapping[0], 0, __ATOMIC_RELAXED);
+	} else {
+		EXPECT_EQ(ioctl(fd, KCOV_RESET_TRACE, 0), 0)
+		{
+			perror("ioctl KCOV_RESET_TRACE");
+		}
+		count = __atomic_load_n(&mapping[0], __ATOMIC_RELAXED);
+		EXPECT_NE(count, 0);
+	}
+}
+
+TEST_F(kcov, kcov_basic_syscall_coverage)
+{
+	unsigned long first, second, before, after, i;
+
+	/* Reset coverage that may be left over from the fixture setup. */
+	reset_coverage(_metadata, variant->fast_reset, self->fd, self->mapping);
+
+	/* Collect the coverage for a single syscall two times in a row. */
+	first = collect_coverage_once(_metadata, self->mapping);
+	second = collect_coverage_once(_metadata, self->mapping);
+	/* Collected coverage should not differ too much. */
+	EXPECT_GT(first * 10, second);
+	EXPECT_GT(second * 10, first);
+
+	/* Now reset the buffer and collect the coverage again. */
+	reset_coverage(_metadata, variant->fast_reset, self->fd, self->mapping);
+	collect_coverage_once(_metadata, self->mapping);
+
+	/* Now try many times to fill up the buffer. */
+	reset_coverage(_metadata, variant->fast_reset, self->fd, self->mapping);
+	while (collect_coverage_unchecked(_metadata, self->mapping,
+					  /*dump*/ false)) {
+		/* Do nothing. */
+	}
+	before = __atomic_load_n(&(self->mapping[0]), __ATOMIC_RELAXED);
+	/*
+	 * Resetting with ioctl may still generate some coverage, but much less
+	 * than there was before.
+	 */
+	reset_coverage(_metadata, variant->fast_reset, self->fd, self->mapping);
+	after = __atomic_load_n(&(self->mapping[0]), __ATOMIC_RELAXED);
+	EXPECT_GT(before, after);
+	/* Collecting coverage after reset will now succeed. */
+	collect_coverage_once(_metadata, self->mapping);
+}
+
+FIXTURE(kcov_uniq)
+{
+	int fd;
+	unsigned long *mapping;
+	size_t mapping_size;
+	unsigned long *bitmap;
+	size_t bitmap_size;
+	unsigned long *cover;
+	size_t cover_size;
+};
+
+FIXTURE_VARIANT(kcov_uniq)
+{
+	bool fast_reset;
+	bool map_readonly;
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(kcov_uniq, fast_rw)
+{
+	/* clang-format on */
+	.fast_reset = true,
+	.map_readonly = false,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(kcov_uniq, slow_rw)
+{
+	/* clang-format on */
+	.fast_reset = false,
+	.map_readonly = false,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(kcov_uniq, slow_ro)
+{
+	/* clang-format on */
+	.fast_reset = false,
+	.map_readonly = true,
+};
+
+FIXTURE_SETUP(kcov_uniq)
+{
+	int prot = variant->map_readonly ? PROT_READ : (PROT_READ | PROT_WRITE);
+
+	/* Read-only mapping is incompatible with fast reset. */
+	ASSERT_FALSE(variant->map_readonly && variant->fast_reset);
+
+	self->mapping_size = COVER_SIZE;
+	self->fd = kcov_open_init(_metadata, self->mapping_size, prot,
+				  &(self->mapping));
+
+	/* Enable coverage collection on the current thread. */
+	EXPECT_EQ(ioctl(self->fd, KCOV_UNIQUE_ENABLE, BITMAP_SIZE), 0)
+	{
+		perror("ioctl KCOV_ENABLE");
+		munmap(self->mapping, COVER_SIZE * sizeof(unsigned long));
+		close(self->fd);
+	}
+}
+
+FIXTURE_TEARDOWN(kcov_uniq)
+{
+	kcov_uninit_close(_metadata, self->fd, self->mapping,
+			  self->mapping_size);
+}
+
+TEST_F(kcov_uniq, kcov_uniq_coverage)
+{
+	unsigned long first, second, before, after, i;
+
+	/* Reset coverage that may be left over from the fixture setup. */
+	reset_coverage(_metadata, variant->fast_reset, self->fd, self->mapping);
+
+	/*
+	 * Collect the coverage for a single syscall two times in a row.
+	 * Use collect_coverage_unchecked(), because it may return zero coverage.
+	 */
+	first = collect_coverage_unchecked(_metadata, self->mapping,
+					   /*dump*/ true);
+	second = collect_coverage_unchecked(_metadata, self->mapping,
+					    /*dump*/ true);
+
+	/* Now reset the buffer and collect the coverage again. */
+	reset_coverage(_metadata, variant->fast_reset, self->fd, self->mapping);
+	collect_coverage_once(_metadata, self->mapping);
+
+	/* Now try many times to saturate the unique coverage bitmap. */
+	reset_coverage(_metadata, variant->fast_reset, self->fd, self->mapping);
+	for (i = 0; i < 1000; i++)
+		collect_coverage_unchecked(_metadata, self->mapping,
+					   /*dump*/ false);
+	/* Another invocation of collect_coverage_unchecked() should not produce new coverage. */
+	EXPECT_EQ(collect_coverage_unchecked(_metadata, self->mapping,
+					     /*dump*/ false),
+		  0);
+
+	before = __atomic_load_n(&(self->mapping[0]), __ATOMIC_RELAXED);
+	/*
+	 * Resetting with ioctl may still generate some coverage, but much less
+	 * than there was before.
+	 */
+	reset_coverage(_metadata, variant->fast_reset, self->fd, self->mapping);
+	after = __atomic_load_n(&(self->mapping[0]), __ATOMIC_RELAXED);
+	EXPECT_GT(before, after);
+	/* Collecting coverage after reset will now succeed. */
+	collect_coverage_once(_metadata, self->mapping);
+}
+
+TEST_HARNESS_MAIN
-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v2 11/11] kcov: use enum kcov_mode in kcov_mode_enabled()

[Alexander Potapenko]

Replace the remaining declarations of `unsigned int mode` with
`enum kcov_mode mode`. No functional change.

Signed-off-by: Alexander Potapenko <glider@google.com>
---
 kernel/kcov.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/kcov.c b/kernel/kcov.c
index 1693004d89764..62ce4c65f79fa 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -951,7 +951,7 @@ static const struct file_operations kcov_fops = {
  * collecting coverage and copies all collected coverage into the kcov area.
  */
 
-static inline bool kcov_mode_enabled(unsigned int mode)
+static inline bool kcov_mode_enabled(enum kcov_mode mode)
 {
 	return (mode & ~KCOV_IN_CTXSW) != KCOV_MODE_DISABLED;
 }
@@ -959,7 +959,7 @@ static inline bool kcov_mode_enabled(unsigned int mode)
 static void kcov_remote_softirq_start(struct task_struct *t)
 {
 	struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
-	unsigned int mode;
+	enum kcov_mode mode;
 
 	mode = READ_ONCE(t->kcov_mode);
 	barrier();
@@ -1135,7 +1135,7 @@ void kcov_remote_stop(void)
 {
 	struct task_struct *t = current;
 	struct kcov *kcov;
-	unsigned int mode;
+	enum kcov_mode mode;
 	void *area, *trace;
 	unsigned int size, trace_size;
 	int sequence;
-- 
2.50.0.727.gbf7dc18ff4-goog

Re: [PATCH v2 01/11] x86: kcov: disable instrumentation of arch/x86/kernel/tsc.c

[Peter Zijlstra]

On Thu, Jun 26, 2025 at 03:41:48PM +0200, Alexander Potapenko wrote:
> sched_clock() appears to be called from interrupts, producing spurious
> coverage, as reported by CONFIG_KCOV_SELFTEST:

NMI context even. But I'm not sure how this leads to problems. What does
spurious coverage even mean?

Re: [PATCH v2 02/11] kcov: apply clang-format to kcov code

[Peter Zijlstra]

On Thu, Jun 26, 2025 at 03:41:49PM +0200, Alexander Potapenko wrote:
> kcov used to obey clang-format style, but somehow diverged over time.
> This patch applies clang-format to kernel/kcov.c and
> include/linux/kcov.h, no functional change.

I'm not sure I agree this is in fact a good thing. Very questionable
style choices made.

I had to kill clang-format hard in my nvim-lsp-clangd setup, because
clang-format is such a piece of shit.


> -static inline void kcov_task_init(struct task_struct *t) {}
> -static inline void kcov_task_exit(struct task_struct *t) {}
> -static inline void kcov_prepare_switch(struct task_struct *t) {}
> -static inline void kcov_finish_switch(struct task_struct *t) {}
> -static inline void kcov_remote_start(u64 handle) {}
> -static inline void kcov_remote_stop(void) {}
> +static inline void kcov_task_init(struct task_struct *t)
> +{
> +}
> +static inline void kcov_task_exit(struct task_struct *t)
> +{
> +}
> +static inline void kcov_prepare_switch(struct task_struct *t)
> +{
> +}
> +static inline void kcov_finish_switch(struct task_struct *t)
> +{
> +}
> +static inline void kcov_remote_start(u64 handle)
> +{
> +}
> +static inline void kcov_remote_stop(void)
> +{
> +}

This is not an improvement.

> @@ -52,36 +53,36 @@ struct kcov {
>  	 *  - task with enabled coverage (we can't unwire it from another task)
>  	 *  - each code section for remote coverage collection
>  	 */
> -	refcount_t		refcount;
> +	refcount_t refcount;
>  	/* The lock protects mode, size, area and t. */
> -	spinlock_t		lock;
> -	enum kcov_mode		mode;
> +	spinlock_t lock;
> +	enum kcov_mode mode;
>  	/* Size of arena (in long's). */
> -	unsigned int		size;
> +	unsigned int size;
>  	/* Coverage buffer shared with user space. */
> -	void			*area;
> +	void *area;
>  	/* Task for which we collect coverage, or NULL. */
> -	struct task_struct	*t;
> +	struct task_struct *t;
>  	/* Collecting coverage from remote (background) threads. */
> -	bool			remote;
> +	bool remote;
>  	/* Size of remote area (in long's). */
> -	unsigned int		remote_size;
> +	unsigned int remote_size;
>  	/*
>  	 * Sequence is incremented each time kcov is reenabled, used by
>  	 * kcov_remote_stop(), see the comment there.
>  	 */
> -	int			sequence;
> +	int sequence;
>  };
>  
>  struct kcov_remote_area {
> -	struct list_head	list;
> -	unsigned int		size;
> +	struct list_head list;
> +	unsigned int size;
>  };
>  
>  struct kcov_remote {
> -	u64			handle;
> -	struct kcov		*kcov;
> -	struct hlist_node	hnode;
> +	u64 handle;
> +	struct kcov *kcov;
> +	struct hlist_node hnode;
>  };
>  
>  static DEFINE_SPINLOCK(kcov_remote_lock);
> @@ -89,14 +90,14 @@ static DEFINE_HASHTABLE(kcov_remote_map, 4);
>  static struct list_head kcov_remote_areas = LIST_HEAD_INIT(kcov_remote_areas);
>  
>  struct kcov_percpu_data {
> -	void			*irq_area;
> -	local_lock_t		lock;
> -
> -	unsigned int		saved_mode;
> -	unsigned int		saved_size;
> -	void			*saved_area;
> -	struct kcov		*saved_kcov;
> -	int			saved_sequence;
> +	void *irq_area;
> +	local_lock_t lock;
> +
> +	unsigned int saved_mode;
> +	unsigned int saved_size;
> +	void *saved_area;
> +	struct kcov *saved_kcov;
> +	int saved_sequence;
>  };
>  
>  static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {

This is just plain wrong. Making something that was readable into a
trainwreck.


Please either teach clang-format sensible style choices, or refrain from
using it.

Re: [PATCH v2 06/11] kcov: x86: introduce CONFIG_KCOV_UNIQUE

[Peter Zijlstra]

On Thu, Jun 26, 2025 at 03:41:53PM +0200, Alexander Potapenko wrote:
> The new config switches coverage instrumentation to using
>   __sanitizer_cov_trace_pc_guard(u32 *guard)
> instead of
>   __sanitizer_cov_trace_pc(void)
> 
> This relies on Clang's -fsanitize-coverage=trace-pc-guard flag [1].
> 
> Each callback receives a unique 32-bit guard variable residing in the
> __sancov_guards section. Those guards can be used by kcov to deduplicate
> the coverage on the fly.

This sounds like a *LOT* of data; how big is this for a typical kernel
build?

Re: [PATCH v2 08/11] kcov: add ioctl(KCOV_UNIQUE_ENABLE)

[Peter Zijlstra]

On Thu, Jun 26, 2025 at 03:41:55PM +0200, Alexander Potapenko wrote:
> ioctl(KCOV_UNIQUE_ENABLE) enables collection of deduplicated coverage
> in the presence of CONFIG_KCOV_ENABLE_GUARDS.
> 
> The buffer shared with the userspace is divided in two parts, one holding
> a bitmap, and the other one being the trace. The single parameter of
> ioctl(KCOV_UNIQUE_ENABLE) determines the number of words used for the
> bitmap.
> 
> Each __sanitizer_cov_trace_pc_guard() instrumentation hook receives a
> pointer to a unique guard variable. Upon the first call of each hook,
> the guard variable is initialized with a unique integer, which is used to
> map those hooks to bits in the bitmap. In the new coverage collection mode,
> the kernel first checks whether the bit corresponding to a particular hook
> is set, and then, if it is not, the PC is written into the trace buffer,
> and the bit is set.

I am somewhat confused; the clang documentation states that every edge
will have a guard variable.

So if I have code like:

foo:	Jcc	foobar
...
bar:	Jcc	foobar
...
foobar:

Then we get two guard variables for the one foobar target?

But from a coverage PoV you don't particularly care about the edges; you
only care you hit the instruction. Combined with the naming of the hook:
'trace_pc_guard', which reads to me like: program-counter guard, suggesting
the guard is in fact per PC or target node, not per edge.

So which is it?

Also, dynamic edges are very hard to allocate guard variables for, while
target guards are trivial, even in the face of dynamic edges.

A further consideration is that the number of edges can vastly outnumber
the number of nodes, again suggesting that node guards might be better.

Re: [PATCH v2 01/11] x86: kcov: disable instrumentation of arch/x86/kernel/tsc.c

[Alexander Potapenko]

On Fri, Jun 27, 2025 at 9:59 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Jun 26, 2025 at 03:41:48PM +0200, Alexander Potapenko wrote:
> > sched_clock() appears to be called from interrupts, producing spurious
> > coverage, as reported by CONFIG_KCOV_SELFTEST:
>
> NMI context even. But I'm not sure how this leads to problems. What does
> spurious coverage even mean?

This leads to KCOV collecting slightly different coverage when
executing the same syscall multiple times.
For syzkaller that means higher chance to pick a less interesting
input incorrectly assuming it produced some new coverage.

There's a similar discussion at
https://lore.kernel.org/all/20240619111936.GK31592@noisy.programming.kicks-ass.net/T/#u

Re: [PATCH v2 07/11] kcov: add trace and trace_size to struct kcov_state

[kernel test robot]

Hi Alexander,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/x86/core]
[cannot apply to akpm-mm/mm-everything tip/sched/core arnd-asm-generic/master akpm-mm/mm-nonmm-unstable masahiroy-kbuild/for-next masahiroy-kbuild/fixes shuah-kselftest/next shuah-kselftest/fixes linus/master mcgrof/modules-next v6.16-rc3 next-20250627]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Alexander-Potapenko/x86-kcov-disable-instrumentation-of-arch-x86-kernel-tsc-c/20250626-214703
base:   tip/x86/core
patch link:    https://lore.kernel.org/r/20250626134158.3385080-8-glider%40google.com
patch subject: [PATCH v2 07/11] kcov: add trace and trace_size to struct kcov_state
config: x86_64-buildonly-randconfig-004-20250627 (https://download.01.org/0day-ci/archive/20250627/202506271946.HACEE9U0-lkp@intel.com/config)
compiler: clang version 20.1.7 (https://github.com/llvm/llvm-project 6146a88f60492b520a36f8f8f3231e15f3cc6082)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250627/202506271946.HACEE9U0-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506271946.HACEE9U0-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> kernel/kcov.c:1013:15: warning: variable 'trace' set but not used [-Wunused-but-set-variable]
    1013 |         void *area, *trace;
         |                      ^
>> kernel/kcov.c:1014:21: warning: variable 'trace_size' set but not used [-Wunused-but-set-variable]
    1014 |         unsigned int size, trace_size;
         |                            ^
   2 warnings generated.


vim +/trace +1013 kernel/kcov.c

  1006	
  1007	/* See the comment before kcov_remote_start() for usage details. */
  1008	void kcov_remote_stop(void)
  1009	{
  1010		struct task_struct *t = current;
  1011		struct kcov *kcov;
  1012		unsigned int mode;
> 1013		void *area, *trace;
> 1014		unsigned int size, trace_size;
  1015		int sequence;
  1016		unsigned long flags;
  1017	
  1018		if (!in_task() && !in_softirq_really())
  1019			return;
  1020	
  1021		local_lock_irqsave(&kcov_percpu_data.lock, flags);
  1022	
  1023		mode = READ_ONCE(t->kcov_mode);
  1024		barrier();
  1025		if (!kcov_mode_enabled(mode)) {
  1026			local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
  1027			return;
  1028		}
  1029		/*
  1030		 * When in softirq, check if the corresponding kcov_remote_start()
  1031		 * actually found the remote handle and started collecting coverage.
  1032		 */
  1033		if (in_serving_softirq() && !t->kcov_softirq) {
  1034			local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
  1035			return;
  1036		}
  1037		/* Make sure that kcov_softirq is only set when in softirq. */
  1038		if (WARN_ON(!in_serving_softirq() && t->kcov_softirq)) {
  1039			local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
  1040			return;
  1041		}
  1042	
  1043		kcov = t->kcov;
  1044		area = t->kcov_state.area;
  1045		size = t->kcov_state.size;
  1046		trace = t->kcov_state.trace;
  1047		trace_size = t->kcov_state.trace_size;
  1048		sequence = t->kcov_state.sequence;
  1049	
  1050		kcov_stop(t);
  1051		if (in_serving_softirq()) {
  1052			t->kcov_softirq = 0;
  1053			kcov_remote_softirq_stop(t);
  1054		}
  1055	
  1056		spin_lock(&kcov->lock);
  1057		/*
  1058		 * KCOV_DISABLE could have been called between kcov_remote_start()
  1059		 * and kcov_remote_stop(), hence the sequence check.
  1060		 */
  1061		if (sequence == kcov->state.sequence && kcov->remote)
  1062			kcov_move_area(kcov->mode, kcov->state.area, kcov->state.size,
  1063				       area);
  1064		spin_unlock(&kcov->lock);
  1065	
  1066		if (in_task()) {
  1067			spin_lock(&kcov_remote_lock);
  1068			kcov_remote_area_put(area, size);
  1069			spin_unlock(&kcov_remote_lock);
  1070		}
  1071	
  1072		local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
  1073	
  1074		/* Get in kcov_remote_start(). */
  1075		kcov_put(kcov);
  1076	}
  1077	EXPORT_SYMBOL(kcov_remote_stop);
  1078	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 02/11] kcov: apply clang-format to kcov code

[Alexander Potapenko]

On Fri, Jun 27, 2025 at 10:02 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Jun 26, 2025 at 03:41:49PM +0200, Alexander Potapenko wrote:
> > kcov used to obey clang-format style, but somehow diverged over time.
> > This patch applies clang-format to kernel/kcov.c and
> > include/linux/kcov.h, no functional change.
>
> I'm not sure I agree this is in fact a good thing. Very questionable
> style choices made.

Adding Miguel, who maintains clang-format.

> I had to kill clang-format hard in my nvim-lsp-clangd setup, because
> clang-format is such a piece of shit.

Random fact that I didn't know before: 1788 out of 35503 kernel .c
files are already formatted according to the clang-format style.
(I expected the number to be much lower)

>
> > -static inline void kcov_task_init(struct task_struct *t) {}
> > -static inline void kcov_task_exit(struct task_struct *t) {}
> > -static inline void kcov_prepare_switch(struct task_struct *t) {}
> > -static inline void kcov_finish_switch(struct task_struct *t) {}
> > -static inline void kcov_remote_start(u64 handle) {}
> > -static inline void kcov_remote_stop(void) {}
> > +static inline void kcov_task_init(struct task_struct *t)
> > +{
> > +}
> > +static inline void kcov_task_exit(struct task_struct *t)
> > +{
> > +}
> > +static inline void kcov_prepare_switch(struct task_struct *t)
> > +{
> > +}
> > +static inline void kcov_finish_switch(struct task_struct *t)
> > +{
> > +}
> > +static inline void kcov_remote_start(u64 handle)
> > +{
> > +}
> > +static inline void kcov_remote_stop(void)
> > +{
> > +}
>
> This is not an improvement.

Fair enough.
I think we can fix this by setting AllowShortFunctionsOnASingleLine:
Empty, SplitEmptyFunction: false in .clang-format

Miguel, do you think this is a reasonable change?


> >
> >  struct kcov_percpu_data {
> > -     void                    *irq_area;
> > -     local_lock_t            lock;
> > -
> > -     unsigned int            saved_mode;
> > -     unsigned int            saved_size;
> > -     void                    *saved_area;
> > -     struct kcov             *saved_kcov;
> > -     int                     saved_sequence;
> > +     void *irq_area;
> > +     local_lock_t lock;
> > +
> > +     unsigned int saved_mode;
> > +     unsigned int saved_size;
> > +     void *saved_area;
> > +     struct kcov *saved_kcov;
> > +     int saved_sequence;
> >  };
> >
> >  static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {
>
> This is just plain wrong. Making something that was readable into a
> trainwreck.

Setting AlignConsecutiveDeclarations: AcrossEmptyLinesAndComments will
replace the above with the following diff:

 struct kcov_percpu_data {
-       void                    *irq_area;
-       local_lock_t            lock;
-
-       unsigned int            saved_mode;
-       unsigned int            saved_size;
-       void                    *saved_area;
-       struct kcov             *saved_kcov;
-       int                     saved_sequence;
+       void        *irq_area;
+       local_lock_t lock;
+
+       unsigned int saved_mode;
+       unsigned int saved_size;
+       void        *saved_area;
+       struct kcov *saved_kcov;
+       int          saved_sequence;
 };

(a bit denser, plus it aligns the variable names, not the pointer signs)
Does this look better?

>
> Please either teach clang-format sensible style choices, or refrain from
> using it.



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Re: [PATCH v2 08/11] kcov: add ioctl(KCOV_UNIQUE_ENABLE)

[Alexander Potapenko]

On Fri, Jun 27, 2025 at 10:27 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Jun 26, 2025 at 03:41:55PM +0200, Alexander Potapenko wrote:
> > ioctl(KCOV_UNIQUE_ENABLE) enables collection of deduplicated coverage
> > in the presence of CONFIG_KCOV_ENABLE_GUARDS.
> >
> > The buffer shared with the userspace is divided in two parts, one holding
> > a bitmap, and the other one being the trace. The single parameter of
> > ioctl(KCOV_UNIQUE_ENABLE) determines the number of words used for the
> > bitmap.
> >
> > Each __sanitizer_cov_trace_pc_guard() instrumentation hook receives a
> > pointer to a unique guard variable. Upon the first call of each hook,
> > the guard variable is initialized with a unique integer, which is used to
> > map those hooks to bits in the bitmap. In the new coverage collection mode,
> > the kernel first checks whether the bit corresponding to a particular hook
> > is set, and then, if it is not, the PC is written into the trace buffer,
> > and the bit is set.
>
> I am somewhat confused; the clang documentation states that every edge
> will have a guard variable.

There are two modes, -fsanitize-coverage=edge and
-fsanitize-coverage=bb, with edge being the default one.

When instrumenting basic blocks, the compiler inserts a call to
__sanitizer_cov_trace_pc at the beginning of every basic block in the
LLVM IR (well, not exactly, because some basic blocks are considered
redundant; this behavior can be disabled by passing
-fsanitize-coverage=no-prune).

Now, instrumenting the edges is actually very similar to basic blocks:
we just find critical edges of the callgraph, add a new basic block in
the middle of those edges, then instrument basic blocks like we did
before.
For what it's worth, the number of coverage hooks does not usually
become quadratic when instrumenting edges, we only add a handful of
new basic blocks.

>
> So if I have code like:
>
> foo:    Jcc     foobar
> ...
> bar:    Jcc     foobar
> ...
> foobar:
>
> Then we get two guard variables for the one foobar target?

Correct.
Note that in this sense coverage guards behave exactly similar to
-fsanitize-coverage=trace-pc that we used before.

Consider the following example (also available at
https://godbolt.org/z/TcMT8W45o):

void bar();
void foo(int *a) {
  if (a)
    *a = 0;
  bar();
}

Compiling it with different coverage options may give an idea of how
{trace-pc,trace-pc-guard}x{bb,edge} relate to each other:

# Coverage we use today, instrumenting edges:
$ clang -fsanitize-coverage=trace-pc -S -O2
# Guard coverage proposed in the patch, instrumenting edges
$ clang -fsanitize-coverage=trace-pc-guard -S -O2
# PC coverage with basic block instrumentation
$ clang -fsanitize-coverage=trace-pc,bb -S -O2
# Guard coverage with basic block instrumentation
$ clang -fsanitize-coverage=trace-pc-guard,bb -S -O2

The number of coverage calls doesn't change if I change trace-pc to
trace-pc-guard.
-fsanitize-coverage=bb produces one call less than
-fsanitize-coverage=edge (aka the default mode).

>
> But from a coverage PoV you don't particularly care about the edges; you
> only care you hit the instruction.

Fuzzing engines care about various signals of program state, not just
basic block coverage.
There's a tradeoff between precisely distinguishing between two states
(e.g. "last time I called a()-->b()->c() to get to this line, now this
is a()->d()->e()->f()-c(), let's treat it differently") and bloating
the fuzzing corpus with redundant information.
Our experience shows that using such makeshift edge coverage produces
better results than just instrumenting basic blocks, but collecting
longer traces of basic blocks is unnecessary.

> Combined with the naming of the hook:
> 'trace_pc_guard', which reads to me like: program-counter guard, suggesting
> the guard is in fact per PC or target node, not per edge.
>
> So which is it?

The same hook is used in both the BB and the edge modes, because in
both cases we are actually instrumenting basic blocks.

>
> Also, dynamic edges are very hard to allocate guard variables for, while
> target guards are trivial, even in the face of dynamic edges.

All edges are known statically, because they are within the same
function - calls between functions are not considered edges.

> A further consideration is that the number of edges can vastly outnumber
> the number of nodes, again suggesting that node guards might be better.
>

For the above reason, this isn't a problem.
In the current setup (with edges, without guards), the number of
instrumentation points in vmlinux is on the order of single-digit
millions.

Re: [PATCH v2 06/11] kcov: x86: introduce CONFIG_KCOV_UNIQUE

[Alexander Potapenko]

On Fri, Jun 27, 2025 at 10:11 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Jun 26, 2025 at 03:41:53PM +0200, Alexander Potapenko wrote:
> > The new config switches coverage instrumentation to using
> >   __sanitizer_cov_trace_pc_guard(u32 *guard)
> > instead of
> >   __sanitizer_cov_trace_pc(void)
> >
> > This relies on Clang's -fsanitize-coverage=trace-pc-guard flag [1].
> >
> > Each callback receives a unique 32-bit guard variable residing in the
> > __sancov_guards section. Those guards can be used by kcov to deduplicate
> > the coverage on the fly.
>
> This sounds like a *LOT* of data; how big is this for a typical kernel
> build?

I have a 1.6Gb sized vmlinux, which has a .text section of 176Mb.
There are 1809419 calls to __sanitizer_cov_trace_pc_guard, and the
__sancov_guards section has a size of 6Mb, which are only allocated at
runtime.

If we take a vmlinux image from syzbot (e.g.
https://storage.googleapis.com/syzbot-assets/dadedf20b2e3/vmlinux-67a99386.xz),
its .text section is 166Mb, and there are 1893023 calls to
__sanitizer_cov_trace_pc, which will translate to exactly the same
number of __sanitizer_cov_trace_pc_guard, if we apply the unique
coverage instrumentation.

Re: [PATCH v2 06/11] kcov: x86: introduce CONFIG_KCOV_UNIQUE

[Alexander Potapenko]

On Fri, Jun 27, 2025 at 4:24 PM Alexander Potapenko <glider@google.com> wrote:
>
> On Fri, Jun 27, 2025 at 10:11 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Thu, Jun 26, 2025 at 03:41:53PM +0200, Alexander Potapenko wrote:
> > > The new config switches coverage instrumentation to using
> > >   __sanitizer_cov_trace_pc_guard(u32 *guard)
> > > instead of
> > >   __sanitizer_cov_trace_pc(void)
> > >
> > > This relies on Clang's -fsanitize-coverage=trace-pc-guard flag [1].
> > >
> > > Each callback receives a unique 32-bit guard variable residing in the
> > > __sancov_guards section. Those guards can be used by kcov to deduplicate
> > > the coverage on the fly.
> >
> > This sounds like a *LOT* of data; how big is this for a typical kernel
> > build?
>
> I have a 1.6Gb sized vmlinux, which has a .text section of 176Mb.
> There are 1809419 calls to __sanitizer_cov_trace_pc_guard, and the
> __sancov_guards section has a size of 6Mb, which are only allocated at
> runtime.

Also note that most of this array will be containing zeroes.
The high coverage watermark across all syzbot instances is below 900K
coverage points: https://syzkaller.appspot.com/upstream
But that is coverage aggregated from multiple runs of the same kernel binary.
CONFIG_KCOV_UNIQUE will be only initializing the guards for the code
that was executed during a single run (<= 1 hour), and only when
coverage collection was enabled for the current process, so background
tasks won't be polluting them.

>
> If we take a vmlinux image from syzbot (e.g.
> https://storage.googleapis.com/syzbot-assets/dadedf20b2e3/vmlinux-67a99386.xz),
> its .text section is 166Mb, and there are 1893023 calls to
> __sanitizer_cov_trace_pc, which will translate to exactly the same
> number of __sanitizer_cov_trace_pc_guard, if we apply the unique
> coverage instrumentation.

Re: [PATCH v2 02/11] kcov: apply clang-format to kcov code

[Miguel Ojeda]

On Fri, Jun 27, 2025 at 2:50 PM Alexander Potapenko <glider@google.com> wrote:
>
> Random fact that I didn't know before: 1788 out of 35503 kernel .c
> files are already formatted according to the clang-format style.
> (I expected the number to be much lower)

Nice :)

> I think we can fix this by setting AllowShortFunctionsOnASingleLine:
> Empty, SplitEmptyFunction: false in .clang-format
>
> Miguel, do you think this is a reasonable change?

I have a few changes in the backlog for clang-format that I hope to
get to soon -- the usual constraints are that the options are
supported in all LLVMs we support (there are some options that I have
to take a look into that weren't available back when we added the
config), and to try to match the style of as much as the kernel as
possible (i.e. since different files in the kernel do different
things).

Cheers,
Miguel

Re: [PATCH v2 02/11] kcov: apply clang-format to kcov code

[Alexander Potapenko]

On Sun, Jun 29, 2025 at 9:25 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Fri, Jun 27, 2025 at 2:50 PM Alexander Potapenko <glider@google.com> wrote:
> >
> > Random fact that I didn't know before: 1788 out of 35503 kernel .c
> > files are already formatted according to the clang-format style.
> > (I expected the number to be much lower)
>
> Nice :)
>
> > I think we can fix this by setting AllowShortFunctionsOnASingleLine:
> > Empty, SplitEmptyFunction: false in .clang-format
> >
> > Miguel, do you think this is a reasonable change?
>
> I have a few changes in the backlog for clang-format that I hope to
> get to soon -- the usual constraints are that the options are
> supported in all LLVMs we support (there are some options that I have
> to take a look into that weren't available back when we added the
> config), and to try to match the style of as much as the kernel as
> possible (i.e. since different files in the kernel do different
> things).

Okay, then for the sake of velocity I can drop this change in v3 and
get back to formatting kcov.c once your changes land.

Re: [PATCH v2 01/11] x86: kcov: disable instrumentation of arch/x86/kernel/tsc.c

[Peter Zijlstra]

On Fri, Jun 27, 2025 at 12:51:47PM +0200, Alexander Potapenko wrote:
> On Fri, Jun 27, 2025 at 9:59 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Thu, Jun 26, 2025 at 03:41:48PM +0200, Alexander Potapenko wrote:
> > > sched_clock() appears to be called from interrupts, producing spurious
> > > coverage, as reported by CONFIG_KCOV_SELFTEST:
> >
> > NMI context even. But I'm not sure how this leads to problems. What does
> > spurious coverage even mean?
> 
> This leads to KCOV collecting slightly different coverage when
> executing the same syscall multiple times.
> For syzkaller that means higher chance to pick a less interesting
> input incorrectly assuming it produced some new coverage.
> 
> There's a similar discussion at
> https://lore.kernel.org/all/20240619111936.GK31592@noisy.programming.kicks-ass.net/T/#u

Clearly I'm not remembering any of that :-)

Anyway, looking at kcov again, all the __sanitize_*() hooks seem to have
check_kcov_mode(), which in turn has something like:

 if (!in_task() ..)
   return false;

Which should be filtering out all these things, no? If this filter
'broken' ?

Re: [PATCH v2 06/11] kcov: x86: introduce CONFIG_KCOV_UNIQUE

[Peter Zijlstra]

On Fri, Jun 27, 2025 at 04:24:36PM +0200, Alexander Potapenko wrote:
> On Fri, Jun 27, 2025 at 10:11 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Thu, Jun 26, 2025 at 03:41:53PM +0200, Alexander Potapenko wrote:
> > > The new config switches coverage instrumentation to using
> > >   __sanitizer_cov_trace_pc_guard(u32 *guard)
> > > instead of
> > >   __sanitizer_cov_trace_pc(void)
> > >
> > > This relies on Clang's -fsanitize-coverage=trace-pc-guard flag [1].
> > >
> > > Each callback receives a unique 32-bit guard variable residing in the
> > > __sancov_guards section. Those guards can be used by kcov to deduplicate
> > > the coverage on the fly.
> >
> > This sounds like a *LOT* of data; how big is this for a typical kernel
> > build?
> 
> I have a 1.6Gb sized vmlinux, which has a .text section of 176Mb.
> There are 1809419 calls to __sanitizer_cov_trace_pc_guard, and the
> __sancov_guards section has a size of 6Mb, which are only allocated at
> runtime.

OK, that's less than I feared. That's ~3.5% of .text, and should be
quite manageable.

Re: [PATCH v2 08/11] kcov: add ioctl(KCOV_UNIQUE_ENABLE)

[Peter Zijlstra]

On Fri, Jun 27, 2025 at 03:58:59PM +0200, Alexander Potapenko wrote:

> There are two modes, -fsanitize-coverage=edge and
> -fsanitize-coverage=bb, with edge being the default one.

Thanks for the details!

> > Also, dynamic edges are very hard to allocate guard variables for, while
> > target guards are trivial, even in the face of dynamic edges.
> 
> All edges are known statically, because they are within the same
> function - calls between functions are not considered edges.

Oooh, that simplifies things a bit.

I suppose that even in the case of computed gotos you can create these
intermediate thunks because you have the whole thing at compile time
(not that there are a lot of those in the kernel).

Re: [PATCH v2 02/11] kcov: apply clang-format to kcov code

[Peter Zijlstra]

On Fri, Jun 27, 2025 at 02:50:18PM +0200, Alexander Potapenko wrote:

> Setting AlignConsecutiveDeclarations: AcrossEmptyLinesAndComments will
> replace the above with the following diff:
> 
>  struct kcov_percpu_data {
> -       void                    *irq_area;
> -       local_lock_t            lock;
> -
> -       unsigned int            saved_mode;
> -       unsigned int            saved_size;
> -       void                    *saved_area;
> -       struct kcov             *saved_kcov;
> -       int                     saved_sequence;
> +       void        *irq_area;
> +       local_lock_t lock;
> +
> +       unsigned int saved_mode;
> +       unsigned int saved_size;
> +       void        *saved_area;
> +       struct kcov *saved_kcov;
> +       int          saved_sequence;
>  };
> 
> (a bit denser, plus it aligns the variable names, not the pointer signs)
> Does this look better?

Better yes, but still not really nice.

Re: [PATCH v2 02/11] kcov: apply clang-format to kcov code

[Peter Zijlstra]

On Sun, Jun 29, 2025 at 09:25:34PM +0200, Miguel Ojeda wrote:

> > I think we can fix this by setting AllowShortFunctionsOnASingleLine:
> > Empty, SplitEmptyFunction: false in .clang-format
> >
> > Miguel, do you think this is a reasonable change?
> 
> I have a few changes in the backlog for clang-format that I hope to
> get to soon -- the usual constraints are that the options are
> supported in all LLVMs we support (there are some options that I have
> to take a look into that weren't available back when we added the
> config),

Since clang format is an entirely optional thing, I don't think we
should care about old versions when inconvenient. Perhaps stick to the
very latest version.

> and to try to match the style of as much as the kernel as
> possible (i.e. since different files in the kernel do different
> things).

You can have per directory .clang-format files to account for this. Eg.
net/ can have its own file that allows their silly comment style etc.

I suppose include/linux/ is going to be a wee problem..

Still, in general I don't like linters, they're too rigid, its either
all or nothing with those things.


And like I said, in my neovim-lsp adventures, I had to stomp hard on
clang-format, it got in the way far more than it was helpful.

Re: [PATCH v2 01/11] x86: kcov: disable instrumentation of arch/x86/kernel/tsc.c

[Alexander Potapenko]

> Anyway, looking at kcov again, all the __sanitize_*() hooks seem to have
> check_kcov_mode(), which in turn has something like:
>
>  if (!in_task() ..)
>    return false;
>
> Which should be filtering out all these things, no? If this filter
> 'broken' ?

I think this is one of the cases where we are transitioning to the IRQ
context (so the coverage isn't really interesting for the fuzzer), but
still haven't bumped preempt_count.

In this particular case in_task() is 1, in_softirq_really() is 0, and
preempt_count() is 2.

Re: [PATCH v2 02/11] kcov: apply clang-format to kcov code

[Miguel Ojeda]

On Mon, Jun 30, 2025 at 10:09 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> Since clang format is an entirely optional thing, I don't think we
> should care about old versions when inconvenient. Perhaps stick to the
> very latest version.

I would love that, but I am not sure about others that use their
distribution toolchains (including for clang-format).

Hmm...

If it would now allow us to get very close to the average kernel
style, then we should definitely consider it -- years ago it wasn't
the case.

> You can have per directory .clang-format files to account for this. Eg.
> net/ can have its own file that allows their silly comment style etc.

Yeah, that is what I recommended in:

    https://docs.kernel.org/dev-tools/clang-format.html

But nobody actually added their own files so far. (Which I guess, in a
sense, is good... :)

> Still, in general I don't like linters, they're too rigid, its either
> all or nothing with those things.

There is `// clang-format off/on` to locally disable it, so that is an
escape hatch, but it is ugly, because we would still need to use it
too much with the current setup.

> And like I said, in my neovim-lsp adventures, I had to stomp hard on
> clang-format, it got in the way far more than it was helpful.

Yeah, clang-format for the kernel so far is most useful for getting
reasonable formatting on e.g. snippets of existing code in an IDE, and
possibly for new files where the maintainer is OK with the style (I
mention a bit of that in the docs above).

But we are not (or, at least, back then) at the point where we could
consider using it on existing files (e.g. just the alignment on
`#define`s makes it a mess in many cases).

I will take a look at the config later this cycle and see how we would
fare nowadays. It would be nice to get to the point where some
subsystems can just use it for new files and look good enough.

Thanks!

Cheers,
Miguel

Re: [PATCH v2 02/11] kcov: apply clang-format to kcov code

[David Laight]

On Thu, 26 Jun 2025 15:41:49 +0200
Alexander Potapenko <glider@google.com> wrote:

> kcov used to obey clang-format style, but somehow diverged over time.
> This patch applies clang-format to kernel/kcov.c and
> include/linux/kcov.h, no functional change.
> 
... 
> -#define kcov_prepare_switch(t)			\
> -do {						\
> -	(t)->kcov_mode |= KCOV_IN_CTXSW;	\
> -} while (0)
> +#define kcov_prepare_switch(t)                   \
> +	do {                                     \
> +		(t)->kcov_mode |= KCOV_IN_CTXSW; \
> +	} while (0)
>  

Too many level of indent.

(and too much churn I just deleted)

	David

Re: [PATCH v2 03/11] kcov: elaborate on using the shared buffer

[Dmitry Vyukov]

On Thu, 26 Jun 2025 at 15:42, Alexander Potapenko <glider@google.com> wrote:
>
> Add a paragraph about the shared buffer usage to kcov.rst.
>
> Signed-off-by: Alexander Potapenko <glider@google.com>
> ---
> Change-Id: Ia47ef7c3fcc74789fe57a6e1d93e29a42dbc0a97
> ---
>  Documentation/dev-tools/kcov.rst | 55 ++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
>
> diff --git a/Documentation/dev-tools/kcov.rst b/Documentation/dev-tools/kcov.rst
> index 6611434e2dd24..abf3ad2e784e8 100644
> --- a/Documentation/dev-tools/kcov.rst
> +++ b/Documentation/dev-tools/kcov.rst
> @@ -137,6 +137,61 @@ mmaps coverage buffer, and then forks child processes in a loop. The child
>  processes only need to enable coverage (it gets disabled automatically when
>  a thread exits).
>
> +Shared buffer for coverage collection
> +-------------------------------------
> +KCOV employs a shared memory buffer as a central mechanism for efficient and
> +direct transfer of code coverage information between the kernel and userspace
> +applications.
> +
> +Calling ``ioctl(fd, KCOV_INIT_TRACE, size)`` initializes coverage collection for
> +the current thread associated with the file descriptor ``fd``. The buffer
> +allocated will hold ``size`` unsigned long values, as interpreted by the kernel.
> +Notably, even in a 32-bit userspace program on a 64-bit kernel, each entry will
> +occupy 64 bits.
> +
> +Following initialization, the actual shared memory buffer is created using::
> +
> +    mmap(NULL, size * sizeof(unsigned long), PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0)
> +
> +The size of this memory mapping, calculated as ``size * sizeof(unsigned long)``,
> +must be a multiple of ``PAGE_SIZE``.
> +
> +This buffer is then shared between the kernel and the userspace. The first
> +element of the buffer contains the number of PCs stored in it.
> +Both the userspace and the kernel may write to the shared buffer, so to avoid
> +race conditions each userspace thread should only update its own buffer.
> +
> +Normally the shared buffer is used as follows::
> +
> +              Userspace                                         Kernel
> +    -----------------------------------------+-------------------------------------------
> +    ioctl(fd, KCOV_INIT_TRACE, size)         |
> +                                             |    Initialize coverage for current thread
> +    mmap(..., MAP_SHARED, fd, 0)             |
> +                                             |    Allocate the buffer, initialize it
> +                                             |    with zeroes
> +    ioctl(fd, KCOV_ENABLE, KCOV_TRACE_PC)    |
> +                                             |    Enable PC collection for current thread
> +                                             |    starting at buffer[1] (KCOV_ENABLE will
> +                                             |    already write some coverage)
> +    Atomically write 0 to buffer[0] to       |
> +    reset the coverage                       |
> +                                             |
> +    Execute some syscall(s)                  |
> +                                             |    Write new coverage starting at
> +                                             |    buffer[1]
> +    Atomically read buffer[0] to get the     |
> +    total coverage size at this point in     |
> +    time                                     |
> +                                             |
> +    ioctl(fd, KCOV_DISABLE, 0)               |
> +                                             |    Write some more coverage for ioctl(),
> +                                             |    then disable PC collection for current
> +                                             |    thread
> +    Safely read and process the coverage     |
> +    up to the buffer[0] value saved above    |
> +
> +
>  Comparison operands collection
>  ------------------------------

Reviewed-by: Dmitry Vyukov <dvyukov@google.com>

Re: [PATCH v2 04/11] kcov: factor out struct kcov_state

[Dmitry Vyukov]

On Thu, 26 Jun 2025 at 15:42, Alexander Potapenko <glider@google.com> wrote:
>
> Group several kcov-related fields (area, size, sequence) that are
> stored in various structures, into `struct kcov_state`, so that
> these fields can be easily passed around and manipulated.
> Note that now the spinlock in struct kcov applies to every member
> of struct kcov_state, including the sequence number.
>
> This prepares us for the upcoming change that will introduce more
> kcov state.
>
> Also update the MAINTAINERS entry: add include/linux/kcov_types..h,
> add myself as kcov reviewer.
>
> Signed-off-by: Alexander Potapenko <glider@google.com>
> ---
> Change-Id: If225682ea2f6e91245381b3270de16e7ea40df39
>
> v2:
>  - add myself to kcov MAINTAINERS
>  - rename kcov-state.h to kcov_types.h
>  - update the description
>  - do not move mode into struct kcov_state
>  - use '{ }' instead of '{ 0 }'
> ---
>  MAINTAINERS                |   2 +
>  include/linux/kcov.h       |   2 +-
>  include/linux/kcov_types.h |  22 +++++++
>  include/linux/sched.h      |  13 +----
>  kernel/kcov.c              | 115 ++++++++++++++++---------------------
>  5 files changed, 78 insertions(+), 76 deletions(-)
>  create mode 100644 include/linux/kcov_types.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index dd844ac8d9107..5bbc78b0fa6ed 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12823,11 +12823,13 @@ F:    include/linux/kcore.h
>  KCOV
>  R:     Dmitry Vyukov <dvyukov@google.com>
>  R:     Andrey Konovalov <andreyknvl@gmail.com>
> +R:     Alexander Potapenko <glider@google.com>
>  L:     kasan-dev@googlegroups.com
>  S:     Maintained
>  B:     https://bugzilla.kernel.org/buglist.cgi?component=Sanitizers&product=Memory%20Management
>  F:     Documentation/dev-tools/kcov.rst
>  F:     include/linux/kcov.h
> +F:     include/linux/kcov_types.h
>  F:     include/uapi/linux/kcov.h
>  F:     kernel/kcov.c
>  F:     scripts/Makefile.kcov
> diff --git a/include/linux/kcov.h b/include/linux/kcov.h
> index 932b4face1005..0e425c3524b86 100644
> --- a/include/linux/kcov.h
> +++ b/include/linux/kcov.h
> @@ -2,7 +2,7 @@
>  #ifndef _LINUX_KCOV_H
>  #define _LINUX_KCOV_H
>
> -#include <linux/sched.h>
> +#include <linux/kcov_types.h>
>  #include <uapi/linux/kcov.h>
>
>  struct task_struct;
> diff --git a/include/linux/kcov_types.h b/include/linux/kcov_types.h
> new file mode 100644
> index 0000000000000..53b25b6f0addd
> --- /dev/null
> +++ b/include/linux/kcov_types.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_KCOV_STATE_H
> +#define _LINUX_KCOV_STATE_H
> +
> +#ifdef CONFIG_KCOV
> +/* See kernel/kcov.c for more details. */
> +struct kcov_state {
> +       /* Size of the area (in long's). */
> +       unsigned int size;
> +
> +       /* Buffer for coverage collection, shared with the userspace. */
> +       void *area;
> +
> +       /*
> +        * KCOV sequence number: incremented each time kcov is reenabled, used
> +        * by kcov_remote_stop(), see the comment there.
> +        */
> +       int sequence;
> +};
> +#endif /* CONFIG_KCOV */
> +
> +#endif /* _LINUX_KCOV_STATE_H */
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index f96ac19828934..68af8d6eaee3a 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -42,6 +42,7 @@
>  #include <linux/restart_block.h>
>  #include <uapi/linux/rseq.h>
>  #include <linux/seqlock_types.h>
> +#include <linux/kcov_types.h>
>  #include <linux/kcsan.h>
>  #include <linux/rv.h>
>  #include <linux/livepatch_sched.h>
> @@ -1512,16 +1513,11 @@ struct task_struct {
>  #endif /* CONFIG_TRACING */
>
>  #ifdef CONFIG_KCOV
> -       /* See kernel/kcov.c for more details. */
> -
>         /* Coverage collection mode enabled for this task (0 if disabled): */
>         unsigned int                    kcov_mode;
>
> -       /* Size of the kcov_area: */
> -       unsigned int                    kcov_size;
> -
> -       /* Buffer for coverage collection: */
> -       void                            *kcov_area;
> +       /* kcov buffer state for this task. */

For consistency: s/kcov/KCOV/

> +       struct kcov_state               kcov_state;
>
>         /* KCOV descriptor wired with this task or NULL: */
>         struct kcov                     *kcov;
> @@ -1529,9 +1525,6 @@ struct task_struct {
>         /* KCOV common handle for remote coverage collection: */
>         u64                             kcov_handle;
>
> -       /* KCOV sequence number: */
> -       int                             kcov_sequence;
> -
>         /* Collect coverage from softirq context: */
>         unsigned int                    kcov_softirq;
>  #endif
> diff --git a/kernel/kcov.c b/kernel/kcov.c
> index 0dd42b78694c9..ff7f118644f49 100644
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -13,6 +13,7 @@
>  #include <linux/init.h>
>  #include <linux/jiffies.h>
>  #include <linux/kcov.h>
> +#include <linux/kcov_types.h>
>  #include <linux/kmsan-checks.h>
>  #include <linux/log2.h>
>  #include <linux/mm.h>
> @@ -54,24 +55,17 @@ struct kcov {
>          *  - each code section for remote coverage collection
>          */
>         refcount_t refcount;
> -       /* The lock protects mode, size, area and t. */
> +       /* The lock protects state and t. */
>         spinlock_t lock;
>         enum kcov_mode mode;
> -       /* Size of arena (in long's). */
> -       unsigned int size;
> -       /* Coverage buffer shared with user space. */
> -       void *area;
> +       struct kcov_state state;
> +
>         /* Task for which we collect coverage, or NULL. */
>         struct task_struct *t;
>         /* Collecting coverage from remote (background) threads. */
>         bool remote;
>         /* Size of remote area (in long's). */
>         unsigned int remote_size;
> -       /*
> -        * Sequence is incremented each time kcov is reenabled, used by
> -        * kcov_remote_stop(), see the comment there.
> -        */
> -       int sequence;
>  };
>
>  struct kcov_remote_area {
> @@ -92,12 +86,9 @@ static struct list_head kcov_remote_areas = LIST_HEAD_INIT(kcov_remote_areas);
>  struct kcov_percpu_data {
>         void *irq_area;
>         local_lock_t lock;
> -
> -       unsigned int saved_mode;
> -       unsigned int saved_size;
> -       void *saved_area;
> +       enum kcov_mode saved_mode;
>         struct kcov *saved_kcov;
> -       int saved_sequence;
> +       struct kcov_state saved_state;
>  };
>
>  static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {
> @@ -219,10 +210,10 @@ void notrace __sanitizer_cov_trace_pc(void)
>         if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t))
>                 return;
>
> -       area = t->kcov_area;
> +       area = t->kcov_state.area;
>         /* The first 64-bit word is the number of subsequent PCs. */
>         pos = READ_ONCE(area[0]) + 1;
> -       if (likely(pos < t->kcov_size)) {
> +       if (likely(pos < t->kcov_state.size)) {
>                 /* Previously we write pc before updating pos. However, some
>                  * early interrupt code could bypass check_kcov_mode() check
>                  * and invoke __sanitizer_cov_trace_pc(). If such interrupt is
> @@ -252,10 +243,10 @@ static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip)
>
>         /*
>          * We write all comparison arguments and types as u64.
> -        * The buffer was allocated for t->kcov_size unsigned longs.
> +        * The buffer was allocated for t->kcov_state.size unsigned longs.
>          */
> -       area = (u64 *)t->kcov_area;
> -       max_pos = t->kcov_size * sizeof(unsigned long);
> +       area = (u64 *)t->kcov_state.area;
> +       max_pos = t->kcov_state.size * sizeof(unsigned long);
>
>         count = READ_ONCE(area[0]);
>
> @@ -356,17 +347,15 @@ EXPORT_SYMBOL(__sanitizer_cov_trace_switch);
>  #endif /* ifdef CONFIG_KCOV_ENABLE_COMPARISONS */
>
>  static void kcov_start(struct task_struct *t, struct kcov *kcov,
> -                      unsigned int size, void *area, enum kcov_mode mode,
> -                      int sequence)
> +                      enum kcov_mode mode, struct kcov_state *state)
>  {
> -       kcov_debug("t = %px, size = %u, area = %px\n", t, size, area);
> +       kcov_debug("t = %px, size = %u, area = %px\n", t, state->size,
> +                  state->area);
>         t->kcov = kcov;
>         /* Cache in task struct for performance. */
> -       t->kcov_size = size;
> -       t->kcov_area = area;
> -       t->kcov_sequence = sequence;
> -       /* See comment in check_kcov_mode(). */
> +       t->kcov_state = *state;
>         barrier();
> +       /* See comment in check_kcov_mode(). */
>         WRITE_ONCE(t->kcov_mode, mode);
>  }
>
> @@ -375,14 +364,14 @@ static void kcov_stop(struct task_struct *t)
>         WRITE_ONCE(t->kcov_mode, KCOV_MODE_DISABLED);
>         barrier();
>         t->kcov = NULL;
> -       t->kcov_size = 0;
> -       t->kcov_area = NULL;
> +       t->kcov_state.size = 0;
> +       t->kcov_state.area = NULL;
>  }
>
>  static void kcov_task_reset(struct task_struct *t)
>  {
>         kcov_stop(t);
> -       t->kcov_sequence = 0;
> +       t->kcov_state.sequence = 0;
>         t->kcov_handle = 0;
>  }
>
> @@ -398,7 +387,7 @@ static void kcov_reset(struct kcov *kcov)
>         kcov->mode = KCOV_MODE_INIT;
>         kcov->remote = false;
>         kcov->remote_size = 0;
> -       kcov->sequence++;
> +       kcov->state.sequence++;
>  }
>
>  static void kcov_remote_reset(struct kcov *kcov)
> @@ -438,7 +427,7 @@ static void kcov_put(struct kcov *kcov)
>  {
>         if (refcount_dec_and_test(&kcov->refcount)) {
>                 kcov_remote_reset(kcov);
> -               vfree(kcov->area);
> +               vfree(kcov->state.area);
>                 kfree(kcov);
>         }
>  }
> @@ -495,8 +484,8 @@ static int kcov_mmap(struct file *filep, struct vm_area_struct *vma)
>         unsigned long flags;
>
>         spin_lock_irqsave(&kcov->lock, flags);
> -       size = kcov->size * sizeof(unsigned long);
> -       if (kcov->area == NULL || vma->vm_pgoff != 0 ||
> +       size = kcov->state.size * sizeof(unsigned long);
> +       if (kcov->state.area == NULL || vma->vm_pgoff != 0 ||
>             vma->vm_end - vma->vm_start != size) {
>                 res = -EINVAL;
>                 goto exit;
> @@ -504,7 +493,7 @@ static int kcov_mmap(struct file *filep, struct vm_area_struct *vma)
>         spin_unlock_irqrestore(&kcov->lock, flags);
>         vm_flags_set(vma, VM_DONTEXPAND);
>         for (off = 0; off < size; off += PAGE_SIZE) {
> -               page = vmalloc_to_page(kcov->area + off);
> +               page = vmalloc_to_page(kcov->state.area + off);
>                 res = vm_insert_page(vma, vma->vm_start + off, page);
>                 if (res) {
>                         pr_warn_once("kcov: vm_insert_page() failed\n");
> @@ -525,7 +514,7 @@ static int kcov_open(struct inode *inode, struct file *filep)
>         if (!kcov)
>                 return -ENOMEM;
>         kcov->mode = KCOV_MODE_DISABLED;
> -       kcov->sequence = 1;
> +       kcov->state.sequence = 1;
>         refcount_set(&kcov->refcount, 1);
>         spin_lock_init(&kcov->lock);
>         filep->private_data = kcov;
> @@ -560,10 +549,10 @@ static int kcov_get_mode(unsigned long arg)
>  static void kcov_fault_in_area(struct kcov *kcov)
>  {
>         unsigned long stride = PAGE_SIZE / sizeof(unsigned long);
> -       unsigned long *area = kcov->area;
> +       unsigned long *area = kcov->state.area;
>         unsigned long offset;
>
> -       for (offset = 0; offset < kcov->size; offset += stride)
> +       for (offset = 0; offset < kcov->state.size; offset += stride)
>                 READ_ONCE(area[offset]);
>  }
>
> @@ -602,7 +591,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
>                  * at task exit or voluntary by KCOV_DISABLE. After that it can
>                  * be enabled for another task.
>                  */
> -               if (kcov->mode != KCOV_MODE_INIT || !kcov->area)
> +               if (kcov->mode != KCOV_MODE_INIT || !kcov->state.area)
>                         return -EINVAL;
>                 t = current;
>                 if (kcov->t != NULL || t->kcov != NULL)
> @@ -612,8 +601,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
>                         return mode;
>                 kcov_fault_in_area(kcov);
>                 kcov->mode = mode;
> -               kcov_start(t, kcov, kcov->size, kcov->area, kcov->mode,
> -                          kcov->sequence);
> +               kcov_start(t, kcov, mode, &kcov->state);
>                 kcov->t = t;
>                 /* Put either in kcov_task_exit() or in KCOV_DISABLE. */
>                 kcov_get(kcov);
> @@ -630,7 +618,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
>                 kcov_put(kcov);
>                 return 0;
>         case KCOV_REMOTE_ENABLE:
> -               if (kcov->mode != KCOV_MODE_INIT || !kcov->area)
> +               if (kcov->mode != KCOV_MODE_INIT || !kcov->state.area)
>                         return -EINVAL;
>                 t = current;
>                 if (kcov->t != NULL || t->kcov != NULL)
> @@ -725,8 +713,8 @@ static long kcov_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
>                         vfree(area);
>                         return -EBUSY;
>                 }
> -               kcov->area = area;
> -               kcov->size = size;
> +               kcov->state.area = area;
> +               kcov->state.size = size;
>                 kcov->mode = KCOV_MODE_INIT;
>                 spin_unlock_irqrestore(&kcov->lock, flags);
>                 return 0;
> @@ -825,10 +813,8 @@ static void kcov_remote_softirq_start(struct task_struct *t)
>         mode = READ_ONCE(t->kcov_mode);
>         barrier();
>         if (kcov_mode_enabled(mode)) {
> +               data->saved_state = t->kcov_state;
>                 data->saved_mode = mode;
> -               data->saved_size = t->kcov_size;
> -               data->saved_area = t->kcov_area;
> -               data->saved_sequence = t->kcov_sequence;
>                 data->saved_kcov = t->kcov;
>                 kcov_stop(t);
>         }
> @@ -839,13 +825,9 @@ static void kcov_remote_softirq_stop(struct task_struct *t)
>         struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
>
>         if (data->saved_kcov) {
> -               kcov_start(t, data->saved_kcov, data->saved_size,
> -                          data->saved_area, data->saved_mode,
> -                          data->saved_sequence);
> -               data->saved_mode = 0;
> -               data->saved_size = 0;
> -               data->saved_area = NULL;
> -               data->saved_sequence = 0;
> +               kcov_start(t, data->saved_kcov, t->kcov_mode,

We used to pass data->saved_mode, now we pass t->kcov_mode.
Are they the same here? This makes me a bit nervous.

> +                          &data->saved_state);
> +               data->saved_state = (struct kcov_state){};
>                 data->saved_kcov = NULL;
>         }
>  }
> @@ -854,12 +836,12 @@ void kcov_remote_start(u64 handle)
>  {
>         struct task_struct *t = current;
>         struct kcov_remote *remote;
> +       struct kcov_state state;
> +       enum kcov_mode mode;
> +       unsigned long flags;
> +       unsigned int size;
>         struct kcov *kcov;
> -       unsigned int mode;
>         void *area;
> -       unsigned int size;
> -       int sequence;
> -       unsigned long flags;
>
>         if (WARN_ON(!kcov_check_handle(handle, true, true, true)))
>                 return;
> @@ -904,7 +886,7 @@ void kcov_remote_start(u64 handle)
>          * KCOV_DISABLE / kcov_remote_reset().
>          */
>         mode = kcov->mode;
> -       sequence = kcov->sequence;
> +       state.sequence = kcov->state.sequence;
>         if (in_task()) {
>                 size = kcov->remote_size;
>                 area = kcov_remote_area_get(size);
> @@ -927,12 +909,14 @@ void kcov_remote_start(u64 handle)
>
>         /* Reset coverage size. */
>         *(u64 *)area = 0;
> +       state.area = area;
> +       state.size = size;
>
>         if (in_serving_softirq()) {
>                 kcov_remote_softirq_start(t);
>                 t->kcov_softirq = 1;
>         }
> -       kcov_start(t, kcov, size, area, mode, sequence);
> +       kcov_start(t, kcov, t->kcov_mode, &state);

We used to pass kcov->mode here, now it's t->kcov_mode.
Are they the same here? I would prefer to restore the current version,
if there is no specific reason to change it.

>
>         local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
>  }
> @@ -1030,9 +1014,9 @@ void kcov_remote_stop(void)
>         }
>
>         kcov = t->kcov;
> -       area = t->kcov_area;
> -       size = t->kcov_size;
> -       sequence = t->kcov_sequence;
> +       area = t->kcov_state.area;
> +       size = t->kcov_state.size;
> +       sequence = t->kcov_state.sequence;
>
>         kcov_stop(t);
>         if (in_serving_softirq()) {
> @@ -1045,8 +1029,9 @@ void kcov_remote_stop(void)
>          * KCOV_DISABLE could have been called between kcov_remote_start()
>          * and kcov_remote_stop(), hence the sequence check.
>          */
> -       if (sequence == kcov->sequence && kcov->remote)
> -               kcov_move_area(kcov->mode, kcov->area, kcov->size, area);
> +       if (sequence == kcov->state.sequence && kcov->remote)
> +               kcov_move_area(kcov->mode, kcov->state.area, kcov->state.size,
> +                              area);
>         spin_unlock(&kcov->lock);
>
>         if (in_task()) {
> --
> 2.50.0.727.gbf7dc18ff4-goog
>

Re: [PATCH v2 05/11] mm/kasan: define __asan_before_dynamic_init, __asan_after_dynamic_init

[Dmitry Vyukov]

On Thu, 26 Jun 2025 at 15:42, Alexander Potapenko <glider@google.com> wrote:
>
> Calls to __asan_before_dynamic_init() and __asan_after_dynamic_init()
> are inserted by Clang when building with coverage guards.
> These functions can be used to detect initialization order fiasco bugs
> in the userspace, but it is fine for them to be no-ops in the kernel.
>
> Signed-off-by: Alexander Potapenko <glider@google.com>

Reviewed-by: Dmitry Vyukov <dvyukov@google.com>

>
> ---
> Change-Id: I7f8eb690a3d96f7d122205e8f1cba8039f6a68eb
>
> v2:
>  - Address comments by Dmitry Vyukov:
>    - rename CONFIG_KCOV_ENABLE_GUARDS to CONFIG_KCOV_UNIQUE
>  - Move this patch before the one introducing CONFIG_KCOV_UNIQUE,
>    per Marco Elver's request.
> ---
>  mm/kasan/generic.c | 18 ++++++++++++++++++
>  mm/kasan/kasan.h   |  2 ++
>  2 files changed, 20 insertions(+)
>
> diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
> index d54e89f8c3e76..b0b7781524348 100644
> --- a/mm/kasan/generic.c
> +++ b/mm/kasan/generic.c
> @@ -238,6 +238,24 @@ void __asan_unregister_globals(void *ptr, ssize_t size)
>  }
>  EXPORT_SYMBOL(__asan_unregister_globals);
>
> +#if defined(CONFIG_KCOV_UNIQUE)
> +/*
> + * __asan_before_dynamic_init() and __asan_after_dynamic_init() are inserted
> + * when the user requests building with coverage guards. In the userspace, these
> + * two functions can be used to detect initialization order fiasco bugs, but in
> + * the kernel they can be no-ops.
> + */
> +void __asan_before_dynamic_init(const char *module_name)
> +{
> +}
> +EXPORT_SYMBOL(__asan_before_dynamic_init);
> +
> +void __asan_after_dynamic_init(void)
> +{
> +}
> +EXPORT_SYMBOL(__asan_after_dynamic_init);
> +#endif
> +
>  #define DEFINE_ASAN_LOAD_STORE(size)                                   \
>         void __asan_load##size(void *addr)                              \
>         {                                                               \
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index 129178be5e649..c817c46b4fcd2 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -582,6 +582,8 @@ void kasan_restore_multi_shot(bool enabled);
>
>  void __asan_register_globals(void *globals, ssize_t size);
>  void __asan_unregister_globals(void *globals, ssize_t size);
> +void __asan_before_dynamic_init(const char *module_name);
> +void __asan_after_dynamic_init(void);
>  void __asan_handle_no_return(void);
>  void __asan_alloca_poison(void *, ssize_t size);
>  void __asan_allocas_unpoison(void *stack_top, ssize_t stack_bottom);
> --
> 2.50.0.727.gbf7dc18ff4-goog
>

Re: [PATCH v2 06/11] kcov: x86: introduce CONFIG_KCOV_UNIQUE

[Dmitry Vyukov]

On Thu, 26 Jun 2025 at 15:42, Alexander Potapenko <glider@google.com> wrote:
>
> The new config switches coverage instrumentation to using
>   __sanitizer_cov_trace_pc_guard(u32 *guard)
> instead of
>   __sanitizer_cov_trace_pc(void)
>
> This relies on Clang's -fsanitize-coverage=trace-pc-guard flag [1].
>
> Each callback receives a unique 32-bit guard variable residing in the
> __sancov_guards section. Those guards can be used by kcov to deduplicate
> the coverage on the fly.
>
> As a first step, we make the new instrumentation mode 1:1 compatible
> with the old one.
>
> [1] https://clang.llvm.org/docs/SanitizerCoverage.html#tracing-pcs-with-guards
>
> Cc: x86@kernel.org
> Signed-off-by: Alexander Potapenko <glider@google.com>
>
> ---
> Change-Id: Iacb1e71fd061a82c2acadf2347bba4863b9aec39
>
> v2:
>  - Address comments by Dmitry Vyukov
>    - rename CONFIG_KCOV_ENABLE_GUARDS to CONFIG_KCOV_UNIQUE
>    - update commit description and config description
>  - Address comments by Marco Elver
>    - rename sanitizer_cov_write_subsequent() to kcov_append_to_buffer()
>    - make config depend on X86_64 (via ARCH_HAS_KCOV_UNIQUE)
>    - swap #ifdef branches
>    - tweak config description
>    - remove redundant check for CONFIG_CC_HAS_SANCOV_TRACE_PC_GUARD
> ---
>  arch/x86/Kconfig                  |  1 +
>  arch/x86/kernel/vmlinux.lds.S     |  1 +
>  include/asm-generic/vmlinux.lds.h | 14 ++++++-
>  include/linux/kcov.h              |  2 +
>  kernel/kcov.c                     | 61 +++++++++++++++++++++----------
>  lib/Kconfig.debug                 | 24 ++++++++++++
>  scripts/Makefile.kcov             |  4 ++
>  scripts/module.lds.S              | 23 ++++++++++++
>  tools/objtool/check.c             |  1 +
>  9 files changed, 110 insertions(+), 21 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index e21cca404943e..d104c5a193bdf 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -93,6 +93,7 @@ config X86
>         select ARCH_HAS_FORTIFY_SOURCE
>         select ARCH_HAS_GCOV_PROFILE_ALL
>         select ARCH_HAS_KCOV                    if X86_64
> +       select ARCH_HAS_KCOV_UNIQUE             if X86_64
>         select ARCH_HAS_KERNEL_FPU_SUPPORT
>         select ARCH_HAS_MEM_ENCRYPT
>         select ARCH_HAS_MEMBARRIER_SYNC_CORE
> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> index cda5f8362e9da..8076e8953fddc 100644
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -372,6 +372,7 @@ SECTIONS
>                 . = ALIGN(PAGE_SIZE);
>                 __bss_stop = .;
>         }
> +       SANCOV_GUARDS_BSS
>
>         /*
>          * The memory occupied from _text to here, __end_of_kernel_reserve, is
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 58a635a6d5bdf..875c4deb66208 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -102,7 +102,8 @@
>   * sections to be brought in with rodata.
>   */
>  #if defined(CONFIG_LD_DEAD_CODE_DATA_ELIMINATION) || defined(CONFIG_LTO_CLANG) || \
> -defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
> +       defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG) || \
> +       defined(CONFIG_KCOV_UNIQUE)
>  #define TEXT_MAIN .text .text.[0-9a-zA-Z_]*
>  #else
>  #define TEXT_MAIN .text
> @@ -121,6 +122,17 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
>  #define SBSS_MAIN .sbss
>  #endif
>
> +#if defined(CONFIG_KCOV_UNIQUE)
> +#define SANCOV_GUARDS_BSS                      \
> +       __sancov_guards(NOLOAD) : {             \
> +               __start___sancov_guards = .;    \
> +               *(__sancov_guards);             \
> +               __stop___sancov_guards = .;     \
> +       }
> +#else
> +#define SANCOV_GUARDS_BSS
> +#endif
> +
>  /*
>   * GCC 4.5 and later have a 32 bytes section alignment for structures.
>   * Except GCC 4.9, that feels the need to align on 64 bytes.
> diff --git a/include/linux/kcov.h b/include/linux/kcov.h
> index 0e425c3524b86..dd8bbee6fe274 100644
> --- a/include/linux/kcov.h
> +++ b/include/linux/kcov.h
> @@ -107,6 +107,8 @@ typedef unsigned long long kcov_u64;
>  #endif
>
>  void __sanitizer_cov_trace_pc(void);
> +void __sanitizer_cov_trace_pc_guard(u32 *guard);
> +void __sanitizer_cov_trace_pc_guard_init(uint32_t *start, uint32_t *stop);
>  void __sanitizer_cov_trace_cmp1(u8 arg1, u8 arg2);
>  void __sanitizer_cov_trace_cmp2(u16 arg1, u16 arg2);
>  void __sanitizer_cov_trace_cmp4(u32 arg1, u32 arg2);
> diff --git a/kernel/kcov.c b/kernel/kcov.c
> index ff7f118644f49..8e98ca8d52743 100644
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -195,27 +195,15 @@ static notrace unsigned long canonicalize_ip(unsigned long ip)
>         return ip;
>  }
>
> -/*
> - * Entry point from instrumented code.
> - * This is called once per basic-block/edge.
> - */
> -void notrace __sanitizer_cov_trace_pc(void)
> +static notrace void kcov_append_to_buffer(unsigned long *area, int size,
> +                                         unsigned long ip)
>  {
> -       struct task_struct *t;
> -       unsigned long *area;
> -       unsigned long ip = canonicalize_ip(_RET_IP_);
> -       unsigned long pos;
> -
> -       t = current;
> -       if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t))
> -               return;
> -
> -       area = t->kcov_state.area;
>         /* The first 64-bit word is the number of subsequent PCs. */
> -       pos = READ_ONCE(area[0]) + 1;
> -       if (likely(pos < t->kcov_state.size)) {
> -               /* Previously we write pc before updating pos. However, some
> -                * early interrupt code could bypass check_kcov_mode() check
> +       unsigned long pos = READ_ONCE(area[0]) + 1;
> +
> +       if (likely(pos < size)) {
> +               /*
> +                * Some early interrupt code could bypass check_kcov_mode() check
>                  * and invoke __sanitizer_cov_trace_pc(). If such interrupt is
>                  * raised between writing pc and updating pos, the pc could be
>                  * overitten by the recursive __sanitizer_cov_trace_pc().
> @@ -226,7 +214,40 @@ void notrace __sanitizer_cov_trace_pc(void)
>                 area[pos] = ip;
>         }
>  }
> +
> +/*
> + * Entry point from instrumented code.
> + * This is called once per basic-block/edge.
> + */
> +#ifdef CONFIG_KCOV_UNIQUE
> +void notrace __sanitizer_cov_trace_pc_guard(u32 *guard)
> +{
> +       if (!check_kcov_mode(KCOV_MODE_TRACE_PC, current))
> +               return;
> +
> +       kcov_append_to_buffer(current->kcov_state.area,
> +                             current->kcov_state.size,
> +                             canonicalize_ip(_RET_IP_));
> +}
> +EXPORT_SYMBOL(__sanitizer_cov_trace_pc_guard);
> +
> +void notrace __sanitizer_cov_trace_pc_guard_init(uint32_t *start,
> +                                                uint32_t *stop)
> +{
> +}
> +EXPORT_SYMBOL(__sanitizer_cov_trace_pc_guard_init);
> +#else /* !CONFIG_KCOV_UNIQUE */
> +void notrace __sanitizer_cov_trace_pc(void)
> +{
> +       if (!check_kcov_mode(KCOV_MODE_TRACE_PC, current))
> +               return;
> +
> +       kcov_append_to_buffer(current->kcov_state.area,
> +                             current->kcov_state.size,
> +                             canonicalize_ip(_RET_IP_));
> +}
>  EXPORT_SYMBOL(__sanitizer_cov_trace_pc);
> +#endif
>
>  #ifdef CONFIG_KCOV_ENABLE_COMPARISONS
>  static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip)
> @@ -254,7 +275,7 @@ static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip)
>         start_index = 1 + count * KCOV_WORDS_PER_CMP;
>         end_pos = (start_index + KCOV_WORDS_PER_CMP) * sizeof(u64);
>         if (likely(end_pos <= max_pos)) {
> -               /* See comment in __sanitizer_cov_trace_pc(). */
> +               /* See comment in kcov_append_to_buffer(). */
>                 WRITE_ONCE(area[0], count + 1);
>                 barrier();
>                 area[start_index] = type;
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index f9051ab610d54..24dcb721dbb0b 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2156,6 +2156,8 @@ config ARCH_HAS_KCOV
>  config CC_HAS_SANCOV_TRACE_PC
>         def_bool $(cc-option,-fsanitize-coverage=trace-pc)
>
> +config CC_HAS_SANCOV_TRACE_PC_GUARD
> +       def_bool $(cc-option,-fsanitize-coverage=trace-pc-guard)
>
>  config KCOV
>         bool "Code coverage for fuzzing"
> @@ -2172,6 +2174,28 @@ config KCOV
>
>           For more details, see Documentation/dev-tools/kcov.rst.
>
> +config ARCH_HAS_KCOV_UNIQUE
> +       bool
> +       help
> +         An architecture should select this when it can successfully
> +         build and run with CONFIG_KCOV_UNIQUE.
> +
> +config KCOV_UNIQUE
> +       depends on KCOV
> +       depends on CC_HAS_SANCOV_TRACE_PC_GUARD && ARCH_HAS_KCOV_UNIQUE
> +       bool "Use coverage guards for KCOV"
> +       help
> +         Use coverage guards instrumentation for KCOV, passing
> +         -fsanitize-coverage=trace-pc-guard to the compiler.

I think this should talk about the new mode, the new ioctl's, and
visible differences for end users first.

> +         Every coverage callback is associated with a global variable that
> +         allows to efficiently deduplicate coverage at collection time.
> +         This drastically reduces the buffer size required for coverage
> +         collection.
> +
> +         This config comes at a cost of increased binary size (4 bytes of .bss
> +         plus 1-2 instructions to pass an extra parameter, per basic block).
> +
>  config KCOV_ENABLE_COMPARISONS
>         bool "Enable comparison operands collection by KCOV"
>         depends on KCOV
> diff --git a/scripts/Makefile.kcov b/scripts/Makefile.kcov
> index 67e8cfe3474b7..0b17533ef35f6 100644
> --- a/scripts/Makefile.kcov
> +++ b/scripts/Makefile.kcov
> @@ -1,5 +1,9 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> +ifeq ($(CONFIG_KCOV_UNIQUE),y)
> +kcov-flags-y                                   += -fsanitize-coverage=trace-pc-guard
> +else
>  kcov-flags-$(CONFIG_CC_HAS_SANCOV_TRACE_PC)    += -fsanitize-coverage=trace-pc
> +endif
>  kcov-flags-$(CONFIG_KCOV_ENABLE_COMPARISONS)   += -fsanitize-coverage=trace-cmp
>  kcov-flags-$(CONFIG_GCC_PLUGIN_SANCOV)         += -fplugin=$(objtree)/scripts/gcc-plugins/sancov_plugin.so
>
> diff --git a/scripts/module.lds.S b/scripts/module.lds.S
> index 450f1088d5fd3..314b56680ea1a 100644
> --- a/scripts/module.lds.S
> +++ b/scripts/module.lds.S
> @@ -64,6 +64,29 @@ SECTIONS {
>                 MOD_CODETAG_SECTIONS()
>         }
>  #endif
> +
> +#ifdef CONFIG_KCOV_UNIQUE
> +       __sancov_guards(NOLOAD) : {
> +               __start___sancov_guards = .;
> +               *(__sancov_guards);
> +               __stop___sancov_guards = .;
> +       }
> +
> +       .text : {
> +               *(.text .text.[0-9a-zA-Z_]*)
> +               *(.text..L*)
> +       }

Why do we need these here? .text does not look specific to CONFIG_KCOV_UNIQUE.
Is it because of constructors/destructors emitted by the compiler, and
.init.text/.exit.text don't work w/o .text?
A comment here would be useful.

> +
> +       .init.text : {
> +               *(.init.text .init.text.[0-9a-zA-Z_]*)
> +               *(.init.text..L*)
> +       }
> +       .exit.text : {
> +               *(.exit.text .exit.text.[0-9a-zA-Z_]*)
> +               *(.exit.text..L*)
> +       }
> +#endif
> +
>         MOD_SEPARATE_CODETAG_SECTIONS()
>  }
>
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index b21b12ec88d96..62fbe9b2aa077 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -1154,6 +1154,7 @@ static const char *uaccess_safe_builtin[] = {
>         "write_comp_data",
>         "check_kcov_mode",
>         "__sanitizer_cov_trace_pc",
> +       "__sanitizer_cov_trace_pc_guard",
>         "__sanitizer_cov_trace_const_cmp1",
>         "__sanitizer_cov_trace_const_cmp2",
>         "__sanitizer_cov_trace_const_cmp4",
> --
> 2.50.0.727.gbf7dc18ff4-goog
>

Re: [PATCH v2 07/11] kcov: add trace and trace_size to struct kcov_state

[Dmitry Vyukov]

On Thu, 26 Jun 2025 at 15:42, Alexander Potapenko <glider@google.com> wrote:
>
> Keep kcov_state.area as the pointer to the memory buffer used by
> kcov and shared with the userspace. Store the pointer to the trace
> (part of the buffer holding sequential events) separately, as we will
> be splitting that buffer in multiple parts.
> No functional changes so far.
>
> Signed-off-by: Alexander Potapenko <glider@google.com>
>
> ---
> Change-Id: I50b5589ef0e0b6726aa0579334093c648f76790a
>
> v2:
>  - Address comments by Dmitry Vyukov:
>    - tweak commit description
>  - Address comments by Marco Elver:
>    - rename sanitizer_cov_write_subsequent() to kcov_append_to_buffer()
>  - Update code to match the new description of struct kcov_state
> ---
>  include/linux/kcov_types.h |  9 ++++++-
>  kernel/kcov.c              | 54 ++++++++++++++++++++++----------------
>  2 files changed, 39 insertions(+), 24 deletions(-)
>
> diff --git a/include/linux/kcov_types.h b/include/linux/kcov_types.h
> index 53b25b6f0addd..233e7a682654b 100644
> --- a/include/linux/kcov_types.h
> +++ b/include/linux/kcov_types.h
> @@ -7,9 +7,16 @@
>  struct kcov_state {
>         /* Size of the area (in long's). */
>         unsigned int size;
> +       /*
> +        * Pointer to user-provided memory used by kcov. This memory may

s/kcov/KCOV/ for consistency

> +        * contain multiple buffers.
> +        */
> +       void *area;
>
> +       /* Size of the trace (in long's). */
> +       unsigned int trace_size;
>         /* Buffer for coverage collection, shared with the userspace. */
> -       void *area;
> +       unsigned long *trace;
>
>         /*
>          * KCOV sequence number: incremented each time kcov is reenabled, used
> diff --git a/kernel/kcov.c b/kernel/kcov.c
> index 8e98ca8d52743..038261145cf93 100644
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -195,11 +195,11 @@ static notrace unsigned long canonicalize_ip(unsigned long ip)
>         return ip;
>  }
>
> -static notrace void kcov_append_to_buffer(unsigned long *area, int size,
> +static notrace void kcov_append_to_buffer(unsigned long *trace, int size,
>                                           unsigned long ip)
>  {
>         /* The first 64-bit word is the number of subsequent PCs. */
> -       unsigned long pos = READ_ONCE(area[0]) + 1;
> +       unsigned long pos = READ_ONCE(trace[0]) + 1;
>
>         if (likely(pos < size)) {
>                 /*
> @@ -209,9 +209,9 @@ static notrace void kcov_append_to_buffer(unsigned long *area, int size,
>                  * overitten by the recursive __sanitizer_cov_trace_pc().
>                  * Update pos before writing pc to avoid such interleaving.
>                  */
> -               WRITE_ONCE(area[0], pos);
> +               WRITE_ONCE(trace[0], pos);
>                 barrier();
> -               area[pos] = ip;
> +               trace[pos] = ip;
>         }
>  }
>
> @@ -225,8 +225,8 @@ void notrace __sanitizer_cov_trace_pc_guard(u32 *guard)
>         if (!check_kcov_mode(KCOV_MODE_TRACE_PC, current))
>                 return;
>
> -       kcov_append_to_buffer(current->kcov_state.area,
> -                             current->kcov_state.size,
> +       kcov_append_to_buffer(current->kcov_state.trace,
> +                             current->kcov_state.trace_size,
>                               canonicalize_ip(_RET_IP_));
>  }
>  EXPORT_SYMBOL(__sanitizer_cov_trace_pc_guard);
> @@ -242,8 +242,8 @@ void notrace __sanitizer_cov_trace_pc(void)
>         if (!check_kcov_mode(KCOV_MODE_TRACE_PC, current))
>                 return;
>
> -       kcov_append_to_buffer(current->kcov_state.area,
> -                             current->kcov_state.size,
> +       kcov_append_to_buffer(current->kcov_state.trace,
> +                             current->kcov_state.trace_size,
>                               canonicalize_ip(_RET_IP_));
>  }
>  EXPORT_SYMBOL(__sanitizer_cov_trace_pc);
> @@ -252,9 +252,9 @@ EXPORT_SYMBOL(__sanitizer_cov_trace_pc);
>  #ifdef CONFIG_KCOV_ENABLE_COMPARISONS
>  static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip)
>  {
> -       struct task_struct *t;
> -       u64 *area;
>         u64 count, start_index, end_pos, max_pos;
> +       struct task_struct *t;
> +       u64 *trace;
>
>         t = current;
>         if (!check_kcov_mode(KCOV_MODE_TRACE_CMP, t))
> @@ -266,22 +266,22 @@ static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip)
>          * We write all comparison arguments and types as u64.
>          * The buffer was allocated for t->kcov_state.size unsigned longs.
>          */
> -       area = (u64 *)t->kcov_state.area;
> +       trace = (u64 *)t->kcov_state.trace;
>         max_pos = t->kcov_state.size * sizeof(unsigned long);
>
> -       count = READ_ONCE(area[0]);
> +       count = READ_ONCE(trace[0]);
>
>         /* Every record is KCOV_WORDS_PER_CMP 64-bit words. */
>         start_index = 1 + count * KCOV_WORDS_PER_CMP;
>         end_pos = (start_index + KCOV_WORDS_PER_CMP) * sizeof(u64);
>         if (likely(end_pos <= max_pos)) {
>                 /* See comment in kcov_append_to_buffer(). */
> -               WRITE_ONCE(area[0], count + 1);
> +               WRITE_ONCE(trace[0], count + 1);
>                 barrier();
> -               area[start_index] = type;
> -               area[start_index + 1] = arg1;
> -               area[start_index + 2] = arg2;
> -               area[start_index + 3] = ip;
> +               trace[start_index] = type;
> +               trace[start_index + 1] = arg1;
> +               trace[start_index + 2] = arg2;
> +               trace[start_index + 3] = ip;
>         }
>  }
>
> @@ -382,11 +382,13 @@ static void kcov_start(struct task_struct *t, struct kcov *kcov,
>
>  static void kcov_stop(struct task_struct *t)
>  {
> +       int saved_sequence = t->kcov_state.sequence;
> +
>         WRITE_ONCE(t->kcov_mode, KCOV_MODE_DISABLED);
>         barrier();
>         t->kcov = NULL;
> -       t->kcov_state.size = 0;
> -       t->kcov_state.area = NULL;
> +       t->kcov_state = (typeof(t->kcov_state)){ 0 };

In a previous patch you used the following syntax, let's stick to one
of these forms:

data->saved_state = (struct kcov_state){};


> +       t->kcov_state.sequence = saved_sequence;
>  }
>
>  static void kcov_task_reset(struct task_struct *t)
> @@ -736,6 +738,8 @@ static long kcov_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
>                 }
>                 kcov->state.area = area;
>                 kcov->state.size = size;
> +               kcov->state.trace = area;
> +               kcov->state.trace_size = size;
>                 kcov->mode = KCOV_MODE_INIT;
>                 spin_unlock_irqrestore(&kcov->lock, flags);
>                 return 0;
> @@ -928,10 +932,12 @@ void kcov_remote_start(u64 handle)
>                 local_lock_irqsave(&kcov_percpu_data.lock, flags);
>         }
>
> -       /* Reset coverage size. */
> -       *(u64 *)area = 0;
>         state.area = area;
>         state.size = size;
> +       state.trace = area;
> +       state.trace_size = size;
> +       /* Reset coverage size. */
> +       state.trace[0] = 0;
>
>         if (in_serving_softirq()) {
>                 kcov_remote_softirq_start(t);
> @@ -1004,8 +1010,8 @@ void kcov_remote_stop(void)
>         struct task_struct *t = current;
>         struct kcov *kcov;
>         unsigned int mode;
> -       void *area;
> -       unsigned int size;
> +       void *area, *trace;
> +       unsigned int size, trace_size;
>         int sequence;
>         unsigned long flags;
>
> @@ -1037,6 +1043,8 @@ void kcov_remote_stop(void)
>         kcov = t->kcov;
>         area = t->kcov_state.area;
>         size = t->kcov_state.size;
> +       trace = t->kcov_state.trace;
> +       trace_size = t->kcov_state.trace_size;
>         sequence = t->kcov_state.sequence;
>
>         kcov_stop(t);
> --
> 2.50.0.727.gbf7dc18ff4-goog
>

Re: [PATCH v2 10/11] kcov: selftests: add kcov_test

[Dmitry Vyukov]

On Thu, 26 Jun 2025 at 15:42, Alexander Potapenko <glider@google.com> wrote:
>
> Implement test fixtures for testing different combinations of coverage
> collection modes:
>  - unique and non-unique coverage;
>  - collecting PCs and comparison arguments;
>  - mapping the buffer as RO and RW.
>
> To build:
>  $ make -C tools/testing/selftests/kcov kcov_test
>
> Signed-off-by: Alexander Potapenko <glider@google.com>
> ---
>  MAINTAINERS                              |   1 +
>  tools/testing/selftests/kcov/Makefile    |   6 +
>  tools/testing/selftests/kcov/kcov_test.c | 364 +++++++++++++++++++++++

Let's also add 'config' fragment (see e.g. ./dma/config)
Otherwise it's impossible to run these tests in automated fashion.

>  3 files changed, 371 insertions(+)
>  create mode 100644 tools/testing/selftests/kcov/Makefile
>  create mode 100644 tools/testing/selftests/kcov/kcov_test.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5bbc78b0fa6ed..0ec909e085077 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12833,6 +12833,7 @@ F:      include/linux/kcov_types.h
>  F:     include/uapi/linux/kcov.h
>  F:     kernel/kcov.c
>  F:     scripts/Makefile.kcov
> +F:     tools/testing/selftests/kcov/
>
>  KCSAN
>  M:     Marco Elver <elver@google.com>
> diff --git a/tools/testing/selftests/kcov/Makefile b/tools/testing/selftests/kcov/Makefile
> new file mode 100644
> index 0000000000000..08abf8b60bcf9
> --- /dev/null
> +++ b/tools/testing/selftests/kcov/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +LDFLAGS += -static
> +
> +TEST_GEN_PROGS := kcov_test
> +
> +include ../lib.mk
> diff --git a/tools/testing/selftests/kcov/kcov_test.c b/tools/testing/selftests/kcov/kcov_test.c
> new file mode 100644
> index 0000000000000..4d3ca41f28af4
> --- /dev/null
> +++ b/tools/testing/selftests/kcov/kcov_test.c
> @@ -0,0 +1,364 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Test the kernel coverage (/sys/kernel/debug/kcov).
> + *
> + * Copyright 2025 Google LLC.
> + */
> +#include <fcntl.h>
> +#include <linux/kcov.h>
> +#include <stdint.h>
> +#include <stddef.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <sys/ioctl.h>
> +#include <sys/mman.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#include "../kselftest_harness.h"
> +
> +/* Normally these defines should be provided by linux/kcov.h, but they aren't there yet. */

Then I think we need to do:
#ifndef KCOV_UNIQUE_ENABLE



> +#define KCOV_UNIQUE_ENABLE _IOW('c', 103, unsigned long)
> +#define KCOV_RESET_TRACE _IO('c', 104)
> +
> +#define COVER_SIZE (64 << 10)
> +#define BITMAP_SIZE (4 << 10)
> +
> +#define DEBUG_COVER_PCS 0
> +
> +FIXTURE(kcov)
> +{
> +       int fd;
> +       unsigned long *mapping;
> +       size_t mapping_size;
> +};
> +
> +FIXTURE_VARIANT(kcov)
> +{
> +       int mode;
> +       bool fast_reset;
> +       bool map_readonly;
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(kcov, mode_trace_pc)
> +{
> +       /* clang-format on */
> +       .mode = KCOV_TRACE_PC,
> +       .fast_reset = true,
> +       .map_readonly = false,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(kcov, mode_trace_cmp)
> +{
> +       /* clang-format on */
> +       .mode = KCOV_TRACE_CMP,
> +       .fast_reset = true,
> +       .map_readonly = false,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(kcov, reset_ioctl_rw)
> +{
> +       /* clang-format on */
> +       .mode = KCOV_TRACE_PC,
> +       .fast_reset = false,
> +       .map_readonly = false,
> +};
> +
> +FIXTURE_VARIANT_ADD(kcov, reset_ioctl_ro)
> +/* clang-format off */
> +{
> +       /* clang-format on */
> +       .mode = KCOV_TRACE_PC,
> +       .fast_reset = false,
> +       .map_readonly = true,
> +};
> +
> +int kcov_open_init(struct __test_metadata *_metadata, unsigned long size,
> +                  int prot, unsigned long **out_mapping)
> +{
> +       unsigned long *mapping;
> +
> +       /* A single fd descriptor allows coverage collection on a single thread. */
> +       int fd = open("/sys/kernel/debug/kcov", O_RDWR);
> +
> +       ASSERT_NE(fd, -1)
> +       {
> +               perror("open");
> +       }
> +
> +       EXPECT_EQ(ioctl(fd, KCOV_INIT_TRACE, COVER_SIZE), 0)
> +       {
> +               perror("ioctl KCOV_INIT_TRACE");
> +               close(fd);
> +       }
> +
> +       /* Mmap buffer shared between kernel- and user-space. */
> +       mapping = (unsigned long *)mmap(NULL, size * sizeof(unsigned long),
> +                                       prot, MAP_SHARED, fd, 0);
> +       ASSERT_NE((void *)mapping, MAP_FAILED)
> +       {
> +               perror("mmap");
> +               close(fd);
> +       }
> +       *out_mapping = mapping;
> +
> +       return fd;
> +}
> +
> +FIXTURE_SETUP(kcov)
> +{
> +       int prot = variant->map_readonly ? PROT_READ : (PROT_READ | PROT_WRITE);
> +
> +       /* Read-only mapping is incompatible with fast reset. */
> +       ASSERT_FALSE(variant->map_readonly && variant->fast_reset);
> +
> +       self->mapping_size = COVER_SIZE;
> +       self->fd = kcov_open_init(_metadata, self->mapping_size, prot,
> +                                 &(self->mapping));
> +
> +       /* Enable coverage collection on the current thread. */
> +       EXPECT_EQ(ioctl(self->fd, KCOV_ENABLE, variant->mode), 0)
> +       {
> +               perror("ioctl KCOV_ENABLE");
> +               munmap(self->mapping, COVER_SIZE * sizeof(unsigned long));
> +               close(self->fd);
> +       }
> +}
> +
> +void kcov_uninit_close(struct __test_metadata *_metadata, int fd,
> +                      unsigned long *mapping, size_t size)
> +{
> +       /* Disable coverage collection for the current thread. */
> +       EXPECT_EQ(ioctl(fd, KCOV_DISABLE, 0), 0)
> +       {
> +               perror("ioctl KCOV_DISABLE");
> +       }
> +
> +       /* Free resources. */
> +       EXPECT_EQ(munmap(mapping, size * sizeof(unsigned long)), 0)
> +       {
> +               perror("munmap");
> +       }
> +
> +       EXPECT_EQ(close(fd), 0)
> +       {
> +               perror("close");
> +       }
> +}
> +
> +FIXTURE_TEARDOWN(kcov)
> +{
> +       kcov_uninit_close(_metadata, self->fd, self->mapping,
> +                         self->mapping_size);
> +}
> +
> +void dump_collected_pcs(struct __test_metadata *_metadata, unsigned long *cover,
> +                       size_t start, size_t end)
> +{
> +       int i = 0;
> +
> +       TH_LOG("Collected %lu PCs", end - start);
> +#if DEBUG_COVER_PCS
> +       for (i = start; i < end; i++)
> +               TH_LOG("0x%lx", cover[i + 1]);
> +#endif
> +}
> +
> +/* Coverage collection helper without assertions. */
> +unsigned long collect_coverage_unchecked(struct __test_metadata *_metadata,
> +                                        unsigned long *cover, bool dump)
> +{
> +       unsigned long before, after;
> +
> +       before = __atomic_load_n(&cover[0], __ATOMIC_RELAXED);
> +       /*
> +        * Call the target syscall call. Here we use read(-1, NULL, 0) as an example.
> +        * This will likely return an error (-EFAULT or -EBADF), but the goal is to
> +        * collect coverage for the syscall's entry/exit paths.
> +        */
> +       read(-1, NULL, 0);
> +
> +       after = __atomic_load_n(&cover[0], __ATOMIC_RELAXED);
> +
> +       if (dump)
> +               dump_collected_pcs(_metadata, cover, before, after);
> +       return after - before;
> +}
> +
> +unsigned long collect_coverage_once(struct __test_metadata *_metadata,
> +                                   unsigned long *cover)
> +{
> +       unsigned long collected =
> +               collect_coverage_unchecked(_metadata, cover, /*dump*/ true);
> +
> +       /* Coverage must be non-zero. */
> +       EXPECT_GT(collected, 0);
> +       return collected;
> +}
> +
> +void reset_coverage(struct __test_metadata *_metadata, bool fast, int fd,
> +                   unsigned long *mapping)
> +{
> +       unsigned long count;
> +
> +       if (fast) {
> +               __atomic_store_n(&mapping[0], 0, __ATOMIC_RELAXED);
> +       } else {
> +               EXPECT_EQ(ioctl(fd, KCOV_RESET_TRACE, 0), 0)
> +               {
> +                       perror("ioctl KCOV_RESET_TRACE");
> +               }
> +               count = __atomic_load_n(&mapping[0], __ATOMIC_RELAXED);
> +               EXPECT_NE(count, 0);
> +       }
> +}
> +
> +TEST_F(kcov, kcov_basic_syscall_coverage)
> +{
> +       unsigned long first, second, before, after, i;
> +
> +       /* Reset coverage that may be left over from the fixture setup. */
> +       reset_coverage(_metadata, variant->fast_reset, self->fd, self->mapping);
> +
> +       /* Collect the coverage for a single syscall two times in a row. */
> +       first = collect_coverage_once(_metadata, self->mapping);
> +       second = collect_coverage_once(_metadata, self->mapping);
> +       /* Collected coverage should not differ too much. */
> +       EXPECT_GT(first * 10, second);
> +       EXPECT_GT(second * 10, first);
> +
> +       /* Now reset the buffer and collect the coverage again. */
> +       reset_coverage(_metadata, variant->fast_reset, self->fd, self->mapping);
> +       collect_coverage_once(_metadata, self->mapping);
> +
> +       /* Now try many times to fill up the buffer. */
> +       reset_coverage(_metadata, variant->fast_reset, self->fd, self->mapping);
> +       while (collect_coverage_unchecked(_metadata, self->mapping,
> +                                         /*dump*/ false)) {
> +               /* Do nothing. */
> +       }
> +       before = __atomic_load_n(&(self->mapping[0]), __ATOMIC_RELAXED);
> +       /*
> +        * Resetting with ioctl may still generate some coverage, but much less
> +        * than there was before.
> +        */
> +       reset_coverage(_metadata, variant->fast_reset, self->fd, self->mapping);
> +       after = __atomic_load_n(&(self->mapping[0]), __ATOMIC_RELAXED);
> +       EXPECT_GT(before, after);
> +       /* Collecting coverage after reset will now succeed. */
> +       collect_coverage_once(_metadata, self->mapping);
> +}
> +
> +FIXTURE(kcov_uniq)
> +{
> +       int fd;
> +       unsigned long *mapping;
> +       size_t mapping_size;
> +       unsigned long *bitmap;
> +       size_t bitmap_size;
> +       unsigned long *cover;
> +       size_t cover_size;
> +};
> +
> +FIXTURE_VARIANT(kcov_uniq)
> +{
> +       bool fast_reset;
> +       bool map_readonly;
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(kcov_uniq, fast_rw)
> +{
> +       /* clang-format on */
> +       .fast_reset = true,
> +       .map_readonly = false,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(kcov_uniq, slow_rw)
> +{
> +       /* clang-format on */
> +       .fast_reset = false,
> +       .map_readonly = false,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(kcov_uniq, slow_ro)
> +{
> +       /* clang-format on */
> +       .fast_reset = false,
> +       .map_readonly = true,
> +};
> +
> +FIXTURE_SETUP(kcov_uniq)
> +{
> +       int prot = variant->map_readonly ? PROT_READ : (PROT_READ | PROT_WRITE);
> +
> +       /* Read-only mapping is incompatible with fast reset. */
> +       ASSERT_FALSE(variant->map_readonly && variant->fast_reset);
> +
> +       self->mapping_size = COVER_SIZE;
> +       self->fd = kcov_open_init(_metadata, self->mapping_size, prot,
> +                                 &(self->mapping));
> +
> +       /* Enable coverage collection on the current thread. */
> +       EXPECT_EQ(ioctl(self->fd, KCOV_UNIQUE_ENABLE, BITMAP_SIZE), 0)
> +       {
> +               perror("ioctl KCOV_ENABLE");
> +               munmap(self->mapping, COVER_SIZE * sizeof(unsigned long));
> +               close(self->fd);
> +       }
> +}
> +
> +FIXTURE_TEARDOWN(kcov_uniq)
> +{
> +       kcov_uninit_close(_metadata, self->fd, self->mapping,
> +                         self->mapping_size);
> +}
> +
> +TEST_F(kcov_uniq, kcov_uniq_coverage)
> +{
> +       unsigned long first, second, before, after, i;
> +
> +       /* Reset coverage that may be left over from the fixture setup. */
> +       reset_coverage(_metadata, variant->fast_reset, self->fd, self->mapping);
> +
> +       /*
> +        * Collect the coverage for a single syscall two times in a row.
> +        * Use collect_coverage_unchecked(), because it may return zero coverage.
> +        */
> +       first = collect_coverage_unchecked(_metadata, self->mapping,
> +                                          /*dump*/ true);
> +       second = collect_coverage_unchecked(_metadata, self->mapping,
> +                                           /*dump*/ true);
> +
> +       /* Now reset the buffer and collect the coverage again. */
> +       reset_coverage(_metadata, variant->fast_reset, self->fd, self->mapping);
> +       collect_coverage_once(_metadata, self->mapping);
> +
> +       /* Now try many times to saturate the unique coverage bitmap. */
> +       reset_coverage(_metadata, variant->fast_reset, self->fd, self->mapping);
> +       for (i = 0; i < 1000; i++)
> +               collect_coverage_unchecked(_metadata, self->mapping,
> +                                          /*dump*/ false);
> +       /* Another invocation of collect_coverage_unchecked() should not produce new coverage. */
> +       EXPECT_EQ(collect_coverage_unchecked(_metadata, self->mapping,
> +                                            /*dump*/ false),
> +                 0);
> +
> +       before = __atomic_load_n(&(self->mapping[0]), __ATOMIC_RELAXED);
> +       /*
> +        * Resetting with ioctl may still generate some coverage, but much less
> +        * than there was before.
> +        */
> +       reset_coverage(_metadata, variant->fast_reset, self->fd, self->mapping);
> +       after = __atomic_load_n(&(self->mapping[0]), __ATOMIC_RELAXED);
> +       EXPECT_GT(before, after);
> +       /* Collecting coverage after reset will now succeed. */
> +       collect_coverage_once(_metadata, self->mapping);
> +}
> +
> +TEST_HARNESS_MAIN
> --
> 2.50.0.727.gbf7dc18ff4-goog
>

Re: [PATCH v2 04/11] kcov: factor out struct kcov_state

[Alexander Potapenko]

> > -       /* Buffer for coverage collection: */
> > -       void                            *kcov_area;
> > +       /* kcov buffer state for this task. */
>
> For consistency: s/kcov/KCOV/
Ack


> >         if (data->saved_kcov) {
> > -               kcov_start(t, data->saved_kcov, data->saved_size,
> > -                          data->saved_area, data->saved_mode,
> > -                          data->saved_sequence);
> > -               data->saved_mode = 0;
> > -               data->saved_size = 0;
> > -               data->saved_area = NULL;
> > -               data->saved_sequence = 0;
> > +               kcov_start(t, data->saved_kcov, t->kcov_mode,
>
> We used to pass data->saved_mode, now we pass t->kcov_mode.
> Are they the same here? This makes me a bit nervous.

Thanks for noticing! I'll fix this one in v3.



> > -       kcov_start(t, kcov, size, area, mode, sequence);
> > +       kcov_start(t, kcov, t->kcov_mode, &state);
>
> We used to pass kcov->mode here, now it's t->kcov_mode.
> Are they the same here? I would prefer to restore the current version,
> if there is no specific reason to change it.

Ditto.

Re: [PATCH v2 06/11] kcov: x86: introduce CONFIG_KCOV_UNIQUE

[Alexander Potapenko]

On Wed, Jul 9, 2025 at 5:01 PM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Thu, 26 Jun 2025 at 15:42, Alexander Potapenko <glider@google.com> wrote:
> >
> > The new config switches coverage instrumentation to using
> >   __sanitizer_cov_trace_pc_guard(u32 *guard)
> > instead of
> >   __sanitizer_cov_trace_pc(void)
> >
> > This relies on Clang's -fsanitize-coverage=trace-pc-guard flag [1].
> >
> > Each callback receives a unique 32-bit guard variable residing in the
> > __sancov_guards section. Those guards can be used by kcov to deduplicate
> > the coverage on the fly.
> >
> > As a first step, we make the new instrumentation mode 1:1 compatible
> > with the old one.
> >
> > [1] https://clang.llvm.org/docs/SanitizerCoverage.html#tracing-pcs-with-guards
> >
> > Cc: x86@kernel.org
> > Signed-off-by: Alexander Potapenko <glider@google.com>
> >
> > ---
> > Change-Id: Iacb1e71fd061a82c2acadf2347bba4863b9aec39
> >
> > v2:
> >  - Address comments by Dmitry Vyukov
> >    - rename CONFIG_KCOV_ENABLE_GUARDS to CONFIG_KCOV_UNIQUE
> >    - update commit description and config description
> >  - Address comments by Marco Elver
> >    - rename sanitizer_cov_write_subsequent() to kcov_append_to_buffer()
> >    - make config depend on X86_64 (via ARCH_HAS_KCOV_UNIQUE)
> >    - swap #ifdef branches
> >    - tweak config description
> >    - remove redundant check for CONFIG_CC_HAS_SANCOV_TRACE_PC_GUARD
> > ---
> >  arch/x86/Kconfig                  |  1 +
> >  arch/x86/kernel/vmlinux.lds.S     |  1 +
> >  include/asm-generic/vmlinux.lds.h | 14 ++++++-
> >  include/linux/kcov.h              |  2 +
> >  kernel/kcov.c                     | 61 +++++++++++++++++++++----------
> >  lib/Kconfig.debug                 | 24 ++++++++++++
> >  scripts/Makefile.kcov             |  4 ++
> >  scripts/module.lds.S              | 23 ++++++++++++
> >  tools/objtool/check.c             |  1 +
> >  9 files changed, 110 insertions(+), 21 deletions(-)
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index e21cca404943e..d104c5a193bdf 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -93,6 +93,7 @@ config X86
> >         select ARCH_HAS_FORTIFY_SOURCE
> >         select ARCH_HAS_GCOV_PROFILE_ALL
> >         select ARCH_HAS_KCOV                    if X86_64
> > +       select ARCH_HAS_KCOV_UNIQUE             if X86_64
> >         select ARCH_HAS_KERNEL_FPU_SUPPORT
> >         select ARCH_HAS_MEM_ENCRYPT
> >         select ARCH_HAS_MEMBARRIER_SYNC_CORE
> > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> > index cda5f8362e9da..8076e8953fddc 100644
> > --- a/arch/x86/kernel/vmlinux.lds.S
> > +++ b/arch/x86/kernel/vmlinux.lds.S
> > @@ -372,6 +372,7 @@ SECTIONS
> >                 . = ALIGN(PAGE_SIZE);
> >                 __bss_stop = .;
> >         }
> > +       SANCOV_GUARDS_BSS
> >
> >         /*
> >          * The memory occupied from _text to here, __end_of_kernel_reserve, is
> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > index 58a635a6d5bdf..875c4deb66208 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -102,7 +102,8 @@
> >   * sections to be brought in with rodata.
> >   */
> >  #if defined(CONFIG_LD_DEAD_CODE_DATA_ELIMINATION) || defined(CONFIG_LTO_CLANG) || \
> > -defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
> > +       defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG) || \
> > +       defined(CONFIG_KCOV_UNIQUE)
> >  #define TEXT_MAIN .text .text.[0-9a-zA-Z_]*
> >  #else
> >  #define TEXT_MAIN .text
> > @@ -121,6 +122,17 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
> >  #define SBSS_MAIN .sbss
> >  #endif
> >
> > +#if defined(CONFIG_KCOV_UNIQUE)
> > +#define SANCOV_GUARDS_BSS                      \
> > +       __sancov_guards(NOLOAD) : {             \
> > +               __start___sancov_guards = .;    \
> > +               *(__sancov_guards);             \
> > +               __stop___sancov_guards = .;     \
> > +       }
> > +#else
> > +#define SANCOV_GUARDS_BSS
> > +#endif
> > +
> >  /*
> >   * GCC 4.5 and later have a 32 bytes section alignment for structures.
> >   * Except GCC 4.9, that feels the need to align on 64 bytes.
> > diff --git a/include/linux/kcov.h b/include/linux/kcov.h
> > index 0e425c3524b86..dd8bbee6fe274 100644
> > --- a/include/linux/kcov.h
> > +++ b/include/linux/kcov.h
> > @@ -107,6 +107,8 @@ typedef unsigned long long kcov_u64;
> >  #endif
> >
> >  void __sanitizer_cov_trace_pc(void);
> > +void __sanitizer_cov_trace_pc_guard(u32 *guard);
> > +void __sanitizer_cov_trace_pc_guard_init(uint32_t *start, uint32_t *stop);
> >  void __sanitizer_cov_trace_cmp1(u8 arg1, u8 arg2);
> >  void __sanitizer_cov_trace_cmp2(u16 arg1, u16 arg2);
> >  void __sanitizer_cov_trace_cmp4(u32 arg1, u32 arg2);
> > diff --git a/kernel/kcov.c b/kernel/kcov.c
> > index ff7f118644f49..8e98ca8d52743 100644
> > --- a/kernel/kcov.c
> > +++ b/kernel/kcov.c
> > @@ -195,27 +195,15 @@ static notrace unsigned long canonicalize_ip(unsigned long ip)
> >         return ip;
> >  }
> >
> > -/*
> > - * Entry point from instrumented code.
> > - * This is called once per basic-block/edge.
> > - */
> > -void notrace __sanitizer_cov_trace_pc(void)
> > +static notrace void kcov_append_to_buffer(unsigned long *area, int size,
> > +                                         unsigned long ip)
> >  {
> > -       struct task_struct *t;
> > -       unsigned long *area;
> > -       unsigned long ip = canonicalize_ip(_RET_IP_);
> > -       unsigned long pos;
> > -
> > -       t = current;
> > -       if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t))
> > -               return;
> > -
> > -       area = t->kcov_state.area;
> >         /* The first 64-bit word is the number of subsequent PCs. */
> > -       pos = READ_ONCE(area[0]) + 1;
> > -       if (likely(pos < t->kcov_state.size)) {
> > -               /* Previously we write pc before updating pos. However, some
> > -                * early interrupt code could bypass check_kcov_mode() check
> > +       unsigned long pos = READ_ONCE(area[0]) + 1;
> > +
> > +       if (likely(pos < size)) {
> > +               /*
> > +                * Some early interrupt code could bypass check_kcov_mode() check
> >                  * and invoke __sanitizer_cov_trace_pc(). If such interrupt is
> >                  * raised between writing pc and updating pos, the pc could be
> >                  * overitten by the recursive __sanitizer_cov_trace_pc().
> > @@ -226,7 +214,40 @@ void notrace __sanitizer_cov_trace_pc(void)
> >                 area[pos] = ip;
> >         }
> >  }
> > +
> > +/*
> > + * Entry point from instrumented code.
> > + * This is called once per basic-block/edge.
> > + */
> > +#ifdef CONFIG_KCOV_UNIQUE
> > +void notrace __sanitizer_cov_trace_pc_guard(u32 *guard)
> > +{
> > +       if (!check_kcov_mode(KCOV_MODE_TRACE_PC, current))
> > +               return;
> > +
> > +       kcov_append_to_buffer(current->kcov_state.area,
> > +                             current->kcov_state.size,
> > +                             canonicalize_ip(_RET_IP_));
> > +}
> > +EXPORT_SYMBOL(__sanitizer_cov_trace_pc_guard);
> > +
> > +void notrace __sanitizer_cov_trace_pc_guard_init(uint32_t *start,
> > +                                                uint32_t *stop)
> > +{
> > +}
> > +EXPORT_SYMBOL(__sanitizer_cov_trace_pc_guard_init);
> > +#else /* !CONFIG_KCOV_UNIQUE */
> > +void notrace __sanitizer_cov_trace_pc(void)
> > +{
> > +       if (!check_kcov_mode(KCOV_MODE_TRACE_PC, current))
> > +               return;
> > +
> > +       kcov_append_to_buffer(current->kcov_state.area,
> > +                             current->kcov_state.size,
> > +                             canonicalize_ip(_RET_IP_));
> > +}
> >  EXPORT_SYMBOL(__sanitizer_cov_trace_pc);
> > +#endif
> >
> >  #ifdef CONFIG_KCOV_ENABLE_COMPARISONS
> >  static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip)
> > @@ -254,7 +275,7 @@ static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip)
> >         start_index = 1 + count * KCOV_WORDS_PER_CMP;
> >         end_pos = (start_index + KCOV_WORDS_PER_CMP) * sizeof(u64);
> >         if (likely(end_pos <= max_pos)) {
> > -               /* See comment in __sanitizer_cov_trace_pc(). */
> > +               /* See comment in kcov_append_to_buffer(). */
> >                 WRITE_ONCE(area[0], count + 1);
> >                 barrier();
> >                 area[start_index] = type;
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index f9051ab610d54..24dcb721dbb0b 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -2156,6 +2156,8 @@ config ARCH_HAS_KCOV
> >  config CC_HAS_SANCOV_TRACE_PC
> >         def_bool $(cc-option,-fsanitize-coverage=trace-pc)
> >
> > +config CC_HAS_SANCOV_TRACE_PC_GUARD
> > +       def_bool $(cc-option,-fsanitize-coverage=trace-pc-guard)
> >
> >  config KCOV
> >         bool "Code coverage for fuzzing"
> > @@ -2172,6 +2174,28 @@ config KCOV
> >
> >           For more details, see Documentation/dev-tools/kcov.rst.
> >
> > +config ARCH_HAS_KCOV_UNIQUE
> > +       bool
> > +       help
> > +         An architecture should select this when it can successfully
> > +         build and run with CONFIG_KCOV_UNIQUE.
> > +
> > +config KCOV_UNIQUE
> > +       depends on KCOV
> > +       depends on CC_HAS_SANCOV_TRACE_PC_GUARD && ARCH_HAS_KCOV_UNIQUE
> > +       bool "Use coverage guards for KCOV"
> > +       help
> > +         Use coverage guards instrumentation for KCOV, passing
> > +         -fsanitize-coverage=trace-pc-guard to the compiler.
>
> I think this should talk about the new mode, the new ioctl's, and
> visible differences for end users first.

Something like this, maybe?

          This option enables KCOV's unique program counter (PC)
collection mode,
          which deduplicates PCs on the fly when the KCOV_UNIQUE_ENABLE ioctl is
          used.

          This significantly reduces the memory footprint for coverage data
          collection compared to trace mode, as it prevents the kernel from
          storing the same PC multiple times.
          Enabling this mode incurs a slight increase in kernel binary size.


> > +         Every coverage callback is associated with a global variable that
> > +         allows to efficiently deduplicate coverage at collection time.
> > +         This drastically reduces the buffer size required for coverage
> > +         collection.
> > +
> > +         This config comes at a cost of increased binary size (4 bytes of .bss
> > +         plus 1-2 instructions to pass an extra parameter, per basic block).
> > +
> >  config KCOV_ENABLE_COMPARISONS
> >         bool "Enable comparison operands collection by KCOV"
> >         depends on KCOV
> > diff --git a/scripts/Makefile.kcov b/scripts/Makefile.kcov
> > index 67e8cfe3474b7..0b17533ef35f6 100644
> > --- a/scripts/Makefile.kcov
> > +++ b/scripts/Makefile.kcov
> > @@ -1,5 +1,9 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> > +ifeq ($(CONFIG_KCOV_UNIQUE),y)
> > +kcov-flags-y                                   += -fsanitize-coverage=trace-pc-guard
> > +else
> >  kcov-flags-$(CONFIG_CC_HAS_SANCOV_TRACE_PC)    += -fsanitize-coverage=trace-pc
> > +endif
> >  kcov-flags-$(CONFIG_KCOV_ENABLE_COMPARISONS)   += -fsanitize-coverage=trace-cmp
> >  kcov-flags-$(CONFIG_GCC_PLUGIN_SANCOV)         += -fplugin=$(objtree)/scripts/gcc-plugins/sancov_plugin.so
> >
> > diff --git a/scripts/module.lds.S b/scripts/module.lds.S
> > index 450f1088d5fd3..314b56680ea1a 100644
> > --- a/scripts/module.lds.S
> > +++ b/scripts/module.lds.S
> > @@ -64,6 +64,29 @@ SECTIONS {
> >                 MOD_CODETAG_SECTIONS()
> >         }
> >  #endif
> > +
> > +#ifdef CONFIG_KCOV_UNIQUE
> > +       __sancov_guards(NOLOAD) : {
> > +               __start___sancov_guards = .;
> > +               *(__sancov_guards);
> > +               __stop___sancov_guards = .;
> > +       }
> > +
> > +       .text : {
> > +               *(.text .text.[0-9a-zA-Z_]*)
> > +               *(.text..L*)
> > +       }
>
> Why do we need these here? .text does not look specific to CONFIG_KCOV_UNIQUE.
> Is it because of constructors/destructors emitted by the compiler, and
> .init.text/.exit.text don't work w/o .text?
> A comment here would be useful.

This is because the compiler creates duplicate .init.text/.exit.text,
making the module loader unhappy.
I'll add a comment.

Re: [PATCH v2 06/11] kcov: x86: introduce CONFIG_KCOV_UNIQUE

[Dmitry Vyukov]

On Fri, 25 Jul 2025 at 12:07, Alexander Potapenko <glider@google.com> wrote:
>
> On Wed, Jul 9, 2025 at 5:01 PM Dmitry Vyukov <dvyukov@google.com> wrote:
> >
> > On Thu, 26 Jun 2025 at 15:42, Alexander Potapenko <glider@google.com> wrote:
> > >
> > > The new config switches coverage instrumentation to using
> > >   __sanitizer_cov_trace_pc_guard(u32 *guard)
> > > instead of
> > >   __sanitizer_cov_trace_pc(void)
> > >
> > > This relies on Clang's -fsanitize-coverage=trace-pc-guard flag [1].
> > >
> > > Each callback receives a unique 32-bit guard variable residing in the
> > > __sancov_guards section. Those guards can be used by kcov to deduplicate
> > > the coverage on the fly.
> > >
> > > As a first step, we make the new instrumentation mode 1:1 compatible
> > > with the old one.
> > >
> > > [1] https://clang.llvm.org/docs/SanitizerCoverage.html#tracing-pcs-with-guards
> > >
> > > Cc: x86@kernel.org
> > > Signed-off-by: Alexander Potapenko <glider@google.com>
> > >
> > > ---
> > > Change-Id: Iacb1e71fd061a82c2acadf2347bba4863b9aec39
> > >
> > > v2:
> > >  - Address comments by Dmitry Vyukov
> > >    - rename CONFIG_KCOV_ENABLE_GUARDS to CONFIG_KCOV_UNIQUE
> > >    - update commit description and config description
> > >  - Address comments by Marco Elver
> > >    - rename sanitizer_cov_write_subsequent() to kcov_append_to_buffer()
> > >    - make config depend on X86_64 (via ARCH_HAS_KCOV_UNIQUE)
> > >    - swap #ifdef branches
> > >    - tweak config description
> > >    - remove redundant check for CONFIG_CC_HAS_SANCOV_TRACE_PC_GUARD
> > > ---
> > >  arch/x86/Kconfig                  |  1 +
> > >  arch/x86/kernel/vmlinux.lds.S     |  1 +
> > >  include/asm-generic/vmlinux.lds.h | 14 ++++++-
> > >  include/linux/kcov.h              |  2 +
> > >  kernel/kcov.c                     | 61 +++++++++++++++++++++----------
> > >  lib/Kconfig.debug                 | 24 ++++++++++++
> > >  scripts/Makefile.kcov             |  4 ++
> > >  scripts/module.lds.S              | 23 ++++++++++++
> > >  tools/objtool/check.c             |  1 +
> > >  9 files changed, 110 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > index e21cca404943e..d104c5a193bdf 100644
> > > --- a/arch/x86/Kconfig
> > > +++ b/arch/x86/Kconfig
> > > @@ -93,6 +93,7 @@ config X86
> > >         select ARCH_HAS_FORTIFY_SOURCE
> > >         select ARCH_HAS_GCOV_PROFILE_ALL
> > >         select ARCH_HAS_KCOV                    if X86_64
> > > +       select ARCH_HAS_KCOV_UNIQUE             if X86_64
> > >         select ARCH_HAS_KERNEL_FPU_SUPPORT
> > >         select ARCH_HAS_MEM_ENCRYPT
> > >         select ARCH_HAS_MEMBARRIER_SYNC_CORE
> > > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> > > index cda5f8362e9da..8076e8953fddc 100644
> > > --- a/arch/x86/kernel/vmlinux.lds.S
> > > +++ b/arch/x86/kernel/vmlinux.lds.S
> > > @@ -372,6 +372,7 @@ SECTIONS
> > >                 . = ALIGN(PAGE_SIZE);
> > >                 __bss_stop = .;
> > >         }
> > > +       SANCOV_GUARDS_BSS
> > >
> > >         /*
> > >          * The memory occupied from _text to here, __end_of_kernel_reserve, is
> > > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > > index 58a635a6d5bdf..875c4deb66208 100644
> > > --- a/include/asm-generic/vmlinux.lds.h
> > > +++ b/include/asm-generic/vmlinux.lds.h
> > > @@ -102,7 +102,8 @@
> > >   * sections to be brought in with rodata.
> > >   */
> > >  #if defined(CONFIG_LD_DEAD_CODE_DATA_ELIMINATION) || defined(CONFIG_LTO_CLANG) || \
> > > -defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
> > > +       defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG) || \
> > > +       defined(CONFIG_KCOV_UNIQUE)
> > >  #define TEXT_MAIN .text .text.[0-9a-zA-Z_]*
> > >  #else
> > >  #define TEXT_MAIN .text
> > > @@ -121,6 +122,17 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
> > >  #define SBSS_MAIN .sbss
> > >  #endif
> > >
> > > +#if defined(CONFIG_KCOV_UNIQUE)
> > > +#define SANCOV_GUARDS_BSS                      \
> > > +       __sancov_guards(NOLOAD) : {             \
> > > +               __start___sancov_guards = .;    \
> > > +               *(__sancov_guards);             \
> > > +               __stop___sancov_guards = .;     \
> > > +       }
> > > +#else
> > > +#define SANCOV_GUARDS_BSS
> > > +#endif
> > > +
> > >  /*
> > >   * GCC 4.5 and later have a 32 bytes section alignment for structures.
> > >   * Except GCC 4.9, that feels the need to align on 64 bytes.
> > > diff --git a/include/linux/kcov.h b/include/linux/kcov.h
> > > index 0e425c3524b86..dd8bbee6fe274 100644
> > > --- a/include/linux/kcov.h
> > > +++ b/include/linux/kcov.h
> > > @@ -107,6 +107,8 @@ typedef unsigned long long kcov_u64;
> > >  #endif
> > >
> > >  void __sanitizer_cov_trace_pc(void);
> > > +void __sanitizer_cov_trace_pc_guard(u32 *guard);
> > > +void __sanitizer_cov_trace_pc_guard_init(uint32_t *start, uint32_t *stop);
> > >  void __sanitizer_cov_trace_cmp1(u8 arg1, u8 arg2);
> > >  void __sanitizer_cov_trace_cmp2(u16 arg1, u16 arg2);
> > >  void __sanitizer_cov_trace_cmp4(u32 arg1, u32 arg2);
> > > diff --git a/kernel/kcov.c b/kernel/kcov.c
> > > index ff7f118644f49..8e98ca8d52743 100644
> > > --- a/kernel/kcov.c
> > > +++ b/kernel/kcov.c
> > > @@ -195,27 +195,15 @@ static notrace unsigned long canonicalize_ip(unsigned long ip)
> > >         return ip;
> > >  }
> > >
> > > -/*
> > > - * Entry point from instrumented code.
> > > - * This is called once per basic-block/edge.
> > > - */
> > > -void notrace __sanitizer_cov_trace_pc(void)
> > > +static notrace void kcov_append_to_buffer(unsigned long *area, int size,
> > > +                                         unsigned long ip)
> > >  {
> > > -       struct task_struct *t;
> > > -       unsigned long *area;
> > > -       unsigned long ip = canonicalize_ip(_RET_IP_);
> > > -       unsigned long pos;
> > > -
> > > -       t = current;
> > > -       if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t))
> > > -               return;
> > > -
> > > -       area = t->kcov_state.area;
> > >         /* The first 64-bit word is the number of subsequent PCs. */
> > > -       pos = READ_ONCE(area[0]) + 1;
> > > -       if (likely(pos < t->kcov_state.size)) {
> > > -               /* Previously we write pc before updating pos. However, some
> > > -                * early interrupt code could bypass check_kcov_mode() check
> > > +       unsigned long pos = READ_ONCE(area[0]) + 1;
> > > +
> > > +       if (likely(pos < size)) {
> > > +               /*
> > > +                * Some early interrupt code could bypass check_kcov_mode() check
> > >                  * and invoke __sanitizer_cov_trace_pc(). If such interrupt is
> > >                  * raised between writing pc and updating pos, the pc could be
> > >                  * overitten by the recursive __sanitizer_cov_trace_pc().
> > > @@ -226,7 +214,40 @@ void notrace __sanitizer_cov_trace_pc(void)
> > >                 area[pos] = ip;
> > >         }
> > >  }
> > > +
> > > +/*
> > > + * Entry point from instrumented code.
> > > + * This is called once per basic-block/edge.
> > > + */
> > > +#ifdef CONFIG_KCOV_UNIQUE
> > > +void notrace __sanitizer_cov_trace_pc_guard(u32 *guard)
> > > +{
> > > +       if (!check_kcov_mode(KCOV_MODE_TRACE_PC, current))
> > > +               return;
> > > +
> > > +       kcov_append_to_buffer(current->kcov_state.area,
> > > +                             current->kcov_state.size,
> > > +                             canonicalize_ip(_RET_IP_));
> > > +}
> > > +EXPORT_SYMBOL(__sanitizer_cov_trace_pc_guard);
> > > +
> > > +void notrace __sanitizer_cov_trace_pc_guard_init(uint32_t *start,
> > > +                                                uint32_t *stop)
> > > +{
> > > +}
> > > +EXPORT_SYMBOL(__sanitizer_cov_trace_pc_guard_init);
> > > +#else /* !CONFIG_KCOV_UNIQUE */
> > > +void notrace __sanitizer_cov_trace_pc(void)
> > > +{
> > > +       if (!check_kcov_mode(KCOV_MODE_TRACE_PC, current))
> > > +               return;
> > > +
> > > +       kcov_append_to_buffer(current->kcov_state.area,
> > > +                             current->kcov_state.size,
> > > +                             canonicalize_ip(_RET_IP_));
> > > +}
> > >  EXPORT_SYMBOL(__sanitizer_cov_trace_pc);
> > > +#endif
> > >
> > >  #ifdef CONFIG_KCOV_ENABLE_COMPARISONS
> > >  static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip)
> > > @@ -254,7 +275,7 @@ static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip)
> > >         start_index = 1 + count * KCOV_WORDS_PER_CMP;
> > >         end_pos = (start_index + KCOV_WORDS_PER_CMP) * sizeof(u64);
> > >         if (likely(end_pos <= max_pos)) {
> > > -               /* See comment in __sanitizer_cov_trace_pc(). */
> > > +               /* See comment in kcov_append_to_buffer(). */
> > >                 WRITE_ONCE(area[0], count + 1);
> > >                 barrier();
> > >                 area[start_index] = type;
> > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > > index f9051ab610d54..24dcb721dbb0b 100644
> > > --- a/lib/Kconfig.debug
> > > +++ b/lib/Kconfig.debug
> > > @@ -2156,6 +2156,8 @@ config ARCH_HAS_KCOV
> > >  config CC_HAS_SANCOV_TRACE_PC
> > >         def_bool $(cc-option,-fsanitize-coverage=trace-pc)
> > >
> > > +config CC_HAS_SANCOV_TRACE_PC_GUARD
> > > +       def_bool $(cc-option,-fsanitize-coverage=trace-pc-guard)
> > >
> > >  config KCOV
> > >         bool "Code coverage for fuzzing"
> > > @@ -2172,6 +2174,28 @@ config KCOV
> > >
> > >           For more details, see Documentation/dev-tools/kcov.rst.
> > >
> > > +config ARCH_HAS_KCOV_UNIQUE
> > > +       bool
> > > +       help
> > > +         An architecture should select this when it can successfully
> > > +         build and run with CONFIG_KCOV_UNIQUE.
> > > +
> > > +config KCOV_UNIQUE
> > > +       depends on KCOV
> > > +       depends on CC_HAS_SANCOV_TRACE_PC_GUARD && ARCH_HAS_KCOV_UNIQUE
> > > +       bool "Use coverage guards for KCOV"
> > > +       help
> > > +         Use coverage guards instrumentation for KCOV, passing
> > > +         -fsanitize-coverage=trace-pc-guard to the compiler.
> >
> > I think this should talk about the new mode, the new ioctl's, and
> > visible differences for end users first.
>
> Something like this, maybe?
>
>           This option enables KCOV's unique program counter (PC)
> collection mode,
>           which deduplicates PCs on the fly when the KCOV_UNIQUE_ENABLE ioctl is
>           used.
>
>           This significantly reduces the memory footprint for coverage data
>           collection compared to trace mode, as it prevents the kernel from
>           storing the same PC multiple times.
>           Enabling this mode incurs a slight increase in kernel binary size.
>
>

Looks good to me.

> > > +         Every coverage callback is associated with a global variable that
> > > +         allows to efficiently deduplicate coverage at collection time.
> > > +         This drastically reduces the buffer size required for coverage
> > > +         collection.
> > > +
> > > +         This config comes at a cost of increased binary size (4 bytes of .bss
> > > +         plus 1-2 instructions to pass an extra parameter, per basic block).
> > > +
> > >  config KCOV_ENABLE_COMPARISONS
> > >         bool "Enable comparison operands collection by KCOV"
> > >         depends on KCOV
> > > diff --git a/scripts/Makefile.kcov b/scripts/Makefile.kcov
> > > index 67e8cfe3474b7..0b17533ef35f6 100644
> > > --- a/scripts/Makefile.kcov
> > > +++ b/scripts/Makefile.kcov
> > > @@ -1,5 +1,9 @@
> > >  # SPDX-License-Identifier: GPL-2.0-only
> > > +ifeq ($(CONFIG_KCOV_UNIQUE),y)
> > > +kcov-flags-y                                   += -fsanitize-coverage=trace-pc-guard
> > > +else
> > >  kcov-flags-$(CONFIG_CC_HAS_SANCOV_TRACE_PC)    += -fsanitize-coverage=trace-pc
> > > +endif
> > >  kcov-flags-$(CONFIG_KCOV_ENABLE_COMPARISONS)   += -fsanitize-coverage=trace-cmp
> > >  kcov-flags-$(CONFIG_GCC_PLUGIN_SANCOV)         += -fplugin=$(objtree)/scripts/gcc-plugins/sancov_plugin.so
> > >
> > > diff --git a/scripts/module.lds.S b/scripts/module.lds.S
> > > index 450f1088d5fd3..314b56680ea1a 100644
> > > --- a/scripts/module.lds.S
> > > +++ b/scripts/module.lds.S
> > > @@ -64,6 +64,29 @@ SECTIONS {
> > >                 MOD_CODETAG_SECTIONS()
> > >         }
> > >  #endif
> > > +
> > > +#ifdef CONFIG_KCOV_UNIQUE
> > > +       __sancov_guards(NOLOAD) : {
> > > +               __start___sancov_guards = .;
> > > +               *(__sancov_guards);
> > > +               __stop___sancov_guards = .;
> > > +       }
> > > +
> > > +       .text : {
> > > +               *(.text .text.[0-9a-zA-Z_]*)
> > > +               *(.text..L*)
> > > +       }
> >
> > Why do we need these here? .text does not look specific to CONFIG_KCOV_UNIQUE.
> > Is it because of constructors/destructors emitted by the compiler, and
> > .init.text/.exit.text don't work w/o .text?
> > A comment here would be useful.
>
> This is because the compiler creates duplicate .init.text/.exit.text,
> making the module loader unhappy.
> I'll add a comment.

Yes, a comment would help.

Re: [PATCH v2 07/11] kcov: add trace and trace_size to struct kcov_state

[Alexander Potapenko]

> > +        * Pointer to user-provided memory used by kcov. This memory may
>
> s/kcov/KCOV/ for consistency
Ack.


> > @@ -382,11 +382,13 @@ static void kcov_start(struct task_struct *t, struct kcov *kcov,
> >
> >  static void kcov_stop(struct task_struct *t)
> >  {
> > +       int saved_sequence = t->kcov_state.sequence;
> > +
> >         WRITE_ONCE(t->kcov_mode, KCOV_MODE_DISABLED);
> >         barrier();
> >         t->kcov = NULL;
> > -       t->kcov_state.size = 0;
> > -       t->kcov_state.area = NULL;
> > +       t->kcov_state = (typeof(t->kcov_state)){ 0 };
>
> In a previous patch you used the following syntax, let's stick to one
> of these forms:
>
> data->saved_state = (struct kcov_state){};

Yeah, I did some research recently and figured out {} is more preferred.

Re: [PATCH v2 10/11] kcov: selftests: add kcov_test

[Alexander Potapenko]

On Wed, Jul 9, 2025 at 5:15 PM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Thu, 26 Jun 2025 at 15:42, Alexander Potapenko <glider@google.com> wrote:
> >
> > Implement test fixtures for testing different combinations of coverage
> > collection modes:
> >  - unique and non-unique coverage;
> >  - collecting PCs and comparison arguments;
> >  - mapping the buffer as RO and RW.
> >
> > To build:
> >  $ make -C tools/testing/selftests/kcov kcov_test
> >
> > Signed-off-by: Alexander Potapenko <glider@google.com>
> > ---
> >  MAINTAINERS                              |   1 +
> >  tools/testing/selftests/kcov/Makefile    |   6 +
> >  tools/testing/selftests/kcov/kcov_test.c | 364 +++++++++++++++++++++++
>
> Let's also add 'config' fragment (see e.g. ./dma/config)
> Otherwise it's impossible to run these tests in automated fashion.
Done in v3.

> > +/* Normally these defines should be provided by linux/kcov.h, but they aren't there yet. */
>
> Then I think we need to do:
> #ifndef KCOV_UNIQUE_ENABLE

Good point!