diff mbox series

[v2,19/21] gcc-plugins: test GCC plugin support in Kconfig

Message ID 1522128575-5326-20-git-send-email-yamada.masahiro@socionext.com
State New
Headers show
Series kconfig: move compiler capability tests to Kconfig | expand

Commit Message

Masahiro Yamada March 27, 2018, 5:29 a.m. UTC 
Run scripts/gcc-plugin.sh from Kconfig.  Users can enable GCC_PLUGINS
only when it is supported.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

---

Changes in v2: None

 arch/Kconfig                 |  4 +++
 scripts/Makefile.gcc-plugins | 82 ++++++++++++++++----------------------------
 scripts/gcc-plugin.sh        |  1 -
 3 files changed, 33 insertions(+), 54 deletions(-)

-- 
2.7.4

Comments

Kees Cook March 28, 2018, 11:44 a.m. UTC | #1 
On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Run scripts/gcc-plugin.sh from Kconfig.  Users can enable GCC_PLUGINS

> only when it is supported.

>

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> ---

>

> Changes in v2: None

>

>  arch/Kconfig                 |  4 +++

>  scripts/Makefile.gcc-plugins | 82 ++++++++++++++++----------------------------

>  scripts/gcc-plugin.sh        |  1 -

>  3 files changed, 33 insertions(+), 54 deletions(-)

>

> diff --git a/arch/Kconfig b/arch/Kconfig

> index b42378d..88cc925 100644

> --- a/arch/Kconfig

> +++ b/arch/Kconfig

> @@ -407,9 +407,13 @@ config HAVE_GCC_PLUGINS

>           An arch should select this symbol if it supports building with

>           GCC plugins.

>

> +config CC_HAS_GCC_PLUGINS

> +       bool

> +


This doesn't seem used anywhere?

>  menuconfig GCC_PLUGINS

>         bool "GCC plugins"

>         depends on HAVE_GCC_PLUGINS

> +       depends on $(success $srctree/scripts/gcc-plugin.sh $HOSTCXX $CC)

>         depends on !COMPILE_TEST

>         help

>           GCC plugins are loadable modules that provide extra features to the

> diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins

> index 25da4c0..19d0d5b 100644

> --- a/scripts/Makefile.gcc-plugins

> +++ b/scripts/Makefile.gcc-plugins

> [...]

> -# If plugins aren't supported, abort the build before hard-to-read compiler

> -# errors start getting spewed by the main build.

> -PHONY += gcc-plugins-check

> -gcc-plugins-check: FORCE

> -ifdef CONFIG_GCC_PLUGINS

> -  ifeq ($(PLUGINCC),)

> -    ifneq ($(GCC_PLUGINS_CFLAGS),)

> -      $(Q)$(srctree)/scripts/gcc-plugin.sh --show-error $(HOSTCXX) $(CC) || true

> -      @echo "Cannot use CONFIG_GCC_PLUGINS: your gcc installation does not support plugins, perhaps the necessary headers are missing?" >&2 && exit 1


As mentioned in the other email, we lose the error reporting. Now the
lack of plugins is just a silent =n in menuconfig. Keeping
--show-error in the Kconfig call and retaining stderr would be nice.

I need to do some further testing with SANCOV, but otherwise this all
looks correct, and my testing shows it behaving correctly.

-Kees

-- 
Kees Cook
Pixel Security
Masahiro Yamada April 11, 2018, 3:55 p.m. UTC | #2 
2018-03-28 20:44 GMT+09:00 Kees Cook <keescook@chromium.org>:
> On Mon, Mar 26, 2018 at 10:29 PM, Masahiro Yamada

> <yamada.masahiro@socionext.com> wrote:

>> Run scripts/gcc-plugin.sh from Kconfig.  Users can enable GCC_PLUGINS

>> only when it is supported.

>>

>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

>> ---

>>

>> Changes in v2: None

>>

>>  arch/Kconfig                 |  4 +++

>>  scripts/Makefile.gcc-plugins | 82 ++++++++++++++++----------------------------

