Re: [PATCH][1-3] New configure options that make the compiler use -fPIE and -pie as default option

[Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>]

Hi Magnus,

a couple of comments, mostly nits.

> 2014-07-31  Magnus Granberg  <zorry@gentoo.org>
>
> 	/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

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University