Collecting both remote and "local" coverage with KCOV

Collecting both remote and "local" coverage with KCOV

[Alexander Bulekov]

Hello,
I'm trying to collect coverage over the syscalls issued by my process,
as well as the kthreads spawned as a result of these syscalls
(eg coverage over vhost ioctls and the worker kthread). Is there a way
to collect coverage with both KCOV_REMOTE_ENABLE(with common_handle) and
KCOV_ENABLE, simultaneously? 

Based on the code it seems that these two modes are mutually
exclusive within a single task, but I don't think this is mentioned in
the Documentation, so I want to make sure I'm not missing something.
Thank you
-Alex

Re: Collecting both remote and "local" coverage with KCOV

[Dmitry Vyukov]

On Mon, Nov 16, 2020 at 3:39 AM Alexander Bulekov <alxndr@bu.edu> wrote:
>
> Hello,
> I'm trying to collect coverage over the syscalls issued by my process,
> as well as the kthreads spawned as a result of these syscalls
> (eg coverage over vhost ioctls and the worker kthread). Is there a way
> to collect coverage with both KCOV_REMOTE_ENABLE(with common_handle) and
> KCOV_ENABLE, simultaneously?
>
> Based on the code it seems that these two modes are mutually
> exclusive within a single task, but I don't think this is mentioned in
> the Documentation, so I want to make sure I'm not missing something.

Hi Alex,

Yes, it's probably not supported within a single task. The easiest way
to verify is to try it ;)

It is possible to collect both coverages, but you will need 2 threads
(one just to set up remote KCOV).

Unless I am missing any fundamental limitations, I would say it would
be reasonable to support this within a single task as well.

Re: Collecting both remote and "local" coverage with KCOV

[Andrey Konovalov]

On Mon, Nov 16, 2020 at 9:35 AM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Mon, Nov 16, 2020 at 3:39 AM Alexander Bulekov <alxndr@bu.edu> wrote:
> >
> > Hello,
> > I'm trying to collect coverage over the syscalls issued by my process,
> > as well as the kthreads spawned as a result of these syscalls
> > (eg coverage over vhost ioctls and the worker kthread). Is there a way
> > to collect coverage with both KCOV_REMOTE_ENABLE(with common_handle) and
> > KCOV_ENABLE, simultaneously?
> >
> > Based on the code it seems that these two modes are mutually
> > exclusive within a single task, but I don't think this is mentioned in
> > the Documentation, so I want to make sure I'm not missing something.
>
> Hi Alex,
>
> Yes, it's probably not supported within a single task. The easiest way
> to verify is to try it ;)
>
> It is possible to collect both coverages, but you will need 2 threads
> (one just to set up remote KCOV).
>
> Unless I am missing any fundamental limitations, I would say it would
> be reasonable to support this within a single task as well.

I think the reason we did that initially, is because we don't care
about normal coverage for USB emitting pseudo-syscalls. Filed a bug
for this: https://bugzilla.kernel.org/show_bug.cgi?id=210225

Re: Collecting both remote and "local" coverage with KCOV

[Alexander Bulekov]

On 201116 1805, Andrey Konovalov wrote:
> On Mon, Nov 16, 2020 at 9:35 AM Dmitry Vyukov <dvyukov@google.com> wrote:
> >
> > On Mon, Nov 16, 2020 at 3:39 AM Alexander Bulekov <alxndr@bu.edu> wrote:
> > >
> > > Hello,
> > > I'm trying to collect coverage over the syscalls issued by my process,
> > > as well as the kthreads spawned as a result of these syscalls
> > > (eg coverage over vhost ioctls and the worker kthread). Is there a way
> > > to collect coverage with both KCOV_REMOTE_ENABLE(with common_handle) and
> > > KCOV_ENABLE, simultaneously?
> > >
> > > Based on the code it seems that these two modes are mutually
> > > exclusive within a single task, but I don't think this is mentioned in
> > > the Documentation, so I want to make sure I'm not missing something.
> >
> > Hi Alex,
> >
> > Yes, it's probably not supported within a single task. The easiest way
> > to verify is to try it ;)
> >
> > It is possible to collect both coverages, but you will need 2 threads
> > (one just to set up remote KCOV).
> >
> > Unless I am missing any fundamental limitations, I would say it would
> > be reasonable to support this within a single task as well.
> 
> I think the reason we did that initially, is because we don't care
> about normal coverage for USB emitting pseudo-syscalls. Filed a bug
> for this: https://bugzilla.kernel.org/show_bug.cgi?id=210225

I'm interested in adding support for this. Looking through the code, I
can think of ~two approaches: 

1.) Allow the same kcov fd to be used to track coverage with both
KCOV_REMOTE_ENABLE and KCOV_ENABLE. If we try to use the same coverage
bitmap for both the remote and the local coverage, I think the local
part would have to deal with the kcov_remote_lock. If the local part
continues to write directly into the user-space coverage-area, as it
does now, it seems it would require locking for each __sanitizer_cov
call.  Alternatively, the local and the remote parts could write into
different coverage-bitmaps, but I'm not sure if there is a neat way to
do this.

