fredag 01 augusti 2014 10.52.27 skrev Rainer Orth: > Hi Magnus, > > a couple of comments, mostly nits. > > > 2014-07-31 Magnus Granberg > > > > /gcc > > * config/gnu-user.h: Define PIE_DRIVER_SELF_SPECS for PIE > > as default and GNU_DRIVER_SELF_SPECS. > > * config/i386/gnu-user-common.h: Define DRIVER_SELF_SPECS > > * configure.ac: Add new option that enable PIE as default. > > * configure, config.in: Rebuild. > > * Makefile.in: Disable PIE when building the compiler. > > * doc/install.texi: Add the new configure option default PIE. > > * doc/invoke.texi: Add note for the new configure option default PIE. > > Many of those entries are mis-formatted. See other examples and the GNU > Coding Standards for details. E.g. the first would be > > * config/gnu-user.h (PIE_DRIVER_SELF_SPECS): Define. > > In general, you need to mention which macro, variable, manual section > you change. Emacs' add-change-log-entry does the basics for you. > Besides, you only state what changed, not why. > > Apart from that, I don't think defining PIE_DRIVER_SELF_SPECS in > gnu-user.h is a good idea. This way, every other target supporting the > option would have to duplicate that stuff. > > * testsuite/gcc/default-pie.c: New test for new configure option > --enale-default-pie > > gcc/testsuite has its own ChangeLog file. Typo for --enale-... > > * testsuite/gcc.dg/other/anon5.C: Add skip test as it fail to link > on effective_target default_pie. > > should be > > * g++.dg/other/anon5.C: Skip if default_pie. > > No explanations in ChangeLog entries; they belong into the code. > Besides, you had the first dir component wrong. Again, Emacs does this > for you. > > * testsuite/lib/target-supports.exp (check_profiling_available): > We can't use profiling on effective target default_pie. > (check_effective_target_pie): Add check_effective_target_default_pie. > > Wrong: should be > > * lib/target-supports.exp (check_effective_target_default_pie): > New proc. > > The new default_pic effective-target keyword needs to be documented in > doc/sourcebuild.texi. > > --- a/gcc/testsuite/gcc.dg/default-pie.c 2013-11-09 21:07:16.741479728 +0100 > +++ b/gcc/testsuite/gcc.dg/default-pie.c 2013-11-09 21:05:07.801479218 > +0100 @@ -0,0 +1,12 @@ > +/* { dg-do compile { target *-*-linux* *-*-gnu* } } */ > +/* { dg-require-effective-target default_pie } */ > > Why restrict to Linux, GNU? default_pie should be enough once other > targets add this. > > --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-store-ccp-3.c 2012-03-14 > 17:33:37.000000000 +0100 +++ > b/gcc/testsuite/gcc.dg/tree-ssa/ssa-store-ccp-3.c 2014-07-29 > 00:55:17.421086416 +0200 @@ -2,6 +2,9 @@ > /* Skipped on MIPS GNU/Linux target because __PIC__ can be > defined for executables as well as shared libraries. */ > /* { dg-skip-if "" { *-*-darwin* hppa*64*-*-* mips*-*-linux* *-*-mingw* } { > "*" } { "" } } */ +/* Skipped on default_pie targets because __PIC__ is > + defined for executables. */ > +/* { dg-skip-if "" { default_pie } { "*" } { "" } } */ > > Emit those default args, they're unnecessary. Also in g++.dg/other/anon5.C. > > --- a/gcc/testsuite/g++.dg/other/anon5.C 2012-11-10 15:34:42.000000000 +0100 > +++ b/gcc/testsuite/g++.dg/other/anon5.C 2013-11-09 14:49:52.281390127 > +0100 @@ -1,5 +1,6 @@ > // PR c++/34094 > // { dg-do link { target { ! { *-*-darwin* *-*-hpux* *-*-solaris2.* } } } } > +// { dg-skip-if "" { default_pie } { "*" } { "" } } > > The first arg to dg-skip-if should explain why you're skipping the test. > > --- a/gcc/testsuite/lib/target-supports.exp 2013-10-01 11:18:30.000000000 > +0200 +++ b/gcc/testsuite/lib/target-supports.exp 2013-10-25 > 22:01:46.743388469 +0200 @@ -474,6 +474,11 @@ proc > check_profiling_available { test_wh > } > } > > + # Profiling don't work with default -fPIE -pie. > > Grammar: "doesn't work". > > +# Return 1 if -pie, -fPIE are default enable, 0 otherwise. > + > +proc check_effective_target_default_pie { } { > > Hard to understand, perhaps > > # Return 1 if -pie -fPIE are enabled by default, 0 otherwise. > > --- a/gcc/doc/invoke.texi 2013-10-03 19:13:50.000000000 +0200 > +++ b/gcc/doc/invoke.texi 2013-11-17 21:30:02.784220111 +0100 > @@ -10535,6 +10535,12 @@ For predictable results, you must also s > used for compilation (@option{-fpie}, @option{-fPIE}, > or model suboptions) when you specify this linker option. > > +NOTE: With configure --enable-default-pie this option is enabled by default > > With the @option{--enable-default-pie} configure option, ... > > +for C, C++, ObjC, ObjC++, if none of @option{-fno-PIE}, @option{-fno-pie}, > +@option{-fPIC}, @option{-fpic}, @option{-fno-PIC}, @option{-fno-pic}, > +@option{-nostdlib}, @option{-nostartfiles}, @option{-shared}, > +@option{-nodefaultlibs}, nor @option{static} are found. > > @option{-static}. > > Rainer Thanks Rainer for the nits and comments. Have updated the patches and Changelogs. But i still use PIE_DRIVER_SELF_SPECS, do you have a ide where move it so i don't need to duplicate that stuff or how to do it? Magnus G 2014-11-10 Magnus Granberg /gcc * config/gnu-user.h (PIE_DRIVER_SELF_SPECS) and (GNU_DRIVER_SELF_SPECS): Define. * config/i386/gnu-user-common.h (DRIVER_SELF_SPECS): Define * configure.ac: Add new option. * configure, config.in: Rebuild. * Makefile.in (ALL_CFLAGS) and (ALL_CXXFLAGS): Disable PIE. * doc/install.texi: New configure option. * doc/invoke.texi: Add note to PIE. * doc/sourcebuild.texi: New effective target. gcc/testsuite * gcc/default-pie.c: New test * gcc.dg/tree-ssa/ssa-store-ccp-3.c: Skip if default_pie * g++.dg/other/anon5.C: Skip if default_pie * lib/target-supports.exp (check_effective_target_default_pie): New proc. /libgcc * Makefile.in (CRTSTUFF_CFLAGS): Disable PIE.