* 2006-05-11 change to pe-dll.c breaks Windows DLLs @ 2006-08-17 7:20 Christopher Faylor 2006-08-17 8:09 ` Christopher Faylor 0 siblings, 1 reply; 4+ messages in thread From: Christopher Faylor @ 2006-08-17 7:20 UTC (permalink / raw) To: binutils; +Cc: pedro_alves The change following the "Huh?" below causes problems for x86 Windows DLLs. Index: pe-dll.c =================================================================== RCS file: /cvs/src/src/ld/pe-dll.c,v retrieving revision 1.83 retrieving revision 1.84 diff -u -r1.83 -r1.84 --- pe-dll.c 31 Jan 2006 22:08:14 -0000 1.83 +++ pe-dll.c 11 May 2006 08:48:58 -0000 1.84 @@ -1131,8 +1137,13 @@ int nsyms, symsize; /* If it's not loaded, we don't need to relocate it this way. */ +#if 0 /* Some toolchains can generate .data sections without a SEC_LOAD flag but with relocs. */ if (!(s->output_section->flags & SEC_LOAD)) continue; +#endif + /* Huh ? */ + if (strncmp (bfd_get_section_name (abfd, s), ".idata",6) == 0) + continue; /* I don't know why there would be a reloc for these, but I've seen it happen - DJ */ I don't understand the rationale for the change. How can we skip idata$6 sections? And, as a meta-issue the "#if 0" and /* Huh ? */ comment don't provide a lot of information into what is going on here. Shouldn't there be some more informative comments there? This is the ChangeLog from this change: 2006-05-11 Pedro Alves <pedro_alves...> * pe-dll.c (autofilter_symbollist): Add Dllmain, DllMainCRTStartup, _DllMainCRTStartup and .text. (autofilter_liblist): Add libcegcc. (autofilter_symbolprefixlist): Add __imp_ and .idata$. (generate_reloc): Do not skip sections without a SEC_LOAD flag, they can still contain relocs that need processing. Skip the .idata$6 section. (jmp_arm_bytes): New array: Contains byte codes for an ARM jump. (make_one): Use the new array. (make_import_fixup_entry): Use .idata$2 instead of .idata$3. * emultempl/pe.em (MajorSubsystemVersion): Set to 3 for armpe. Pedro can you explain why this is needed? Should it possibly be conditionalized only for your ARM target? cgf ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: 2006-05-11 change to pe-dll.c breaks Windows DLLs 2006-08-17 7:20 2006-05-11 change to pe-dll.c breaks Windows DLLs Christopher Faylor @ 2006-08-17 8:09 ` Christopher Faylor 2006-08-17 8:15 ` Pedro Alves 0 siblings, 1 reply; 4+ messages in thread From: Christopher Faylor @ 2006-08-17 8:09 UTC (permalink / raw) To: pedro_alves, binutils On Thu, Aug 17, 2006 at 01:00:36AM -0400, Christopher Faylor wrote: >The change following the "Huh?" below causes problems for x86 Windows DLLs. Btw, the problem that I'm seeing is that Windows DLLs produced by ld can no longer be relocated -- which sort of makes sense given this change. cgf >Index: pe-dll.c >=================================================================== >RCS file: /cvs/src/src/ld/pe-dll.c,v >retrieving revision 1.83 >retrieving revision 1.84 >diff -u -r1.83 -r1.84 >--- pe-dll.c 31 Jan 2006 22:08:14 -0000 1.83 >+++ pe-dll.c 11 May 2006 08:48:58 -0000 1.84 >@@ -1131,8 +1137,13 @@ > int nsyms, symsize; > > /* If it's not loaded, we don't need to relocate it this way. */ >+#if 0 /* Some toolchains can generate .data sections without a SEC_LOAD flag but with relocs. */ > if (!(s->output_section->flags & SEC_LOAD)) > continue; >+#endif >+ /* Huh ? */ >+ if (strncmp (bfd_get_section_name (abfd, s), ".idata",6) == 0) >+ continue; > > /* I don't know why there would be a reloc for these, but I've > seen it happen - DJ */ > >I don't understand the rationale for the change. How can we skip idata$6 sections? > >And, as a meta-issue the "#if 0" and /* Huh ? */ comment don't provide >a lot of information into what is going on here. Shouldn't there be >some more informative comments there? > >This is the ChangeLog from this change: > >2006-05-11 Pedro Alves <pedro_alves...> > > * pe-dll.c (autofilter_symbollist): Add Dllmain, > DllMainCRTStartup, _DllMainCRTStartup and .text. > (autofilter_liblist): Add libcegcc. > (autofilter_symbolprefixlist): Add __imp_ and .idata$. > (generate_reloc): Do not skip sections without a SEC_LOAD flag, > they can still contain relocs that need processing. > Skip the .idata$6 section. > (jmp_arm_bytes): New array: Contains byte codes for an ARM jump. > (make_one): Use the new array. > (make_import_fixup_entry): Use .idata$2 instead of .idata$3. > * emultempl/pe.em (MajorSubsystemVersion): Set to 3 for armpe. > >Pedro can you explain why this is needed? Should it possibly be >conditionalized only for your ARM target? > >cgf > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: 2006-05-11 change to pe-dll.c breaks Windows DLLs 2006-08-17 8:09 ` Christopher Faylor @ 2006-08-17 8:15 ` Pedro Alves 2006-08-18 5:12 ` Christopher Faylor 0 siblings, 1 reply; 4+ messages in thread From: Pedro Alves @ 2006-08-17 8:15 UTC (permalink / raw) To: binutils [-- Attachment #1: Type: text/plain, Size: 2698 bytes --] Christopher Faylor wrote: > On Thu, Aug 17, 2006 at 01:00:36AM -0400, Christopher Faylor wrote: >> The change following the "Huh?" below causes problems for x86 Windows DLLs. > > Btw, the problem that I'm seeing is that Windows DLLs produced by ld can > no longer be relocated -- which sort of makes sense given this change. > > Outch! >> >> /* If it's not loaded, we don't need to relocate it this way. */ >> +#if 0 /* Some toolchains can generate .data sections without a SEC_LOAD flag but with relocs. */ >> if (!(s->output_section->flags & SEC_LOAD)) >> continue; >> +#endif >> + /* Huh ? */ >> + if (strncmp (bfd_get_section_name (abfd, s), ".idata",6) == 0) >> + continue; >> >> /* I don't know why there would be a reloc for these, but I've >> seen it happen - DJ */ >> >> I don't understand the rationale for the change. How can we skip idata$6 sections? >> >> And, as a meta-issue the "#if 0" and /* Huh ? */ comment don't provide >> a lot of information into what is going on here. Shouldn't there be >> some more informative comments there? >> >> This is the ChangeLog from this change: >> >> 2006-05-11 Pedro Alves <pedro_alves...> >> >> * pe-dll.c (autofilter_symbollist): Add Dllmain, >> DllMainCRTStartup, _DllMainCRTStartup and .text. >> (autofilter_liblist): Add libcegcc. >> (autofilter_symbolprefixlist): Add __imp_ and .idata$. >> (generate_reloc): Do not skip sections without a SEC_LOAD flag, >> they can still contain relocs that need processing. >> Skip the .idata$6 section. >> (jmp_arm_bytes): New array: Contains byte codes for an ARM jump. >> (make_one): Use the new array. >> (make_import_fixup_entry): Use .idata$2 instead of .idata$3. >> * emultempl/pe.em (MajorSubsystemVersion): Set to 3 for armpe. >> >> Pedro can you explain why this is needed? Should it possibly be >> conditionalized only for your ARM target? >> I can't say I understand it myself. It was part of some pending patches I had that I found somewhere else when I started hacking binutils (as explained on the mail where I posted that patch), for which I don't know the author (I think it was Mamaich). That part has been making me nervous too, and I have it removed in my local tree for a while, in the hope I would see the reason for it. I don't. I am *very* sorry that it caused trouble. Proposed patch attached that reverts the offending parts. Please review and commit. ld/Changelog 2006-08-17 Pedro Alves <pedro_alves@portugalmail.pt> * pe-dll.c (autofilter_symbolprefixlist): Remove .idata$. (generate_reloc): Revert to skipping sections without a SEC_LOAD flag, and to not skipping .idata* sections. Cheers, Pedro Alves [-- Attachment #2: pe-dll.c.diff --] [-- Type: text/plain, Size: 896 bytes --] --- pe-dll.c.prev 2006-08-17 08:03:18.000000000 +0100 +++ pe-dll.c 2006-08-17 08:12:24.000000000 +0100 @@ -296,7 +296,6 @@ static autofilter_entry_type autofilter_ { "__imp_", 6 }, /* Do __imp_ explicitly to save time. */ { "__rtti_", 7 }, - { ".idata$", 7 }, /* Don't re-export auto-imported symbols. */ { "_nm_", 4 }, { "__builtin_", 10 }, @@ -1155,13 +1154,8 @@ generate_reloc (bfd *abfd, struct bfd_li int nsyms, symsize; /* If it's not loaded, we don't need to relocate it this way. */ -#if 0 /* Some toolchains can generate .data sections without a SEC_LOAD flag but with relocs. */ if (!(s->output_section->flags & SEC_LOAD)) continue; -#endif - /* Huh ? */ - if (strncmp (bfd_get_section_name (abfd, s), ".idata",6) == 0) - continue; /* I don't know why there would be a reloc for these, but I've seen it happen - DJ */ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: 2006-05-11 change to pe-dll.c breaks Windows DLLs 2006-08-17 8:15 ` Pedro Alves @ 2006-08-18 5:12 ` Christopher Faylor 0 siblings, 0 replies; 4+ messages in thread From: Christopher Faylor @ 2006-08-18 5:12 UTC (permalink / raw) To: Pedro Alves, binutils On Thu, Aug 17, 2006 at 08:19:52AM +0100, Pedro Alves wrote: >Christopher Faylor wrote: >>On Thu, Aug 17, 2006 at 01:00:36AM -0400, Christopher Faylor wrote: >>>The change following the "Huh?" below causes problems for x86 Windows >>>DLLs. >> >>Btw, the problem that I'm seeing is that Windows DLLs produced by ld can >>no longer be relocated -- which sort of makes sense given this change. >> >>[snip] >>>Pedro can you explain why this is needed? Should it possibly be >>>conditionalized only for your ARM target? >>> > >I can't say I understand it myself. It was part of some pending patches >I had that I found somewhere else when I started hacking binutils (as >explained on the mail where I posted that patch), for which I don't know >the author (I think it was Mamaich). >That part has been making me nervous too, and I have it removed in my >local tree for a while, in the hope I would see the reason for it. >I don't. >I am *very* sorry that it caused trouble. It's really no problem. I'm afraid that I will have to demand a refund, however. :-) >Proposed patch attached that reverts the offending parts. Please >review and commit. > >ld/Changelog > > 2006-08-17 Pedro Alves <pedro_alves@portugalmail.pt> > > * pe-dll.c (autofilter_symbolprefixlist): Remove .idata$. > (generate_reloc): Revert to skipping sections without a > SEC_LOAD flag, and to not skipping .idata* sections. Reviewed and committed. Thanks for the quick response. cgf ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2006-08-17 17:04 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2006-08-17 7:20 2006-05-11 change to pe-dll.c breaks Windows DLLs Christopher Faylor 2006-08-17 8:09 ` Christopher Faylor 2006-08-17 8:15 ` Pedro Alves 2006-08-18 5:12 ` Christopher Faylor
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).