diff mbox

Add -lssp_nonshared to LINK_SSP_SPEC

Message ID 20120207085443.GU18768@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Feb. 7, 2012, 8:54 a.m. UTC
On Mon, Jan 23, 2012 at 12:03:27PM +0100, Richard Guenther wrote:
> On Mon, Jan 23, 2012 at 12:23 AM, Gerald Pfeifer <gerald@pfeifer.com> wrote:
> > On Sat, 21 Jan 2012, Tijl Coosemans wrote:
> >> I've been using this patch now. It's similar to the above url, but
> >> conditional on TARGET_LIBC_PROVIDES_SSP to support older FreeBSD
> >> versions.
> >>
> >> Gerald volunteered to commit. Gerald, just trunk for now or older
> >> branches too?
> >
> > If Richi agries, I'd apply this on trunk and the GCC 4.6 branch,
> > since that is the stable release our users employ.
> 
> Sure, go ahead.
> 
> Richard.
> 
> > Gerald
> >
> > PS: We also need to update the copyright date at the top, and I'll
> > take care of that when committing.
> >
> >> 2011-01-20  Tijl Coosemans  <tijl@coosemans.org>
> >>
> >>       * gcc/config/freebsd-spec.h [TARGET_LIBC_PROVIDES_SSP] (LINK_SSP_SPEC): Define.

This change unfortunately broke all non-freebsd powerpc* targets.
For some weird reason freebsd-spec.h is included for all powerpc* targets
(to support -mcall-freebsd), which means that e.g. on powerpc64-linux
the above results in -fstack-protector being broken.

So, if this mess is really needed (does anybody actually use -mcall-freebsd
on non-freebsd targets?), IMHO freebsd-spec.h must avoid defining non-FBSD_
prefixed macros, as the following (completely untested) patch.  Don't have
access to FreeBSD to test it there though, can test on powerpc64-linux.

2012-02-07  Jakub Jelinek  <jakub@redhat.com>

	* config/freebsd-spec.h (LINK_EH_SPEC, LINK_SSP_SPEC,
	USE_LD_AS_NEEDED): Don't define.
	(FBSD_LINK_EH_SPEC, FBSD_LINK_SSP_SPEC, FBSD_USE_LD_AS_NEEDED):
	Define these instead.
	* config/freebsd.h (LINK_EH_SPEC, LINK_SSP_SPEC, USE_LD_AS_NEEDED):
	Redefine to FBSD_* macros if those are defined.


	Jakub

Comments

Richard Biener Feb. 7, 2012, 9:50 a.m. UTC | #1
On Tue, Feb 7, 2012 at 9:54 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Mon, Jan 23, 2012 at 12:03:27PM +0100, Richard Guenther wrote:
>> On Mon, Jan 23, 2012 at 12:23 AM, Gerald Pfeifer <gerald@pfeifer.com> wrote:
>> > On Sat, 21 Jan 2012, Tijl Coosemans wrote:
>> >> I've been using this patch now. It's similar to the above url, but
>> >> conditional on TARGET_LIBC_PROVIDES_SSP to support older FreeBSD
>> >> versions.
>> >>
>> >> Gerald volunteered to commit. Gerald, just trunk for now or older
>> >> branches too?
>> >
>> > If Richi agries, I'd apply this on trunk and the GCC 4.6 branch,
>> > since that is the stable release our users employ.
>>
>> Sure, go ahead.
>>
>> Richard.
>>
>> > Gerald
>> >
>> > PS: We also need to update the copyright date at the top, and I'll
>> > take care of that when committing.
>> >
>> >> 2011-01-20  Tijl Coosemans  <tijl@coosemans.org>
>> >>
>> >>       * gcc/config/freebsd-spec.h [TARGET_LIBC_PROVIDES_SSP] (LINK_SSP_SPEC): Define.
>
> This change unfortunately broke all non-freebsd powerpc* targets.
> For some weird reason freebsd-spec.h is included for all powerpc* targets
> (to support -mcall-freebsd), which means that e.g. on powerpc64-linux
> the above results in -fstack-protector being broken.
>
> So, if this mess is really needed (does anybody actually use -mcall-freebsd
> on non-freebsd targets?), IMHO freebsd-spec.h must avoid defining non-FBSD_
> prefixed macros, as the following (completely untested) patch.  Don't have
> access to FreeBSD to test it there though, can test on powerpc64-linux.

Ugh.

I propose to revert the original patch for now.

Richard.

