Bug 210337

Summary: KCOV: allow nested remote coverage sections in task context
Product: Memory Management Reporter: Andrey Konovalov (andreyknvl)
Component: SanitizersAssignee: MM/Sanitizers virtual assignee (mm_sanitizers)
Status: NEW ---    
Severity: normal CC: dvyukov, kasan-dev
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: upstream Subsystem:
Regression: No Bisected commit-id:

Description Andrey Konovalov 2020-11-23 22:44:57 UTC
KCOV currently supports collecting coverage when an interrupt with a remote coverage collection section comes in the middle of a remote coverage collection section for a task. However, there's no support for having nested remote coverage collection sections when both happen in the task context. Support for this would be useful at least for USB/IP, see [1] for details.


[1] https://lkml.org/lkml/2020/10/16/592
Comment 1 Dmitry Vyukov 2020-11-24 08:03:12 UTC
2 partial, but potentially simpler solutions, just for discussion.

1. If we assume that recursion only happens for the same target KCOV descriptor, then instead of full recursion support kcov_remote_start could just check that we need to collect coverage for the already active KCOV descriptor and increment a recursion counter. Then kcov_remote_stop will decrement the recursion counter and if it's not 0, bail out.
This will allow us to avoid the WARNING and collect coverage for USB without full fledged recursion support.

2. Maybe USB code could be restructured slightly. Namely instead of:

void foo() {
  kcov_remote_start(...);
  ...
  kcov_remote_stop(...);
}

void bar() {
  kcov_remote_start(...);
  ...
  foo();
  ...
  kcov_remote_stop(...);
}

we do:

void __foo() {
  kcov_remote_start(...);
  ...
  kcov_remote_stop(...);
}

void foo() {
  kcov_remote_start(...);
  __foo();
  kcov_remote_stop(...);
}

void bar() {
  kcov_remote_start(...);
  ...
  __foo();
  ...
  kcov_remote_stop(...);
}

In some sense similar to "locked" and "unlocked" versions of the same function. If you already hold the lock, you need to call the internal "unlocked" function.
So this does not too unreasonable.

I have not looked at the USB code, though.