>>  scripts/gcc-plugin.sh        |  1 -

>>  3 files changed, 33 insertions(+), 54 deletions(-)

>>

>> diff --git a/arch/Kconfig b/arch/Kconfig

>> index b42378d..88cc925 100644

>> --- a/arch/Kconfig

>> +++ b/arch/Kconfig

>> @@ -407,9 +407,13 @@ config HAVE_GCC_PLUGINS

>>           An arch should select this symbol if it supports building with

>>           GCC plugins.

>>

>> +config CC_HAS_GCC_PLUGINS

>> +       bool

>> +

>

> This doesn't seem used anywhere?

>

>>  menuconfig GCC_PLUGINS

>>         bool "GCC plugins"

>>         depends on HAVE_GCC_PLUGINS

>> +       depends on $(success $srctree/scripts/gcc-plugin.sh $HOSTCXX $CC)

>>         depends on !COMPILE_TEST

>>         help

>>           GCC plugins are loadable modules that provide extra features to the

>> diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins

>> index 25da4c0..19d0d5b 100644

>> --- a/scripts/Makefile.gcc-plugins

>> +++ b/scripts/Makefile.gcc-plugins

>> [...]

>> -# If plugins aren't supported, abort the build before hard-to-read compiler

>> -# errors start getting spewed by the main build.

>> -PHONY += gcc-plugins-check

>> -gcc-plugins-check: FORCE

>> -ifdef CONFIG_GCC_PLUGINS

>> -  ifeq ($(PLUGINCC),)

>> -    ifneq ($(GCC_PLUGINS_CFLAGS),)

>> -      $(Q)$(srctree)/scripts/gcc-plugin.sh --show-error $(HOSTCXX) $(CC) || true

>> -      @echo "Cannot use CONFIG_GCC_PLUGINS: your gcc installation does not support plugins, perhaps the necessary headers are missing?" >&2 && exit 1

>

> As mentioned in the other email, we lose the error reporting. Now the

> lack of plugins is just a silent =n in menuconfig.


This is the right thing to do.
Features unsupported by the compiler should be silently disable.


> Keeping

> --show-error in the Kconfig call and retaining stderr would be nice.


No.
There is no problem to use a compiler without plugin support.

If a user does not want to use plugin in the first place,
why does he/she need to be bothered by such information in stderr?


> I need to do some further testing with SANCOV, but otherwise this all

> looks correct, and my testing shows it behaving correctly.

>

> -Kees

>

> --

> Kees Cook

> Pixel Security

> --

> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in

> the body of a message to majordomo@vger.kernel.org

> More majordomo info at  http://vger.kernel.org/majordomo-info.html




-- 
Best Regards
Masahiro Yamada
Kees Cook April 11, 2018, 4:09 p.m. UTC | #3 
On Wed, Apr 11, 2018 at 8:55 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> No.

> There is no problem to use a compiler without plugin support.

>

> If a user does not want to use plugin in the first place,

> why does he/she need to be bothered by such information in stderr?


So, I don't think it's needed for the first version of this, but it'd
be nice to have a way for end users to discover _why_ some option they
want is unavailable. Having them dig through the Kconfigs, edit
scripts to see stderr again, running those by hand, etc, is really
unfriendly. If, instead, stderr was visible from the menuconfig "help"
for a Kconfig, it could serve as automatic documentation for why
something wasn't enabled. I don't mean for stderr to get sprayed out
during a regular "make"; I agree: that would be ugly and pointless.
I'd just like some way for the reason behind an option being disabled
to be discoverable in some way.

-Kees

-- 
Kees Cook
Pixel Security
diff mbox series

Patch

diff --git a/arch/Kconfig b/arch/Kconfig
index b42378d..88cc925 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -407,9 +407,13 @@  config HAVE_GCC_PLUGINS
 	  An arch should select this symbol if it supports building with
 	  GCC plugins.
 
+config CC_HAS_GCC_PLUGINS
+	bool
+
 menuconfig GCC_PLUGINS
 	bool "GCC plugins"
 	depends on HAVE_GCC_PLUGINS
