public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [patch] bfd: Core files with p_filesz < p_memsz (build-id)
@ 2007-07-28 20:16 Jan Kratochvil
  2007-07-29 12:30 ` Jan Kratochvil
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Jan Kratochvil @ 2007-07-28 20:16 UTC (permalink / raw)
  To: binutils; +Cc: Roland McGrath

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: multipart/mixed; boundary="jI8keyz6grp/JLjh", Size: 28 bytes --]

<<< No Message Collected >>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [patch] bfd: Core files with p_filesz < p_memsz (build-id)
  2007-07-28 20:16 [patch] bfd: Core files with p_filesz < p_memsz (build-id) Jan Kratochvil
@ 2007-07-29 12:30 ` Jan Kratochvil
  2007-07-29 15:29 ` Roland McGrath
  2007-07-30 14:07 ` [resent] " Jan Kratochvil
  2 siblings, 0 replies; 12+ messages in thread
From: Jan Kratochvil @ 2007-07-29 12:30 UTC (permalink / raw)
  To: binutils; +Cc: Roland McGrath

[-- Attachment #1: Type: text/plain, Size: 2217 bytes --]

Hi,

there is now a pending patch for Linux kernels producing core files with the
first page of the ELF file for the build-id note identification.

So far BFD handled either p_filesz == 0 or p_filesz == p_memsz.  Patch handles
the case 0 < p_filesz < p_memsz (p_filesz == PAGE_SIZE for the build-id case).

`0 < p_filesz < p_memsz' meaning was so far undefined for the ET_CORE files.


Regards,
Jan


New build-id enhanced Linux kernels produce core files:
Program Headers:
  Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
  NOTE           0x000660 0x0000000000000000 0x0000000000000000 0x000324 0x000000     0x0
  LOAD           0x001000 0x0000000000400000 0x0000000000000000 0x001000 0x0b1000 R E 0x1000
                                                                ^^^^^^^^ ^^^^^^^^
  LOAD           0x002000 0x00000000006b1000 0x0000000000000000 0x00a000 0x00a000 RW  0x1000

	BFD-patched gdb `info files'
		0x0000000000400000 - 0x0000000000401000 is load1
		0x00000000006b1000 - 0x00000000006bb000 is load2
	0x0000000000419f00 <main+0>:    push   %r15
	0x0000000000419f02 <main+2>:    push   %r14

	BROKEN: original gdb `info files' (it sees the code sections zeroed)
		0x0000000000400000 - 0x0000000000401000 is load1a
		0x0000000000401000 - 0x00000000004b1000 is load1b
		^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
		0x00000000006b1000 - 0x00000000006bb000 is load2
	0x0000000000419f00 <main+0>:    add    %al,(%rax)
	0x0000000000419f02 <main+2>:    add    %al,(%rax)
	(as 0x419f00 >= 0x401000)

Legacy kernel core files:
Program Headers:
  Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
  NOTE           0x000660 0x0000000000000000 0x0000000000000000 0x000324 0x000000     0x0
  LOAD           0x001000 0x0000000000400000 0x0000000000000000 0x000000 0x0b1000 R E 0x1000
                                                                ^^^^^^^^ ^^^^^^^^
  LOAD           0x001000 0x00000000006b1000 0x0000000000000000 0x00a000 0x00a000 RW  0x1000

	original gdb `info files'
		0x00000000006b1000 - 0x00000000006bb000 is load2
	0x0000000000419f00 <main+0>:    push   %r15
	0x0000000000419f02 <main+2>:    push   %r14

[-- Attachment #2: bfd-dump_elf_headers.patch --]
[-- Type: text/plain, Size: 1055 bytes --]

2007-07-28  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* bfd/elf.c (_bfd_elf_new_section_hook): New comment for ET_CORE files
	with p_filesz shorter than p_memsz.  The data/bss split case has been
	restricted only for non-ET_CORE files.

--- bfd/elf.c	26 Jul 2007 18:15:46 -0000	1.401
+++ bfd/elf.c	28 Jul 2007 19:00:08 -0000
@@ -2225,6 +2225,9 @@ _bfd_elf_new_section_hook (bfd *abfd, as
    by the difference between the two sizes.  In effect, the segment is split
    into it's initialized and uninitialized parts.
 
+   This notion does not apply in ET_CORE files, where a shorter p_filesz means
+   that the data is not available in the dump.
+
  */
 
 bfd_boolean
@@ -2239,8 +2242,8 @@ _bfd_elf_make_section_from_phdr (bfd *ab
   size_t len;
   int split;
 
-  split = ((hdr->p_memsz > 0)
-	    && (hdr->p_filesz > 0)
+  split = (abfd->format != bfd_core
+	    && (hdr->p_memsz > 0) && (hdr->p_filesz > 0)
 	    && (hdr->p_memsz > hdr->p_filesz));
   sprintf (namebuf, "%s%d%s", typename, index, split ? "a" : "");
   len = strlen (namebuf) + 1;

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] bfd: Core files with p_filesz < p_memsz (build-id)
  2007-07-28 20:16 [patch] bfd: Core files with p_filesz < p_memsz (build-id) Jan Kratochvil
  2007-07-29 12:30 ` Jan Kratochvil
@ 2007-07-29 15:29 ` Roland McGrath
  2007-07-29 18:32   ` Jan Kratochvil
  2007-07-30 14:07 ` [resent] " Jan Kratochvil
  2 siblings, 1 reply; 12+ messages in thread
From: Roland McGrath @ 2007-07-29 15:29 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: binutils

Are you sure that changing _bfd_elf_make_section_from_phdr is the right way
to fix gdb?

It is in a certain sense accurate to split the one segment into two
sections, a leading SEC_LOAD one and a trailing one without SEC_LOAD.

What does your change do to e.g. objcopy on a core file?


Thanks,
Roland

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] bfd: Core files with p_filesz < p_memsz (build-id)
  2007-07-29 15:29 ` Roland McGrath
@ 2007-07-29 18:32   ` Jan Kratochvil
  2007-08-01 13:05     ` Alan Modra
  0 siblings, 1 reply; 12+ messages in thread
From: Jan Kratochvil @ 2007-07-29 18:32 UTC (permalink / raw)
  To: Roland McGrath; +Cc: binutils

[-- Attachment #1: Type: text/plain, Size: 2072 bytes --]

On Sat, 28 Jul 2007 22:16:04 +0200, Roland McGrath wrote:
> Are you sure that changing _bfd_elf_make_section_from_phdr is the right way
> to fix gdb?
> 
> It is in a certain sense accurate to split the one segment into two
> sections, a leading SEC_LOAD one and a trailing one without SEC_LOAD.

The core files with `p_filesz == 0' were working before.
_bfd_elf_make_section_from_phdr() can create the sections as two parts, with
the second problematic one emulating the former `p_filesz == 0' core segments.
No new memory structure layout was introduced.  [attached]

Another issue would be changing the sections layout as currently the original
p_memsz is still lost.  Going to post later an add-on incompatible change using
SEC_NEVER_LOAD so that the p_memsz is retained for the bfd library applications
+ across objcopy.


> What does your change do to e.g. objcopy on a core file?

(the previous version was stripping the `load1b' part), thanks for finding it.



Best Regards,
Jan


Patched bfd version with the new `p_filesz == 4096' kernel core file:
	CVS HEAD gdb `info files':
		0x0000000000400000 - 0x0000000000401000 is load1a
		0x0000000000401000 - 0x0000000000401000 is load1b
		0x00000000006b1000 - 0x00000000006bb000 is load2
	objcopy:
	Section Headers:
	[Nr] Name                 Type         Addr             Off      Size     ES Flags Lk Inf Al
	[ 5] load1a               PROGBITS     0000000000400000 00001000 00001000  0 AX     0   0 4096
	[ 6] load1b               PROGBITS     0000000000401000 00002000 00000000  0 AX     0   0 4096
	[ 7] load2                PROGBITS     00000000006b1000 00002000 0000a000  0 WA     0   0 4096

Former bfd version with the former `p_filesz == 0' kernel core file:
	CVS HEAD gdb `info files':
		0x0000000000400000 - 0x0000000000400000 is load1
		0x00000000006b1000 - 0x00000000006bb000 is load2
	objcopy:
	Section Headers:
	[ 5] load1                PROGBITS     0000000000400000 00001000 00000000  0 AX     0   0 4096
	[ 6] load2                PROGBITS     00000000006b1000 00001000 0000a000  0 WA     0   0 4096

[-- Attachment #2: bfd-dump_elf_headers.patch --]
[-- Type: text/plain, Size: 1395 bytes --]

2007-07-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* bfd/elf.c (_bfd_elf_new_section_hook): New comment for ET_CORE files
	with p_filesz shorter than p_memsz.  Behave for such split sections in
	a backward compatible way for both its parts.

--- bfd/elf.c	26 Jul 2007 18:15:46 -0000	1.401
+++ bfd/elf.c	29 Jul 2007 13:46:21 -0000
@@ -2225,6 +2225,9 @@ _bfd_elf_new_section_hook (bfd *abfd, as
    by the difference between the two sizes.  In effect, the segment is split
    into it's initialized and uninitialized parts.
 
+   This notion does not apply in ET_CORE files, where a shorter p_filesz means
+   that the data is not available in the dump.
+
  */
 
 bfd_boolean
@@ -2286,10 +2289,20 @@ _bfd_elf_make_section_from_phdr (bfd *ab
 	return FALSE;
       newsect->vma = hdr->p_vaddr + hdr->p_filesz;
       newsect->lma = hdr->p_paddr + hdr->p_filesz;
-      newsect->size = hdr->p_memsz - hdr->p_filesz;
+      if (abfd->format != bfd_core)
+	newsect->size = hdr->p_memsz - hdr->p_filesz;
+      else
+        {
+	  newsect->size = 0;
+	  newsect->filepos = hdr->p_offset;
+	  newsect->flags |= SEC_HAS_CONTENTS;
+	  newsect->alignment_power = bfd_log2 (hdr->p_align);
+	}
       if (hdr->p_type == PT_LOAD)
 	{
 	  newsect->flags |= SEC_ALLOC;
+	  if (abfd->format == bfd_core)
+	    newsect->flags |= SEC_LOAD;
 	  if (hdr->p_flags & PF_X)
 	    newsect->flags |= SEC_CODE;
 	}

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [resent] [patch] bfd: Core files with p_filesz < p_memsz (build-id)
  2007-07-28 20:16 [patch] bfd: Core files with p_filesz < p_memsz (build-id) Jan Kratochvil
  2007-07-29 12:30 ` Jan Kratochvil
  2007-07-29 15:29 ` Roland McGrath
@ 2007-07-30 14:07 ` Jan Kratochvil
  2 siblings, 0 replies; 12+ messages in thread
From: Jan Kratochvil @ 2007-07-30 14:07 UTC (permalink / raw)
  To: binutils; +Cc: Roland McGrath

[-- Attachment #1: Type: text/plain, Size: 2569 bytes --]

The original mail got corrupted, resent here (thanks for the notice, Daniel).
The patch here is broken - the followup mails are:
	http://sourceware.org/ml/binutils/2007-07/msg00516.html
with the corrected patch at:
	http://sourceware.org/ml/binutils/2007-07/msg00520.html
------------------------------------------------------------------------------

Hi,

there is now a pending patch for Linux kernels producing core files with the
first page of the ELF file for the build-id note identification.

So far BFD handled either p_filesz == 0 or p_filesz == p_memsz.  Patch handles
the case 0 < p_filesz < p_memsz (p_filesz == PAGE_SIZE for the build-id case).

`0 < p_filesz < p_memsz' meaning was so far undefined for the ET_CORE files.


Regards,
Jan


New build-id enhanced Linux kernels produce core files:
Program Headers:
  Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
  NOTE           0x000660 0x0000000000000000 0x0000000000000000 0x000324 0x000000     0x0
  LOAD           0x001000 0x0000000000400000 0x0000000000000000 0x001000 0x0b1000 R E 0x1000
                                                                ^^^^^^^^ ^^^^^^^^
  LOAD           0x002000 0x00000000006b1000 0x0000000000000000 0x00a000 0x00a000 RW  0x1000

	BFD-patched gdb `info files'
		0x0000000000400000 - 0x0000000000401000 is load1
		0x00000000006b1000 - 0x00000000006bb000 is load2
	0x0000000000419f00 <main+0>:    push   %r15
	0x0000000000419f02 <main+2>:    push   %r14

	BROKEN: original gdb `info files' (it sees the code sections zeroed)
		0x0000000000400000 - 0x0000000000401000 is load1a
		0x0000000000401000 - 0x00000000004b1000 is load1b
		^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
		0x00000000006b1000 - 0x00000000006bb000 is load2
	0x0000000000419f00 <main+0>:    add    %al,(%rax)
	0x0000000000419f02 <main+2>:    add    %al,(%rax)
	(as 0x419f00 >= 0x401000)

Legacy kernel core files:
Program Headers:
  Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
  NOTE           0x000660 0x0000000000000000 0x0000000000000000 0x000324 0x000000     0x0
  LOAD           0x001000 0x0000000000400000 0x0000000000000000 0x000000 0x0b1000 R E 0x1000
                                                                ^^^^^^^^ ^^^^^^^^
  LOAD           0x001000 0x00000000006b1000 0x0000000000000000 0x00a000 0x00a000 RW  0x1000

	original gdb `info files'
		0x00000000006b1000 - 0x00000000006bb000 is load2
	0x0000000000419f00 <main+0>:    push   %r15
	0x0000000000419f02 <main+2>:    push   %r14

[-- Attachment #2: bfd-dump_elf_headers0.patch --]
[-- Type: text/plain, Size: 1055 bytes --]

2007-07-28  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* bfd/elf.c (_bfd_elf_new_section_hook): New comment for ET_CORE files
	with p_filesz shorter than p_memsz.  The data/bss split case has been
	restricted only for non-ET_CORE files.

--- bfd/elf.c	26 Jul 2007 18:15:46 -0000	1.401
+++ bfd/elf.c	28 Jul 2007 19:00:08 -0000
@@ -2225,6 +2225,9 @@ _bfd_elf_new_section_hook (bfd *abfd, as
    by the difference between the two sizes.  In effect, the segment is split
    into it's initialized and uninitialized parts.
 
+   This notion does not apply in ET_CORE files, where a shorter p_filesz means
+   that the data is not available in the dump.
+
  */
 
 bfd_boolean
@@ -2239,8 +2242,8 @@ _bfd_elf_make_section_from_phdr (bfd *ab
   size_t len;
   int split;
 
-  split = ((hdr->p_memsz > 0)
-	    && (hdr->p_filesz > 0)
+  split = (abfd->format != bfd_core
+	    && (hdr->p_memsz > 0) && (hdr->p_filesz > 0)
 	    && (hdr->p_memsz > hdr->p_filesz));
   sprintf (namebuf, "%s%d%s", typename, index, split ? "a" : "");
   len = strlen (namebuf) + 1;

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] bfd: Core files with p_filesz < p_memsz (build-id)
  2007-07-29 18:32   ` Jan Kratochvil
@ 2007-08-01 13:05     ` Alan Modra
  2007-08-02 20:03       ` Daniel Jacobowitz
  0 siblings, 1 reply; 12+ messages in thread
From: Alan Modra @ 2007-08-01 13:05 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Roland McGrath, binutils

On Sun, Jul 29, 2007 at 06:53:15PM +0200, Jan Kratochvil wrote:
> On Sat, 28 Jul 2007 22:16:04 +0200, Roland McGrath wrote:
> > Are you sure that changing _bfd_elf_make_section_from_phdr is the right way
> > to fix gdb?
> > 
> > It is in a certain sense accurate to split the one segment into two
> > sections, a leading SEC_LOAD one and a trailing one without SEC_LOAD.
> 
> The core files with `p_filesz == 0' were working before.

I think BFD is still doing the right thing.  Please fix this in gdb.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] bfd: Core files with p_filesz < p_memsz (build-id)
  2007-08-01 13:05     ` Alan Modra
@ 2007-08-02 20:03       ` Daniel Jacobowitz
  2007-08-03  0:13         ` Alan Modra
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Jacobowitz @ 2007-08-02 20:03 UTC (permalink / raw)
  To: Jan Kratochvil, Roland McGrath, binutils

On Wed, Aug 01, 2007 at 10:35:47PM +0930, Alan Modra wrote:
> On Sun, Jul 29, 2007 at 06:53:15PM +0200, Jan Kratochvil wrote:
> > On Sat, 28 Jul 2007 22:16:04 +0200, Roland McGrath wrote:
> > > Are you sure that changing _bfd_elf_make_section_from_phdr is the right way
> > > to fix gdb?
> > > 
> > > It is in a certain sense accurate to split the one segment into two
> > > sections, a leading SEC_LOAD one and a trailing one without SEC_LOAD.
> > 
> > The core files with `p_filesz == 0' were working before.
> 
> I think BFD is still doing the right thing.  Please fix this in gdb.

Could you elaborate?  Consider a core with these headers:

  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  NOTE           0x0000000000000388 0x0000000000000000  0x0000000000000000
                 0x0000000000000314 0x0000000000000000         0
  LOAD           0x0000000000001000 0x0000000000400000  0x0000000000000000
                 0x0000000000000000 0x0000000000005000  R E    1000
  LOAD           0x0000000000001000 0x0000000000504000  0x0000000000000000
                 0x0000000000000100 0x0000000000001000  RW     1000

That's 20K at 0x400000, none of which is in the core dump, and 4K at
0x504000, 256 bytes of which is in the core dump.  A reasonably
current objdump produces these pseudo-sections:

  4 load1         00000000  0000000000400000  0000000000000000  00001000  2**12
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  5 load2a        00000100  0000000000504000  0000000000000000  00001000  2**12
                  CONTENTS, ALLOC, LOAD
  6 load2b        00000f00  0000000000504100  0000000000000100  00000000  2**0
                  ALLOC

So we report the BSS portion of an entirely BSS PT_LOAD as having size
zero (plus contents flag, bizarre).  But the BSS portion of a partially
filled PT_LOAD is shown with size equal to its memory size.  I don't
see why they should be inconsistent, and it makes handling this in GDB
a little awkward.

I don't think the zero-sized fake sections have any value.  Certainly
GDB just skips over them when it's walking its tables.  That suggests
we don't need either load1 or load2b.  Obviously we could make a more
faithful representation of the program headers; that will make an
unmodified GDB start reading zeros from undumped segments though.

-- 
Daniel Jacobowitz
CodeSourcery

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] bfd: Core files with p_filesz < p_memsz (build-id)
  2007-08-02 20:03       ` Daniel Jacobowitz
@ 2007-08-03  0:13         ` Alan Modra
  2007-08-03  0:21           ` Daniel Jacobowitz
  0 siblings, 1 reply; 12+ messages in thread
From: Alan Modra @ 2007-08-03  0:13 UTC (permalink / raw)
  To: Jan Kratochvil, Roland McGrath, binutils

On Thu, Aug 02, 2007 at 04:02:49PM -0400, Daniel Jacobowitz wrote:
> On Wed, Aug 01, 2007 at 10:35:47PM +0930, Alan Modra wrote:
> > I think BFD is still doing the right thing.  Please fix this in gdb.
> 
> Could you elaborate?  Consider a core with these headers:
[snip]
> So we report the BSS portion of an entirely BSS PT_LOAD as having size
> zero (plus contents flag, bizarre).  But the BSS portion of a partially
> filled PT_LOAD is shown with size equal to its memory size.  I don't
> see why they should be inconsistent, and it makes handling this in GDB
> a little awkward.

Oh, I see.  I was just looking at the split case.  I'd say the
non-split behaviour is a bug.  A program header with p_filesz zero and
p_memsz non-zero really ought to create a bfd section with size equal
to p_memsz, without SEC_HAS_CONTENTS and SEC_LOAD.

So I think we should apply the following, and possibly on top of this
do something special for core files.

Index: bfd/elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.404
diff -u -p -w -r1.404 elf.c
--- bfd/elf.c	1 Aug 2007 19:55:10 -0000	1.404
+++ bfd/elf.c	2 Aug 2007 23:58:56 -0000
@@ -2223,7 +2223,7 @@ _bfd_elf_new_section_hook (bfd *abfd, as
    for the single program segment.  The first has the length specified by
    the file size of the segment, and the second has the length specified
    by the difference between the two sizes.  In effect, the segment is split
-   into it's initialized and uninitialized parts.
+   into its initialized and uninitialized parts.
 
  */
 
@@ -2242,6 +2242,9 @@ _bfd_elf_make_section_from_phdr (bfd *ab
   split = ((hdr->p_memsz > 0)
 	    && (hdr->p_filesz > 0)
 	    && (hdr->p_memsz > hdr->p_filesz));
+
+  if (hdr->p_filesz > 0)
+    {
   sprintf (namebuf, "%s%d%s", typename, index, split ? "a" : "");
   len = strlen (namebuf) + 1;
   name = bfd_alloc (abfd, len);
@@ -2272,10 +2275,11 @@ _bfd_elf_make_section_from_phdr (bfd *ab
     {
       newsect->flags |= SEC_READONLY;
     }
+    }
 
-  if (split)
+  if (hdr->p_memsz > hdr->p_filesz)
     {
-      sprintf (namebuf, "%s%db", typename, index);
+      sprintf (namebuf, "%s%d%s", typename, index, split ? "b" : "");
       len = strlen (namebuf) + 1;
       name = bfd_alloc (abfd, len);
       if (!name)


-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] bfd: Core files with p_filesz < p_memsz (build-id)
  2007-08-03  0:13         ` Alan Modra
@ 2007-08-03  0:21           ` Daniel Jacobowitz
  2007-08-03  2:50             ` Alan Modra
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Jacobowitz @ 2007-08-03  0:21 UTC (permalink / raw)
  To: Jan Kratochvil, Roland McGrath, binutils

On Fri, Aug 03, 2007 at 09:43:00AM +0930, Alan Modra wrote:
> Oh, I see.  I was just looking at the split case.  I'd say the
> non-split behaviour is a bug.  A program header with p_filesz zero and
> p_memsz non-zero really ought to create a bfd section with size equal
> to p_memsz, without SEC_HAS_CONTENTS and SEC_LOAD.
> 
> So I think we should apply the following, and possibly on top of this
> do something special for core files.

Yes, I agree.  But keep in mind that this patch on its own is going to
lay waste to GDB's core debugging :-)  It will now disassemble all
functions as zeroes.

What we really want in GDB is now a bit complicated:
  - Prefer data from core file if within p_filesz.
  - Prefer data from executable if between p_filesz and p_memsz.
  - Show zeroes (rather than read errors) for data between p_filesz
    and p_memsz if not present in the executable.

Messy...

-- 
Daniel Jacobowitz
CodeSourcery

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] bfd: Core files with p_filesz < p_memsz (build-id)
  2007-08-03  0:21           ` Daniel Jacobowitz
@ 2007-08-03  2:50             ` Alan Modra
  2007-08-06 19:32               ` Daniel Jacobowitz
  2007-08-09 16:04               ` Jan Kratochvil
  0 siblings, 2 replies; 12+ messages in thread
From: Alan Modra @ 2007-08-03  2:50 UTC (permalink / raw)
  To: Jan Kratochvil, Roland McGrath, binutils

On Thu, Aug 02, 2007 at 08:21:03PM -0400, Daniel Jacobowitz wrote:
> On Fri, Aug 03, 2007 at 09:43:00AM +0930, Alan Modra wrote:
> > Oh, I see.  I was just looking at the split case.  I'd say the
> > non-split behaviour is a bug.  A program header with p_filesz zero and
> > p_memsz non-zero really ought to create a bfd section with size equal
> > to p_memsz, without SEC_HAS_CONTENTS and SEC_LOAD.
> > 
> > So I think we should apply the following, and possibly on top of this
> > do something special for core files.
> 
> Yes, I agree.  But keep in mind that this patch on its own is going to
> lay waste to GDB's core debugging :-)  It will now disassemble all
> functions as zeroes.
> 
> What we really want in GDB is now a bit complicated:
>   - Prefer data from core file if within p_filesz.
>   - Prefer data from executable if between p_filesz and p_memsz.
>   - Show zeroes (rather than read errors) for data between p_filesz
>     and p_memsz if not present in the executable.
> 
> Messy...

I'd be happy with the following.  Please check that this doesn't break
gdb..

	* elf.c (_bfd_elf_make_section_from_phdr): Properly handle
	bss segments.

Index: bfd/elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.404
diff -u -p -w -r1.404 elf.c
--- bfd/elf.c	1 Aug 2007 19:55:10 -0000	1.404
+++ bfd/elf.c	3 Aug 2007 02:44:28 -0000
@@ -2223,7 +2223,7 @@ _bfd_elf_new_section_hook (bfd *abfd, as
    for the single program segment.  The first has the length specified by
    the file size of the segment, and the second has the length specified
    by the difference between the two sizes.  In effect, the segment is split
-   into it's initialized and uninitialized parts.
+   into its initialized and uninitialized parts.
 
  */
 
@@ -2242,6 +2242,9 @@ _bfd_elf_make_section_from_phdr (bfd *ab
   split = ((hdr->p_memsz > 0)
 	    && (hdr->p_filesz > 0)
 	    && (hdr->p_memsz > hdr->p_filesz));
+
+  if (hdr->p_filesz > 0)
+    {
   sprintf (namebuf, "%s%d%s", typename, index, split ? "a" : "");
   len = strlen (namebuf) + 1;
   name = bfd_alloc (abfd, len);
@@ -2272,10 +2275,13 @@ _bfd_elf_make_section_from_phdr (bfd *ab
     {
       newsect->flags |= SEC_READONLY;
     }
+    }
 
-  if (split)
+  if (hdr->p_memsz > hdr->p_filesz)
     {
-      sprintf (namebuf, "%s%db", typename, index);
+      bfd_vma align;
+
+      sprintf (namebuf, "%s%d%s", typename, index, split ? "b" : "");
       len = strlen (namebuf) + 1;
       name = bfd_alloc (abfd, len);
       if (!name)
@@ -2287,8 +2293,21 @@ _bfd_elf_make_section_from_phdr (bfd *ab
       newsect->vma = hdr->p_vaddr + hdr->p_filesz;
       newsect->lma = hdr->p_paddr + hdr->p_filesz;
       newsect->size = hdr->p_memsz - hdr->p_filesz;
+      newsect->filepos = hdr->p_offset + hdr->p_filesz;
+      align = newsect->vma & -newsect->vma;
+      if (align == 0 || align > hdr->p_align)
+	align = hdr->p_align;
+      newsect->alignment_power = bfd_log2 (align);
       if (hdr->p_type == PT_LOAD)
 	{
+	  /* Hack for gdb.  Segments that have not been modified do
+	     not have their contents written to a core file, on the
+	     assumption that a debugger can find the contents in the
+	     executable.  We flag this case by setting the fake
+	     section size to zero.  Note that "real" bss sections will
+	     always have their contents dumped to the core file.  */
+	  if (bfd_get_format (abfd) == bfd_core)
+	    newsect->size = 0;
 	  newsect->flags |= SEC_ALLOC;
 	  if (hdr->p_flags & PF_X)
 	    newsect->flags |= SEC_CODE;

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] bfd: Core files with p_filesz < p_memsz (build-id)
  2007-08-03  2:50             ` Alan Modra
@ 2007-08-06 19:32               ` Daniel Jacobowitz
  2007-08-09 16:04               ` Jan Kratochvil
  1 sibling, 0 replies; 12+ messages in thread
From: Daniel Jacobowitz @ 2007-08-06 19:32 UTC (permalink / raw)
  To: binutils; +Cc: Jan Kratochvil, Roland McGrath

On Fri, Aug 03, 2007 at 12:20:00PM +0930, Alan Modra wrote:
> I'd be happy with the following.  Please check that this doesn't break
> gdb..
> 
> 	* elf.c (_bfd_elf_make_section_from_phdr): Properly handle
> 	bss segments.

Thanks.  This version doesn't change GDB test results, and it seems to
do the right thing on a core file I hacked together with a hex editor.

-- 
Daniel Jacobowitz
CodeSourcery

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] bfd: Core files with p_filesz < p_memsz (build-id)
  2007-08-03  2:50             ` Alan Modra
  2007-08-06 19:32               ` Daniel Jacobowitz
@ 2007-08-09 16:04               ` Jan Kratochvil
  1 sibling, 0 replies; 12+ messages in thread
From: Jan Kratochvil @ 2007-08-09 16:04 UTC (permalink / raw)
  To: Alan Modra; +Cc: Roland McGrath, binutils

On Fri, 03 Aug 2007 04:50:00 +0200, Alan Modra wrote:
[snip]
> I'd be happy with the following.  Please check that this doesn't break
> gdb..
> 
> 	* elf.c (_bfd_elf_make_section_from_phdr): Properly handle
> 	bss segments.

[ This patch got applied in the meantime modulo whitespaces. ]

Works fine, thanks.


Jan


Functionality of the patch on a p_filesz < p_memsz (build-id) core file:

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  LOAD           0x0000000000001000 0x0000000000400000 0x0000000000000000
                 0x0000000000001000 0x00000000000b1000  R E    1000
  LOAD           0x0000000000002000 0x00000000006b1000 0x0000000000000000
                 0x000000000000a000 0x000000000000a000  RW     1000

Sections:
Idx Name          Size      VMA               LMA               File off  Algn
  4 load1a        00001000  0000000000400000  0000000000000000  00001000  2**12
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  5 load1b        00000000  0000000000401000  0000000000001000  00002000  2**12
                  ALLOC, READONLY, CODE
  6 load2         0000a000  00000000006b1000  0000000000000000  00002000  2**12
                  CONTENTS, ALLOC, LOAD

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2007-08-09 16:04 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-07-28 20:16 [patch] bfd: Core files with p_filesz < p_memsz (build-id) Jan Kratochvil
2007-07-29 12:30 ` Jan Kratochvil
2007-07-29 15:29 ` Roland McGrath
2007-07-29 18:32   ` Jan Kratochvil
2007-08-01 13:05     ` Alan Modra
2007-08-02 20:03       ` Daniel Jacobowitz
2007-08-03  0:13         ` Alan Modra
2007-08-03  0:21           ` Daniel Jacobowitz
2007-08-03  2:50             ` Alan Modra
2007-08-06 19:32               ` Daniel Jacobowitz
2007-08-09 16:04               ` Jan Kratochvil
2007-07-30 14:07 ` [resent] " Jan Kratochvil

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