Fwd: [patch ld]: Enable autoimport for x64 by default

Fwd: [patch ld]: Enable autoimport for x64 by default

[Kai Tietz]

[-- Attachment #1: Type: text/plain, Size: 1888 bytes --]

Ups xD


---------- Forwarded message ----------
From: Kai Tietz <ktietz70@googlemail.com>
Date: 2010/5/16
Subject: [patch ld]: Enable autoimport for x64 by default
To: Binutils <binutils@gmail.com>, Danny Smith <dansmister@gmail.com>,
Dave Korn <dave.korn.cygwin@googlemail.com>


Hello,

this patch set autoimport enabled for x64 targets by default.
Additionally it removes the option for pseudo-relocation-v1, as for
this target it would lead to an crash if using it.
The major issue (which could be possibly merged to pe.em too - if
autoimport enabled by default is wished - is the
gld_${EMULATION_NAME}_get_script function. At the moment it uses the
script, which moves rdata into data, which is caused by the v1
pseudo-relocation. As now for all targets v2 is default and v2 don't
need text/rdata to be marked as writable, this condition for
pei386_auto_import == 1 is just of interest when
pei386_runtime_pseudo_reloc != 2. By this pe.em could use also for x86
mingw the autoimport default enabling.

ChangeLog

2010-05-16  Kai Tietz

       * pep.em (gld_${EMULATION_NAME}_before_parse): Set auto-import
       default enabled.
       (gld_${EMULATION_NAME}_get_script): Check for rdata merge into data
       by pseudo-relocation-kind, too.
       (enum options): Remove OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1.
       (gld${EMULATION_NAME}_add_options): Likewise.
       (gld${EMULATION_NAME}_handle_option): Likewise.
       (pep_find_data_imports): Remove warning about back-door
       enable of auto-import.

Tested for x86_64-pc-mingw32. Ok for apply?

Regards,
Kai

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



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

[-- Attachment #2: do_autoimp.diff --]
[-- Type: application/octet-stream, Size: 3249 bytes --]

Index: src/ld/emultempl/pep.em
===================================================================
--- src.orig/ld/emultempl/pep.em	2010-05-15 21:49:19.000000000 +0200
+++ src/ld/emultempl/pep.em	2010-05-16 11:58:44.524052000 +0200
@@ -139,7 +139,7 @@ gld_${EMULATION_NAME}_before_parse (void
 #ifdef DLL_SUPPORT
   config.dynamic_link = TRUE;
   config.has_shared = 1;
-  link_info.pei386_auto_import = -1;
+  link_info.pei386_auto_import = 1;
   link_info.pei386_runtime_pseudo_reloc = 2; /* Use by default version 2.  */
 #endif
 }
@@ -185,7 +185,6 @@ enum options
   OPTION_EXCLUDE_LIBS,
   OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC,
   OPTION_DLL_DISABLE_RUNTIME_PSEUDO_RELOC,
-  OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1,
   OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V2,
   OPTION_EXCLUDE_MODULES_FOR_IMPLIB,
   OPTION_USE_NUL_PREFIXED_IMPORT_TABLES,
@@ -263,7 +262,6 @@ gld${EMULATION_NAME}_add_options
     {"enable-extra-pep-debug", no_argument, NULL, OPTION_ENABLE_EXTRA_PE_DEBUG},
     {"enable-runtime-pseudo-reloc", no_argument, NULL, OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC},
     {"disable-runtime-pseudo-reloc", no_argument, NULL, OPTION_DLL_DISABLE_RUNTIME_PSEUDO_RELOC},
-    {"enable-runtime-pseudo-reloc-v1", no_argument, NULL, OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1},
     {"enable-runtime-pseudo-reloc-v2", no_argument, NULL, OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V2},
 #endif
     {"enable-long-section-names", no_argument, NULL, OPTION_ENABLE_LONG_SECTION_NAMES},
@@ -736,9 +734,6 @@ gld${EMULATION_NAME}_handle_option (int
     case OPTION_DLL_DISABLE_RUNTIME_PSEUDO_RELOC:
       link_info.pei386_runtime_pseudo_reloc = 0;
       break;
-    case OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1:
-      link_info.pei386_runtime_pseudo_reloc = 1;
-      break;
     case OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V2:
       link_info.pei386_runtime_pseudo_reloc = 2;
       break;
@@ -1097,22 +1092,6 @@ pep_find_data_imports (void)
 	      asymbol **symbols;
 	      int nsyms, i;
 
-	      if (link_info.pei386_auto_import == -1)
-		{
-		  static bfd_boolean warned = FALSE;
-
-		  info_msg (_("Info: resolving %s by linking to %s (auto-import)\n"),
-			    undef->root.string, buf);
-
-		  /* PR linker/4844.  */
-		  if (! warned)
-		    {
-		      warned = TRUE;
-		      einfo (_("%P: warning: auto-importing has been activated without --enable-auto-import specified on the command line.\n\
-This should work unless it involves constant data structures referencing symbols from auto-imported DLLs.\n"));
-		    }
-		}
-
 	      if (!bfd_generic_link_read_symbols (b))
 		{
 		  einfo (_("%B%F: could not read symbols: %E\n"), b);
@@ -1928,7 +1907,7 @@ sed $sc ldscripts/${EMULATION_NAME}.xbn
 echo '  ; else if (!config.magic_demand_paged) return'	>> e${EMULATION_NAME}.c
 sed $sc ldscripts/${EMULATION_NAME}.xn			>> e${EMULATION_NAME}.c
 if test -n "$GENERATE_AUTO_IMPORT_SCRIPT" ; then
-echo '  ; else if (link_info.pei386_auto_import == 1) return'	>> e${EMULATION_NAME}.c
+echo '  ; else if (link_info.pei386_auto_import == 1 && link_info.pei386_runtime_pseudo_reloc != 2) return'	>> e${EMULATION_NAME}.c
 sed $sc ldscripts/${EMULATION_NAME}.xa			>> e${EMULATION_NAME}.c
 fi
 echo '  ; else return'					>> e${EMULATION_NAME}.c

Re: Fwd: [patch ld]: Enable autoimport for x64 by default

[Dave Korn]

On 16/05/2010 14:49, Kai Tietz wrote:

> this patch set autoimport enabled for x64 targets by default.
> Additionally it removes the option for pseudo-relocation-v1, as for
> this target it would lead to an crash if using it.

  So, after this change, the linker will have two options,
"enable-runtime-pseudo-reloc" and "enable-runtime-pseudo-reloc-v2", both of
which mean the same thing and do nothing because they're the default anyway,
and no option at all that refers to -v1.  I think that's going to seem a bit
peculiar to users!

  Perhaps the -v1 option should trigger an error (deprecation message?) rather
than be removed altogether?  Or perhaps you should remove both -v1 and -v2
variants and just have the generic enable/disable options?

>        * pep.em (gld_${EMULATION_NAME}_before_parse): Set auto-import
>        default enabled.
>        (gld_${EMULATION_NAME}_get_script): Check for rdata merge into data
>        by pseudo-relocation-kind, too.
>        (enum options): Remove OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1.
>        (gld${EMULATION_NAME}_add_options): Likewise.
>        (gld${EMULATION_NAME}_handle_option): Likewise.
>        (pep_find_data_imports): Remove warning about back-door
>        enable of auto-import.
> 
> Tested for x86_64-pc-mingw32. Ok for apply?

  The patch will be OK by me once we've discussed what to do about the
confusing options (perhaps nothing at all if you really want).

    cheers,
      DaveK

Re: Fwd: [patch ld]: Enable autoimport for x64 by default

[Kai Tietz]

2010/5/16 Dave Korn <dave.korn.cygwin@gmail.com>:
> On 16/05/2010 14:49, Kai Tietz wrote:
>
>> this patch set autoimport enabled for x64 targets by default.
>> Additionally it removes the option for pseudo-relocation-v1, as for
>> this target it would lead to an crash if using it.
>
>  So, after this change, the linker will have two options,
> "enable-runtime-pseudo-reloc" and "enable-runtime-pseudo-reloc-v2", both of
> which mean the same thing and do nothing because they're the default anyway,
> and no option at all that refers to -v1.  I think that's going to seem a bit
> peculiar to users!

Well, the -v2 part I can remove here, too. The v1 makes absolutely no
sense for x64. Therefore I want to remove the possibiltiy for user
producing something not working.

>  Perhaps the -v1 option should trigger an error (deprecation message?) rather
> than be removed altogether?  Or perhaps you should remove both -v1 and -v2
> variants and just have the generic enable/disable options?

Ok, I'll remove -v2. I wanted to keep this for having same options for
x86 and x64, but on a second thought, the v1 has to be ignored and
warned, or even errored. So maybe better to remove it for x64
completely.

>>        * pep.em (gld_${EMULATION_NAME}_before_parse): Set auto-import
>>        default enabled.
>>        (gld_${EMULATION_NAME}_get_script): Check for rdata merge into data
>>        by pseudo-relocation-kind, too.
>>        (enum options): Remove OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1.
>>        (gld${EMULATION_NAME}_add_options): Likewise.
>>        (gld${EMULATION_NAME}_handle_option): Likewise.
>>        (pep_find_data_imports): Remove warning about back-door
>>        enable of auto-import.
>>
>> Tested for x86_64-pc-mingw32. Ok for apply?
>
>  The patch will be OK by me once we've discussed what to do about the
> confusing options (perhaps nothing at all if you really want).
>
>    cheers,
>      DaveK
>

Regards,
Kai

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

Re: Fwd: [patch ld]: Enable autoimport for x64 by default

[Kai Tietz]

2010/5/16 Kai Tietz <ktietz70@googlemail.com>:
> 2010/5/16 Dave Korn <dave.korn.cygwin@gmail.com>:
>> On 16/05/2010 14:49, Kai Tietz wrote:
>>
>>> this patch set autoimport enabled for x64 targets by default.
>>> Additionally it removes the option for pseudo-relocation-v1, as for
>>> this target it would lead to an crash if using it.
>>
>>  So, after this change, the linker will have two options,
>> "enable-runtime-pseudo-reloc" and "enable-runtime-pseudo-reloc-v2", both of
>> which mean the same thing and do nothing because they're the default anyway,
>> and no option at all that refers to -v1.  I think that's going to seem a bit
>> peculiar to users!
>
> Well, the -v2 part I can remove here, too. The v1 makes absolutely no
> sense for x64. Therefore I want to remove the possibiltiy for user
> producing something not working.
>
>>  Perhaps the -v1 option should trigger an error (deprecation message?) rather
>> than be removed altogether?  Or perhaps you should remove both -v1 and -v2
>> variants and just have the generic enable/disable options?
>
> Ok, I'll remove -v2. I wanted to keep this for having same options for
> x86 and x64, but on a second thought, the v1 has to be ignored and
> warned, or even errored. So maybe better to remove it for x64
> completely.
>
>>>        * pep.em (gld_${EMULATION_NAME}_before_parse): Set auto-import
>>>        default enabled.
>>>        (gld_${EMULATION_NAME}_get_script): Check for rdata merge into data
>>>        by pseudo-relocation-kind, too.
>>>        (enum options): Remove OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1.
>>>        (gld${EMULATION_NAME}_add_options): Likewise.
>>>        (gld${EMULATION_NAME}_handle_option): Likewise.
>>>        (pep_find_data_imports): Remove warning about back-door
>>>        enable of auto-import.
>>>
>>> Tested for x86_64-pc-mingw32. Ok for apply?
>>
>>  The patch will be OK by me once we've discussed what to do about the
>> confusing options (perhaps nothing at all if you really want).

Well, as there are maybe some users using the -v2 option, I would like
to keep it for now. it is deprecated, but I think we should keep it at
least until next release, and well, in case that there will be a v3
version in future (not that I am planning for it), it can still make
sense


Cheers,
Kai

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

Re: Fwd: [patch ld]: Enable autoimport for x64 by default

[Dave Korn]

On 16/05/2010 16:44, Kai Tietz wrote:

> Well, as there are maybe some users using the -v2 option, I would like
> to keep it for now. it is deprecated, but I think we should keep it at
> least until next release, and well, in case that there will be a v3
> version in future (not that I am planning for it), it can still make
> sense

  Yes, although do bear in mind that we've never documented the -v1 and -v2
options, so we're not honour-bound to support them forever.  (My understanding
is that they're mostly there as 'get out of jail free cards' in case we
discovered there was a problem with the default setting at any time.)

  Anyway, I'm happy with what you want to do for your users.  If you're going
to change the removal of the -v1 option so it gives a specific warning or
error (rather than just 'unknown option'), please post the tweaked version of
your patch before you commit it; if you're happy with the current version
as-is, then just go ahead.

    cheers,
      DaveK

Re: Fwd: [patch ld]: Enable autoimport for x64 by default

[Kai Tietz]

[-- Attachment #1: Type: text/plain, Size: 684 bytes --]

Hello,

I reworked patch a bit and kept the v1 option. The v1 will be ignored
and warned.

ChangeLog
2010-05-19  Kai Tietz

        * pep.em (gld_${EMULATION_NAME}_before_parse): Enable by
        default auto_import.
        (gld${EMULATION_NAME}_handle_option): Warn about v1.
        (pep_find_data_imports): Remove superflous warnings about
        auto-import.
        (gld_${EMULATION_NAME}_get_script): Don't merge for auto-import
        and active pseudo-relocation-v2 rdata into data section.

Tested for x86_64-*-mingw*. Ok for apply?

Regards,
Kai

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

[-- Attachment #2: pep_autoimport.diff --]
[-- Type: application/octet-stream, Size: 2352 bytes --]

Index: src/ld/emultempl/pep.em
===================================================================
--- src.orig/ld/emultempl/pep.em	2010-05-17 13:47:52.000000000 +0200
+++ src/ld/emultempl/pep.em	2010-05-19 14:34:45.311008000 +0200
@@ -139,7 +139,7 @@ gld_${EMULATION_NAME}_before_parse (void
 #ifdef DLL_SUPPORT
   config.dynamic_link = TRUE;
   config.has_shared = 1;
-  link_info.pei386_auto_import = -1;
+  link_info.pei386_auto_import = 1;
   link_info.pei386_runtime_pseudo_reloc = 2; /* Use by default version 2.  */
 #endif
 }
@@ -737,7 +737,7 @@ gld${EMULATION_NAME}_handle_option (int 
       link_info.pei386_runtime_pseudo_reloc = 0;
       break;
     case OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V1:
-      link_info.pei386_runtime_pseudo_reloc = 1;
+      einfo (_("%P: warning: Pseudo-relocation version 1 isn't possible for this target - option is ignored.\n"));
       break;
     case OPTION_DLL_ENABLE_RUNTIME_PSEUDO_RELOC_V2:
       link_info.pei386_runtime_pseudo_reloc = 2;
@@ -1097,22 +1097,6 @@ pep_find_data_imports (void)
 	      asymbol **symbols;
 	      int nsyms, i;
 
-	      if (link_info.pei386_auto_import == -1)
-		{
-		  static bfd_boolean warned = FALSE;
-
-		  info_msg (_("Info: resolving %s by linking to %s (auto-import)\n"),
-			    undef->root.string, buf);
-
-		  /* PR linker/4844.  */
-		  if (! warned)
-		    {
-		      warned = TRUE;
-		      einfo (_("%P: warning: auto-importing has been activated without --enable-auto-import specified on the command line.\n\
-This should work unless it involves constant data structures referencing symbols from auto-imported DLLs.\n"));
-		    }
-		}
-
 	      if (!bfd_generic_link_read_symbols (b))
 		{
 		  einfo (_("%B%F: could not read symbols: %E\n"), b);
@@ -1928,7 +1912,7 @@ sed $sc ldscripts/${EMULATION_NAME}.xbn	
 echo '  ; else if (!config.magic_demand_paged) return'	>> e${EMULATION_NAME}.c
 sed $sc ldscripts/${EMULATION_NAME}.xn			>> e${EMULATION_NAME}.c
 if test -n "$GENERATE_AUTO_IMPORT_SCRIPT" ; then
-echo '  ; else if (link_info.pei386_auto_import == 1) return'	>> e${EMULATION_NAME}.c
+echo '  ; else if (link_info.pei386_auto_import == 1 && link_info.pei386_runtime_pseudo_reloc != 2) return'	>> e${EMULATION_NAME}.c
 sed $sc ldscripts/${EMULATION_NAME}.xa			>> e${EMULATION_NAME}.c
 fi
 echo '  ; else return'					>> e${EMULATION_NAME}.c

Re: Fwd: [patch ld]: Enable autoimport for x64 by default

[Nick Clifton]

Hi Kai,

> 2010-05-19  Kai Tietz
>
>          * pep.em (gld_${EMULATION_NAME}_before_parse): Enable by
>          default auto_import.
>          (gld${EMULATION_NAME}_handle_option): Warn about v1.
>          (pep_find_data_imports): Remove superflous warnings about
>          auto-import.
>          (gld_${EMULATION_NAME}_get_script): Don't merge for auto-import
>          and active pseudo-relocation-v2 rdata into data section.

Approved - please apply.

Cheers
   Nick

Re: Fwd: [patch ld]: Enable autoimport for x64 by default

[Kai Tietz]

2010/5/25 Nick Clifton <nickc@redhat.com>:
> Hi Kai,
>
>> 2010-05-19  Kai Tietz
>>
>>         * pep.em (gld_${EMULATION_NAME}_before_parse): Enable by
>>         default auto_import.
>>         (gld${EMULATION_NAME}_handle_option): Warn about v1.
>>         (pep_find_data_imports): Remove superflous warnings about
>>         auto-import.
>>         (gld_${EMULATION_NAME}_get_script): Don't merge for auto-import
>>         and active pseudo-relocation-v2 rdata into data section.
>
> Approved - please apply.
>
> Cheers
>  Nick
>
>
>

Applied.

Thanks,
Kai

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