> 2012-02-07  Jakub Jelinek  <jakub@redhat.com>
>
>        * config/freebsd-spec.h (LINK_EH_SPEC, LINK_SSP_SPEC,
>        USE_LD_AS_NEEDED): Don't define.
>        (FBSD_LINK_EH_SPEC, FBSD_LINK_SSP_SPEC, FBSD_USE_LD_AS_NEEDED):
>        Define these instead.
>        * config/freebsd.h (LINK_EH_SPEC, LINK_SSP_SPEC, USE_LD_AS_NEEDED):
>        Redefine to FBSD_* macros if those are defined.
>
> --- gcc/config/freebsd-spec.h.jj        2012-01-30 00:10:01.000000000 +0100
> +++ gcc/config/freebsd-spec.h   2012-02-07 09:46:05.031256945 +0100
> @@ -135,14 +135,15 @@ is built with the --enable-threads confi
>  #endif
>
>  #if defined(HAVE_LD_EH_FRAME_HDR)
> -#define LINK_EH_SPEC "%{!static:--eh-frame-hdr} "
> +#define FBSD_LINK_EH_SPEC "%{!static:--eh-frame-hdr} "
>  #endif
>
>  #ifdef TARGET_LIBC_PROVIDES_SSP
> -#define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-all:-lssp_nonshared}"
> +#define FBSD_LINK_SSP_SPEC \
> +  "%{fstack-protector|fstack-protector-all:-lssp_nonshared}"
>  #endif
>
>  /* Use --as-needed -lgcc_s for eh support.  */
>  #ifdef HAVE_LD_AS_NEEDED
> -#define USE_LD_AS_NEEDED 1
> +#define FBSD_USE_LD_AS_NEEDED 1
>  #endif
> --- gcc/config/freebsd.h.jj     2010-11-26 18:39:09.000000000 +0100
> +++ gcc/config/freebsd.h        2012-02-07 09:48:50.872294367 +0100
> @@ -1,6 +1,6 @@
>  /* Base configuration file for all FreeBSD targets.
>    Copyright (C) 1999, 2000, 2001, 2007, 2008, 2009,
> -   2010 Free Software Foundation, Inc.
> +   2010, 2011, 2012 Free Software Foundation, Inc.
>
>  This file is part of GCC.
>
> @@ -45,6 +45,21 @@ along with GCC; see the file COPYING3.
>  #undef  LIB_SPEC
>  #define LIB_SPEC FBSD_LIB_SPEC
>
> +#ifdef FBSD_LINK_EH_SPEC
> +#undef LINK_EH_SPEC
> +#define        LINK_EH_SPEC FBSD_LINK_EH_SPEC
> +#endif
> +
> +#ifdef FBSD_LINK_SSP_SPEC
> +#undef LINK_SSP_SPEC
> +#define        LINK_SSP_SPEC FBSD_LINK_SSP_SPEC
> +#endif
> +
> +#ifdef FBSD_USE_LD_AS_NEEDED
> +#undef USE_LD_AS_NEEDED
> +#define        USE_LD_AS_NEEDED FBSD_USE_LD_AS_NEEDED
> +#endif
> +
>  /************************[  Target stuff  ]***********************************/
>
>  /* All FreeBSD Architectures support the ELF object file format.  */
>
>        Jakub
Tijl Coosemans Feb. 7, 2012, 11:17 a.m. UTC | #2
On Tuesday 07 February 2012 09:54:43 Jakub Jelinek wrote:
> On Mon, Jan 23, 2012 at 12:03:27PM +0100, Richard Guenther wrote:
>> On Mon, Jan 23, 2012 at 12:23 AM, Gerald Pfeifer <gerald@pfeifer.com> wrote:
>>> On Sat, 21 Jan 2012, Tijl Coosemans wrote:
>>>> I've been using this patch now. It's similar to the above url, but
>>>> conditional on TARGET_LIBC_PROVIDES_SSP to support older FreeBSD
>>>> versions.
>>>>
>>>> Gerald volunteered to commit. Gerald, just trunk for now or older
>>>> branches too?
>>>
>>> If Richi agries, I'd apply this on trunk and the GCC 4.6 branch,
>>> since that is the stable release our users employ.
>> 
>> Sure, go ahead.
>> 
>> Richard.
>> 
>>> Gerald
>>>
>>> PS: We also need to update the copyright date at the top, and I'll
>>> take care of that when committing.
>>>
>>>> 2011-01-20  Tijl Coosemans  <tijl@coosemans.org>
>>>>
>>>>       * gcc/config/freebsd-spec.h [TARGET_LIBC_PROVIDES_SSP] (LINK_SSP_SPEC): Define.
> 
> This change unfortunately broke all non-freebsd powerpc* targets.
> For some weird reason freebsd-spec.h is included for all powerpc* targets
> (to support -mcall-freebsd), which means that e.g. on powerpc64-linux
> the above results in -fstack-protector being broken.
> 
> So, if this mess is really needed (does anybody actually use -mcall-freebsd
> on non-freebsd targets?), IMHO freebsd-spec.h must avoid defining non-FBSD_
> prefixed macros, as the following (completely untested) patch.  Don't have
> access to FreeBSD to test it there though, can test on powerpc64-linux.

Everything still works on FreeBSD.

> --- gcc/config/freebsd.h.jj	2010-11-26 18:39:09.000000000 +0100
> +++ gcc/config/freebsd.h	2012-02-07 09:48:50.872294367 +0100
> @@ -45,6 +45,21 @@ along with GCC; see the file COPYING3.
>  #undef  LIB_SPEC
>  #define LIB_SPEC FBSD_LIB_SPEC
>  
> +#ifdef	FBSD_LINK_EH_SPEC
> +#undef	LINK_EH_SPEC
> +#define	LINK_EH_SPEC FBSD_LINK_EH_SPEC
> +#endif
> +
> +#ifdef	FBSD_LINK_SSP_SPEC
> +#undef	LINK_SSP_SPEC
> +#define	LINK_SSP_SPEC FBSD_LINK_SSP_SPEC
> +#endif
> +     

