All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: linux-kernel@vger.kernel.org, linux-um@lists.infradead.org
Cc: Jessica Yu <jeyu@kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 0/6] um: fix up CONFIG_GCOV support
Date: Fri, 12 Mar 2021 15:33:01 +0100	[thread overview]
Message-ID: <d36ea54d8c0a8dd706826ba844a6f27691f45d55.camel@sipsolutions.net> (raw)
In-Reply-To: <20210312095526.197739-1-johannes@sipsolutions.net>

On Fri, 2021-03-12 at 10:55 +0100, Johannes Berg wrote:
> CONFIG_GCOV is fairly useful for ARCH=um (e.g. with kunit, though
> my main use case is a bit different) since it writes coverage data
> directly out like a normal userspace binary. Theoretically, that
> is.
> 
> Unfortunately, it's broken in multiple ways today:
> 
>  1) it doesn't like, due to 'mangle_path' in seq_file, and the only
>     solution to that seems to be to rename our symbol, but that's
>     not so bad, and "mangle_path" sounds very generic anyway, which
>     it isn't quite
> 
>  2) gcov requires exit handlers to write out the data, and those are
>     never called for modules, config CONSTRUCTORS exists for init
>     handlers, so add CONFIG_MODULE_DESTRUCTORS here that we can then
>     select in ARCH=um

Yeah, I wish.

None of this can really work. Thing is, __gcov_init(), called from the
constructors, will add the local data structure for the object file into
the global list (__gcov_root). So far, so good.

However, __gcov_exit(), which is called from the destructors (fini_array
which I added support for here) never gets passed the local data
structure pointer. It dumps __gcov_root, and that's it.

That basically means each executable/shared object should have its own
instance of __gcov_root and the functions. But the code in UML was set
up to export __gcov_exit(), which obviously then dumps the kernel's gcov
data.

So to make this really work we should treat modules like shared objects,
and link libgcov.a into each one of them. That might even work, but we
get

ERROR: modpost: "free" [module.ko] undefined!
ERROR: modpost: "vfprintf" [module.ko] undefined!
ERROR: modpost: "fcntl" [module.ko] undefined!
ERROR: modpost: "setbuf" [module.ko] undefined!
ERROR: modpost: "exit" [module.ko] undefined!
ERROR: modpost: "fwrite" [module.ko] undefined!
ERROR: modpost: "stderr" [module.ko] undefined!
ERROR: modpost: "fclose" [module.ko] undefined!
ERROR: modpost: "ftell" [module.ko] undefined!
ERROR: modpost: "fopen" [module.ko] undefined!
ERROR: modpost: "fread" [module.ko] undefined!
ERROR: modpost: "fdopen" [module.ko] undefined!
ERROR: modpost: "fseek" [module.ko] undefined!
ERROR: modpost: "fprintf" [module.ko] undefined!
ERROR: modpost: "strtol" [module.ko] undefined!
ERROR: modpost: "malloc" [module.ko] undefined!
ERROR: modpost: "getpid" [module.ko] undefined!
ERROR: modpost: "getenv" [module.ko] undefined!

We could of course export those, but that makes me nervous, e.g.
printf() is known to use a LOT of stack, far more than we have in the
kernel.

Also, we see:

WARNING: modpost: "__gcov_var" [module] is COMMON symbol
WARNING: modpost: "__gcov_root" [module] is COMMON symbol

which means the module cannot be loaded.

I think I'll just make CONFIG_GCOV depend on !MODULE instead, and for my
use case use CONFIG_GCOV_KERNEL.

Or maybe just kill CONFIG_GCOV entirely, since obviously nobody has ever
tried to use it with modules or with recent toolchains (gcc 9 or newer,
the mangle_path conflict).

johannes


WARNING: multiple messages have this Message-ID (diff)
From: Johannes Berg <johannes@sipsolutions.net>
To: linux-kernel@vger.kernel.org, linux-um@lists.infradead.org
Cc: Jessica Yu <jeyu@kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 0/6] um: fix up CONFIG_GCOV support
Date: Fri, 12 Mar 2021 15:33:01 +0100	[thread overview]
Message-ID: <d36ea54d8c0a8dd706826ba844a6f27691f45d55.camel@sipsolutions.net> (raw)
In-Reply-To: <20210312095526.197739-1-johannes@sipsolutions.net>