+	depends on $(success $srctree/scripts/gcc-plugin.sh $HOSTCXX $CC)
 	depends on !COMPILE_TEST
 	help
 	  GCC plugins are loadable modules that provide extra features to the
diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
index 25da4c0..19d0d5b 100644
--- a/scripts/Makefile.gcc-plugins
+++ b/scripts/Makefile.gcc-plugins
@@ -1,71 +1,47 @@ 
 # SPDX-License-Identifier: GPL-2.0
-ifdef CONFIG_GCC_PLUGINS
-  PLUGINCC := $(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-plugin.sh $(HOSTCXX) $(CC))
-
-  SANCOV_PLUGIN := -fplugin=$(objtree)/scripts/gcc-plugins/sancov_plugin.so
+SANCOV_PLUGIN := -fplugin=$(objtree)/scripts/gcc-plugins/sancov_plugin.so
 
-  gcc-plugin-$(CONFIG_GCC_PLUGIN_CYC_COMPLEXITY)	+= cyc_complexity_plugin.so
+gcc-plugin-$(CONFIG_GCC_PLUGIN_CYC_COMPLEXITY)	+= cyc_complexity_plugin.so
 
-  gcc-plugin-$(CONFIG_GCC_PLUGIN_LATENT_ENTROPY)	+= latent_entropy_plugin.so
-  gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_LATENT_ENTROPY)	+= -DLATENT_ENTROPY_PLUGIN
-  ifdef CONFIG_GCC_PLUGIN_LATENT_ENTROPY
-    DISABLE_LATENT_ENTROPY_PLUGIN			+= -fplugin-arg-latent_entropy_plugin-disable
-  endif
+gcc-plugin-$(CONFIG_GCC_PLUGIN_LATENT_ENTROPY)	+= latent_entropy_plugin.so
+gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_LATENT_ENTROPY)	+= -DLATENT_ENTROPY_PLUGIN
+ifdef CONFIG_GCC_PLUGIN_LATENT_ENTROPY
+  DISABLE_LATENT_ENTROPY_PLUGIN			+= -fplugin-arg-latent_entropy_plugin-disable
+endif
 
-  ifdef CONFIG_GCC_PLUGIN_SANCOV
-    ifeq ($(CFLAGS_KCOV),)
-      # It is needed because of the gcc-plugin.sh and gcc version checks.
-      gcc-plugin-$(CONFIG_GCC_PLUGIN_SANCOV)           += sancov_plugin.so
+ifdef CONFIG_GCC_PLUGIN_SANCOV
+  ifeq ($(CFLAGS_KCOV),)
+    # It is needed because of the gcc-plugin.sh and gcc version checks.
+    gcc-plugin-$(CONFIG_GCC_PLUGIN_SANCOV)           += sancov_plugin.so
 
-      ifneq ($(PLUGINCC),)
-        CFLAGS_KCOV := $(SANCOV_PLUGIN)
-      else
-        $(warning warning: cannot use CONFIG_KCOV: -fsanitize-coverage=trace-pc is not supported by compiler)
-      endif
-    endif
+    CFLAGS_KCOV := $(SANCOV_PLUGIN)
   endif
+endif
 
-  gcc-plugin-$(CONFIG_GCC_PLUGIN_STRUCTLEAK)	+= structleak_plugin.so
-  gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_VERBOSE)	+= -fplugin-arg-structleak_plugin-verbose
-  gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL)	+= -fplugin-arg-structleak_plugin-byref-all
-  gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK)	+= -DSTRUCTLEAK_PLUGIN
-
-  gcc-plugin-$(CONFIG_GCC_PLUGIN_RANDSTRUCT)	+= randomize_layout_plugin.so
-  gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_RANDSTRUCT)	+= -DRANDSTRUCT_PLUGIN
-  gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_RANDSTRUCT_PERFORMANCE)	+= -fplugin-arg-randomize_layout_plugin-performance-mode
+gcc-plugin-$(CONFIG_GCC_PLUGIN_STRUCTLEAK)	+= structleak_plugin.so
+gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_VERBOSE)	+= -fplugin-arg-structleak_plugin-verbose
+gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL)	+= -fplugin-arg-structleak_plugin-byref-all
+gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK)	+= -DSTRUCTLEAK_PLUGIN
 