FYI, there are extra spaces on this line.
Joseph Myers Feb. 7, 2012, 1:13 p.m. UTC | #3
On Tue, 7 Feb 2012, Jakub Jelinek wrote:

> So, if this mess is really needed (does anybody actually use -mcall-freebsd
> on non-freebsd targets?), IMHO freebsd-spec.h must avoid defining non-FBSD_

I've argued for a long time that the -mcall-* support should be removed 
and targets using rs6000/sysv4.h should move to the standard approach of 
each configuration defining and using its own specs where the specs differ 
between targets (in particular, making powerpc*-linux* use linux.h and 
gnu-user.h like most other targets using the Linux kernel do).  There's a 
point about this in the development conventions document at 
<https://docs.google.com/document/pub?id=10LO8y0YhjlKHya_PKM3jEGrJu0rllv-Nc9qP5LXqH_I>.  
I don't think -mcall-* will form any useful part of proper multi-target 
support.
Jakub Jelinek Feb. 7, 2012, 1:18 p.m. UTC | #4
On Tue, Feb 07, 2012 at 01:13:35PM +0000, Joseph S. Myers wrote:
> On Tue, 7 Feb 2012, Jakub Jelinek wrote:
> 
> > So, if this mess is really needed (does anybody actually use -mcall-freebsd
> > on non-freebsd targets?), IMHO freebsd-spec.h must avoid defining non-FBSD_
> 
> I've argued for a long time that the -mcall-* support should be removed 
> and targets using rs6000/sysv4.h should move to the standard approach of 
> each configuration defining and using its own specs where the specs differ 
> between targets (in particular, making powerpc*-linux* use linux.h and 
> gnu-user.h like most other targets using the Linux kernel do).  There's a 
> point about this in the development conventions document at 
> <https://docs.google.com/document/pub?id=10LO8y0YhjlKHya_PKM3jEGrJu0rllv-Nc9qP5LXqH_I>.  
> I don't think -mcall-* will form any useful part of proper multi-target 
> support.

I agree, not sure if I'll have time for that though.  Not a stage4 material
though IMHO.

	Jakub
diff mbox

Patch

--- gcc/config/freebsd-spec.h.jj	2012-01-30 00:10:01.000000000 +0100
+++ gcc/config/freebsd-spec.h	2012-02-07 09:46:05.031256945 +0100
@@ -135,14 +135,15 @@  is built with the --enable-threads confi
 #endif
 
 #if defined(HAVE_LD_EH_FRAME_HDR)
-#define LINK_EH_SPEC "%{!static:--eh-frame-hdr} "
+#define FBSD_LINK_EH_SPEC "%{!static:--eh-frame-hdr} "
 #endif
 
 #ifdef TARGET_LIBC_PROVIDES_SSP
-#define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-all:-lssp_nonshared}"
+#define FBSD_LINK_SSP_SPEC \
+  "%{fstack-protector|fstack-protector-all:-lssp_nonshared}"
 #endif
 
 /* Use --as-needed -lgcc_s for eh support.  */
 #ifdef HAVE_LD_AS_NEEDED
-#define USE_LD_AS_NEEDED 1
+#define FBSD_USE_LD_AS_NEEDED 1
 #endif
--- gcc/config/freebsd.h.jj	2010-11-26 18:39:09.000000000 +0100
+++ gcc/config/freebsd.h	2012-02-07 09:48:50.872294367 +0100
@@ -1,6 +1,6 @@ 
 /* Base configuration file for all FreeBSD targets.
    Copyright (C) 1999, 2000, 2001, 2007, 2008, 2009,
-   2010 Free Software Foundation, Inc.
+   2010, 2011, 2012 Free Software Foundation, Inc.
 
 This file is part of GCC.
 
@@ -45,6 +45,21 @@  along with GCC; see the file COPYING3.
 #undef  LIB_SPEC
 #define LIB_SPEC FBSD_LIB_SPEC
 
+#ifdef	FBSD_LINK_EH_SPEC
+#undef	LINK_EH_SPEC
+#define	LINK_EH_SPEC FBSD_LINK_EH_SPEC
+#endif
+
+#ifdef	FBSD_LINK_SSP_SPEC
+#undef	LINK_SSP_SPEC
+#define	LINK_SSP_SPEC FBSD_LINK_SSP_SPEC
+#endif
+     
+#ifdef	FBSD_USE_LD_AS_NEEDED
+#undef	USE_LD_AS_NEEDED
+#define	USE_LD_AS_NEEDED FBSD_USE_LD_AS_NEEDED
+#endif
+
 /************************[  Target stuff  ]***********************************/
 
 /* All FreeBSD Architectures support the ELF object file format.  */