[patch]: ld set pseudo-relocation version 2 as default for x86 pe targets

[Kai Tietz-2]

Hello,

as now all x86 pe-coff runtimes are supporting in their startup code
pseudo-relocation version 2, I would think it is time to change for
them the default in pe.em.

2009-11-06  Kai Tietz  <kai.tietz@...>

        (USE_PSEUDO_RELOC): New macro.
        (gld_XXX_before_parse): Set pseudo-relocation default
        value to USE_PSEUDO_RELOC.
        (gldXXX_handle_option): Likewise.


Tested for i686-pc-cygwin, i686-pc-mingw32, and i686-w64-mingw32. Ok for apply?

Cheers,
Kai

--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[Christopher Faylor-9]

On Fri, Nov 06, 2009 at 02:09:06PM +0100, Kai Tietz wrote:

>Hello,
>
>as now all x86 pe-coff runtimes are supporting in their startup code
>pseudo-relocation version 2, I would think it is time to change for
>them the default in pe.em.
>
>2009-11-06  Kai Tietz  <kai.tietz@...>
>
> (USE_PSEUDO_RELOC): New macro.
> (gld_XXX_before_parse): Set pseudo-relocation default
> value to USE_PSEUDO_RELOC.
> (gldXXX_handle_option): Likewise.
>
>
>Tested for i686-pc-cygwin, i686-pc-mingw32, and i686-w64-mingw32. Ok for apply?

In theory, yes.  I have a very minor nit with the name
"USE_PSEUDO_RELOC".  It doesn't quite sound like what you're trying to
do to me.  I think something like "PSEUDO_RELOC_VERSION" or "PSEUDO_RELOC_VAL"
might be better.

cgf

[Kai Tietz-2]

2009/11/6 Christopher Faylor <cgf-use-the-mailinglist-please@...>:
> In theory, yes.  I have a very minor nit with the name
> "USE_PSEUDO_RELOC".  It doesn't quite sound like what you're trying to
> do to me.  I think something like "PSEUDO_RELOC_VERSION" or "PSEUDO_RELOC_VAL"
> might be better.
>
> cgf
>

Ok, I agree that this name is a bit weak. I changed patch and used
instead DEFAULT_PSEUDO_RELOC_VERSION, which fits meaning IMHO best.

2009-11-06  Kai Tietz  <kai.tietz@...>

        (DEFAULT_PSEUDO_RELOC_VERSION): New macro.
        (gld_XXX_before_parse): Set pseudo-relocation default
        value to DEFAULT_PSEUDO_RELOC_VERSION.
        (gldXXX_handle_option): Likewise.


Ok for apply?

Cheers,
Kai

--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[Kai Tietz-2]

2009/11/6 Dave Korn <dave.korn.cygwin@...>:
>  Rather than YA "#ifdef i386" in the .em file, how about adding the
> definition of DEFAULT_PSEUDO_RELOC_VERSION=2 to the relevant emulparams script
> files and use ${DEFAULT_PSEUDO_RELOC_VERSION:-1} in the .em file?

Well, I think that this emulscripts are tending to collect parameters
nobody maintains here (as my last cleanup patch for them had shown for
me). So I think it is more clear to keep such default values in
source, as people begin to search here for them, but well, this is
just my opinion.

>  Also, any reason not to keep pep.em in sync here?  That'd be one advantage
> of avoiding the #ifdef: you could apply the same change to both emulscripts
> and just not define the default anywhere you didn't want to change it.

This isn't necessary (and even makes no sense for x64 at all), as x64
depends on version 2. Version 1 is not usable for x64 pe at all. So
there is just version 2 as default in fact. The mechanism version one
is based on, can't handle different relocation sizes, which is a must
for x64.

>    cheers,
>      DaveK

Cheers,
Kai
--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[Dave Korn-6]

Kai Tietz wrote:

