| Message ID | 20161206062800.21800-3-andrew.donnellan@au1.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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?
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!
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.
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.
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
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
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.
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
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(+)