PATCH COMMITTED: Fix -fsplit-stack build with old binutils

On Mon, Sep 27, 2010 at 5:07 PM, Ian Lance Taylor <iant@google.com> wrote:
> http://gcc.gnu.org/ml/gcc-regression/2010-09/msg00356.html shows a build
> failure with the -fsplit-stack patches:
>
> /home/regress/tbox/svn-gcc/libgcc/config/i386/morestack.S
> /home/regress/tbox/svn-gcc/libgcc/config/i386/morestack.S: Assembler messages:
> /home/regress/tbox/svn-gcc/libgcc/config/i386/morestack.S:138: Error: unknown pseudo-op: `.cfi_personality'
> /home/regress/tbox/svn-gcc/libgcc/config/i386/morestack.S:139: Error: unknown pseudo-op: `.cfi_lsda'
> make[3]: *** [morestack_s.o] Error 1
>
> gas picked up support for .cfi_personality 2006-11-03, which was in the
> binutils 2.18 release.
>
> This patch changes gcc to only support -fsplit-stack if the .cfi
> pseudo-ops are available in the assembler.  Another approach would be to
> spell out the information in morestack.S, but that does not seem to me
> like a useful way to spend time considering how long the GNU binutils
> have supported the pseudo-ops.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.  Committed to
> mainline.
>
> Ian
>
>
> gcc/ChangeLog:
>
> 2010-09-27  Ian Lance Taylor  <iant@google.com>
>
>        * config/i386/i386.c (ix86_supports_split_stack): -fsplit-stack
>        requires assembler support for CFI directives.
>
> libgcc/ChangeLog:
>
> 2010-09-27  Ian Lance Taylor  <iant@google.com>
>
>        * configure.ac: Test whether assembler supports CFI directives.
>        * config.host: Only add t-stack and i386/t-stack-i386 to
>        tmake_file if libgcc_cv_cfi is "yes".
>        * configure: Rebuild.
>

I checked in this patch to add the missing `$'.

On Fri, Oct 01, 2010 at 04:32:51AM -0700, H.J. Lu wrote:
> > 2010-09-27  Ian Lance Taylor  <iant@google.com>
> >
> >        * configure.ac: Test whether assembler supports CFI directives.
> >        * config.host: Only add t-stack and i386/t-stack-i386 to
> >        tmake_file if libgcc_cv_cfi is "yes".
> >        * configure: Rebuild.
> >
> 
> I checked in this patch to add the missing `$'.
>
> Index: config.host
> ===================================================================
> --- config.host	(revision 164874)
> +++ config.host	(working copy)
> @@ -610,7 +610,7 @@ i[34567]86-*-linux* | x86_64-*-linux* |
>    i[34567]86-*-gnu*)
>  	tmake_file="${tmake_file} t-tls"
>  	if test "$libgcc_cv_cfi" = "yes"; then
> -		tmake_file="{$tmake_file} t-stack i386/t-stack-i386"
> +		tmake_file="${$tmake_file} t-stack i386/t-stack-i386"
>  	fi
>  	;;
>  esac

That looks *really* odd.  Are you sure that's not supposed to be:

> -		tmake_file="{$tmake_file} t-stack i386/t-stack-i386"
> +		tmake_file="${tmake_file} t-stack i386/t-stack-i386"

That makes more sense.

-Nathan

On Fri, Oct 1, 2010 at 4:50 AM, Nathan Froyd <froydnj@codesourcery.com> wrote:
> On Fri, Oct 01, 2010 at 04:32:51AM -0700, H.J. Lu wrote:
>> > 2010-09-27  Ian Lance Taylor  <iant@google.com>
>> >
>> >        * configure.ac: Test whether assembler supports CFI directives.
>> >        * config.host: Only add t-stack and i386/t-stack-i386 to
>> >        tmake_file if libgcc_cv_cfi is "yes".
>> >        * configure: Rebuild.
>> >
>>
>> I checked in this patch to add the missing `$'.
>>
>> Index: config.host
>> ===================================================================
>> --- config.host       (revision 164874)
>> +++ config.host       (working copy)
>> @@ -610,7 +610,7 @@ i[34567]86-*-linux* | x86_64-*-linux* |
>>    i[34567]86-*-gnu*)
>>       tmake_file="${tmake_file} t-tls"
>>       if test "$libgcc_cv_cfi" = "yes"; then
>> -             tmake_file="{$tmake_file} t-stack i386/t-stack-i386"
>> +             tmake_file="${$tmake_file} t-stack i386/t-stack-i386"
>>       fi
>>       ;;
>>  esac
>
> That looks *really* odd.  Are you sure that's not supposed to be:
>
>> -             tmake_file="{$tmake_file} t-stack i386/t-stack-i386"
>> +             tmake_file="${tmake_file} t-stack i386/t-stack-i386"
>
> That makes more sense.

I somehow missed it. You are right. I fixed it.

Thanks.

"H.J. Lu" <hjl.tools@gmail.com> writes:

> I checked in this patch to add the missing `$'.

Thanks.

Ian