| Message ID | yddtwv6t3wp.fsf@lokon.CeBiTec.Uni-Bielefeld.DE |
|---|---|
| State | New |
| Headers | show |
On Thu, 21 May 2015, Rainer Orth wrote: > @@ -1864,6 +1873,12 @@ libgcc.mvars: config.status Makefile specs xgcc$(exeext) > echo GCC_CFLAGS = '$(GCC_CFLAGS)' >> tmp-libgcc.mvars > echo INHIBIT_LIBC_CFLAGS = '$(INHIBIT_LIBC_CFLAGS)' >> tmp-libgcc.mvars > echo TARGET_SYSTEM_ROOT = '$(TARGET_SYSTEM_ROOT)' >> tmp-libgcc.mvars > + if test @enable_default_pie@ = yes; then \ > + NO_PIE_CFLAGS="-fno-PIE"; \ > > Why literal -fno-PIE instead of @NO_PIE_CFLAGS@? Because this is for the target, but @NO_PIE_CFLAGS@ is for the host.
On Thu, May 21, 2015 at 7:13 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote: > "H.J. Lu" <hjl.tools@gmail.com> writes: > >> Here is the complete patch. Tested on Linux/x86-64. It is also >> available on hjl/pie/master branch in git mirror. > > As always, please keep generated files like configure and config.in out > of the submission: it simplifies review. Will do. > diff --git a/gcc/Makefile.in b/gcc/Makefile.in > index ab9b637..e429274 100644 > --- a/gcc/Makefile.in > +++ b/gcc/Makefile.in > @@ -253,6 +253,12 @@ LINKER = $(CC) > LINKER_FLAGS = $(CFLAGS) > endif > > +# We don't want to compile the compiler with -fPIE, it make PCH fail. > ^s > +COMPILER += @NO_PIE_CFLAGS@ > > @@ -750,6 +756,8 @@ CC_FOR_BUILD = @CC_FOR_BUILD@ > CXX_FOR_BUILD = @CXX_FOR_BUILD@ > BUILD_CFLAGS= @BUILD_CFLAGS@ -DGENERATOR_FILE > BUILD_CXXFLAGS = @BUILD_CXXFLAGS@ -DGENERATOR_FILE > +BUILD_CFLAGS += @NO_PIE_CFLAGS@ > +BUILD_CXXFLAGS += @NO_PIE_CFLAGS@ > > Here and in several other places, you use += instead of just adding > @NO_PIE_CFLAGS@ to the existing BUILD_CFLAGS variable. Please lets > keep to the existing idiom instead of randomly introducing another. I used += because 1. There are things: # library is not introduced. If HOST_LIBS is not set, link with # $(CXX) to pick up -lstdc++. ifeq ($(HOST_LIBS),) LINKER = $(CXX) LINKER_FLAGS = $(CXXFLAGS) else LINKER = $(CC) LINKER_FLAGS = $(CFLAGS) endif Use "+=" only needs one line. 2. There are many usages of "+=" in Makefile.in already. > @@ -761,6 +769,7 @@ BUILD_LINKERFLAGS = $(BUILD_CXXFLAGS) > > # Native linker and preprocessor flags. For x-fragment overrides. > BUILD_LDFLAGS=@BUILD_LDFLAGS@ > +BUILD_LDFLAGS += @NO_PIE_FLAG@ > > Likewise. > > BUILD_CPPFLAGS= -I. -I$(@D) -I$(srcdir) -I$(srcdir)/$(@D) \ > -I$(srcdir)/../include @INCINTL@ $(CPPINC) $(CPPFLAGS) > > @@ -1864,6 +1873,12 @@ libgcc.mvars: config.status Makefile specs xgcc$(exeext) > echo GCC_CFLAGS = '$(GCC_CFLAGS)' >> tmp-libgcc.mvars > echo INHIBIT_LIBC_CFLAGS = '$(INHIBIT_LIBC_CFLAGS)' >> tmp-libgcc.mvars > echo TARGET_SYSTEM_ROOT = '$(TARGET_SYSTEM_ROOT)' >> tmp-libgcc.mvars > + if test @enable_default_pie@ = yes; then \ > + NO_PIE_CFLAGS="-fno-PIE"; \ > > Why literal -fno-PIE instead of @NO_PIE_CFLAGS@? Joseph already commented on it. > + else \ > + NO_PIE_CFLAGS=; \ > + fi; \ > + echo NO_PIE_CFLAGS = "$$NO_PIE_CFLAGS" >> tmp-libgcc.mvars > > mv tmp-libgcc.mvars libgcc.mvars > > Besides, we're trying to get away from libgcc.mvars, moving the > detection to libgcc proper. It would be nice to do so here. It will happen when libgcc.mvars is removed/moved. I don't want to duplicate the logic in libgcc/configure now. > diff --git a/gcc/ada/gcc-interface/Makefile.in b/gcc/ada/gcc-interface/Makefile.in > index ecc443e..90aedb5 100644 > --- a/gcc/ada/gcc-interface/Makefile.in > +++ b/gcc/ada/gcc-interface/Makefile.in > @@ -267,6 +267,9 @@ TOOLS_LIBS = ../link.o ../targext.o ../../ggc-none.o ../../libcommon-target.a \ > ../../libcommon.a ../../../libcpp/libcpp.a $(LIBGNAT) $(LIBINTL) $(LIBICONV) \ > ../$(LIBBACKTRACE) ../$(LIBIBERTY) $(SYSLIBS) $(TGT_LIB) > > +# Add -no-pie to TOOLS_LIBS since some of them are compiled with -fno-PIE. > +TOOLS_LIBS += @NO_PIE_FLAG@ > > Again, avoid += > > diff --git a/gcc/config/sol2.h b/gcc/config/sol2.h > index 4dceb16..adf6f3b 100644 > --- a/gcc/config/sol2.h > +++ b/gcc/config/sol2.h > @@ -127,7 +127,7 @@ along with GCC; see the file COPYING3. If not see > #define ASM_SPEC_BASE \ > "%{v:-V} %{Qy:} %{!Qn:-Qy} %{Ym,*} -s %(asm_cpu)" > > -#define ASM_PIC_SPEC " %{fpic|fpie|fPIC|fPIE:-K PIC}" > +#define ASM_PIC_SPEC " %{" FPIE_OR_FPIC_SPEC ":-K PIC}" > > #undef ASM_CPU_DEFAULT_SPEC > #define ASM_CPU_DEFAULT_SPEC \ > > This is ok once the rest goes in. I haven't reviewed the other > target-specific parts, though. > > diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi > index 04332c1..437a534 100644 > --- a/gcc/doc/install.texi > +++ b/gcc/doc/install.texi > @@ -1585,6 +1585,9 @@ not be built. > Specify that the run-time libraries for stack smashing protection > should not be built. > > +@item --enable-default-pie > +Turn on @option{-fPIE} and @option{-pie} by default. > + > @item --disable-libquadmath > Specify that the GCC quad-precision math library should not be built. > On some systems, the library is required to be linkable when building > > This option was added in a seemingly completely random place, between > options to enable/disable runtime libs. Please find a better place. I will move it to before "@item --enable-secureplt". If you aren't happy with it, can you suggest a better place? > diff --git a/gcc/opts.c b/gcc/opts.c > index 9deb8df..4b6d978 100644 > --- a/gcc/opts.c > +++ b/gcc/opts.c > @@ -739,8 +739,22 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, > opts->x_flag_section_anchors = 0; > } > > +#ifndef ENABLE_DEFAULT_PIE > +#undef DEFAULT_FLAG_PIE > +#define DEFAULT_FLAG_PIE 0 > +#endif > + > > Couldn't this be done in defaults.h, too? It seems confusing to provide > DEFAULT_FLAG_PIE defaults both here and in defaults.h. I will try. Thanks.
"H.J. Lu" <hjl.tools@gmail.com> writes: > On Thu, May 21, 2015 at 7:13 AM, Rainer Orth > <ro@cebitec.uni-bielefeld.de> wrote: >> "H.J. Lu" <hjl.tools@gmail.com> writes: >> >>> Here is the complete patch. Tested on Linux/x86-64. It is also >>> available on hjl/pie/master branch in git mirror. Now that the patch is in, I see a couple of testsuite regressions on both Linux/x86_64 and Solaris (both SPARC and x86, patches not yet submitted) with --enable-default-pie. Do you plan to fix those? Rainer
diff --git a/gcc/Makefile.in b/gcc/Makefile.in index ab9b637..e429274 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -253,6 +253,12 @@ LINKER = $(CC) LINKER_FLAGS = $(CFLAGS) endif +# We don't want to compile the compiler with -fPIE, it make PCH fail. ^s +COMPILER += @NO_PIE_CFLAGS@ @@ -750,6 +756,8 @@ CC_FOR_BUILD = @CC_FOR_BUILD@ CXX_FOR_BUILD = @CXX_FOR_BUILD@ BUILD_CFLAGS= @BUILD_CFLAGS@ -DGENERATOR_FILE BUILD_CXXFLAGS = @BUILD_CXXFLAGS@ -DGENERATOR_FILE +BUILD_CFLAGS += @NO_PIE_CFLAGS@ +BUILD_CXXFLAGS += @NO_PIE_CFLAGS@ Here and in several other places, you use += instead of just adding @NO_PIE_CFLAGS@ to the existing BUILD_CFLAGS variable. Please lets keep to the existing idiom instead of randomly introducing another. @@ -761,6 +769,7 @@ BUILD_LINKERFLAGS = $(BUILD_CXXFLAGS) # Native linker and preprocessor flags. For x-fragment overrides. BUILD_LDFLAGS=@BUILD_LDFLAGS@ +BUILD_LDFLAGS += @NO_PIE_FLAG@ Likewise. BUILD_CPPFLAGS= -I. -I$(@D) -I$(srcdir) -I$(srcdir)/$(@D) \ -I$(srcdir)/../include @INCINTL@ $(CPPINC) $(CPPFLAGS) @@ -1864,6 +1873,12 @@ libgcc.mvars: config.status Makefile specs xgcc$(exeext) echo GCC_CFLAGS = '$(GCC_CFLAGS)' >> tmp-libgcc.mvars echo INHIBIT_LIBC_CFLAGS = '$(INHIBIT_LIBC_CFLAGS)' >> tmp-libgcc.mvars echo TARGET_SYSTEM_ROOT = '$(TARGET_SYSTEM_ROOT)' >> tmp-libgcc.mvars + if test @enable_default_pie@ = yes; then \ + NO_PIE_CFLAGS="-fno-PIE"; \ Why literal -fno-PIE instead of @NO_PIE_CFLAGS@? + else \ + NO_PIE_CFLAGS=; \ + fi; \ + echo NO_PIE_CFLAGS = "$$NO_PIE_CFLAGS" >> tmp-libgcc.mvars mv tmp-libgcc.mvars libgcc.mvars Besides, we're trying to get away from libgcc.mvars, moving the detection to libgcc proper. It would be nice to do so here. diff --git a/gcc/ada/gcc-interface/Makefile.in b/gcc/ada/gcc-interface/Makefile.in index ecc443e..90aedb5 100644 --- a/gcc/ada/gcc-interface/Makefile.in +++ b/gcc/ada/gcc-interface/Makefile.in @@ -267,6 +267,9 @@ TOOLS_LIBS = ../link.o ../targext.o ../../ggc-none.o ../../libcommon-target.a \ ../../libcommon.a ../../../libcpp/libcpp.a $(LIBGNAT) $(LIBINTL) $(LIBICONV) \ ../$(LIBBACKTRACE) ../$(LIBIBERTY) $(SYSLIBS) $(TGT_LIB) +# Add -no-pie to TOOLS_LIBS since some of them are compiled with -fno-PIE. +TOOLS_LIBS += @NO_PIE_FLAG@ Again, avoid += diff --git a/gcc/config/sol2.h b/gcc/config/sol2.h index 4dceb16..adf6f3b 100644 --- a/gcc/config/sol2.h +++ b/gcc/config/sol2.h @@ -127,7 +127,7 @@ along with GCC; see the file COPYING3. If not see #define ASM_SPEC_BASE \ "%{v:-V} %{Qy:} %{!Qn:-Qy} %{Ym,*} -s %(asm_cpu)" -#define ASM_PIC_SPEC " %{fpic|fpie|fPIC|fPIE:-K PIC}" +#define ASM_PIC_SPEC " %{" FPIE_OR_FPIC_SPEC ":-K PIC}" #undef ASM_CPU_DEFAULT_SPEC #define ASM_CPU_DEFAULT_SPEC \ This is ok once the rest goes in. I haven't reviewed the other target-specific parts, though. diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi index 04332c1..437a534 100644 --- a/gcc/doc/install.texi +++ b/gcc/doc/install.texi @@ -1585,6 +1585,9 @@ not be built. Specify that the run-time libraries for stack smashing protection should not be built. +@item --enable-default-pie +Turn on @option{-fPIE} and @option{-pie} by default. + @item --disable-libquadmath Specify that the GCC quad-precision math library should not be built. On some systems, the library is required to be linkable when building This option was added in a seemingly completely random place, between options to enable/disable runtime libs. Please find a better place. diff --git a/gcc/opts.c b/gcc/opts.c index 9deb8df..4b6d978 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -739,8 +739,22 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, opts->x_flag_section_anchors = 0; } +#ifndef ENABLE_DEFAULT_PIE +#undef DEFAULT_FLAG_PIE +#define DEFAULT_FLAG_PIE 0 +#endif + Couldn't this be done in defaults.h, too? It seems confusing to provide DEFAULT_FLAG_PIE defaults both here and in defaults.h. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University