* 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).