* Bug fix for gldelf_i386_place_orphan in elf32.em [version 2.20.1]
@ 2011-01-12 7:01 Sheng, Yongjie
2011-01-12 7:38 ` Alan Modra
0 siblings, 1 reply; 6+ messages in thread
From: Sheng, Yongjie @ 2011-01-12 7:01 UTC (permalink / raw)
To: binutils
Dear Binutils maintainer,
I caught a bug in function gldelf_i386_place_orphan, and a patch is attached at below.
My point on the patch:
for (; lookup != NULL; lookup = lookup->next)
if ((lookup->bfd_section != NULL
&& (lookup->bfd_section->flags & SEC_DEBUGGING) != 0)
|| strcmp (lookup->name, ".comment") == 0)
break;
[Yongjie] The above for loop tries to find a ".comment" or a debug section.
hold[orphan_nonalloc].os = lookup ? lookup->prev : NULL;
hold[orphan_nonalloc].name = ".comment";
[Yongjie] The above code set the hold[orphan_nonalloc] name as ".comment", but os as lookup->prev. It should be lookup itself, right?
Please help to evaluate the patch. Thanks.
Below is the patch
--- elf32.orig.em 2011-01-12 18:16:15.767198643 +0800
+++ elf32.em 2011-01-12 18:16:56.556586465 +0800
@@ -1772,7 +1772,7 @@
&& (lookup->bfd_section->flags & SEC_DEBUGGING) != 0)
|| strcmp (lookup->name, ".comment") == 0)
break;
- hold[orphan_nonalloc].os = lookup ? lookup->prev : NULL;
+ hold[orphan_nonalloc].os = lookup; /* ? lookup->prev : NULL;*/
hold[orphan_nonalloc].name = ".comment";
orphan_init_done = 1;
}
Yours sincerely,
Sheng, Yongjie (Sam)
Digital Home Group
Intel Asia-Pacific Research & Development Ltd.
Telephone: +86-21-61166516/61167853
i-net: 8821-6516/8821-7853
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug fix for gldelf_i386_place_orphan in elf32.em [version 2.20.1]
2011-01-12 7:01 Bug fix for gldelf_i386_place_orphan in elf32.em [version 2.20.1] Sheng, Yongjie
@ 2011-01-12 7:38 ` Alan Modra
2011-01-12 7:51 ` Sheng, Yongjie
0 siblings, 1 reply; 6+ messages in thread
From: Alan Modra @ 2011-01-12 7:38 UTC (permalink / raw)
To: Sheng, Yongjie; +Cc: binutils
On Wed, Jan 12, 2011 at 03:01:15PM +0800, Sheng, Yongjie wrote:
> The above code set the hold[orphan_nonalloc] name as ".comment", but os as lookup->prev. It should be lookup itself, right?
No. The idea is to place orphan non-alloc sections before debug
sections, or when there are no debug sections, before .comment.
The setting of .name is really just to ensure we don't call
output_rel_find later in case no debug or .comment sections exist.
(".comment" is a good choice because we know that won't be found by
lang_output_section_find either.)
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: Bug fix for gldelf_i386_place_orphan in elf32.em [version 2.20.1]
2011-01-12 7:38 ` Alan Modra
@ 2011-01-12 7:51 ` Sheng, Yongjie
2011-01-12 13:40 ` Alan Modra
0 siblings, 1 reply; 6+ messages in thread
From: Sheng, Yongjie @ 2011-01-12 7:51 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils
[-- Attachment #1: Type: text/plain, Size: 1256 bytes --]
Alan,
Thanks for your quick response.
Please see my small linker-test.tgz test package.
With "-gstabs" compiling option, a hello world (binutils-2.20.1) links with a mangled-section.o(two section names are mangled, being orphan sections).
It will crash in running (in ld-linux.so.2).
It will not crash if built with binutils-2.17.50 (FC8).
Yours sincerely,
Sheng, Yongjie (Sam)
-----Original Message-----
From: Alan Modra [mailto:amodra@gmail.com]
Sent: Wednesday, January 12, 2011 3:39 PM
To: Sheng, Yongjie
Cc: binutils@sources.redhat.com
Subject: Re: Bug fix for gldelf_i386_place_orphan in elf32.em [version 2.20.1]
On Wed, Jan 12, 2011 at 03:01:15PM +0800, Sheng, Yongjie wrote:
> The above code set the hold[orphan_nonalloc] name as ".comment", but os as lookup->prev. It should be lookup itself, right?
No. The idea is to place orphan non-alloc sections before debug
sections, or when there are no debug sections, before .comment.
The setting of .name is really just to ensure we don't call
output_rel_find later in case no debug or .comment sections exist.
(".comment" is a good choice because we know that won't be found by
lang_output_section_find either.)
--
Alan Modra
Australia Development Lab, IBM
[-- Attachment #2: linker-test.tgz --]
[-- Type: application/x-compressed, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug fix for gldelf_i386_place_orphan in elf32.em [version 2.20.1]
2011-01-12 7:51 ` Sheng, Yongjie
@ 2011-01-12 13:40 ` Alan Modra
2011-01-12 15:21 ` Sheng, Yongjie
2011-01-14 2:28 ` Alan Modra
0 siblings, 2 replies; 6+ messages in thread
From: Alan Modra @ 2011-01-12 13:40 UTC (permalink / raw)
To: Sheng, Yongjie; +Cc: binutils
On Wed, Jan 12, 2011 at 03:48:50PM +0800, Sheng, Yongjie wrote:
> Please see my small linker-test.tgz test package.
> With "-gstabs" compiling option, a hello world (binutils-2.20.1) links with a mangled-section.o(two section names are mangled, being orphan sections).
> It will crash in running (in ld-linux.so.2).
> It will not crash if built with binutils-2.17.50 (FC8).
Thanks. That nicely illuminates the bug, which is that using
lookup->prev can result in hold[orphan_nonalloc].os equal to
hold[orphan_bss].os. If that happens, then orphan bss sections and
orphan non-alloc sections might be intermingled.
I'll test the following patch overnight, to find the inevitable
testsuite failures this will cause..
Index: ld/emultempl/elf32.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/elf32.em,v
retrieving revision 1.216
diff -u -p -r1.216 elf32.em
--- ld/emultempl/elf32.em 20 Dec 2010 13:00:14 -0000 1.216
+++ ld/emultempl/elf32.em 12 Jan 2011 13:30:55 -0000
@@ -1788,7 +1788,7 @@ gld${EMULATION_NAME}_place_orphan (asect
{ ".sdata",
SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD | SEC_DATA | SEC_SMALL_DATA,
0, 0, 0, 0 },
- { 0,
+ { ".comment",
SEC_HAS_CONTENTS,
0, 0, 0, 0 },
};
@@ -1880,7 +1880,6 @@ gld${EMULATION_NAME}_place_orphan (asect
if (!orphan_init_done)
{
- lang_output_section_statement_type *lookup;
struct orphan_save *ho;
for (ho = hold; ho < hold + sizeof (hold) / sizeof (hold[0]); ++ho)
@@ -1890,16 +1889,6 @@ gld${EMULATION_NAME}_place_orphan (asect
if (ho->os != NULL && ho->os->flags == 0)
ho->os->flags = ho->flags;
}
- lookup = hold[orphan_bss].os;
- if (lookup == NULL)
- lookup = &lang_output_section_statement.head->output_section_statement;
- for (; lookup != NULL; lookup = lookup->next)
- if ((lookup->bfd_section != NULL
- && (lookup->bfd_section->flags & SEC_DEBUGGING) != 0)
- || strcmp (lookup->name, ".comment") == 0)
- break;
- hold[orphan_nonalloc].os = lookup ? lookup->prev : NULL;
- hold[orphan_nonalloc].name = ".comment";
orphan_init_done = 1;
}
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: Bug fix for gldelf_i386_place_orphan in elf32.em [version 2.20.1]
2011-01-12 13:40 ` Alan Modra
@ 2011-01-12 15:21 ` Sheng, Yongjie
2011-01-14 2:28 ` Alan Modra
1 sibling, 0 replies; 6+ messages in thread
From: Sheng, Yongjie @ 2011-01-12 15:21 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils
Alan,
Thanks.
That's right, the orphan bss sections and orphan non-alloc sections are intermingled, and causes orphan bss section cannot get a correct virtual address.
Yours sincerely,
Sheng, Yongjie (Sam)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug fix for gldelf_i386_place_orphan in elf32.em [version 2.20.1]
2011-01-12 13:40 ` Alan Modra
2011-01-12 15:21 ` Sheng, Yongjie
@ 2011-01-14 2:28 ` Alan Modra
1 sibling, 0 replies; 6+ messages in thread
From: Alan Modra @ 2011-01-14 2:28 UTC (permalink / raw)
To: Sheng, Yongjie, binutils
On Thu, Jan 13, 2011 at 12:09:45AM +1030, Alan Modra wrote:
> Thanks. That nicely illuminates the bug, which is that using
> lookup->prev can result in hold[orphan_nonalloc].os equal to
> hold[orphan_bss].os. If that happens, then orphan bss sections and
> orphan non-alloc sections might be intermingled.
>
> I'll test the following patch overnight, to find the inevitable
> testsuite failures this will cause..
Applied with this changelog. The testsuite doesn't appear to care
about ordering of non-alloc sections.
* emultempl/elf32.em (gld${EMULATION_NAME}_place_orphan): Don't
attempt to put non-alloc orphans before debug sections, just place
them after .comment.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-01-14 2:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-12 7:01 Bug fix for gldelf_i386_place_orphan in elf32.em [version 2.20.1] Sheng, Yongjie
2011-01-12 7:38 ` Alan Modra
2011-01-12 7:51 ` Sheng, Yongjie
2011-01-12 13:40 ` Alan Modra
2011-01-12 15:21 ` Sheng, Yongjie
2011-01-14 2:28 ` Alan Modra
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).