2.) Allow multiple kcov fds to be used by the same task. In the task,
keep a linked-list of pointers to kcov objects (remote or local). For
each __sanitizer_... call, walk the linked list and check if any of the
kcov objects match the requirements (trace_cmp/trace_pc/remote). This
would also have the side-effect of enabling simultaneous PC and CMP
tracing. Of course, it seems that this would add some overhead (in the
case of a single open fd, there would be extra pointer dereferences to
get the area[], size, etc).

Am I missing a better alternative?
Thanks

Re: Collecting both remote and "local" coverage with KCOV

[Dmitry Vyukov]

On Wed, Nov 18, 2020 at 3:21 AM Alexander Bulekov <alxndr@bu.edu> wrote:
>
> On 201116 1805, Andrey Konovalov wrote:
> > On Mon, Nov 16, 2020 at 9:35 AM Dmitry Vyukov <dvyukov@google.com> wrote:
> > >
> > > On Mon, Nov 16, 2020 at 3:39 AM Alexander Bulekov <alxndr@bu.edu> wrote:
> > > >
> > > > Hello,
> > > > I'm trying to collect coverage over the syscalls issued by my process,
> > > > as well as the kthreads spawned as a result of these syscalls
> > > > (eg coverage over vhost ioctls and the worker kthread). Is there a way
> > > > to collect coverage with both KCOV_REMOTE_ENABLE(with common_handle) and
> > > > KCOV_ENABLE, simultaneously?
> > > >
> > > > Based on the code it seems that these two modes are mutually
> > > > exclusive within a single task, but I don't think this is mentioned in
> > > > the Documentation, so I want to make sure I'm not missing something.
> > >
> > > Hi Alex,
> > >
> > > Yes, it's probably not supported within a single task. The easiest way
> > > to verify is to try it ;)
> > >
> > > It is possible to collect both coverages, but you will need 2 threads
> > > (one just to set up remote KCOV).
> > >
> > > Unless I am missing any fundamental limitations, I would say it would
> > > be reasonable to support this within a single task as well.
> >
> > I think the reason we did that initially, is because we don't care
> > about normal coverage for USB emitting pseudo-syscalls. Filed a bug
> > for this: https://bugzilla.kernel.org/show_bug.cgi?id=210225
>
> I'm interested in adding support for this. Looking through the code, I
> can think of ~two approaches:
>
> 1.) Allow the same kcov fd to be used to track coverage with both
> KCOV_REMOTE_ENABLE and KCOV_ENABLE. If we try to use the same coverage
> bitmap for both the remote and the local coverage, I think the local
> part would have to deal with the kcov_remote_lock. If the local part
> continues to write directly into the user-space coverage-area, as it
> does now, it seems it would require locking for each __sanitizer_cov
> call.  Alternatively, the local and the remote parts could write into
> different coverage-bitmaps, but I'm not sure if there is a neat way to
> do this.

This has 2 problems:
 - performance (__sanitizer_cov is by far the most performance
critical part of kernel with KCOV=y)
 - recurions, locks are also traced, so it's not that we really can
call anything there

> 2.) Allow multiple kcov fds to be used by the same task. In the task,
> keep a linked-list of pointers to kcov objects (remote or local). For
> each __sanitizer_... call, walk the linked list and check if any of the
> kcov objects match the requirements (trace_cmp/trace_pc/remote). This
> would also have the side-effect of enabling simultaneous PC and CMP
> tracing. Of course, it seems that this would add some overhead (in the
> case of a single open fd, there would be extra pointer dereferences to
> get the area[], size, etc).

Walking linked list in __sanitizer_... has the same performance
problems, but I think we don't really need to do it.
Assuming we have at most 1 KCOV that traces the task itself we can
continue keeping it cached in task_struct:
https://elixir.bootlin.com/linux/v5.10-rc4/source/include/linux/sched.h#L1254
and __sanitizer_... will continue using these fields.

For the kcov pointer in task struct:
https://elixir.bootlin.com/linux/v5.10-rc4/source/include/linux/sched.h#L1257
we either have a linked list, or 1 pointer for local tracking and a
separate list for remote kcov's:
struct kcov *kcov; // local tracing
struct kcov *remote_kcovs; // remote tracing, can be more than 1
Whichever is better I am not sure, it seems that some functions would
benefit from a single list (KCOV_DISABLE), while others would benefit
from separate fields (KCOV_ENABLE).
Maybe the simplest code will be if we use both approaches -- put all
kcov's into a list, but also cache the local kcov into a separate
field? Then KCOV_DISABLE could just walk the list, but KCOV_ENABLE can
continue checking 1 field.