* Fwd: [patch ld]: Enable autoimport for x64 by default [not found] <AANLkTilPUz8aNxQ4dPIi_RMThDK4nZv3tV-XrLOR6jqK@mail.gmail.com> @ 2010-05-16 13:49 ` Kai Tietz 2010-05-16 14:13 ` Dave Korn 0 siblings, 1 reply; 8+ messages in thread From: Kai Tietz @ 2010-05-16 13:49 UTC (permalink / raw) To: Binutils [-- 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fwd: [patch ld]: Enable autoimport for x64 by default 2010-05-16 13:49 ` Fwd: [patch ld]: Enable autoimport for x64 by default Kai Tietz @ 2010-05-16 14:13 ` Dave Korn 2010-05-16 14:25 ` Kai Tietz 0 siblings, 1 reply; 8+ messages in thread From: Dave Korn @ 2010-05-16 14:13 UTC (permalink / raw) To: Kai Tietz; +Cc: Binutils 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fwd: [patch ld]: Enable autoimport for x64 by default 2010-05-16 14:13 ` Dave Korn @ 2010-05-16 14:25 ` Kai Tietz 2010-05-16 15:44 ` Kai Tietz 0 siblings, 1 reply; 8+ messages in thread From: Kai Tietz @ 2010-05-16 14:25 UTC (permalink / raw) To: Dave Korn; +Cc: Binutils 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fwd: [patch ld]: Enable autoimport for x64 by default 2010-05-16 14:25 ` Kai Tietz @ 2010-05-16 15:44 ` Kai Tietz 2010-05-17 15:09 ` Dave Korn 0 siblings, 1 reply; 8+ messages in thread From: Kai Tietz @ 2010-05-16 15:44 UTC (permalink / raw) To: Dave Korn; +Cc: Binutils 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fwd: [patch ld]: Enable autoimport for x64 by default 2010-05-16 15:44 ` Kai Tietz @ 2010-05-17 15:09 ` Dave Korn 2010-05-19 12:49 ` Kai Tietz 0 siblings, 1 reply; 8+ messages in thread From: Dave Korn @ 2010-05-17 15:09 UTC (permalink / raw) To: Kai Tietz; +Cc: Binutils 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fwd: [patch ld]: Enable autoimport for x64 by default 2010-05-17 15:09 ` Dave Korn @ 2010-05-19 12:49 ` Kai Tietz 2010-05-25 9:31 ` Nick Clifton 0 siblings, 1 reply; 8+ messages in thread From: Kai Tietz @ 2010-05-19 12:49 UTC (permalink / raw) To: Dave Korn; +Cc: Binutils [-- 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fwd: [patch ld]: Enable autoimport for x64 by default 2010-05-19 12:49 ` Kai Tietz @ 2010-05-25 9:31 ` Nick Clifton 2010-05-25 10:06 ` Kai Tietz 0 siblings, 1 reply; 8+ messages in thread From: Nick Clifton @ 2010-05-25 9:31 UTC (permalink / raw) To: Kai Tietz; +Cc: Dave Korn, Binutils 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fwd: [patch ld]: Enable autoimport for x64 by default 2010-05-25 9:31 ` Nick Clifton @ 2010-05-25 10:06 ` Kai Tietz 0 siblings, 0 replies; 8+ messages in thread From: Kai Tietz @ 2010-05-25 10:06 UTC (permalink / raw) To: Nick Clifton; +Cc: Dave Korn, Binutils 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-05-25 10:06 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <AANLkTilPUz8aNxQ4dPIi_RMThDK4nZv3tV-XrLOR6jqK@mail.gmail.com> 2010-05-16 13:49 ` Fwd: [patch ld]: Enable autoimport for x64 by default Kai Tietz 2010-05-16 14:13 ` Dave Korn 2010-05-16 14:25 ` Kai Tietz 2010-05-16 15:44 ` Kai Tietz 2010-05-17 15:09 ` Dave Korn 2010-05-19 12:49 ` Kai Tietz 2010-05-25 9:31 ` Nick Clifton 2010-05-25 10:06 ` Kai Tietz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).