> Ok, I agree that this name is a bit weak. I changed patch and used
> instead DEFAULT_PSEUDO_RELOC_VERSION, which fits meaning IMHO best.
>
> 2009-11-06  Kai Tietz  <kai.tietz@...>
>
> (DEFAULT_PSEUDO_RELOC_VERSION): New macro.
> (gld_XXX_before_parse): Set pseudo-relocation default
> value to DEFAULT_PSEUDO_RELOC_VERSION.
> (gldXXX_handle_option): Likewise.

  That's misformatted: missing asterisk + filename on the first line.

> Ok for apply?

  Rather than YA "#ifdef i386" in the .em file, how about adding the
definition of DEFAULT_PSEUDO_RELOC_VERSION=2 to the relevant emulparams script
files and use ${DEFAULT_PSEUDO_RELOC_VERSION:-1} in the .em file?

  Also, any reason not to keep pep.em in sync here?  That'd be one advantage
of avoiding the #ifdef: you could apply the same change to both emulscripts
and just not define the default anywhere you didn't want to change it.

    cheers,
      DaveK

[Dave Korn-6]

Kai Tietz wrote:

> 2009/11/6 Dave Korn <dave.korn.cygwin@...>:
>>  Rather than YA "#ifdef i386" in the .em file, how about adding the
>> definition of DEFAULT_PSEUDO_RELOC_VERSION=2 to the relevant emulparams script
>> files and use ${DEFAULT_PSEUDO_RELOC_VERSION:-1} in the .em file?
>
> Well, I think that this emulscripts are tending to collect parameters
> nobody maintains here (as my last cleanup patch for them had shown for
> me). So I think it is more clear to keep such default values in
> source, as people begin to search here for them, but well, this is
> just my opinion.

  Fair enough.  I think it would be a good plan long-term to try and get rid
of all the #ifdeffery there and replace it by variables defined in the
emulparams scripts, and I don't think it's really all that obscure a location
to have definitions; the curly braces make it clear that it's a shell
variable, not a #define, and so it ought to be obvious that there's no other
place it could have come from, but I won't ask you to worry about that now.

>>  Also, any reason not to keep pep.em in sync here?  That'd be one advantage
>> of avoiding the #ifdef: you could apply the same change to both emulscripts
>> and just not define the default anywhere you didn't want to change it.
>
> This isn't necessary (and even makes no sense for x64 at all), as x64
> depends on version 2. Version 1 is not usable for x64 pe at all. So
> there is just version 2 as default in fact. The mechanism version one
> is based on, can't handle different relocation sizes, which is a must
> for x64.

  Oh, of course, silly me.  The patch is OK (with fixed changelog of course).

    cheers,
      DaveK

[Dave Korn-6]

Dave Korn wrote:

>   The patch is OK (with fixed changelog of course).

  And of course assuming the new name satisfies cgf's nit, which I'd assume it
does.

    cheers,
      DaveK

[Kai Tietz-2]

2009/11/6 Dave Korn <dave.korn.cygwin@...>:

> Dave Korn wrote:
>
>>   The patch is OK (with fixed changelog of course).
>
>  And of course assuming the new name satisfies cgf's nit, which I'd assume it
> does.
>
>    cheers,
>      DaveK
>
>

Applied with corrected changelog entry.

Thanks,
Kai

--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[Kai Tietz-2]

2009/11/6 Dave Korn <dave.korn.cygwin@...>:
>  Fair enough.  I think it would be a good plan long-term to try and get rid
> of all the #ifdeffery there and replace it by variables defined in the
> emulparams scripts, and I don't think it's really all that obscure a location
> to have definitions; the curly braces make it clear that it's a shell
> variable, not a #define, and so it ought to be obvious that there's no other
> place it could have come from, but I won't ask you to worry about that now.

Well, for some I agree. But for example the UNDERSCORING default is
already present in target bfd structure. So in those scripts there
should be just those definitions, which aren't already present in
other locations. But in general I agree that it would be very good to
minimize some of this #if guard magic. Especially some files in bfd
are here really gross.

Cheers,
Kai


--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination