linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] gcc-plugins: fix definition of DISABLE_LATENT_ENTROPY_PLUGIN
@ 2016-12-06  6:27 Andrew Donnellan
  2016-12-06  6:27 ` [PATCH 2/3] powerpc: correctly disable latent entropy GCC plugin on prom_init.o Andrew Donnellan
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Andrew Donnellan @ 2016-12-06  6:27 UTC (permalink / raw)
  To: linuxppc-dev, kernel-hardening, keescook, re.emese
  Cc: linux-kernel, linux-kbuild, pageexec, spender, mmarek

The variable DISABLE_LATENT_ENTROPY_PLUGIN is defined when
CONFIG_PAX_LATENT_ENTROPY is set. This is leftover from the original PaX
version of the plugin code and doesn't actually exist. Change the condition
to depend on CONFIG_GCC_PLUGIN_LATENT_ENTROPY instead.

Fixes: 38addce8b600 ("gcc-plugins: Add latent_entropy plugin")
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
---
 scripts/Makefile.gcc-plugins | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
index 060d2cb..26c67b7 100644
--- a/scripts/Makefile.gcc-plugins
+++ b/scripts/Makefile.gcc-plugins
@@ -8,7 +8,7 @@ ifdef CONFIG_GCC_PLUGINS
 
   gcc-plugin-$(CONFIG_GCC_PLUGIN_LATENT_ENTROPY)	+= latent_entropy_plugin.so
   gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_LATENT_ENTROPY)	+= -DLATENT_ENTROPY_PLUGIN
-  ifdef CONFIG_PAX_LATENT_ENTROPY
+  ifdef CONFIG_GCC_PLUGIN_LATENT_ENTROPY
     DISABLE_LATENT_ENTROPY_PLUGIN			+= -fplugin-arg-latent_entropy_plugin-disable
   endif
 
-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/3] powerpc: correctly disable latent entropy GCC plugin on prom_init.o
  2016-12-06  6:27 [PATCH 1/3] gcc-plugins: fix definition of DISABLE_LATENT_ENTROPY_PLUGIN Andrew Donnellan
@ 2016-12-06  6:27 ` Andrew Donnellan
  2016-12-06  6:28 ` [PATCH 3/3] powerpc: enable support for GCC plugins Andrew Donnellan
  2017-02-06 20:37 ` [1/3] gcc-plugins: fix definition of DISABLE_LATENT_ENTROPY_PLUGIN Michael Ellerman
  2 siblings, 0 replies; 15+ messages in thread
From: Andrew Donnellan @ 2016-12-06  6:27 UTC (permalink / raw)
  To: linuxppc-dev, kernel-hardening, keescook, re.emese
  Cc: linux-kernel, linux-kbuild, pageexec, spender, mmarek

