linux-kernel - Re: [PATCH] kbuild: Allow gcov to be enabled on the command line

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK7LNASQ+btvNOZ8yU6JLXBHVzPaEwj-7z0_dFouw2EUKd=3uA@mail.gmail.com>
Date:   Fri, 24 Nov 2023 11:02:00 +0900
From:   Masahiro Yamada <masahiroy@...nel.org>
To:     Kent Overstreet <kent.overstreet@...ux.dev>
Cc:     linux-kernel@...r.kernel.org,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Nicolas Schier <nicolas@...sle.eu>,
        linux-kbuild@...r.kernel.org
Subject: Re: [PATCH] kbuild: Allow gcov to be enabled on the command line

On Thu, Nov 23, 2023 at 8:55 AM Kent Overstreet
<kent.overstreet@...ux.dev> wrote:
>
> This allows gcov to be enabled for a particular kernel source
> subdirectory on the command line, without editing makefiles, like so:
>
>   make GCOV_PROFILE_fs_bcachefs=y
>
> Cc: Masahiro Yamada <masahiroy@...nel.org>
> Cc: Nathan Chancellor <nathan@...nel.org>
> Cc: Nick Desaulniers <ndesaulniers@...gle.com>
> Cc: Nicolas Schier <nicolas@...sle.eu>
> Cc: linux-kbuild@...r.kernel.org
> Signed-off-by: Kent Overstreet <kent.overstreet@...ux.dev>
> ---
>  scripts/Kbuild.include | 10 ++++++++++
>  scripts/Makefile.lib   |  2 +-
>  2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 7778cc97a4e0..5341736f2e30 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -277,3 +277,13 @@ ifneq ($(and $(filter notintermediate, $(.FEATURES)),$(filter-out 4.4,$(MAKE_VER
>  else
>  .SECONDARY:
>  endif
> +
> + # expand_parents(a/b/c) = a/b/c a/b a
> +expand_parents2 = $(if $(subst .,,$(1)),$(call expand_parents,$(1)),)
> +expand_parents  = $(1) $(call expand_parents2,$(patsubst %/,%,$(dir $(1))))
> +
> +# flatten_dirs(a/b/c) = a_b_c a_b a
> +flatten_dirs = $(subst /,_,$(call expand_parents,$(1)))
> +
> +# eval_vars(X_,a/b/c) = $(X_a_b_c) $(X_a_b) $(X_a)
> +eval_vars = $(foreach var,$(call flatten_dirs,$(2)),$($(1)$(var)))



I do not like tricky code like this.

Also, with "fs_bcachefs", it is unclear which directory
is enabled.




How about this?



[1] Specify the list of directories by GCOV_PROFILE_DIRS


diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 1a965fe68e01..286a569556b3 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -147,8 +147,12 @@ _cpp_flags     = $(KBUILD_CPPFLAGS) $(cppflags-y)
$(CPPFLAGS_$(target-stem).lds)
 # (in this order)
 #
 ifeq ($(CONFIG_GCOV_KERNEL),y)
+ifneq ($(filter $(obj),$(GCOV_PROFILE_DIRS)),)
+export GCOV_PROFILE_SUBDIR := y
+endif
+
 _c_flags += $(if $(patsubst n%,, \
-
$(GCOV_PROFILE_$(basetarget).o)$(GCOV_PROFILE)$(CONFIG_GCOV_PROFILE_ALL)),
\
+
$(GCOV_PROFILE_$(basetarget).o)$(GCOV_PROFILE)$(GCOV_PROFILE_SUBDIR)$(CONFIG_GCOV_PROFILE_ALL)),
\
                $(CFLAGS_GCOV))
 endif



Usage:

  $ make GCOV_PROFILE_DIRS=fs/bcachefs

   ->  enable GCOV in fs/bachefs and its subdirectories.

or

  $ make GCOV_PROFILE_DIRS="drivers/gpio drivers/pinctrl"

   -> enable GCOV in drivers/gpio, drivers/pinctrl, and their subdirectories.




[2] Do equivalent, but from a CONFIG option


config GCOV_PROFILE_DIRS
      string "Directories to enable GCOV"


Then, you can set CONFIG_GCOV_PROFILE_DIRS="fs/bcachefs"


This might be a more natural approach because we already have
CONFIG_GCOV_PROFILE_ALL, although it might eventually go away
because CONFIG_GCOV_PROFILE_ALL=y is almost equivalent to
CONFIG_GCOV_PROFILE_DIRS="."




diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 1a965fe68e01..286a569556b3 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -147,8 +147,12 @@ _cpp_flags     = $(KBUILD_CPPFLAGS) $(cppflags-y)
$(CPPFLAGS_$(target-stem).lds)
 # (in this order)
 #
 ifeq ($(CONFIG_GCOV_KERNEL),y)
+ifneq ($(filter $(obj),$(CONFIG_GCOV_PROFILE_DIRS)),)
+export GCOV_PROFILE_SUBDIR := y
+endif
+
 _c_flags += $(if $(patsubst n%,, \
-
$(GCOV_PROFILE_$(basetarget).o)$(GCOV_PROFILE)$(CONFIG_GCOV_PROFILE_ALL)),
\
+
$(GCOV_PROFILE_$(basetarget).o)$(GCOV_PROFILE)$(GCOV_PROFILE_SUBDIR)$(CONFIG_GCOV_PROFILE_ALL)),
\
                $(CFLAGS_GCOV))
 endif







> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 1a965fe68e01..0b4581a8bc33 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -148,7 +148,7 @@ _cpp_flags     = $(KBUILD_CPPFLAGS) $(cppflags-y) $(CPPFLAGS_$(target-stem).lds)
>  #
>  ifeq ($(CONFIG_GCOV_KERNEL),y)
>  _c_flags += $(if $(patsubst n%,, \
> -               $(GCOV_PROFILE_$(basetarget).o)$(GCOV_PROFILE)$(CONFIG_GCOV_PROFILE_ALL)), \
> +               $(GCOV_PROFILE_$(basetarget).o)$(call eval_vars,GCOV_PROFILE_,$(src))$(GCOV_PROFILE)$(CONFIG_GCOV_PROFILE_ALL)), \
>                 $(CFLAGS_GCOV))
>  endif
>
> --
> 2.42.0
>


--
Best Regards
Masahiro Yamada