| Message ID | 20210312095526.197739-1-johannes@sipsolutions.net |
|---|---|
| Headers | show |
| Series | um: fix up CONFIG_GCOV support | expand |
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
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 3) As mentioned above, gcov requires init/exit handlers, but they aren't linked into binary properly, that's easy to fix. 4) gcda files are then written, so .gitignore them 5) it's not always useful to create coverage data for the *entire* kernel, so I've split off CONFIG_GCOV_BASE from CONFIG_GCOV to allow option in only in some places, which of course requires adding the necessary "subdir-cflags" or "CFLAGS_obj" changes in the places where it's desired, as local patches. None of these changes (hopefully) seem too controversional, biggest are the module changes but obviously they compile to nothing if the architecture doesn't WANT_MODULE_DESTRUCTORS. Any thoughts on how to merge this? The seq_file/.gitignore changes are independent at least code-wise, though of course it only works with the seq_file changes (.gitignore doesn't matter, of course), while the module changes are a requirement for the later ARCH=um patches since the Kconfig symbol has to exist. Perhaps I can just get ACKs on all the patches and then they can go through the UML tree? Thanks, johannes