On Fri, 2021-03-12 at 10:55 +0100, Johannes Berg wrote:
> CONFIG_GCOV is fairly useful for ARCH=um (e.g. with kunit, though
> my main use case is a bit different) since it writes coverage data
> directly out like a normal userspace binary. Theoretically, that
> is.
> 
> Unfortunately, it's broken in multiple ways today:
> 
>  1) it doesn't like, due to 'mangle_path' in seq_file, and the only
>     solution to that seems to be to rename our symbol, but that's
>     not so bad, and "mangle_path" sounds very generic anyway, which
>     it isn't quite
> 
>  2) gcov requires exit handlers to write out the data, and those are
>     never called for modules, config CONSTRUCTORS exists for init
>     handlers, so add CONFIG_MODULE_DESTRUCTORS here that we can then
>     select in ARCH=um

Yeah, I wish.

None of this can really work. Thing is, __gcov_init(), called from the
constructors, will add the local data structure for the object file into
the global list (__gcov_root). So far, so good.

However, __gcov_exit(), which is called from the destructors (fini_array
which I added support for here) never gets passed the local data
structure pointer. It dumps __gcov_root, and that's it.

That basically means each executable/shared object should have its own
instance of __gcov_root and the functions. But the code in UML was set
up to export __gcov_exit(), which obviously then dumps the kernel's gcov
data.

So to make this really work we should treat modules like shared objects,
and link libgcov.a into each one of them. That might even work, but we
get

ERROR: modpost: "free" [module.ko] undefined!
ERROR: modpost: "vfprintf" [module.ko] undefined!
ERROR: modpost: "fcntl" [module.ko] undefined!
ERROR: modpost: "setbuf" [module.ko] undefined!
ERROR: modpost: "exit" [module.ko] undefined!
ERROR: modpost: "fwrite" [module.ko] undefined!
ERROR: modpost: "stderr" [module.ko] undefined!
ERROR: modpost: "fclose" [module.ko] undefined!
ERROR: modpost: "ftell" [module.ko] undefined!
ERROR: modpost: "fopen" [module.ko] undefined!
ERROR: modpost: "fread" [module.ko] undefined!
ERROR: modpost: "fdopen" [module.ko] undefined!
ERROR: modpost: "fseek" [module.ko] undefined!
ERROR: modpost: "fprintf" [module.ko] undefined!
ERROR: modpost: "strtol" [module.ko] undefined!
ERROR: modpost: "malloc" [module.ko] undefined!
ERROR: modpost: "getpid" [module.ko] undefined!
ERROR: modpost: "getenv" [module.ko] undefined!

We could of course export those, but that makes me nervous, e.g.
printf() is known to use a LOT of stack, far more than we have in the
kernel.

Also, we see:

WARNING: modpost: "__gcov_var" [module] is COMMON symbol
WARNING: modpost: "__gcov_root" [module] is COMMON symbol

which means the module cannot be loaded.

I think I'll just make CONFIG_GCOV depend on !MODULE instead, and for my
use case use CONFIG_GCOV_KERNEL.

Or maybe just kill CONFIG_GCOV entirely, since obviously nobody has ever
tried to use it with modules or with recent toolchains (gcc 9 or newer,
the mangle_path conflict).

johannes


_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

  parent reply	other threads:[~2021-03-12 14:34 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-12  9:55 [PATCH 0/6] um: fix up CONFIG_GCOV support Johannes Berg
2021-03-12  9:55 ` Johannes Berg
2021-03-12  9:55 ` [PATCH 1/6] seq_file: rename mangle_path to seq_mangle_path Johannes Berg
2021-03-12  9:55   ` Johannes Berg
2021-03-12  9:55 ` [PATCH 2/6] module: add support for CONFIG_MODULE_DESTRUCTORS Johannes Berg
2021-03-12  9:55   ` Johannes Berg
2021-03-12 10:26   ` Johannes Berg
2021-03-12 10:26     ` Johannes Berg
2021-03-12  9:55 ` [PATCH 3/6] .gitignore: also ignore gcda files Johannes Berg
2021-03-12  9:55   ` Johannes Berg
2021-03-12  9:55 ` [PATCH 4/6] um: split up CONFIG_GCOV Johannes Berg
2021-03-12  9:55   ` Johannes Berg
2021-03-18 21:27   ` Brendan Higgins
2021-03-18 21:27     ` Brendan Higgins
2021-03-18 21:30     ` Johannes Berg
2021-03-12  9:55 ` [PATCH 5/6] um: fix CONFIG_GCOV for built-in code Johannes Berg
2021-03-12  9:55   ` Johannes Berg
2021-03-12  9:55 ` [PATCH 6/6] um: fix CONFIG_GCOV for modules Johannes Berg
2021-03-12  9:55   ` Johannes Berg
2021-03-12 14:33 ` Johannes Berg [this message]
2021-03-12 14:33   ` [PATCH 0/6] um: fix up CONFIG_GCOV support Johannes Berg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d36ea54d8c0a8dd706826ba844a6f27691f45d55.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=jeyu@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-um@lists.infradead.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.