| Summary: | [4.6] spec rules matching preprocessor flags don't work anymore | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | Alexey Shvetsov <alexxy> |
| Component: | New packages | Assignee: | Gentoo Toolchain Maintainers <toolchain> |
| Status: | RESOLVED FIXED | ||
| Severity: | blocker | CC: | ansla80, chris, hardened, lk4d4math, Marc-Antoine, pchrist, StormByte, taaroa |
| Priority: | High | ||
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| URL: | http://gcc.gnu.org/PR48524 | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | |||
| Bug Blocks: | 346809, 364319 | ||
| Attachments: |
/var/tmp/portage/sys-libs/glibc-2.13-r2/temp/build.log.gz
01_all_joined-cpp-defs.patch support switches with separated form like -D/-U support switches with separated form like -D/-U |
||
|
Description
Alexey Shvetsov
2011-04-03 12:06:20 UTC
Created attachment 268321 [details]
/var/tmp/portage/sys-libs/glibc-2.13-r2/temp/build.log.gz
gzipped build.log
Upstream or is gentoo related? looks to me like -U_FORTIFY_SOURCE isnt working in disabling fortify source ... For some reason it seems to split it up now and doesn't match the spec pattern we were using. $ echo "" | gcc -E -v -U_FORTIFY_SOURCE ... /usr/libexec/gcc/x86_64-unknown-linux-gnu/4.6.0-pre9999/cc1 -E -quiet -v -U _FORTIFY_SOURCE - -mtune=generic -march=x86-64 ... Anyways I'm testing a patch. Fixed in 4.6.0 p1.1. http://sources.gentoo.org/viewcvs.py/gentoo/src/patchsets/gcc/4.6.0/gentoo/10_all_default-fortify-source.patch?r1=1.1&r2=1.2 i dont think we want to go that route. the point of doing it via specs is that people are free to use their own custom specs and bypass the whole thing. okay. i'm not sure why this stopped working though. it looks like -D and -U options are now getting split into two parts:
$ gcc-4.6.0 -E -v -D_FORTIFY_SOURCE=2 -U_FORTIFY_SOURCE - < /dev/null
COLLECT_GCC_OPTIONS='-E' '-v' '-D' '_FORTIFY_SOURCE=2' '-U' '_FORTIFY_SOURCE' '-mtune=generic' '-march=x86-64'
$ gcc-4.5.2 -E -v -D_FORTIFY_SOURCE=2 -U_FORTIFY_SOURCE - < /dev/null
COLLECT_GCC_OPTIONS='-E' '-v' '-D_FORTIFY_SOURCE=2' '-U_FORTIFY_SOURCE' '-mtune=generic' '-march=x86-64'
but the specs for -D and -U options are %{D*&U*&A*} in both.
so if we use `append-cppflags -U _FORTIFY_SOURCE`, things work ? Nope.
$ cat myspec
.test: @test
@test:
echo Test1: %{D*&U*&A*}
echo Test2: %{!U_FORTIFY_SOURCE:-D_FORTIFY_SOURCE=2}
echo Test3: %{!D_FORTIFY_SOURCE:%{!D_FORTIFY_SOURCE=*:%{!U_FORTIFY_SOURCE:-D_FORTIFY_SOURCE=2}}}
$ touch foo.test
$ gcc -E -specs myspec -U_FORTIFY_SOURCE -U _FORTIFY_SOURCE foo.test
Test1: -U _FORTIFY_SOURCE
Test2: -D_FORTIFY_SOURCE=2
Test3: -D_FORTIFY_SOURCE=2
And you can't use spaces or wildcards in the middle of a switch name. Unless I'm missing something obvious it seems impossible to write a spec rule for a preprocessor definition now. I'll bug upstream about it.
hardened: this probably breaks your kernel builds since this and similar stopped working:
%{!D__KERNEL__:%{!nopie:%(esp_options_pie) %(esp_link_pie)}}
(In reply to comment #12) > hardened: this probably breaks your kernel builds since this and similar > stopped working: > > %{!D__KERNEL__:%{!nopie:%(esp_options_pie) %(esp_link_pie)}} It does. hard-sixtyfour linux-2.6.38-hardened # cat .config | grep STACKPROTECTOR CONFIG_CC_STACKPROTECTOR=y hard-sixtyfour linux-2.6.38-hardened # make HOSTLD scripts/kconfig/conf ... kernel/bounds.c:1:0: error: code model kernel does not support PIC mode kernel/bounds.c:1:0: error: code model 'kernel' not supported in the 64 bit mode make[1]: *** [kernel/bounds.s] Error 1 make: *** [prepare0] Error 2 i wonder if there's a piece of core logic we can revert for the time being so we get the old behavior. that way we can focus on the other problems without this tripping us up while upstream resolves the issue. upstream says specs are for internal use only, so yeah, we're on our own. If we add the preprocessor flags we need to the opt file we can get the behaviour we want but we'd have to add each flag individually. That sucks but if we only have to handle _FORTIFY_SOURCE and __KERNEL__ it might be manageable. I've been messing with the option handling code but haven't found a way around it yet that doesn't break the separated arg case or mess up other options. Created attachment 269455 [details, diff]
01_all_joined-cpp-defs.patch
Force -D and -U to use joined as their canonical forms.
As an added bonus this also fixes stuff like "-U _FORTIFY_SOURCE" which never worked before. It survived a testsuite run with no regressions but I'm going to do a system rebuild to make sure I didn't horribly mess something up.
(In reply to comment #16) > If we add the preprocessor flags we need to the opt file we can get the > behaviour we want but we'd have to add each flag individually. That sucks but > if we only have to handle _FORTIFY_SOURCE and __KERNEL__ it might be > manageable. I've been messing with the option handling code but haven't found > a way around it yet that doesn't break the separated arg case or mess up other > options. Zorry and I were thinking of an adhoc solution along these lines, and it does suck handling each manually. A gcc -dumpspecs of hardened gcc gives the following -D and -U options: -D_POSIX_SOURCE -D_REENTRANT -D_MUDFLAP -D_MUDFLAPTH in additon to the -D_FORTIFY_SOURCES -D__KERNEL__. I think we would have to deal with those too. http://sources.gentoo.org/viewcvs.py/gentoo/src/patchsets/gcc/4.6.0/gentoo/01_all_joined-cpp-defs.patch?rev=1.1&view=markup http://sources.gentoo.org/viewcvs.py/gentoo/src/patchsets/gcc/4.6.0/gentoo/10_all_default-fortify-source.patch?r1=1.2&r2=1.3 Fixed in 4.6.0 p1.2. *** Bug 365239 has been marked as a duplicate of this bug. *** Ryan: did you get a chance to mention this cpp patch upstream ? I didn't, but I don't think it would be accepted. They changed it pretty deliberately to pave the way for further cleanups. So I don't even know if this patch will keep working in 4.7. I didn't use the Ubuntu fortify patch at the time because hardened would still be broken, but maybe it's the way to go. I'm releasing 4.6.2 with this patch + the fix for gcj in bug #364319. I don't want to carry this patch forward to 4.7. We have an alternative for FORTIFY_SOURCE. Can hardened migrate away from spec rules or do we have to have some kind of whitelist? (In reply to comment #24) > I'm releasing 4.6.2 with this patch + the fix for gcj in bug #364319. I don't > want to carry this patch forward to 4.7. We have an alternative for > FORTIFY_SOURCE. Can hardened migrate away from spec rules or do we have to > have some kind of whitelist? Can check for __KERNEL__ insted of D__KERNEL__ and it should work so will change that in the patch for hardened gcc 4.7 the ubuntu patch i mentioned (somewhere) doesn't use specs. it uses builtin default defines. shouldn't that work for us ? I looked through all the hardened spec stuff and it's only -D__KERNEL__ we have to worry about. The rest aren't used as patterns to match, just replacement text. Matching on __KERNEL__ doesn't work either in the tests I just did. Since it's one flag let's just special case it and use the ubuntu patch for fortify. I'll try to roll a new patchset this weekend. (In reply to comment #27) i think we should send a patch to lkml to add -no-pie/etc... if the compiler supports it ... (In reply to comment #28) > (In reply to comment #27) > > i think we should send a patch to lkml to add -no-pie/etc... if the compiler > supports it ... The kernel is one thing but then we have all the kernels modules packages. (In reply to comment #29) kernel modules use the kernel source tree to build modules Created attachment 293865 [details, diff]
support switches with separated form like -D/-U
Fist apptempt to support switches with separated form like -D/-U
Created attachment 293867 [details, diff]
support switches with separated form like -D/-U
Did have some error in the last patch
How does that differ from the patch we're using now? 4.6.2 p1.1 switches back to the builtin define method for fortification. Let's table the rest til 4.7. Fixed in gcc 4.7 http://gcc.gnu.org/viewcvs?view=revision&revision=184022 Nice. :) |