* Re: elf.c assign_file_positions_for_segments
@ 2004-09-28 16:15 Michael Chastain
0 siblings, 0 replies; 6+ messages in thread
From: Michael Chastain @ 2004-09-28 16:15 UTC (permalink / raw)
To: amodra, kettenis; +Cc: gdb, binutils
Another data point from my test bed:
I just saw the gcore breakage with gcc -gstabs+ on native
i686-pc-linux-gnu. The problem did not happen with gcc -gdwarf-2.
This was with gdb 2004-09-22 and many different versions of gcc,
gcc 2.95.3 and gcc 3.3.4 and gcc 3.4.2 in particular.
With a current gdb, 2004-09-28, the problem does not happen.
So the fix is all good.
Michael
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: elf.c assign_file_positions_for_segments
2004-09-24 3:09 ` Alan Modra
@ 2004-09-24 6:36 ` Mark Kettenis
0 siblings, 0 replies; 6+ messages in thread
From: Mark Kettenis @ 2004-09-24 6:36 UTC (permalink / raw)
To: amodra; +Cc: binutils, gdb
Date: Fri, 24 Sep 2004 12:39:20 +0930
From: Alan Modra <amodra@bigpond.net.au>
On Fri, Sep 24, 2004 at 09:47:57AM +0930, Alan Modra wrote:
> If we really don't need the SEC_HAS_CONTENTS test, then I'll take it out
> and gdb gcore should be OK, otherwise I might ask you to clear
> SEC_HAS_CONTENTS in gdb.
I think it likely that the SEC_HAS_CONTENTS test is just a hack to work
around another problem. I'm applying the following to go back to the
old behaviour.
Thanks.
You might like to clear SEC_HAS_CONTENTS in gdb too, as it will result
in needless file space being allocated.
I'll investigate this. Can't say I completely understand what's
happening here, but I can always bother Michael Snyder with it, since
he wrote the initial gcore code.
Cheers,
Mark
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: elf.c assign_file_positions_for_segments
2004-09-24 0:18 ` Alan Modra
@ 2004-09-24 3:09 ` Alan Modra
2004-09-24 6:36 ` Mark Kettenis
0 siblings, 1 reply; 6+ messages in thread
From: Alan Modra @ 2004-09-24 3:09 UTC (permalink / raw)
To: Mark Kettenis; +Cc: binutils, gdb
On Fri, Sep 24, 2004 at 09:47:57AM +0930, Alan Modra wrote:
> On Fri, Sep 24, 2004 at 09:12:22AM +0930, Alan Modra wrote:
> > On Thu, Sep 23, 2004 at 05:51:17PM +0200, Mark Kettenis wrote:
> > > I'm not sure where to fix this.
> >
> > I didn't think about gdb using bfd to generate core files. Clearly, I
> > need to fix my breakage of assign_file_positions_for_segments.
>
> While waiting for gdb to build, I took a look at gcore.c. For read-only
> sections, I see you clear SEC_LOAD but leave SEC_HAS_CONTENTS set.
> This is very likely why you're getting filesz non-zero for these
> sections. The new code consistently checks both SEC_LOAD and
> SEC_HAS_CONTENTS whereas the old code just checked SEC_HAS_CONTENTS in
> one place.
>
> I'll take a good look at exactly why the SEC_HAS_CONTENTS check is
> needed, ie. whether the following comment reflects current reality.
> /* We check SEC_HAS_CONTENTS here because if NOLOAD is used in a linker
> script we may have a section with SEC_LOAD clear but which is
> supposed to have contents. */
>
> If we really don't need the SEC_HAS_CONTENTS test, then I'll take it out
> and gdb gcore should be OK, otherwise I might ask you to clear
> SEC_HAS_CONTENTS in gdb.
I think it likely that the SEC_HAS_CONTENTS test is just a hack to work
around another problem. I'm applying the following to go back to the
old behaviour.
You might like to clear SEC_HAS_CONTENTS in gdb too, as it will result
in needless file space being allocated.
* elf.c (IS_LOADED): Delete.
(assign_file_positions_for_segments): Just test SEC_LOAD instead.
Restore SEC_HAS_CONTENTS test to the one place it was used prior
to 2004-09-22.
Index: bfd/elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.246
diff -u -p -r1.246 elf.c
--- bfd/elf.c 22 Sep 2004 06:45:38 -0000 1.246
+++ bfd/elf.c 24 Sep 2004 02:59:52 -0000
@@ -3787,12 +3787,6 @@ vma_page_aligned_bias (bfd_vma vma, ufil
return ((vma - off) % maxpagesize);
}
-/* We check SEC_HAS_CONTENTS here because if NOLOAD is used in a linker
- script we may have a section with SEC_LOAD clear but which is
- supposed to have contents. */
-#define IS_LOADED(FLAGS) \
- (((FLAGS) & SEC_LOAD) != 0 || ((FLAGS) & SEC_HAS_CONTENTS) != 0)
-
/* Assign file positions to the sections based on the mapping from
sections to segments. This function also sets up some fields in
the file header, and writes out the program headers. */
@@ -3959,7 +3953,7 @@ assign_file_positions_for_segments (bfd
.tbss, we need to look at the next section to decide
whether the segment has any loadable sections. */
i = 0;
- while (!IS_LOADED (m->sections[i]->flags))
+ while ((m->sections[i]->flags & SEC_LOAD) == 0)
{
if ((m->sections[i]->flags & SEC_THREAD_LOCAL) == 0
|| ++i >= m->count)
@@ -4107,7 +4101,7 @@ assign_file_positions_for_segments (bfd
{
bfd_signed_vma adjust;
- if (IS_LOADED (flags))
+ if ((flags & SEC_LOAD) != 0)
{
adjust = sec->lma - (p->p_paddr + p->p_filesz);
if (adjust < 0)
@@ -4164,11 +4158,26 @@ assign_file_positions_for_segments (bfd
if (p->p_type == PT_LOAD)
{
sec->filepos = off;
- if (IS_LOADED (flags))
+ /* FIXME: The SEC_HAS_CONTENTS test here dates back to
+ 1997, and the exact reason for it isn't clear. One
+ plausible explanation is that it is to work around
+ a problem we have with linker scripts using data
+ statements in NOLOAD sections. I don't think it
+ makes a great deal of sense to have such a section
+ assigned to a PT_LOAD segment, but apparently
+ people do this. The data statement results in a
+ bfd_data_link_order being built, and these need
+ section contents to write into. Eventually, we get
+ to _bfd_elf_write_object_contents which writes any
+ section with contents to the output. Make room
+ here for the write, so that following segments are
+ not trashed. */
+ if ((flags & SEC_LOAD) != 0
+ || (flags & SEC_HAS_CONTENTS) != 0)
off += sec->size;
}
- if (IS_LOADED (flags))
+ if ((flags & SEC_LOAD) != 0)
{
p->p_filesz += sec->size;
p->p_memsz += sec->size;
--
Alan Modra
IBM OzLabs - Linux Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: elf.c assign_file_positions_for_segments
2004-09-23 23:42 ` Alan Modra
@ 2004-09-24 0:18 ` Alan Modra
2004-09-24 3:09 ` Alan Modra
0 siblings, 1 reply; 6+ messages in thread
From: Alan Modra @ 2004-09-24 0:18 UTC (permalink / raw)
To: Mark Kettenis; +Cc: bintuils, gdb
On Fri, Sep 24, 2004 at 09:12:22AM +0930, Alan Modra wrote:
> On Thu, Sep 23, 2004 at 05:51:17PM +0200, Mark Kettenis wrote:
> > I'm not sure where to fix this.
>
> I didn't think about gdb using bfd to generate core files. Clearly, I
> need to fix my breakage of assign_file_positions_for_segments.
While waiting for gdb to build, I took a look at gcore.c. For read-only
sections, I see you clear SEC_LOAD but leave SEC_HAS_CONTENTS set.
This is very likely why you're getting filesz non-zero for these
sections. The new code consistently checks both SEC_LOAD and
SEC_HAS_CONTENTS whereas the old code just checked SEC_HAS_CONTENTS in
one place.
I'll take a good look at exactly why the SEC_HAS_CONTENTS check is
needed, ie. whether the following comment reflects current reality.
/* We check SEC_HAS_CONTENTS here because if NOLOAD is used in a linker
script we may have a section with SEC_LOAD clear but which is
supposed to have contents. */
If we really don't need the SEC_HAS_CONTENTS test, then I'll take it out
and gdb gcore should be OK, otherwise I might ask you to clear
SEC_HAS_CONTENTS in gdb.
--
Alan Modra
IBM OzLabs - Linux Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: elf.c assign_file_positions_for_segments
2004-09-23 15:51 Mark Kettenis
@ 2004-09-23 23:42 ` Alan Modra
2004-09-24 0:18 ` Alan Modra
0 siblings, 1 reply; 6+ messages in thread
From: Alan Modra @ 2004-09-23 23:42 UTC (permalink / raw)
To: Mark Kettenis; +Cc: bintuils, gdb
On Thu, Sep 23, 2004 at 05:51:17PM +0200, Mark Kettenis wrote:
> I'm not sure where to fix this.
I didn't think about gdb using bfd to generate core files. Clearly, I
need to fix my breakage of assign_file_positions_for_segments.
--
Alan Modra
IBM OzLabs - Linux Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: elf.c assign_file_positions_for_segments
@ 2004-09-23 15:51 Mark Kettenis
2004-09-23 23:42 ` Alan Modra
0 siblings, 1 reply; 6+ messages in thread
From: Mark Kettenis @ 2004-09-23 15:51 UTC (permalink / raw)
To: amodra; +Cc: bintuils, gdb
Hi Alan,
The following patch:
2004-09-22 Alan Modra <amodra@bigpond.net.au>
* elf.c (IS_LOADED): Define.
(assign_file_positions_for_segments): Don't round up file offset of
PT_LOAD segments containing no SEC_LOAD sections, instead round down.
Delete code handling link script adjustment of lma. Do the adjust
in later code handling similar ajustments. Remove dead code error
check. Warn if section lma would require a negative offset
adjustment. Tweak lma adjustment to use p_filesz rather than p_memsz.
Use p_vaddr + p_memsz inside section loop in place of voff. Don't
update voff in section loop. Change voff in segment loop to be an
adjustment on top of "off". Set sec->filepos and update "off" later.
Test for loadable sections consistently using IS_LOADED. Similarly,
test for alloc-only sections other than .tbss consistently.
Don't bother checking SEC_ALLOC in PT_LOAD segments. Remove FIXME.
Tidy PT_NOTE handling. Use %B and %A in error messages.
(assign_file_positions_except_relocs): Use %B in error message.
breaks the GDB gcore command on sparc-sun-solaris2.9, and probably on
other systems too. Here's the output of readelf -l without:
---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---
Elf file type is CORE (Core file)
Entry point 0x0
There are 15 program headers, starting at offset 52
Program Headers:
Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
NOTE 0x000214 0x00000000 0x00000000 0x00578 0x00000 R 0x1
LOAD 0x00078c 0x00010000 0x00000000 0x00000 0x02000 R E 0x1
LOAD 0x00278c 0x00020000 0x00000000 0x02000 0x02000 RWE 0x1
LOAD 0x00478c 0x00022000 0x00000000 0x02000 0x02000 RWE 0x1
LOAD 0x00678c 0xff280000 0x00000000 0x00000 0xaa000 R E 0x1
LOAD 0x0b078c 0xff33a000 0x00000000 0x06000 0x06000 RWE 0x1
LOAD 0x0b678c 0xff340000 0x00000000 0x02000 0x02000 RWE 0x1
LOAD 0x0b878c 0xff350000 0x00000000 0x00000 0x04000 R E 0x1
LOAD 0x0bc78c 0xff370000 0x00000000 0x00000 0x18000 R E 0x1
LOAD 0x0d478c 0xff396000 0x00000000 0x02000 0x02000 RWE 0x1
LOAD 0x0d678c 0xff3a0000 0x00000000 0x00000 0x02000 R E 0x1
LOAD 0x0d878c 0xff3b0000 0x00000000 0x02000 0x02000 RWE 0x1
LOAD 0x0da78c 0xff3c0000 0x00000000 0x26000 0x26000 R E 0x1
LOAD 0x10078c 0xff3f6000 0x00000000 0x02000 0x02000 RWE 0x1
LOAD 0x10278c 0xffbfe000 0x00000000 0x02000 0x02000 RWE 0x1
---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---
and with your patch:
---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---
Elf file type is CORE (Core file)
Entry point 0x0
There are 15 program headers, starting at offset 52
Program Headers:
Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
NOTE 0x000214 0x00000000 0x00000000 0x00578 0x00000 R 0x1
LOAD 0x00078c 0x00010000 0x00000000 0x02000 0x02000 R E 0x1
LOAD 0x00278c 0x00020000 0x00000000 0x02000 0x02000 RWE 0x1
LOAD 0x00478c 0x00022000 0x00000000 0x02000 0x02000 RWE 0x1
LOAD 0x00678c 0xff280000 0x00000000 0xaa000 0xaa000 R E 0x1
LOAD 0x0b078c 0xff33a000 0x00000000 0x06000 0x06000 RWE 0x1
LOAD 0x0b678c 0xff340000 0x00000000 0x02000 0x02000 RWE 0x1
LOAD 0x0b878c 0xff350000 0x00000000 0x04000 0x04000 R E 0x1
LOAD 0x0bc78c 0xff370000 0x00000000 0x18000 0x18000 R E 0x1
LOAD 0x0d478c 0xff396000 0x00000000 0x02000 0x02000 RWE 0x1
LOAD 0x0d678c 0xff3a0000 0x00000000 0x02000 0x02000 R E 0x1
LOAD 0x0d878c 0xff3b0000 0x00000000 0x02000 0x02000 RWE 0x1
LOAD 0x0da78c 0xff3c0000 0x00000000 0x26000 0x26000 R E 0x1
LOAD 0x10078c 0xff3f6000 0x00000000 0x02000 0x02000 RWE 0x1
LOAD 0x10278c 0xffbfe000 0x00000000 0x02000 0x02000 RWE 0x1
---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---
Look at the first LOAD header. Before it had a FileSiz of 0,
afterwards it's 0x02000. Note that this is a read-only section. The
contents of the section in the core file is all zeroes. Presumably
the fact that its FileSiz is 0 causes GDB to read the contents from
the executable instead. However, with FileSiz 0x02000, the contents
get overwritten with all zeroes. GDB's disas command shows rather
dull output in that case.
I'm not sure where to fix this. What we'd like to avoid in GDB is
actually read the contents of read-only sections from memory when
we're synthesizing the core file.
Mark
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-09-28 16:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-09-28 16:15 elf.c assign_file_positions_for_segments Michael Chastain
-- strict thread matches above, loose matches on Subject: below --
2004-09-23 15:51 Mark Kettenis
2004-09-23 23:42 ` Alan Modra
2004-09-24 0:18 ` Alan Modra
2004-09-24 3:09 ` Alan Modra
2004-09-24 6:36 ` Mark Kettenis
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).