-  GCC_PLUGINS_CFLAGS := $(strip $(addprefix -fplugin=$(objtree)/scripts/gcc-plugins/, $(gcc-plugin-y)) $(gcc-plugin-cflags-y))
+gcc-plugin-$(CONFIG_GCC_PLUGIN_RANDSTRUCT)	+= randomize_layout_plugin.so
+gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_RANDSTRUCT)	+= -DRANDSTRUCT_PLUGIN
+gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_RANDSTRUCT_PERFORMANCE)	+= -fplugin-arg-randomize_layout_plugin-performance-mode
 
-  export PLUGINCC GCC_PLUGINS_CFLAGS GCC_PLUGIN GCC_PLUGIN_SUBDIR
-  export SANCOV_PLUGIN DISABLE_LATENT_ENTROPY_PLUGIN
+GCC_PLUGINS_CFLAGS := $(strip $(addprefix -fplugin=$(objtree)/scripts/gcc-plugins/, $(gcc-plugin-y)) $(gcc-plugin-cflags-y))
 
-  ifneq ($(PLUGINCC),)
-    # SANCOV_PLUGIN can be only in CFLAGS_KCOV because avoid duplication.
-    GCC_PLUGINS_CFLAGS := $(filter-out $(SANCOV_PLUGIN), $(GCC_PLUGINS_CFLAGS))
-  endif
+export GCC_PLUGINS_CFLAGS GCC_PLUGIN GCC_PLUGIN_SUBDIR
+export SANCOV_PLUGIN DISABLE_LATENT_ENTROPY_PLUGIN
 
-  KBUILD_CFLAGS += $(GCC_PLUGINS_CFLAGS)
-  GCC_PLUGIN := $(gcc-plugin-y)
-  GCC_PLUGIN_SUBDIR := $(gcc-plugin-subdir-y)
-endif
+# SANCOV_PLUGIN can be only in CFLAGS_KCOV because avoid duplication.
+GCC_PLUGINS_CFLAGS := $(filter-out $(SANCOV_PLUGIN), $(GCC_PLUGINS_CFLAGS))
 
-# If plugins aren't supported, abort the build before hard-to-read compiler
-# errors start getting spewed by the main build.
-PHONY += gcc-plugins-check
-gcc-plugins-check: FORCE
-ifdef CONFIG_GCC_PLUGINS
-  ifeq ($(PLUGINCC),)
-    ifneq ($(GCC_PLUGINS_CFLAGS),)
-      $(Q)$(srctree)/scripts/gcc-plugin.sh --show-error $(HOSTCXX) $(CC) || true
-      @echo "Cannot use CONFIG_GCC_PLUGINS: your gcc installation does not support plugins, perhaps the necessary headers are missing?" >&2 && exit 1
-    endif
-  endif
-endif
-	@:
+KBUILD_CFLAGS += $(GCC_PLUGINS_CFLAGS)
+GCC_PLUGIN := $(gcc-plugin-y)
+GCC_PLUGIN_SUBDIR := $(gcc-plugin-subdir-y)
 
 # Actually do the build, if requested.
 PHONY += gcc-plugins
-gcc-plugins: scripts_basic gcc-plugins-check
+gcc-plugins: scripts_basic
 ifdef CONFIG_GCC_PLUGINS
 	$(Q)$(MAKE) $(build)=scripts/gcc-plugins
 endif
diff --git a/scripts/gcc-plugin.sh b/scripts/gcc-plugin.sh
index 0edbdae..b18c69c 100755
--- a/scripts/gcc-plugin.sh
+++ b/scripts/gcc-plugin.sh
@@ -24,7 +24,6 @@  EOF
 
 if [ $? -eq 0 ]
 then
-	echo "$1"
 	exit 0
 fi