Commit 38addce8b600 ("gcc-plugins: Add latent_entropy plugin") excludes
certain powerpc early boot code from the latent entropy plugin by adding
appropriate CFLAGS. It looks like this was supposed to cover prom_init.o,
but ended up saying init.o (which doesn't exist) instead. Fix the typo.

Fixes: 38addce8b600 ("gcc-plugins: Add latent_entropy plugin")
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

---

I think that we potentially could get rid of some of these disables, but
it's safer to leave it for now.
---
 arch/powerpc/kernel/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 1925341..adb52d1 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -15,7 +15,7 @@ CFLAGS_btext.o		+= -fPIC
 endif
 
 CFLAGS_cputable.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
-CFLAGS_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
+CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 CFLAGS_btext.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 
-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/3] powerpc: enable support for GCC plugins
  2016-12-06  6:27 [PATCH 1/3] gcc-plugins: fix definition of DISABLE_LATENT_ENTROPY_PLUGIN Andrew Donnellan
  2016-12-06  6:27 ` [PATCH 2/3] powerpc: correctly disable latent entropy GCC plugin on prom_init.o Andrew Donnellan
@ 2016-12-06  6:28 ` Andrew Donnellan
  2016-12-06 20:40   ` Kees Cook
                     ` (3 more replies)
  2017-02-06 20:37 ` [1/3] gcc-plugins: fix definition of DISABLE_LATENT_ENTROPY_PLUGIN Michael Ellerman
  2 siblings, 4 replies; 15+ messages in thread
From: Andrew Donnellan @ 2016-12-06  6:28 UTC (permalink / raw)
  To: linuxppc-dev, kernel-hardening, keescook, re.emese
  Cc: linux-kernel, linux-kbuild, pageexec, spender, mmarek

Enable support for GCC plugins on powerpc.

Add an additional version check in gcc-plugins-check to advise users to
upgrade to gcc 5.2+ on powerpc to avoid issues with header files (gcc <=
4.6) or missing copies of rs6000-cpus.def (4.8 to 5.1 on 64-bit targets).

Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

---

Open to bikeshedding on the gcc version check.

Compile tested with all plugins enabled on gcc 4.6-6.2,
x86->ppc{32,64,64le} and 4.8-6.2 ppc64le->ppc{32,64,64le}. Thanks to
Chris Smart for help with this.

I think it's best to take this through powerpc#next with an ACK from
Kees/Emese?
---
 arch/powerpc/Kconfig         | 1 +
 scripts/Makefile.gcc-plugins | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 65fba4c..6efbc08 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -92,6 +92,7 @@ config PPC
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS if MPROFILE_KERNEL
 	select HAVE_FUNCTION_TRACER
 	select HAVE_FUNCTION_GRAPH_TRACER
+	select HAVE_GCC_PLUGINS
 	select SYSCTL_EXCEPTION_TRACE
 	select VIRT_TO_BUS if !PPC64
 	select HAVE_IDE
diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
index 26c67b7..9835a75 100644
--- a/scripts/Makefile.gcc-plugins
+++ b/scripts/Makefile.gcc-plugins
@@ -47,6 +47,14 @@ gcc-plugins-check: FORCE
 ifdef CONFIG_GCC_PLUGINS
   ifeq ($(PLUGINCC),)
     ifneq ($(GCC_PLUGINS_CFLAGS),)
+      # Various gccs between 4.5 and 5.1 have bugs on powerpc due to missing
+      # header files. gcc <= 4.6 doesn't work at all, gccs from 4.8 to 5.1 have
+      # issues with 64-bit targets.
+      ifeq ($(ARCH),powerpc)
+        ifeq ($(call cc-ifversion, -le, 0501, y), y)
+	  @echo "Cannot use CONFIG_GCC_PLUGINS: plugin support on gcc <= 5.1 is buggy on powerpc, please upgrade to gcc 5.2 or newer" >&2 && exit 1
+        endif
+      endif
       ifeq ($(call cc-ifversion, -ge, 0405, y), y)
 	$(Q)$(srctree)/scripts/gcc-plugin.sh --show-error "$(__PLUGINCC)" "$(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
-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] powerpc: enable support for GCC plugins
  2016-12-06  6:28 ` [PATCH 3/3] powerpc: enable support for GCC plugins Andrew Donnellan
@ 2016-12-06 20:40   ` Kees Cook
  2016-12-07  1:05     ` [kernel-hardening] " Andrew Donnellan
  2016-12-06 21:25   ` Emese Revfy
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Kees Cook @ 2016-12-06 20:40 UTC (permalink / raw)
  To: Andrew Donnellan
  Cc: linuxppc-dev, kernel-hardening, Emese Revfy, LKML, linux-kbuild,
	PaX Team, Brad Spengler, Michal Marek

On Mon, Dec 5, 2016 at 10:28 PM, Andrew Donnellan
<andrew.donnellan@au1.ibm.com> wrote:
> Enable support for GCC plugins on powerpc.
>
> Add an additional version check in gcc-plugins-check to advise users to
> upgrade to gcc 5.2+ on powerpc to avoid issues with header files (gcc <=
> 4.6) or missing copies of rs6000-cpus.def (4.8 to 5.1 on 64-bit targets).
>
> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>
> ---
>
> Open to bikeshedding on the gcc version check.

I think this looks fine. Anyone wanting to use gcc plugins on ppc with
an earlier gcc can send patches if they find a sane way to make it
work. :)

> Compile tested with all plugins enabled on gcc 4.6-6.2,
> x86->ppc{32,64,64le} and 4.8-6.2 ppc64le->ppc{32,64,64le}. Thanks to
> Chris Smart for help with this.

I assume also tested on 5.2? :)

> I think it's best to take this through powerpc#next with an ACK from
> Kees/Emese?

That would be fine by me. Please consider the whole series:

Acked-by: Kees Cook <keescook@chromium.org>

Thanks!

-Kees

> ---
>  arch/powerpc/Kconfig         | 1 +
>  scripts/Makefile.gcc-plugins | 8 ++++++++
>  2 files changed, 9 insertions(+)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 65fba4c..6efbc08 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -92,6 +92,7 @@ config PPC
>         select HAVE_DYNAMIC_FTRACE_WITH_REGS if MPROFILE_KERNEL
>         select HAVE_FUNCTION_TRACER
>         select HAVE_FUNCTION_GRAPH_TRACER
> +       select HAVE_GCC_PLUGINS
>         select SYSCTL_EXCEPTION_TRACE
>         select VIRT_TO_BUS if !PPC64
>         select HAVE_IDE
> diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
> index 26c67b7..9835a75 100644
> --- a/scripts/Makefile.gcc-plugins
> +++ b/scripts/Makefile.gcc-plugins
> @@ -47,6 +47,14 @@ gcc-plugins-check: FORCE
>  ifdef CONFIG_GCC_PLUGINS
>    ifeq ($(PLUGINCC),)
>      ifneq ($(GCC_PLUGINS_CFLAGS),)
> +      # Various gccs between 4.5 and 5.1 have bugs on powerpc due to missing
> +      # header files. gcc <= 4.6 doesn't work at all, gccs from 4.8 to 5.1 have
> +      # issues with 64-bit targets.
> +      ifeq ($(ARCH),powerpc)
> +        ifeq ($(call cc-ifversion, -le, 0501, y), y)
> +         @echo "Cannot use CONFIG_GCC_PLUGINS: plugin support on gcc <= 5.1 is buggy on powerpc, please upgrade to gcc 5.2 or newer" >&2 && exit 1
> +        endif
> +      endif
>        ifeq ($(call cc-ifversion, -ge, 0405, y), y)
>         $(Q)$(srctree)/scripts/gcc-plugin.sh --show-error "$(__PLUGINCC)" "$(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
> --
> Andrew Donnellan              OzLabs, ADL Canberra
> andrew.donnellan@au1.ibm.com  IBM Australia Limited
>



-- 
Kees Cook
Nexus Security

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] powerpc: enable support for GCC plugins
  2016-12-06  6:28 ` [PATCH 3/3] powerpc: enable support for GCC plugins Andrew Donnellan
  2016-12-06 20:40   ` Kees Cook
@ 2016-12-06 21:25   ` Emese Revfy
  2016-12-07  5:49     ` Andrew Donnellan
  2016-12-07  5:45   ` Andrew Donnellan
  2016-12-08 14:42   ` PaX Team
  3 siblings, 1 reply; 15+ messages in thread
From: Emese Revfy @ 2016-12-06 21:25 UTC (permalink / raw)
  To: Andrew Donnellan
  Cc: linuxppc-dev, kernel-hardening, keescook, linux-kernel,
	linux-kbuild, pageexec, spender, mmarek

On Tue,  6 Dec 2016 17:28:00 +1100
Andrew Donnellan <andrew.donnellan@au1.ibm.com> wrote:

> +      # Various gccs between 4.5 and 5.1 have bugs on powerpc due to missing
> +      # header files. gcc <= 4.6 doesn't work at all, gccs from 4.8 to 5.1 have
> +      # issues with 64-bit targets.
> +      ifeq ($(ARCH),powerpc)
> +        ifeq ($(call cc-ifversion, -le, 0501, y), y)
> +	  @echo "Cannot use CONFIG_GCC_PLUGINS: plugin support on gcc <= 5.1 is buggy on powerpc, please upgrade to gcc 5.2 or newer" >&2 && exit 1
> +        endif
> +      endif

Hi,

What are these missing headers? Because if they aren't necessary then they can
be removed from gcc-common.h. There were missing headers on arm/arm64 and these
archs are supported. I think this version check is unnecessary because
gcc-plugin.sh also checks the missing headers.

What is the problem on gcc-4.5/gcc-4.6?

-- 
Emese

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [kernel-hardening] Re: [PATCH 3/3] powerpc: enable support for GCC plugins
  2016-12-06 20:40   ` Kees Cook
@ 2016-12-07  1:05     ` Andrew Donnellan
  0 siblings, 0 replies; 15+ messages in thread
From: Andrew Donnellan @ 2016-12-07  1:05 UTC (permalink / raw)
  To: kernel-hardening
  Cc: linuxppc-dev, Emese Revfy, LKML, linux-kbuild, PaX Team,
	Brad Spengler, Michal Marek

On 07/12/16 07:40, Kees Cook wrote:
>> Compile tested with all plugins enabled on gcc 4.6-6.2,
>> x86->ppc{32,64,64le} and 4.8-6.2 ppc64le->ppc{32,64,64le}. Thanks to
>> Chris Smart for help with this.
>
> I assume also tested on 5.2? :)

Tested on the latest subrevision of every release branch up till 6.2, so 
yes :)

>> I think it's best to take this through powerpc#next with an ACK from
>> Kees/Emese?
>
> That would be fine by me. Please consider the whole series:
>
> Acked-by: Kees Cook <keescook@chromium.org>

Thanks!

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] powerpc: enable support for GCC plugins
  2016-12-06  6:28 ` [PATCH 3/3] powerpc: enable support for GCC plugins Andrew Donnellan
  2016-12-06 20:40   ` Kees Cook
  2016-12-06 21:25   ` Emese Revfy
@ 2016-12-07  5:45   ` Andrew Donnellan
  2016-12-08 14:42   ` PaX Team
  3 siblings, 0 replies; 15+ messages in thread
From: Andrew Donnellan @ 2016-12-07  5:45 UTC (permalink / raw)
  To: linuxppc-dev, kernel-hardening, keescook, re.emese
  Cc: pageexec, spender, linux-kernel, mmarek, linux-kbuild

On 06/12/16 17:28, Andrew Donnellan wrote:
> Enable support for GCC plugins on powerpc.
>
> Add an additional version check in gcc-plugins-check to advise users to
> upgrade to gcc 5.2+ on powerpc to avoid issues with header files (gcc <=
> 4.6) or missing copies of rs6000-cpus.def (4.8 to 5.1 on 64-bit targets).
>
> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>
> ---
>
> Open to bikeshedding on the gcc version check.
>
> Compile tested with all plugins enabled on gcc 4.6-6.2,
> x86->ppc{32,64,64le} and 4.8-6.2 ppc64le->ppc{32,64,64le}. Thanks to
> Chris Smart for help with this.
>
> I think it's best to take this through powerpc#next with an ACK from
> Kees/Emese?
> ---
>  arch/powerpc/Kconfig         | 1 +
>  scripts/Makefile.gcc-plugins | 8 ++++++++

Will respin with an update to Documentation/gcc-plugins.txt as well.

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] powerpc: enable support for GCC plugins
  2016-12-06 21:25   ` Emese Revfy
@ 2016-12-07  5:49     ` Andrew Donnellan
  0 siblings, 0 replies; 15+ messages in thread
From: Andrew Donnellan @ 2016-12-07  5:49 UTC (permalink / raw)
  To: Emese Revfy
  Cc: spender, keescook, linux-kbuild, kernel-hardening, linux-kernel,
	mmarek, pageexec, linuxppc-dev

On 07/12/16 08:25, Emese Revfy wrote:
> What are these missing headers? Because if they aren't necessary then they can
> be removed from gcc-common.h. There were missing headers on arm/arm64 and these
> archs are supported. I think this version check is unnecessary because
> gcc-plugin.sh also checks the missing headers.

rs6000-cpus.def, included via tm.h - see 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66840

I realise gcc-plugin.sh does detect this, but the point of the 
additional version check is to provide somewhat more helpful advice to 
the user.

> What is the problem on gcc-4.5/gcc-4.6?

On 4.6.4, c-family/c-common.h:

/scratch/ajd/gcc-test-v2/kernel/scripts/gcc-plugins/gcc-common.h:60:31: 
fatal error: c-family/c-common.h: No such file or directory

ajd@ka1:/scratch/ajd/tmp/cross/gcc-4.6.4-nolibc/powerpc64-linux$ find 
-name c-common.*
./lib/gcc/powerpc64-linux/4.6.4/plugin/include/c-common.h
./lib/gcc/powerpc64-linux/4.6.4/plugin/include/c-family/c-common.def

Are we sure the version check in gcc-common.h:59 is correct, or is this 
just a peculiarity of my particular toolchain?

I need to build another 4.5 toolchain, I'll try to do that this week.

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] powerpc: enable support for GCC plugins
  2016-12-06  6:28 ` [PATCH 3/3] powerpc: enable support for GCC plugins Andrew Donnellan
                     ` (2 preceding siblings ...)
  2016-12-07  5:45   ` Andrew Donnellan
@ 2016-12-08 14:42   ` PaX Team
  2016-12-08 18:06     ` Kees Cook
  3 siblings, 1 reply; 15+ messages in thread
From: PaX Team @ 2016-12-08 14:42 UTC (permalink / raw)
  To: linuxppc-dev, kernel-hardening, keescook, re.emese, Andrew Donnellan
  Cc: linux-kernel, linux-kbuild, spender, mmarek

On 6 Dec 2016 at 17:28, Andrew Donnellan wrote:

> Enable support for GCC plugins on powerpc.
> 
> Add an additional version check in gcc-plugins-check to advise users to
> upgrade to gcc 5.2+ on powerpc to avoid issues with header files (gcc <=
> 4.6) or missing copies of rs6000-cpus.def (4.8 to 5.1 on 64-bit targets).

i don't think that this is the right approach. there's a general and a special
issue here, both of which need different handling.

the general problem is to detect problems related to gcc plugin headers and
notify the users about solutions. emitting various messages from a Makefile
is certainly not a scalable approach, just imagine how it will look when the
other 30+ archs begin to add their own special cases... if anything, they
should be documented in Documentation/gcc-plugins.txt (or a new doc if it
grows too big) and the Makefile message should just point at it.

as for the solutions, the general advice should enable the use of otherwise
failing gcc versions instead of forcing updating to new ones (though the
latter is advisable for other reasons but not everyone's in the position to
do so easily). in my experience all one needs to do is manually install the
missing files from the gcc sources (ideally distros would take care of it).

the specific problem addressed here can (and IMHO should) be solved in
another way: remove the inclusion of the offending headers in gcc-common.h
as neither tm.h nor c-common.h are needed by existing plugins. for background,
i created gcc-common.h to simplify plugin development across all supportable
gcc versions i came across over the years, so it follows the 'everything but
the kitchen sink' approach. that isn't necessarily what the kernel and other
projects need so they should just use my version as a basis and fork/simplify
it (even i maintain private forks of the public version).

as for the location of c-common.h, upstream gcc moved it under c-family in
2010 after the release of 4.5, so it should be where gcc-common.h expects
it and i'm not sure how it ended up at its old location for you.

cheers,
 PaX Team

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] powerpc: enable support for GCC plugins
  2016-12-08 14:42   ` PaX Team
@ 2016-12-08 18:06     ` Kees Cook
  2016-12-09  2:48       ` Andrew Donnellan
  0 siblings, 1 reply; 15+ messages in thread
From: Kees Cook @ 2016-12-08 18:06 UTC (permalink / raw)
  To: PaX Team
  Cc: linuxppc-dev, kernel-hardening, Emese Revfy, Andrew Donnellan,
	LKML, linux-kbuild, Brad Spengler, Michal Marek

On Thu, Dec 8, 2016 at 6:42 AM, PaX Team <pageexec@freemail.hu> wrote:
> On 6 Dec 2016 at 17:28, Andrew Donnellan wrote:
>
>> Enable support for GCC plugins on powerpc.
>>
>> Add an additional version check in gcc-plugins-check to advise users to
>> upgrade to gcc 5.2+ on powerpc to avoid issues with header files (gcc <=
>> 4.6) or missing copies of rs6000-cpus.def (4.8 to 5.1 on 64-bit targets).
>
> i don't think that this is the right approach. there's a general and a special
> issue here, both of which need different handling.
>
> the general problem is to detect problems related to gcc plugin headers and
> notify the users about solutions. emitting various messages from a Makefile
> is certainly not a scalable approach, just imagine how it will look when the
> other 30+ archs begin to add their own special cases... if anything, they
> should be documented in Documentation/gcc-plugins.txt (or a new doc if it
> grows too big) and the Makefile message should just point at it.
>
> as for the solutions, the general advice should enable the use of otherwise
> failing gcc versions instead of forcing updating to new ones (though the
> latter is advisable for other reasons but not everyone's in the position to
> do so easily). in my experience all one needs to do is manually install the
> missing files from the gcc sources (ideally distros would take care of it).
>
> the specific problem addressed here can (and IMHO should) be solved in
> another way: remove the inclusion of the offending headers in gcc-common.h
> as neither tm.h nor c-common.h are needed by existing plugins. for background,
> i created gcc-common.h to simplify plugin development across all supportable
> gcc versions i came across over the years, so it follows the 'everything but
> the kitchen sink' approach. that isn't necessarily what the kernel and other
> projects need so they should just use my version as a basis and fork/simplify
> it (even i maintain private forks of the public version).

If removing those will lower the requirement for PPC, that would be
ideal. Otherwise, I'd like to take the practical approach of making
the plugins available on PPC right now, with an eye towards relaxing
the version requirement as people need it.

> as for the location of c-common.h, upstream gcc moved it under c-family in
> 2010 after the release of 4.5, so it should be where gcc-common.h expects
> it and i'm not sure how it ended up at its old location for you.

That is rather odd. What distro was the PPC test done on? (Or were
these manually built gcc versions?)

-Kees

-- 
Kees Cook
Nexus Security

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] powerpc: enable support for GCC plugins
  2016-12-08 18:06     ` Kees Cook
@ 2016-12-09  2:48       ` Andrew Donnellan
  2016-12-09 10:59         ` PaX Team
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Donnellan @ 2016-12-09  2:48 UTC (permalink / raw)
  To: Kees Cook, PaX Team
  Cc: linuxppc-dev, kernel-hardening, Emese Revfy, LKML, linux-kbuild,
	Brad Spengler, Michal Marek

On 09/12/16 05:06, Kees Cook wrote:
>> i don't think that this is the right approach. there's a general and a special
>> issue here, both of which need different handling.
>>
>> the general problem is to detect problems related to gcc plugin headers and
>> notify the users about solutions. emitting various messages from a Makefile
>> is certainly not a scalable approach, just imagine how it will look when the
>> other 30+ archs begin to add their own special cases... if anything, they
>> should be documented in Documentation/gcc-plugins.txt (or a new doc if it
>> grows too big) and the Makefile message should just point at it.

I think I agree in principle - Makefiles are already unreadable enough 
without a million special cases.

>> as for the solutions, the general advice should enable the use of otherwise
>> failing gcc versions instead of forcing updating to new ones (though the
>> latter is advisable for other reasons but not everyone's in the position to
>> do so easily). in my experience all one needs to do is manually install the
>> missing files from the gcc sources (ideally distros would take care of it).

If someone else is willing to write up that advice, then great.

>> the specific problem addressed here can (and IMHO should) be solved in
>> another way: remove the inclusion of the offending headers in gcc-common.h
>> as neither tm.h nor c-common.h are needed by existing plugins. for background,

We can't build without tm.h: http://pastebin.com/W0azfCr0

And we get warnings without c-common.h: http://pastebin.com/Aw8CAj10

>> as for the location of c-common.h, upstream gcc moved it under c-family in
>> 2010 after the release of 4.5, so it should be where gcc-common.h expects
>> it and i'm not sure how it ended up at its old location for you.
>
> That is rather odd. What distro was the PPC test done on? (Or were
> these manually built gcc versions?)

These were all manually built using a script running on a Debian box. 
Installing precompiled distro versions of rather old gccs would have 
been somewhat challenging. I've just rebuilt 4.6.4 to double check that 
I wasn't just seeing things, but it seems that it definitely is still 
putting c-common.h in the old location.

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] powerpc: enable support for GCC plugins
  2016-12-09  2:48       ` Andrew Donnellan
@ 2016-12-09 10:59         ` PaX Team
  2017-01-27  5:52           ` Andrew Donnellan
  0 siblings, 1 reply; 15+ messages in thread
From: PaX Team @ 2016-12-09 10:59 UTC (permalink / raw)
  To: Kees Cook, Andrew Donnellan
  Cc: linuxppc-dev, kernel-hardening, Emese Revfy, LKML, linux-kbuild,
	Brad Spengler, Michal Marek

On 9 Dec 2016 at 13:48, Andrew Donnellan wrote:

> >> as for the solutions, the general advice should enable the use of otherwise
> >> failing gcc versions instead of forcing updating to new ones (though the
> >> latter is advisable for other reasons but not everyone's in the position to
> >> do so easily). in my experience all one needs to do is manually install the
> >> missing files from the gcc sources (ideally distros would take care of it).
> 
> If someone else is willing to write up that advice, then great.
> 
> >> the specific problem addressed here can (and IMHO should) be solved in
> >> another way: remove the inclusion of the offending headers in gcc-common.h
> >> as neither tm.h nor c-common.h are needed by existing plugins. for background,
> 
> We can't build without tm.h: http://pastebin.com/W0azfCr0

you'll need to repeat the removal of dependent headers. based on a quick
test here across gcc 4.5-6.2, if you remove rtl.h, tm_p.h, hard-reg-set.h
and emit-rtl.h in addition to tm.h, the plugins should build fine.

> And we get warnings without c-common.h: http://pastebin.com/Aw8CAj10

that's not due to c-common.h. gcc versions 4.5-4.6 are compiled as a C program
and gcc 4.7 can be compiled both as a C and a C++ program (IIRC, distros opted
for the latter, i forget what manually built versions default to but i guess you
went with the C compilation for your gcc anyway). couple that with -Wmissing-prototypes
and you get that warning regardless of c-common.h being included. something like
this should fix it:

--- a/scripts/gcc-plugins/gcc-generate-gimple-pass.h 2016-12-06 01:01:54.521724573 +0100
+++ b/scripts/gcc-plugins/gcc-generate-gimple-pass.h      2016-12-09 11:43:32.225226164 +0100
@@ -136,6 +136,7 @@
        return new _PASS_NAME_PASS();
 }
 #else
+struct opt_pass *_MAKE_PASS_NAME_PASS(void);
 struct opt_pass *_MAKE_PASS_NAME_PASS(void)
 {
        return &_PASS_NAME_PASS.pass;

> These were all manually built using a script running on a Debian box. 
> Installing precompiled distro versions of rather old gccs would have 
> been somewhat challenging. I've just rebuilt 4.6.4 to double check that 
> I wasn't just seeing things, but it seems that it definitely is still 
> putting c-common.h in the old location.

for reference, this is the git commit that did the move:

commit 7bedc3a05d34cd81e4835a2d3ff8c0ec7108eeb5
Author: steven <steven@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Sat Jun 5 20:33:22 2010 +0000

    gcc/ChangeLog:
            * c-common.c: Move to c-family/.
            * c-common.def: Likewise.
            * c-common.h: Likewise.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] powerpc: enable support for GCC plugins
  2016-12-09 10:59         ` PaX Team
@ 2017-01-27  5:52           ` Andrew Donnellan
  2017-01-27  5:55             ` Andrew Donnellan
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Donnellan @ 2017-01-27  5:52 UTC (permalink / raw)
  To: pageexec, Kees Cook
  Cc: linuxppc-dev, kernel-hardening, Emese Revfy, LKML, linux-kbuild,
	Brad Spengler, Michal Marek

On 09/12/16 21:59, PaX Team wrote:
>>>> the specific problem addressed here can (and IMHO should) be solved in
>>>> another way: remove the inclusion of the offending headers in gcc-common.h
>>>> as neither tm.h nor c-common.h are needed by existing plugins. for background,
>>
>> We can't build without tm.h: http://pastebin.com/W0azfCr0
>
> you'll need to repeat the removal of dependent headers. based on a quick
> test here across gcc 4.5-6.2, if you remove rtl.h, tm_p.h, hard-reg-set.h
> and emit-rtl.h in addition to tm.h, the plugins should build fine.

OK, I finally have a chance to look at this series again.

basic-block.h includes tm.h, and I don't believe we can remove that. I'm 
not convinced there's a way around this.

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] powerpc: enable support for GCC plugins
  2017-01-27  5:52           ` Andrew Donnellan
@ 2017-01-27  5:55             ` Andrew Donnellan
  0 siblings, 0 replies; 15+ messages in thread
From: Andrew Donnellan @ 2017-01-27  5:55 UTC (permalink / raw)
  To: pageexec, Kees Cook
  Cc: linuxppc-dev, kernel-hardening, Emese Revfy, LKML, linux-kbuild,
	Brad Spengler, Michal Marek

On 27/01/17 16:52, Andrew Donnellan wrote:
> basic-block.h includes tm.h, and I don't believe we can remove that. I'm
> not convinced there's a way around this.

Includes via function.h, I should say.

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [1/3] gcc-plugins: fix definition of DISABLE_LATENT_ENTROPY_PLUGIN
  2016-12-06  6:27 [PATCH 1/3] gcc-plugins: fix definition of DISABLE_LATENT_ENTROPY_PLUGIN Andrew Donnellan
  2016-12-06  6:27 ` [PATCH 2/3] powerpc: correctly disable latent entropy GCC plugin on prom_init.o Andrew Donnellan
  2016-12-06  6:28 ` [PATCH 3/3] powerpc: enable support for GCC plugins Andrew Donnellan
@ 2017-02-06 20:37 ` Michael Ellerman
  2 siblings, 0 replies; 15+ messages in thread
From: Michael Ellerman @ 2017-02-06 20:37 UTC (permalink / raw)
  To: Andrew Donnellan, linuxppc-dev, kernel-hardening, keescook, re.emese
  Cc: pageexec, spender, linux-kernel, mmarek, linux-kbuild

On Tue, 2016-12-06 at 06:27:58 UTC, Andrew Donnellan wrote:
> The variable DISABLE_LATENT_ENTROPY_PLUGIN is defined when
> CONFIG_PAX_LATENT_ENTROPY is set. This is leftover from the original PaX
> version of the plugin code and doesn't actually exist. Change the condition
> to depend on CONFIG_GCC_PLUGIN_LATENT_ENTROPY instead.
> 
> Fixes: 38addce8b600 ("gcc-plugins: Add latent_entropy plugin")
> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/7b4010edff09929c253e6626ab19ca

cheers

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-02-06 20:37 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-06  6:27 [PATCH 1/3] gcc-plugins: fix definition of DISABLE_LATENT_ENTROPY_PLUGIN Andrew Donnellan
2016-12-06  6:27 ` [PATCH 2/3] powerpc: correctly disable latent entropy GCC plugin on prom_init.o Andrew Donnellan
2016-12-06  6:28 ` [PATCH 3/3] powerpc: enable support for GCC plugins Andrew Donnellan
2016-12-06 20:40   ` Kees Cook
2016-12-07  1:05     ` [kernel-hardening] " Andrew Donnellan
2016-12-06 21:25   ` Emese Revfy
2016-12-07  5:49     ` Andrew Donnellan
2016-12-07  5:45   ` Andrew Donnellan
2016-12-08 14:42   ` PaX Team
2016-12-08 18:06     ` Kees Cook
2016-12-09  2:48       ` Andrew Donnellan
2016-12-09 10:59         ` PaX Team
2017-01-27  5:52           ` Andrew Donnellan
2017-01-27  5:55             ` Andrew Donnellan
2017-02-06 20:37 ` [1/3] gcc-plugins: fix definition of DISABLE_LATENT_ENTROPY_PLUGIN Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).