vdso handling

vdso handling

[Metzger, Markus T]

Hello,

My name is Markus.  I work for Intel on GDB in the area of
hardware-supported execution recording.

I noticed that the BFD created for the VDSO (system-provided in-memory
DSO) does not contain any BFD sections.  Is this intentional?  Or has
there just been no need for them?

If it just hasn't been done, yet, how would I best approach this?


Here's the problem I am trying to solve with this.  May as well be
that I'm on the wrong track...

When using the btrace record target, GDB only allows access to
read-only memory during replay.  The btrace record target does not
trace data so read-write memory corresponds to the end of the trace,
not the current replay position.

The implementation uses the same check that is also used for
'trust-readonly-sections", i.e.

	    section = target_section_by_addr (ops, offset);
	    if (section != NULL)
	      {
		/* Check if the section we found is readonly.  */
		if ((bfd_get_section_flags (section->the_bfd_section->owner,
					    section->the_bfd_section)
		     & SEC_READONLY) != 0)

For the vdso, there is no target section, so the check fails.  This
prevents GDB from disassembling vdso instructions during replay.


The vdso is processed in symbol_file_add_from_memory at
gdb/symfile-mem.c:84.  It calls bfd_from_remote_memory to create a BFD
for the vdso and then processes it.  If there were BFD sections, the
following small patch should add the respective target sections to GDB.

diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index e3230de..cf4da38 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -91,6 +91,7 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr, char *name,
   struct section_addr_info *sai;
   unsigned int i;
   struct cleanup *cleanup;
+  struct target_section *sections, *sections_end, *tsec;

   if (bfd_get_flavour (templ) != bfd_target_elf_flavour)
     error (_("add-symbol-file-from-memory not supported for this target"));
@@ -113,6 +114,22 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr, char *name,
     error (_("Got object file from memory but can't read symbols: %s."),
	   bfd_errmsg (bfd_get_error ()));

+  /* Add target sections for this bfd.  */
+  sections = NULL;
+  sections_end = NULL;
+  if (build_section_table (nbfd, &sections, &sections_end))
+    error (_("Failed to build section table"));
+
+  /* Adjust the target section addresses by the load address.  */
+  for (tsec = sections; tsec != sections_end; ++tsec)
+    {
+      tsec->addr += loadbase;
+      tsec->endaddr += loadbase;
+    }
+
+  add_target_sections (&nbfd, sections, sections_end);
+  xfree (sections);
+
   sai = alloc_section_addr_info (bfd_count_sections (nbfd));
   make_cleanup (xfree, sai);
   i = 0;


This should allow me to pass the above read-only section check and
allow GDB to disassemble vdso instructions.

thanks,
markus.


Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: vdso handling

[Alan Modra]

On Mon, Mar 10, 2014 at 01:04:33PM +0000, Metzger, Markus T wrote:
> I noticed that the BFD created for the VDSO (system-provided in-memory
> DSO) does not contain any BFD sections.  Is this intentional?  Or has
> there just been no need for them?
[snip]
> The vdso is processed in symbol_file_add_from_memory at
> gdb/symfile-mem.c:84.  It calls bfd_from_remote_memory to create a BFD
> for the vdso and then processes it.

The underlying cause is that you're trying to debug an ELF binary that
only contains the execution view.  The linking view (of which the
sections are a part) is not loaded, so bfd_from_remote_memory does not
have this information.  See elfcode.h bfd_from_remote_memory.

You can see similar breakage of gdb and binutils if you zap e_shoff,
e_shnum, and e_shstrndx of your favourite hello world program.

I suppose one way to provide something that gdb and other tools expect
would be to treat the vdso like a core file, and create fake sections
corresponding to the program headers.  I'm not really keen on the idea
though, since I know that will open up a can of worms.

Can't you point gdb at a file image for the vdso?

-- 
Alan Modra
Australia Development Lab, IBM

Re: vdso handling

[Mike Frysinger]

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

On Wed 12 Mar 2014 17:47:02 Alan Modra wrote:
> On Mon, Mar 10, 2014 at 01:04:33PM +0000, Metzger, Markus T wrote:
> > I noticed that the BFD created for the VDSO (system-provided in-memory
> > DSO) does not contain any BFD sections.  Is this intentional?  Or has
> > there just been no need for them?
> 
> [snip]
> 
> > The vdso is processed in symbol_file_add_from_memory at
> > gdb/symfile-mem.c:84.  It calls bfd_from_remote_memory to create a BFD
> > for the vdso and then processes it.
> 
> The underlying cause is that you're trying to debug an ELF binary that
> only contains the execution view.  The linking view (of which the
> sections are a part) is not loaded, so bfd_from_remote_memory does not
> have this information.  See elfcode.h bfd_from_remote_memory.
> 
> You can see similar breakage of gdb and binutils if you zap e_shoff,
> e_shnum, and e_shstrndx of your favourite hello world program.
> 
> I suppose one way to provide something that gdb and other tools expect
> would be to treat the vdso like a core file, and create fake sections
> corresponding to the program headers.  I'm not really keen on the idea
> though, since I know that will open up a can of worms.
> 
> Can't you point gdb at a file image for the vdso?

i don't think distros generally ship it ?  the kernel doesn't install it by 
default (e.g. into /lib/modules/$(uname -r)/vdso/).
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: vdso handling

[Doug Evans]

On Wed, Mar 12, 2014 at 12:17 AM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Mar 10, 2014 at 01:04:33PM +0000, Metzger, Markus T wrote:
>> I noticed that the BFD created for the VDSO (system-provided in-memory
>> DSO) does not contain any BFD sections.  Is this intentional?  Or has
>> there just been no need for them?
> [snip]
>> The vdso is processed in symbol_file_add_from_memory at
>> gdb/symfile-mem.c:84.  It calls bfd_from_remote_memory to create a BFD
>> for the vdso and then processes it.
>
> The underlying cause is that you're trying to debug an ELF binary that
> only contains the execution view.  The linking view (of which the
> sections are a part) is not loaded, so bfd_from_remote_memory does not
> have this information.  See elfcode.h bfd_from_remote_memory.
>
> You can see similar breakage of gdb and binutils if you zap e_shoff,
> e_shnum, and e_shstrndx of your favourite hello world program.
>
> I suppose one way to provide something that gdb and other tools expect
> would be to treat the vdso like a core file, and create fake sections
> corresponding to the program headers.  I'm not really keen on the idea
> though, since I know that will open up a can of worms.

I think a case can be made that gdb should be able to use the
"execution view" of the program here.
As for how to achieve that ... "Discuss." :-)

Re: vdso handling

[Cary Coutant]

> I think a case can be made that gdb should be able to use the
> "execution view" of the program here.
> As for how to achieve that ... "Discuss." :-)

Add a PT_DEBUG program header entry? The PT_DEBUG segment would need
to have a small header that allows the debugger to find .debug_abbrev,
.debug_info, etc. (i.e., a mini section table). Or, just add
individual program header entries for each of the standard debug
sections: PT_DEBUG_ABBREV, PT_DEBUG_INFO, etc.

-cary

Re: vdso handling

[Alan Modra]

On Wed, Mar 12, 2014 at 01:22:58PM -0700, Cary Coutant wrote:
> > I think a case can be made that gdb should be able to use the
> > "execution view" of the program here.
> > As for how to achieve that ... "Discuss." :-)
> 
> Add a PT_DEBUG program header entry? The PT_DEBUG segment would need
> to have a small header that allows the debugger to find .debug_abbrev,
> .debug_info, etc. (i.e., a mini section table). Or, just add
> individual program header entries for each of the standard debug
> sections: PT_DEBUG_ABBREV, PT_DEBUG_INFO, etc.

Debug sections are not normally loaded.  For that reason I don't think
it makes any sense to specify program headers for them.  It wouldn't
help in the vdso case anyway, since the problem there is that you only
have the loaded part of the original ELF file.

-- 
Alan Modra
Australia Development Lab, IBM

RE: vdso handling

[Metzger, Markus T]

> -----Original Message-----
> From: Alan Modra [mailto:amodra@gmail.com]
> Sent: Thursday, March 13, 2014 2:02 AM
> To: Cary Coutant
> Cc: Doug Evans; Metzger, Markus T; gdb@sourceware.org;
> binutils@sourceware.org
> Subject: Re: vdso handling
> 
> On Wed, Mar 12, 2014 at 01:22:58PM -0700, Cary Coutant wrote:
> > > I think a case can be made that gdb should be able to use the
> > > "execution view" of the program here.
> > > As for how to achieve that ... "Discuss." :-)
> >
> > Add a PT_DEBUG program header entry? The PT_DEBUG segment would
> need
> > to have a small header that allows the debugger to find .debug_abbrev,
> > .debug_info, etc. (i.e., a mini section table). Or, just add
> > individual program header entries for each of the standard debug
> > sections: PT_DEBUG_ABBREV, PT_DEBUG_INFO, etc.
> 
> Debug sections are not normally loaded.  For that reason I don't think
> it makes any sense to specify program headers for them.  It wouldn't
> help in the vdso case anyway, since the problem there is that you only
> have the loaded part of the original ELF file.

The vdso contains a section table, as well.  When I hack
bfd_from_remote_memory to create BFD sections from them similar
to what elf_object_p does, I get the target sections that I wanted in GDB.
The patch is rather big, though, and duplicating a lot of elf_object_p's code.

I have not tried generating fake sections from segments, yet.

Regards,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

RE: vdso handling

[Metzger, Markus T]

> -----Original Message-----
> From: Metzger, Markus T
> Sent: Thursday, March 13, 2014 9:24 AM


> > On Wed, Mar 12, 2014 at 01:22:58PM -0700, Cary Coutant wrote:
> > > > I think a case can be made that gdb should be able to use the
> > > > "execution view" of the program here.
> > > > As for how to achieve that ... "Discuss." :-)
> > >
> > > Add a PT_DEBUG program header entry? The PT_DEBUG segment would
> > need
> > > to have a small header that allows the debugger to find .debug_abbrev,
> > > .debug_info, etc. (i.e., a mini section table). Or, just add
> > > individual program header entries for each of the standard debug
> > > sections: PT_DEBUG_ABBREV, PT_DEBUG_INFO, etc.
> >
> > Debug sections are not normally loaded.  For that reason I don't think
> > it makes any sense to specify program headers for them.  It wouldn't
> > help in the vdso case anyway, since the problem there is that you only
> > have the loaded part of the original ELF file.
> 
> The vdso contains a section table, as well.  When I hack
> bfd_from_remote_memory to create BFD sections from them similar
> to what elf_object_p does, I get the target sections that I wanted in GDB.
> The patch is rather big, though, and duplicating a lot of elf_object_p's code.
> 
> I have not tried generating fake sections from segments, yet.

This turned out to be rather simple.  Is this the right direction to go?
Should I maybe restrict this to PT_LOAD segments?


diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index 20101be..22aae2a 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -1771,7 +1771,6 @@ NAME(_bfd_elf,bfd_from_remote_memory)
 	    return NULL;
 	  }
       }
-  free (x_phdrs);
 
   /* If the segments visible in memory didn't include the section headers,
      then clear them from the file header.  */
@@ -1791,6 +1790,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   bim = (struct bfd_in_memory *) bfd_malloc (sizeof (struct bfd_in_memory));
   if (bim == NULL)
     {
+      free (x_phdrs);
       free (contents);
       bfd_set_error (bfd_error_no_memory);
       return NULL;
@@ -1799,6 +1799,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   if (nbfd == NULL)
     {
       free (bim);
+      free (x_phdrs);
       free (contents);
       bfd_set_error (bfd_error_no_memory);
       return NULL;
@@ -1815,6 +1816,12 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   nbfd->mtime = time (NULL);
   nbfd->mtime_set = TRUE;
 
+  /* Add fake sections for program headers.  We ignore errors.  */
+  for (i = 0; i < i_ehdr.e_phnum; ++i)
+    (void) bfd_section_from_phdr (nbfd, &i_phdrs[i], i);
+
+  free (x_phdrs);
+
   if (loadbasep)
     *loadbasep = loadbase;
   return nbfd;


regards,
markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: vdso handling

[Pedro Alves]

On 03/13/2014 08:23 AM, Metzger, Markus T wrote:
>> -----Original Message-----
>> From: Alan Modra [mailto:amodra@gmail.com]
>> Sent: Thursday, March 13, 2014 2:02 AM
>> To: Cary Coutant
>> Cc: Doug Evans; Metzger, Markus T; gdb@sourceware.org;
>> binutils@sourceware.org
>> Subject: Re: vdso handling
>>
>> On Wed, Mar 12, 2014 at 01:22:58PM -0700, Cary Coutant wrote:
>>>> I think a case can be made that gdb should be able to use the
>>>> "execution view" of the program here.
>>>> As for how to achieve that ... "Discuss." :-)
>>>
>>> Add a PT_DEBUG program header entry? The PT_DEBUG segment would
>> need
>>> to have a small header that allows the debugger to find .debug_abbrev,
>>> .debug_info, etc. (i.e., a mini section table). Or, just add
>>> individual program header entries for each of the standard debug
>>> sections: PT_DEBUG_ABBREV, PT_DEBUG_INFO, etc.
>>
>> Debug sections are not normally loaded.  For that reason I don't think
>> it makes any sense to specify program headers for them.  It wouldn't
>> help in the vdso case anyway, since the problem there is that you only
>> have the loaded part of the original ELF file.
> 
> The vdso contains a section table, as well.  When I hack
> bfd_from_remote_memory to create BFD sections from them similar
> to what elf_object_p does, I get the target sections that I wanted in GDB.

I got curious.  Indeed:

$ gdb --quiet /bin/ls
Reading symbols from /usr/bin/ls...(no debugging symbols found)...done.
(gdb) catch syscall
Catchpoint 1 (any syscall)
(gdb) r
Starting program: /usr/bin/ls

Catchpoint 1 (call to syscall brk), 0x000000323d0163fa in __brk (addr=addr@entry=0x0) at ../sysdeps/unix/sysv/linux/x86_64/brk.c:32
32        __curbrk = newbrk = (void *) INLINE_SYSCALL (brk, 1, addr);
(gdb) shell cat /proc/23042/maps|grep vdso
7ffff7ffd000-7ffff7fff000 r-xp 00000000 00:00 0                          [vdso]
(gdb) dump memory /tmp/vdso.so 0x7ffff7ffd000 0x7ffff7fff000
(gdb) q

$ objdump -h /tmp/vdso.so

/tmp/vdso.so:     file format elf64-x86-64

Sections:
Idx Name          Size      VMA               LMA               File off  Algn
  0 .hash         00000040  ffffffffff700120  ffffffffff700120  00000120  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  1 .dynsym       00000108  ffffffffff700160  ffffffffff700160  00000160  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  2 .dynstr       0000005e  ffffffffff700268  ffffffffff700268  00000268  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  3 .gnu.version  00000016  ffffffffff7002c6  ffffffffff7002c6  000002c6  2**1
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  4 .gnu.version_d 00000038  ffffffffff7002e0  ffffffffff7002e0  000002e0  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  5 .note         0000003c  ffffffffff700318  ffffffffff700318  00000318  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  6 .eh_frame_hdr 0000003c  ffffffffff700354  ffffffffff700354  00000354  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  7 .eh_frame     00000138  ffffffffff700390  ffffffffff700390  00000390  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  8 .dynamic      000000f0  ffffffffff7004c8  ffffffffff7004c8  000004c8  2**3
                  CONTENTS, ALLOC, LOAD, DATA
  9 .rodata       00000020  ffffffffff7005b8  ffffffffff7005b8  000005b8  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 10 __bug_table   0000000c  ffffffffff7005d8  ffffffffff7005d8  000005d8  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 11 .discard      00000006  ffffffffff7005e4  ffffffffff7005e4  000005e4  2**0
                  CONTENTS, ALLOC, LOAD, DATA
 12 .altinstructions 00000048  ffffffffff7005ea  ffffffffff7005ea  000005ea  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 13 .altinstr_replacement 00000012  ffffffffff700632  ffffffffff700632  00000632  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 14 .text         0000069d  ffffffffff700700  ffffffffff700700  00000700  2**4
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 15 .comment      0000002c  0000000000000000  0000000000000000  00000d9d  2**0
                  CONTENTS, READONLY

> The patch is rather big, though, and duplicating a lot of elf_object_p's code.

Why's that?  Why doesn't the memory-backed bfd paths take the same paths as
a file-backed bfd internally in bfd?  It sounds to me that this should be
doable without duplication.

-- 
Pedro Alves

Re: vdso handling

[Pedro Alves]

On 03/13/2014 10:07 AM, Pedro Alves wrote:
> Why's that?  Why doesn't the memory-backed bfd paths take the same paths as
> a file-backed bfd internally in bfd?  It sounds to me that this should be
> doable without duplication.

BTW, I meant that for vDSO's only.  The vsyscall page is not an elf,
and therefore bfd still needs to be passed a template elf.  For the
latter, GDB would indeed need to work with the segments.  Do we still
care for vsyscall kernels?  But for the former, bfd should just be
able to read the whole DSO as a plain elf.

Some glibc versions even include the vdso in the DSO list (*), and GDB
should be able to tell that that DSO is the vDSO (by matching addresses), and
load it completely from memory, still using a memory backed bfd, but _without_
a template.  So with that in mind, bfd should be able to read the vdso
as a bfd from memory using the same paths as a file-backed bfd, except,
well, the bfd's backing store is in memory rather than in a file.

(*) note how linux-vdso.so.1 is listed by ldd, even if "info shared" in gdb
doesn't show it, on some systems.

-- 
Pedro Alves

Re: vdso handling

[Samuel Bronson]

Pedro Alves <palves@redhat.com> writes:

> On 03/13/2014 10:07 AM, Pedro Alves wrote:
>> Why's that?  Why doesn't the memory-backed bfd paths take the same paths as
>> a file-backed bfd internally in bfd?  It sounds to me that this should be
>> doable without duplication.
>
> BTW, I meant that for vDSO's only.  The vsyscall page is not an elf,
> and therefore bfd still needs to be passed a template elf.  For the
> latter, GDB would indeed need to work with the segments.  Do we still
> care for vsyscall kernels?  But for the former, bfd should just be
> able to read the whole DSO as a plain elf.
>
> Some glibc versions even include the vdso in the DSO list (*), and GDB
> should be able to tell that that DSO is the vDSO (by matching addresses), and
                                                    ^^^^^^^^^^^^^^^^^^^^^
Hmm, why don't we already do that? It's bound to be easier than meeting
the conditions to get glibc to stop falsely cliaming that the vDSO comes
from a file <https://sourceware.org/bugzilla/show_bug.cgi?id=13097#c5>.

That'd be enough to take two bugs off of <http://bugs.debian.org/gdb>
right there.

> load it completely from memory, still using a memory backed bfd, but _without_
> a template.  So with that in mind, bfd should be able to read the vdso
> as a bfd from memory using the same paths as a file-backed bfd, except,
> well, the bfd's backing store is in memory rather than in a file.
>
> (*) note how linux-vdso.so.1 is listed by ldd, even if "info shared" in gdb
> doesn't show it, on some systems.

What versions don't list the vdso under some name or other?  (Mine calls
it linux-gate.so.1 for some reason.)

-- 
Hi! I'm a .signature virus! Copy me into your ~/.signature to help me spread!

Re: vdso handling

[Pedro Alves]

On 06/01/2014 09:31 PM, Samuel Bronson wrote:
> Pedro Alves <palves@redhat.com> writes:

>> Some glibc versions even include the vdso in the DSO list (*), and GDB
>> should be able to tell that that DSO is the vDSO (by matching addresses), and
>                                                     ^^^^^^^^^^^^^^^^^^^^^
> Hmm, why don't we already do that? It's bound to be easier than meeting
> the conditions to get glibc to stop falsely cliaming that the vDSO comes
> from a file <https://sourceware.org/bugzilla/show_bug.cgi?id=13097#c5>.

Dunno.  Because nobody has done it?

I suppose that's what Ulrich meant in
<https://sourceware.org/bugzilla/show_bug.cgi?id=13097#c1>.

>> (*) note how linux-vdso.so.1 is listed by ldd, even if "info shared" in gdb
>> doesn't show it, on some systems.
> 
> What versions don't list the vdso under some name or other?  (Mine calls
> it linux-gate.so.1 for some reason.)

I don't know versions numbers, but all before the glibc commit
mentioned in <https://sourceware.org/bugzilla/show_bug.cgi?id=13097#c1>
I guess, and also, see the rest of the discussion there, indicating
that Fedora carries a reversion of the offending patch.

-- 
Pedro Alves

Re: vdso handling

[Alan Modra]

On Thu, Mar 13, 2014 at 10:07:10AM +0000, Pedro Alves wrote:
> Why doesn't the memory-backed bfd paths take the same paths as
> a file-backed bfd internally in bfd?

I think they do.  In symfile-mem.c:symbol_file_add_from_memory

  nbfd = bfd_elf_bfd_from_remote_memory (templ, addr, &loadbase,
					 target_read_memory_bfd);
..
  if (!bfd_check_format (nbfd, bfd_object))

That bfd_check_format is where elf_object_p is called.  *If*
bfd_from_remote_memory picked up the sections headers, you'll get a
normal bfd with sections..

-- 
Alan Modra
Australia Development Lab, IBM

Re: vdso handling

[Mark Wielaard]

On Thu, 2014-03-13 at 11:31 +1030, Alan Modra wrote:
> It wouldn't
> help in the vdso case anyway, since the problem there is that you only
> have the loaded part of the original ELF file.

Note that the vdso is often special, compared to other ELF dsos, because
the loaded part is just the complete ELF image in memory. Since they are
very simple they will just have one PT_LOAD at offset zero and if the
image is smaller than the page size then the whole file is just simply
mapped into memory completely. So by fetching the vdso ELF image from
remote memory you should be able to get the section headers and the
not-allocated sections too.

Cheers,

Mark

Re: vdso handling

[Alan Modra]

On Thu, Mar 13, 2014 at 10:52:16AM +0100, Mark Wielaard wrote:
> On Thu, 2014-03-13 at 11:31 +1030, Alan Modra wrote:
> > It wouldn't
> > help in the vdso case anyway, since the problem there is that you only
> > have the loaded part of the original ELF file.
> 
> Note that the vdso is often special, compared to other ELF dsos, because
> the loaded part is just the complete ELF image in memory. Since they are
> very simple they will just have one PT_LOAD at offset zero and if the
> image is smaller than the page size then the whole file is just simply
> mapped into memory completely. So by fetching the vdso ELF image from
> remote memory you should be able to get the section headers and the
> not-allocated sections too.

Yes, but if the vdso does not fit in a page (which incidentally is
inferred by program header p_align), then you may lose the section
headers.  I was assuming this was the case.

-- 
Alan Modra
Australia Development Lab, IBM

Re: vdso handling

[Mark Wielaard]

On Thu, 2014-03-13 at 23:33 +1030, Alan Modra wrote:
> On Thu, Mar 13, 2014 at 10:52:16AM +0100, Mark Wielaard wrote:
> > On Thu, 2014-03-13 at 11:31 +1030, Alan Modra wrote:
> > > It wouldn't
> > > help in the vdso case anyway, since the problem there is that you only
> > > have the loaded part of the original ELF file.
> > 
> > Note that the vdso is often special, compared to other ELF dsos, because
> > the loaded part is just the complete ELF image in memory. Since they are
> > very simple they will just have one PT_LOAD at offset zero and if the
> > image is smaller than the page size then the whole file is just simply
> > mapped into memory completely. So by fetching the vdso ELF image from
> > remote memory you should be able to get the section headers and the
> > not-allocated sections too.
> 
> Yes, but if the vdso does not fit in a page (which incidentally is
> inferred by program header p_align), then you may lose the section
> headers.  I was assuming this was the case.

Yes, you are right. Almost everything else is bigger than a page and has
multiple PT_LOAD segments. In which case determining whether the full
shdrs were mapped in from the file is much trickier. And might have to
rely on only the phdrs.

We recently went through this for elfutils, which also contains an
elf_from_remote_memory implementation (if someone needs inspiration).
Currently that implementation just uses the phdrs and so needs some
heuristics using the program header fields to know what part of the file
was mapped where. But if you have access to the actual /proc/PID/maps
ranges you might be able to do something more accurate, or at least be
able to cross check that the program headers view is really how the ELF
image is actually laid out (*).

Cheers,

Mark

(*) Don't make the mistake we made to think p_align is what segments are
actually aligned to, the dynamic loader on GNU/Linux just uses the
actual page size of the architecture to align the mappings:
https://lists.fedorahosted.org/pipermail/elfutils-devel/2014-March/003859.html

Re: vdso handling

[Pedro Alves]

On 03/13/2014 01:03 PM, Alan Modra wrote:
> On Thu, Mar 13, 2014 at 10:52:16AM +0100, Mark Wielaard wrote:
>> On Thu, 2014-03-13 at 11:31 +1030, Alan Modra wrote:
>>> It wouldn't
>>> help in the vdso case anyway, since the problem there is that you only
>>> have the loaded part of the original ELF file.
>>
>> Note that the vdso is often special, compared to other ELF dsos, because
>> the loaded part is just the complete ELF image in memory. Since they are
>> very simple they will just have one PT_LOAD at offset zero and if the
>> image is smaller than the page size then the whole file is just simply
>> mapped into memory completely. So by fetching the vdso ELF image from
>> remote memory you should be able to get the section headers and the
>> not-allocated sections too.
> 
> Yes, but if the vdso does not fit in a page (which incidentally is
> inferred by program header p_align), then you may lose the section
> headers.  I was assuming this was the case.

Hmm.  How so?  On x86 (arch/x86/vdso/vdso.S), the kernel just does:

        .globl vdso_start, vdso_end
        .align PAGE_SIZE
vdso_start:
        .incbin "arch/x86/vdso/vdso.so"
vdso_end:
        .align PAGE_SIZE /* extra data here leaks to userspace. */


And then arch/x86/vdso/vma.c has:

static int __init init_vdso(void)
{
        int npages = (vdso_end - vdso_start + PAGE_SIZE - 1) / PAGE_SIZE;
        int i;

        patch_vdso64(vdso_start, vdso_end - vdso_start);

        vdso_size = npages << PAGE_SHIFT;
        for (i = 0; i < npages; i++)
                vdso_pages[i] = virt_to_page(vdso_start + i*PAGE_SIZE);

#ifdef CONFIG_X86_X32_ABI
        patch_vdsox32(vdsox32_start, vdsox32_end - vdsox32_start);
        npages = (vdsox32_end - vdsox32_start + PAGE_SIZE - 1) / PAGE_SIZE;
        vdsox32_size = npages << PAGE_SHIFT;
        for (i = 0; i < npages; i++)
                vdsox32_pages[i] = virt_to_page(vdsox32_start + i*PAGE_SIZE);
#endif

        return 0;
}

And patch_vdso64 _relies_ on sections being present at runtime:

static void __init patch_vdso64(void *vdso, size_t len)
{
        Elf64_Ehdr *hdr = vdso;
        Elf64_Shdr *sechdrs, *alt_sec = 0;
        char *secstrings;
        void *alt_data;
        int i;

        BUG_ON(len < sizeof(Elf64_Ehdr));
        BUG_ON(memcmp(hdr->e_ident, ELFMAG, SELFMAG) != 0);

        sechdrs = (void *)hdr + hdr->e_shoff;
        secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;

        for (i = 1; i < hdr->e_shnum; i++) {
                Elf64_Shdr *shdr = &sechdrs[i];
                if (!strcmp(secstrings + shdr->sh_name, ".altinstructions")) {
                        alt_sec = shdr;
                        goto found;
                }
        }

        /* If we get here, it's probably a bug. */
        pr_warning("patch_vdso64: .altinstructions not found\n");
        return;  /* nothing to patch */

found:
        alt_data = (void *)hdr + alt_sec->sh_offset;
        apply_alternatives(alt_data, alt_data + alt_sec->sh_size);
}

On e.g., arch/powerpc/kernel/vdso.c, I see even a lot more
code looking at the sections of the vdso.

-- 
Pedro Alves

Re: vdso handling

[Pedro Alves]

On 03/13/2014 02:59 PM, Pedro Alves wrote:

> Hmm.  How so?  On x86 (arch/x86/vdso/vdso.S), the kernel just does:

Hmm, guess I should really be looking at where the vdso is
actually mapped to a process's address space...  (no idea where
that is).

-- 
Pedro Alves

Re: vdso handling

[Pedro Alves]

On 03/13/2014 03:04 PM, Pedro Alves wrote:
> On 03/13/2014 02:59 PM, Pedro Alves wrote:
> 
>> Hmm.  How so?  On x86 (arch/x86/vdso/vdso.S), the kernel just does:
> 
> Hmm, guess I should really be looking at where the vdso is
> actually mapped to a process's address space...  (no idea where
> that is).
> 

I think I found it in the same file (arch/x86/vdso/vma.c):

int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
{
	return setup_additional_pages(bprm, uses_interp, vdso_pages,
				      vdso_size);
}

vdso_size comes from:

static int __init init_vdso(void)
{
	int npages = (vdso_end - vdso_start + PAGE_SIZE - 1) / PAGE_SIZE;
	int i;

	patch_vdso64(vdso_start, vdso_end - vdso_start);

	vdso_size = npages << PAGE_SHIFT;


So it seems like the whole vdso should be always mapped in.

I didn't look at the whole mode compat mess, but if it behaves
differently, it'd sound like a kernel bug to me.

-- 
Pedro Alves

Re: vdso handling

[Alan Modra]

On Thu, Mar 13, 2014 at 03:26:18PM +0000, Pedro Alves wrote:
> So it seems like the whole vdso should be always mapped in.

OK, so I think Markus should be looking at why bfd_from_remote_memory
decides to exclude the section headers.  ie. why the following code
is being executed

  /* If the segments visible in memory didn't include the section headers,
     then clear them from the file header.  */
  if ((bfd_vma) contents_size < (i_ehdr.e_shoff
				 + i_ehdr.e_shnum * i_ehdr.e_shentsize))
    {
      memset (&x_ehdr.e_shoff, 0, sizeof x_ehdr.e_shoff);
      memset (&x_ehdr.e_shnum, 0, sizeof x_ehdr.e_shnum);
      memset (&x_ehdr.e_shstrndx, 0, sizeof x_ehdr.e_shstrndx);
    }

It may be that we can tweak the heuristic.  One thing I noticed is
that the following

  /* Trim the last segment so we don't bother with zeros in the last page
     that are off the end of the file.  However, if the extra bit in that
     page includes the section headers, keep them.  */

will trim off .symtab which might otherwise be available.  GNU ld places
.symtab after the section headers.

-- 
Alan Modra
Australia Development Lab, IBM

RE: vdso handling

[Metzger, Markus T]

> -----Original Message-----
> From: Alan Modra [mailto:amodra@gmail.com]
> Sent: Friday, March 14, 2014 12:54 AM


> OK, so I think Markus should be looking at why bfd_from_remote_memory
> decides to exclude the section headers.  ie. why the following code
> is being executed
> 
>   /* If the segments visible in memory didn't include the section headers,
>      then clear them from the file header.  */
>   if ((bfd_vma) contents_size < (i_ehdr.e_shoff
> 				 + i_ehdr.e_shnum * i_ehdr.e_shentsize))
>     {
>       memset (&x_ehdr.e_shoff, 0, sizeof x_ehdr.e_shoff);
>       memset (&x_ehdr.e_shnum, 0, sizeof x_ehdr.e_shnum);
>       memset (&x_ehdr.e_shstrndx, 0, sizeof x_ehdr.e_shstrndx);
>     }
> 
> It may be that we can tweak the heuristic.  One thing I noticed is
> that the following
> 
>   /* Trim the last segment so we don't bother with zeros in the last page
>      that are off the end of the file.  However, if the extra bit in that
>      page includes the section headers, keep them.  */
> 
> will trim off .symtab which might otherwise be available.  GNU ld places
> .symtab after the section headers.

Something like this?

diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index 20101be..601d7ea 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -1616,7 +1616,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   bfd_byte *contents;
   int err;
   unsigned int i;
-  bfd_vma loadbase;
+  bfd_vma loadbase, shdr_begin, shdr_end;
   bfd_boolean loadbase_set;
 
   /* Read in the ELF header in external format.  */
@@ -1728,20 +1728,16 @@ NAME(_bfd_elf,bfd_from_remote_memory)
     }
 
   /* Trim the last segment so we don't bother with zeros in the last page
-     that are off the end of the file.  However, if the extra bit in that
-     page includes the section headers, keep them.  */
-  if ((bfd_vma) contents_size > last_phdr->p_offset + last_phdr->p_filesz
-      && (bfd_vma) contents_size >= (i_ehdr.e_shoff
-				     + i_ehdr.e_shnum * i_ehdr.e_shentsize))
-    {
-      contents_size = last_phdr->p_offset + last_phdr->p_filesz;
-      if ((bfd_vma) contents_size < (i_ehdr.e_shoff
-				     + i_ehdr.e_shnum * i_ehdr.e_shentsize))
-	contents_size = i_ehdr.e_shoff + i_ehdr.e_shnum * i_ehdr.e_shentsize;
-    }
-  else
+     that are off the end of the file.  */
+  if ((bfd_vma) contents_size > last_phdr->p_offset + last_phdr->p_filesz)
     contents_size = last_phdr->p_offset + last_phdr->p_filesz;
 
+  /* Keep the section headers.  */
+  shdr_begin = i_ehdr.e_shoff;
+  shdr_end = shdr_begin + i_ehdr.e_shnum * i_ehdr.e_shentsize;
+  if (shdr_end > (bfd_vma) contents_size)
+    contents_size = shdr_end;
+
   /* Now we know the size of the whole image we want read in.  */
   contents = (bfd_byte *) bfd_zmalloc (contents_size);
   if (contents == NULL)
@@ -1773,18 +1769,19 @@ NAME(_bfd_elf,bfd_from_remote_memory)
       }
   free (x_phdrs);
 
-  /* If the segments visible in memory didn't include the section headers,
-     then clear them from the file header.  */
-  if ((bfd_vma) contents_size < (i_ehdr.e_shoff
-				 + i_ehdr.e_shnum * i_ehdr.e_shentsize))
-    {
-      memset (&x_ehdr.e_shoff, 0, sizeof x_ehdr.e_shoff);
-      memset (&x_ehdr.e_shnum, 0, sizeof x_ehdr.e_shnum);
-      memset (&x_ehdr.e_shstrndx, 0, sizeof x_ehdr.e_shstrndx);
-    }
+  /* Copy the section headers.  */
+  err = target_read_memory (ehdr_vma + shdr_begin,
+			    contents + shdr_begin, shdr_end - shdr_begin);
+  if (err)
+  {
+    free (contents);
+    bfd_set_error (bfd_error_system_call);
+    errno = err;
+    return NULL;
+  }
 
   /* This will normally have been in the first PT_LOAD segment.  But it
-     conceivably could be missing, and we might have just changed it.  */
+     conceivably could be missing.  */
   memcpy (contents, &x_ehdr, sizeof x_ehdr);
 
   /* Now we have a memory image of the ELF file contents.  Make a BFD.  */


Would it be OK to send this patch as part of a GDB patch series with
binutils-patches and you CC'ed?  Or do you want a separate patch
only to binutils-patches?

This patch alone runs without new fails in the gdb and binutils suite
on x86-64 Fedora 19.  For the latter I ran "make check" from
$build/binutils, where $build is my build directory from I which I called
configure.

In combination with my other GDB changes, it shows regressions in
gdb.base/break-interp.exp.  I'll send the patch once I fixed them.

thanks,
Markus.

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: vdso handling

[Alan Modra]

On Tue, Mar 18, 2014 at 03:09:58PM +0000, Metzger, Markus T wrote:
> diff --git a/bfd/elfcode.h b/bfd/elfcode.h
> index 20101be..601d7ea 100644
> --- a/bfd/elfcode.h
> +++ b/bfd/elfcode.h
> @@ -1616,7 +1616,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
>    bfd_byte *contents;
>    int err;
>    unsigned int i;
> -  bfd_vma loadbase;
> +  bfd_vma loadbase, shdr_begin, shdr_end;
>    bfd_boolean loadbase_set;
>  
>    /* Read in the ELF header in external format.  */
> @@ -1728,20 +1728,16 @@ NAME(_bfd_elf,bfd_from_remote_memory)
>      }
>  
>    /* Trim the last segment so we don't bother with zeros in the last page
> -     that are off the end of the file.  However, if the extra bit in that
> -     page includes the section headers, keep them.  */
> -  if ((bfd_vma) contents_size > last_phdr->p_offset + last_phdr->p_filesz
> -      && (bfd_vma) contents_size >= (i_ehdr.e_shoff
> -				     + i_ehdr.e_shnum * i_ehdr.e_shentsize))
> -    {
> -      contents_size = last_phdr->p_offset + last_phdr->p_filesz;
> -      if ((bfd_vma) contents_size < (i_ehdr.e_shoff
> -				     + i_ehdr.e_shnum * i_ehdr.e_shentsize))
> -	contents_size = i_ehdr.e_shoff + i_ehdr.e_shnum * i_ehdr.e_shentsize;
> -    }
> -  else
> +     that are off the end of the file.  */
> +  if ((bfd_vma) contents_size > last_phdr->p_offset + last_phdr->p_filesz)
>      contents_size = last_phdr->p_offset + last_phdr->p_filesz;
>  
> +  /* Keep the section headers.  */
> +  shdr_begin = i_ehdr.e_shoff;
> +  shdr_end = shdr_begin + i_ehdr.e_shnum * i_ehdr.e_shentsize;
> +  if (shdr_end > (bfd_vma) contents_size)
> +    contents_size = shdr_end;
> +

I don't think this is a good idea.  If/when bfd_from_remote_memory is
used for something other than the linux kernel vdso, we can't assume
the section headers are loaded.  The original code made the assumption
that the highest address loaded from a PT_LOAD header was rounded up
to a page, and that the page size could be inferred from p_align.
Here:
	segment_end = (i_phdrs[i].p_offset + i_phdrs[i].p_filesz
		       + i_phdrs[i].p_align - 1) & -i_phdrs[i].p_align;
	if (segment_end > (bfd_vma) contents_size)
	  contents_size = segment_end;

Now, p_align is generally set from the page size if using GNU ld, but
I'm wondering if your vdso somehow doesn't have that property.  Can
you show us your vdso readelf -e output?  If p_align isn't set to a
page, then the change in heuristic I envision is to make use of
elf_backend_data maxpagesize to figure out which parts of the image
might be loaded.  If that isn't enough then perhaps we should add
another parameter to bfd_from_remote_memory to allow its caller to
specify the end of the image.

> Would it be OK to send this patch as part of a GDB patch series with
> binutils-patches and you CC'ed?  Or do you want a separate patch
> only to binutils-patches?

I don't mind either way.  This part of bfd belongs to gdb, so gdb
maintainers really have the final say on patch approval.  I'm just
someone who happened to become interested in the problem..

-- 
Alan Modra
Australia Development Lab, IBM

RE: vdso handling

[Metzger, Markus T]

> -----Original Message-----
> From: Alan Modra [mailto:amodra@gmail.com]
> Sent: Wednesday, March 19, 2014 12:10 AM

> Now, p_align is generally set from the page size if using GNU ld, but
> I'm wondering if your vdso somehow doesn't have that property.  Can
> you show us your vdso readelf -e output?

ELF Header:
  Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00 
  Class:                             ELF64
  Data:                              2's complement, little endian
  Version:                           1 (current)
  OS/ABI:                            UNIX - System V
  ABI Version:                       0
  Type:                              DYN (Shared object file)
  Machine:                           Advanced Micro Devices X86-64
  Version:                           0x1
  Entry point address:               0xffffffffff700700
  Start of program headers:          64 (bytes into file)
  Start of section headers:          4320 (bytes into file)
  Flags:                             0x0
  Size of this header:               64 (bytes)
  Size of program headers:           56 (bytes)
  Number of program headers:         4
  Size of section headers:           64 (bytes)
  Number of section headers:         18
  Section header string table index: 17

Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            0000000000000000 000000 000000 00      0   0  0
  [ 1] .hash             HASH            ffffffffff700120 000120 000040 04   A  2   0  8
  [ 2] .dynsym           DYNSYM          ffffffffff700160 000160 000108 18   A  3   2  8
  [ 3] .dynstr           STRTAB          ffffffffff700268 000268 00005e 00   A  0   0  1
  [ 4] .gnu.version      VERSYM          ffffffffff7002c6 0002c6 000016 02   A  2   0  2
  [ 5] .gnu.version_d    VERDEF          ffffffffff7002e0 0002e0 000038 00   A  3   2  8
  [ 6] .note             NOTE            ffffffffff700318 000318 00003c 00   A  0   0  4
  [ 7] .eh_frame_hdr     PROGBITS        ffffffffff700354 000354 00002c 00   A  0   0  4
  [ 8] .eh_frame         PROGBITS        ffffffffff700380 000380 0000e8 00   A  0   0  8
  [ 9] .dynamic          DYNAMIC         ffffffffff700468 000468 0000f0 10  WA  3   0  8
  [10] .rodata           PROGBITS        ffffffffff700558 000558 000020 01 AMS  0   0  8
  [11] __bug_table       PROGBITS        ffffffffff700578 000578 000018 00   A  0   0  1
  [12] .discard          PROGBITS        ffffffffff700590 000590 000012 00  WA  0   0  1
  [13] .altinstructions  PROGBITS        ffffffffff7005a2 0005a2 0000d8 00   A  0   0  1
  [14] .altinstr_replacement PROGBITS        ffffffffff70067a 00067a 000036 00  AX  0   0  1
  [15] .text             PROGBITS        ffffffffff700700 000700 0008fd 00  AX  0   0 16
  [16] .comment          PROGBITS        0000000000000000 000ffd 00002c 01  MS  0   0  1
  [17] .shstrtab         STRTAB          0000000000000000 001029 0000b7 00      0   0  1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings), l (large)
  I (info), L (link order), G (group), T (TLS), E (exclude), x (unknown)
  O (extra OS processing required) o (OS specific), p (processor specific)

Program Headers:
  Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
  LOAD           0x000000 0xffffffffff700000 0xffffffffff700000 0x000ffd 0x000ffd R E 0x1000
  DYNAMIC        0x000468 0xffffffffff700468 0xffffffffff700468 0x0000f0 0x0000f0 R   0x8
  NOTE           0x000318 0xffffffffff700318 0xffffffffff700318 0x00003c 0x00003c R   0x4
  GNU_EH_FRAME   0x000354 0xffffffffff700354 0xffffffffff700354 0x00002c 0x00002c R   0x4

 Section to Segment mapping:
  Segment Sections...
   00     .hash .dynsym .dynstr .gnu.version .gnu.version_d .note .eh_frame_hdr .eh_frame .dynamic .rodata __bug_table .discard .altinstructions .altinstr_replacement .text 
   01     .dynamic 
   02     .note 
   03     .eh_frame_hdr

Regards,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

RE: vdso handling

[Metzger, Markus T]

> -----Original Message-----
> From: Alan Modra [mailto:amodra@gmail.com]
> Sent: Wednesday, March 19, 2014 12:10 AM


> > +  /* Keep the section headers.  */
> > +  shdr_begin = i_ehdr.e_shoff;
> > +  shdr_end = shdr_begin + i_ehdr.e_shnum * i_ehdr.e_shentsize;
> > +  if (shdr_end > (bfd_vma) contents_size)
> > +    contents_size = shdr_end;
> > +
> 
> I don't think this is a good idea.  If/when bfd_from_remote_memory is
> used for something other than the linux kernel vdso, we can't assume
> the section headers are loaded.

Shouldn't the ehdr indicate that there are no sections in this case?


> The original code made the assumption
> that the highest address loaded from a PT_LOAD header was rounded up
> to a page, and that the page size could be inferred from p_align.
> Here:
> 	segment_end = (i_phdrs[i].p_offset + i_phdrs[i].p_filesz
> 		       + i_phdrs[i].p_align - 1) & -i_phdrs[i].p_align;
> 	if (segment_end > (bfd_vma) contents_size)
> 	  contents_size = segment_end;
> 
> Now, p_align is generally set from the page size if using GNU ld, but
> I'm wondering if your vdso somehow doesn't have that property.  Can
> you show us your vdso readelf -e output?  If p_align isn't set to a
> page, then the change in heuristic I envision is to make use of
> elf_backend_data maxpagesize to figure out which parts of the image
> might be loaded.  If that isn't enough then perhaps we should add
> another parameter to bfd_from_remote_memory to allow its caller to
> specify the end of the image.

I'm not sure the caller knows.  GDB gets the base address from AUXV's
AT_SYSINFO_EHDR.

If we can't trust the image to contain everything that the ELF header
describes, would it be safer to generate fake sections based on the
program header?  We already assume that the program header is
contained in the image.

Regards,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: vdso handling

[Pedro Alves]

On 03/19/2014 08:29 AM, Metzger, Markus T wrote:
> If we can't trust the image to contain everything that the ELF header
> describes, would it be safer to generate fake sections based on the
> program header?  We already assume that the program header is
> contained in the image.

Please let's try to make this work before giving up and going
in that direction.

-- 
Pedro Alves

Re: vdso handling

[Alan Modra]

On Wed, Mar 19, 2014 at 08:29:47AM +0000, Metzger, Markus T wrote:
> Shouldn't the ehdr indicate that there are no sections in this case?

Nope.  See my other email to Pedro.

> If we can't trust the image to contain everything that the ELF header
> describes, would it be safer to generate fake sections based on the
> program header?  We already assume that the program header is
> contained in the image.

Yes, you're correct that it is wrong to assume program headers are
loaded.  Even worse, the in-memory image doesn't even need to contain
the ELF file header.

-- 
Alan Modra
Australia Development Lab, IBM

Re: vdso handling

[Pedro Alves]

On 03/20/2014 01:59 AM, Alan Modra wrote:
> On Wed, Mar 19, 2014 at 08:29:47AM +0000, Metzger, Markus T wrote:
>> Shouldn't the ehdr indicate that there are no sections in this case?
> 
> Nope.  See my other email to Pedro.
> 
>> If we can't trust the image to contain everything that the ELF header
>> describes, would it be safer to generate fake sections based on the
>> program header?  We already assume that the program header is
>> contained in the image.
> 
> Yes, you're correct that it is wrong to assume program headers are
> loaded.  Even worse, the in-memory image doesn't even need to contain
> the ELF file header.

Yeah, and I was just assuming it didn't, hence my "just trust the
headers" push before.

I'm now thinking that we'll need pseudo-sections from program
headers anyway, so I'd suggest going in that direction, leaving
the add-symbol-file-from-memory command's intention generic,
and leave revisiting how gdb retrieves the vdso itself off of
memory for another day.

-- 
Pedro Alves

RE: vdso handling

[Metzger, Markus T]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Friday, March 21, 2014 4:55 PM


> >> If we can't trust the image to contain everything that the ELF header
> >> describes, would it be safer to generate fake sections based on the
> >> program header?  We already assume that the program header is
> >> contained in the image.
> >
> > Yes, you're correct that it is wrong to assume program headers are
> > loaded.  Even worse, the in-memory image doesn't even need to contain
> > the ELF file header.
> 
> Yeah, and I was just assuming it didn't, hence my "just trust the
> headers" push before.
> 
> I'm now thinking that we'll need pseudo-sections from program
> headers anyway, so I'd suggest going in that direction, leaving
> the add-symbol-file-from-memory command's intention generic,
> and leave revisiting how gdb retrieves the vdso itself off of
> memory for another day.

That would be something like the patch in one of the previous emails
in this thread: https://sourceware.org/ml/gdb/2014-03/msg00028.html,
wouldn't it?

Alan, would you be OK with this, as well?

Thanks,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: vdso handling

[Pedro Alves]

On 03/18/2014 11:09 PM, Alan Modra wrote:
> I don't think this is a good idea.  If/when bfd_from_remote_memory is
> used for something other than the linux kernel vdso, we can't assume
> the section headers are loaded.  

I wonder what use cases are these though.  It'd be odd to me to
load the elf headers, but not all that the headers point at.
Sounds like we should just make that a requirement?

I looked at the history of this whole code, and here's what I found.

Roland originally added this bfd function for reading the Linux
vsyscall dso, in 8d6337fe:

 https://sourceware.org/ml/binutils/2003-05/msg00542.html

As far as I can judge from http://lwn.net/Articles/30258/ , and
from looking at the early days of the vsyscall dso in the Linux
tree, it looks like the vsyscall dso was always included complete
in memory (e.g., at v2.6.12-rc2, the initial git import):

...
/* 32bit VDSOs mapped into user space. */
asm(".section \".init.data\",\"aw\"\n"
    "syscall32_syscall:\n"
    ".incbin \"arch/x86_64/ia32/vsyscall-syscall.so\"\n"
    "syscall32_syscall_end:\n"
    "syscall32_sysenter:\n"
    ".incbin \"arch/x86_64/ia32/vsyscall-sysenter.so\"\n"
    "syscall32_sysenter_end:\n"
    ".previous");
...

# The DSO images are built using a special linker script
quiet_cmd_syscall = SYSCALL $@
      cmd_syscall = $(CC) -m32 -nostdlib -shared -s \
                           -Wl,-soname=linux-gate.so.1 -o $@ \
                           -Wl,-T,$(filter-out FORCE,$^)

...

I found no sign of strip or of any special elf munging.

GDB only uses bfd_elf_bfd_from_remote_memory for the vdso,
and for "add-symbol-file-from-memory".

Roland himself added the "add-symbol-file-from-memory"
command (5417f6dc) to GDB too, at:

 https://www.sourceware.org/ml/gdb-patches/2003-10/msg00045.html

 "This command may not really be worth having, but it serves to exercise the
 underlying function symbol_file_add_from_memory.  That function does the
 work of reading symbols and unwind info from the Linux vsyscall DSO."

-- 
Pedro Alves

Re: vdso handling

[Alan Modra]

On Wed, Mar 19, 2014 at 12:03:40PM +0000, Pedro Alves wrote:
> On 03/18/2014 11:09 PM, Alan Modra wrote:
> > I don't think this is a good idea.  If/when bfd_from_remote_memory is
> > used for something other than the linux kernel vdso, we can't assume
> > the section headers are loaded.  
> 
> I wonder what use cases are these though.  It'd be odd to me to
> load the elf headers, but not all that the headers point at.

Well, no, the section headers are part of the linking view.  The ELF
file header points at their *file offset*.  Thinking it odd that they
are not loaded is exactly the same as thinking it odd that .comment is
not loaded!

> Sounds like we should just make that a requirement?

You indeed could make that a requirement, but it'd mean that object
files loaded by glibc ld.so would not satisfy the requirement.

Taking Markus' vdso as an example, the PT_LOAD header covers file
offsets from 0 to 0xffd and page size is 0x1000.  If ld.so were
loading a similar image, it would just load in 0 to 0xfff (assuming
ld.so's dl_pagesize is also 0x1000).  The section headers are at file
offset 0x10e0 so would miss being loaded.  Nor should they be loaded,
since they are completely extraneous to executing the image it would
be foolish to waste another page to load them.

>  "This command may not really be worth having, but it serves to exercise the
>  underlying function symbol_file_add_from_memory.  That function does the
>  work of reading symbols and unwind info from the Linux vsyscall DSO."

Heh, OK, so don't let my comments about more general uses of
bfd_from_remote_memory get in your way of fixing this problem.

Hmm, if you only want to read vdsos then bfd_from_remote_memory is way
overengineered.  In fact, I think it could disappear entirely.
If you can find the extent of the vdso in gdb, then all of the ELF
version of bfd_from_remote_memory prior to allocating the bim is
unnecessary, because all that code really does is find the extent.
So the tail of bfd_from_remote_memory moves to bfd/opncls.c as
(totally untested):

bfd_boolean
bfd_openr_bim (bfd *templ, void *contents, size_t contents_size)
{
  struct bfd_in_memory *bim;

  bim = (struct bfd_in_memory *) bfd_malloc (sizeof (struct bfd_in_memory));
  if (bim == NULL)
    return NULL;

  nbfd = _bfd_new_bfd ();
  if (nbfd == NULL)
    {
      free (bim);
      return NULL;
    }
  nbfd->filename = "<in-memory>";
  if (templ != NULL)
    nbfd->xvec = templ->xvec;
  bim->size = contents_size;
  bim->buffer = (bfd_byte *) contents;
  nbfd->iostream = bim;
  nbfd->flags = BFD_IN_MEMORY;
  nbfd->iovec = &_bfd_memory_iovec;
  nbfd->origin = 0;
  nbfd->direction = read_direction;
  nbfd->mtime = time (NULL);
  nbfd->mtime_set = TRUE;
  return nbfd;
}

gdb is then responsible for filling in "contents" and determining
"contents_size", which presumably can be done in the same way as
you did in https://sourceware.org/ml/binutils/2014-03/msg00130.html
The loadbase calculation also moves to gdb, which shouldn't be too
hard.  Note that "templ" above is optional, which allows you to get
rid of
       /* FIXME: cagney/2004-05-06: Should not require an existing
         BFD when trying to create a run-time BFD of the VSYSCALL

-- 
Alan Modra
Australia Development Lab, IBM

RE: vdso handling

[Metzger, Markus T]

> -----Original Message-----
> From: Alan Modra [mailto:amodra@gmail.com]
> Sent: Thursday, March 20, 2014 2:33 AM


> On Wed, Mar 19, 2014 at 12:03:40PM +0000, Pedro Alves wrote:
> > On 03/18/2014 11:09 PM, Alan Modra wrote:
> > > I don't think this is a good idea.  If/when bfd_from_remote_memory is
> > > used for something other than the linux kernel vdso, we can't assume
> > > the section headers are loaded.
> >
> > I wonder what use cases are these though.  It'd be odd to me to
> > load the elf headers, but not all that the headers point at.
> 
> Well, no, the section headers are part of the linking view.  The ELF
> file header points at their *file offset*.  Thinking it odd that they
> are not loaded is exactly the same as thinking it odd that .comment is
> not loaded!
> 
> > Sounds like we should just make that a requirement?
> 
> You indeed could make that a requirement, but it'd mean that object
> files loaded by glibc ld.so would not satisfy the requirement.
> 
> Taking Markus' vdso as an example, the PT_LOAD header covers file
> offsets from 0 to 0xffd and page size is 0x1000.  If ld.so were
> loading a similar image, it would just load in 0 to 0xfff (assuming
> ld.so's dl_pagesize is also 0x1000).  The section headers are at file
> offset 0x10e0 so would miss being loaded.  Nor should they be loaded,
> since they are completely extraneous to executing the image it would
> be foolish to waste another page to load them.
> 
> >  "This command may not really be worth having, but it serves to exercise
> the
> >  underlying function symbol_file_add_from_memory.  That function does
> the
> >  work of reading symbols and unwind info from the Linux vsyscall DSO."
> 
> Heh, OK, so don't let my comments about more general uses of
> bfd_from_remote_memory get in your way of fixing this problem.
> 
> Hmm, if you only want to read vdsos then bfd_from_remote_memory is way
> overengineered.  In fact, I think it could disappear entirely.
> If you can find the extent of the vdso in gdb, then all of the ELF
> version of bfd_from_remote_memory prior to allocating the bim is
> unnecessary, because all that code really does is find the extent.
> So the tail of bfd_from_remote_memory moves to bfd/opncls.c as
> (totally untested):
> 
> bfd_boolean
> bfd_openr_bim (bfd *templ, void *contents, size_t contents_size)
> {
>   struct bfd_in_memory *bim;
> 
>   bim = (struct bfd_in_memory *) bfd_malloc (sizeof (struct
> bfd_in_memory));
>   if (bim == NULL)
>     return NULL;
> 
>   nbfd = _bfd_new_bfd ();
>   if (nbfd == NULL)
>     {
>       free (bim);
>       return NULL;
>     }
>   nbfd->filename = "<in-memory>";
>   if (templ != NULL)
>     nbfd->xvec = templ->xvec;
>   bim->size = contents_size;
>   bim->buffer = (bfd_byte *) contents;
>   nbfd->iostream = bim;
>   nbfd->flags = BFD_IN_MEMORY;
>   nbfd->iovec = &_bfd_memory_iovec;
>   nbfd->origin = 0;
>   nbfd->direction = read_direction;
>   nbfd->mtime = time (NULL);
>   nbfd->mtime_set = TRUE;
>   return nbfd;
> }
> 
> gdb is then responsible for filling in "contents" and determining
> "contents_size", which presumably can be done in the same way as
> you did in https://sourceware.org/ml/binutils/2014-03/msg00130.html
> The loadbase calculation also moves to gdb, which shouldn't be too
> hard.  Note that "templ" above is optional, which allows you to get
> rid of
>        /* FIXME: cagney/2004-05-06: Should not require an existing
>          BFD when trying to create a run-time BFD of the VSYSCALL

I was waiting for Pedro to reply but he didn't.

This would move the ELF processing itself into GDB.  That sounds a bit
odd to me but it seems GDB already does a fair amount of ELF reading.

Pedro, are you OK with this?  Will you accept a patch that goes into
the direction that Alan described above?

Thanks,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: vdso handling

[Pedro Alves]

On 03/20/2014 01:33 AM, Alan Modra wrote:
> On Wed, Mar 19, 2014 at 12:03:40PM +0000, Pedro Alves wrote:
>> On 03/18/2014 11:09 PM, Alan Modra wrote:
>>> I don't think this is a good idea.  If/when bfd_from_remote_memory is
>>> used for something other than the linux kernel vdso, we can't assume
>>> the section headers are loaded.  
>>
>> I wonder what use cases are these though.  It'd be odd to me to
>> load the elf headers, but not all that the headers point at.
> 
> Well, no, the section headers are part of the linking view.  The ELF
> file header points at their *file offset*.  Thinking it odd that they
> are not loaded is exactly the same as thinking it odd that .comment is
> not loaded!

:-)  Sorry, I get that, but I was somehow assuming the elf header
wasn't itself covered by a PT_LOAD.  Rookie mistake...

>> Sounds like we should just make that a requirement?
> 
> You indeed could make that a requirement, but it'd mean that object
> files loaded by glibc ld.so would not satisfy the requirement.

Indeed.  So this mechanism could be further extended in future
to be able to read DSOs out of memory, and be able to retrieve
the dynamic symbol table, etc., which would allow getting at
unwind info for already-deleted libraries.  I guess that's what
the elfutils folks had been talking about, but I somehow managed
to not connect the dots.  :-)

I just tried pointing add-symbol-file-from-memory at an already
mapped DSO's elf header, but it doesn't work as is unfortunately:

 (gdb) info shared curses
 0x000000324d006d20  0x000000324d01df58  Yes         /lib64/libncurses.so.5
 (gdb) x /4b 0x000000324d000000
 0x324d000000:   127     69      76      70
 (gdb) add-symbol-file-from-memory 0x000000324d000000
 Failed to read a valid object file image from memory.

I single stepped a little through
bfd_elf_bfd_from_remote_memory - something goes wrong with the
reading of the load segment contents, probably something wrong
with the address computations.

> 
> Taking Markus' vdso as an example, the PT_LOAD header covers file
> offsets from 0 to 0xffd and page size is 0x1000.  If ld.so were
> loading a similar image, it would just load in 0 to 0xfff (assuming
> ld.so's dl_pagesize is also 0x1000).  The section headers are at file
> offset 0x10e0 so would miss being loaded.  Nor should they be loaded,
> since they are completely extraneous to executing the image it would
> be foolish to waste another page to load them.

*nod*

>>  "This command may not really be worth having, but it serves to exercise the
>>  underlying function symbol_file_add_from_memory.  That function does the
>>  work of reading symbols and unwind info from the Linux vsyscall DSO."
> 
> Heh, OK, so don't let my comments about more general uses of
> bfd_from_remote_memory get in your way of fixing this problem.

Hmm.  Thinking further ahead, it would be good to make GDB be able
to read deleted DSOs off of memory like elfutils is now doing.
With that in mind, it does seem to make sense to have this
bfd_elf_bfd_from_remote_memory code in place as is, but extend it to
treat section-less elfs just like cores, creating pseudo bfd
sections from segments...

-- 
Pedro Alves

Re: vdso handling

[Alan Modra]

On Fri, Mar 21, 2014 at 03:48:48PM +0000, Pedro Alves wrote:
> I just tried pointing add-symbol-file-from-memory at an already
> mapped DSO's elf header, but it doesn't work as is unfortunately:
> 
>  (gdb) info shared curses
>  0x000000324d006d20  0x000000324d01df58  Yes         /lib64/libncurses.so.5
>  (gdb) x /4b 0x000000324d000000
>  0x324d000000:   127     69      76      70
>  (gdb) add-symbol-file-from-memory 0x000000324d000000
>  Failed to read a valid object file image from memory.
> 
> I single stepped a little through
> bfd_elf_bfd_from_remote_memory - something goes wrong with the
> reading of the load segment contents, probably something wrong
> with the address computations.

readelf -a --wide on my x86_64 libncurses.so.5 shows

[snip]
  Start of section headers:          132144 (bytes into file)
[snip]
  [25] .shstrtab         STRTAB          0000000000000000 02034c 0000de 00      0   0  1
[snip]
  LOAD           0x000000 0x0000000000000000 0x0000000000000000 0x01efe4 0x01efe4 R E 0x200000
  LOAD           0x01fd50 0x000000000021fd50 0x000000000021fd50 0x0005e4 0x000770 RW  0x200000

So .shstrtab and the section headers might have been loaded by the
second PT_LOAD header, *but* the second PT_LOAD has a bss area.
Anything past 0x220334 will be cleared out by ld.so.  No chance of
getting at section headers then, and this will be true for most
in-memory images.

bfd_from_remote_memory should take note of p_memsz..  Hmm, and there
are quite a few other issues there too, most notably that p_align
on x86_64 these days tends to be *much* larger than the page size used
by ld.so.

Gah, I've been sucked into looking at this long enough that I may as
well fix it.  Does this look OK?

bfd/
	* elfcode.h (bfd_from_remote_memory): Add "size" parameter.
	Consolidate code handling possible section headers past end of
	segment.  Don't use p_align for page size guess, instead use
	minpagesize.  Take note of ld.so clearing section headers when
	p_memsz > p_filesz.  Handle file header specifying no section
	headers.  Handle zero p_align throughout.  Default loadbase to
	zero.  Add comments.  Rename contents_size to high_offset, and
	make it a bfd_vma.  Delete unnecessary bfd_set_error calls.
	* bfd-in.h (bfd_elf_bfd_from_remote_memory): Update prototpe.
	* elf-bfd.h (struct elf_backend_data <elf_backend_from_remote_memory>):
	Likewise.
	(_bfd_elf32_bfd_from_remote_memory): Likewise.
	(_bfd_elf64_bfd_from_remote_memory): Likewise.
	* elf.c (bfd_elf_bfd_from_remote_memory): Adjust.
	* bfd-in2.h: Regnerate.
gdb/
	* symfile-mem.c (symbol_file_add_from_memory): Add size parameter.
	Pass to bfd_elf_bfd_from_remote_memory.  Adjust all callers.
	(struct symbol_file_add_from_memory_args): Add size field.
	(find_vdso_size): New function.
	(add_vsyscall_page): Attempt to find vdso size.

diff --git a/bfd/bfd-in.h b/bfd/bfd-in.h
index da350a5..ddc8270 100644
--- a/bfd/bfd-in.h
+++ b/bfd/bfd-in.h
@@ -680,19 +680,21 @@ extern int bfd_get_elf_phdrs
   (bfd *abfd, void *phdrs);
 
 /* Create a new BFD as if by bfd_openr.  Rather than opening a file,
-   reconstruct an ELF file by reading the segments out of remote memory
-   based on the ELF file header at EHDR_VMA and the ELF program headers it
-   points to.  If not null, *LOADBASEP is filled in with the difference
-   between the VMAs from which the segments were read, and the VMAs the
-   file headers (and hence BFD's idea of each section's VMA) put them at.
-
-   The function TARGET_READ_MEMORY is called to copy LEN bytes from the
-   remote memory at target address VMA into the local buffer at MYADDR; it
-   should return zero on success or an `errno' code on failure.  TEMPL must
-   be a BFD for an ELF target with the word size and byte order found in
-   the remote memory.  */
+   reconstruct an ELF file by reading the segments out of remote
+   memory based on the ELF file header at EHDR_VMA and the ELF program
+   headers it points to.  If non-zero, SIZE is the known extent of the
+   object.  If not null, *LOADBASEP is filled in with the difference
+   between the VMAs from which the segments were read, and the VMAs
+   the file headers (and hence BFD's idea of each section's VMA) put
+   them at.
+
+   The function TARGET_READ_MEMORY is called to copy LEN bytes from
+   the remote memory at target address VMA into the local buffer at
+   MYADDR; it should return zero on success or an `errno' code on
+   failure.  TEMPL must be a BFD for a target with the word size and
+   byte order found in the remote memory.  */
 extern bfd *bfd_elf_bfd_from_remote_memory
-  (bfd *templ, bfd_vma ehdr_vma, bfd_vma *loadbasep,
+  (bfd *templ, bfd_vma ehdr_vma, size_t size, bfd_vma *loadbasep,
    int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr,
 			      bfd_size_type len));
 
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index ee6e6cb..6d07303 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1191,9 +1191,9 @@ struct elf_backend_data
   /* This function implements `bfd_elf_bfd_from_remote_memory';
      see elf.c, elfcode.h.  */
   bfd *(*elf_backend_bfd_from_remote_memory)
-     (bfd *templ, bfd_vma ehdr_vma, bfd_vma *loadbasep,
-      int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr,
-				 bfd_size_type len));
+    (bfd *templ, bfd_vma ehdr_vma, size_t size, bfd_vma *loadbasep,
+     int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr,
+				bfd_size_type len));
 
   /* This function is used by `_bfd_elf_get_synthetic_symtab';
      see elf.c.  */
@@ -2334,10 +2334,10 @@ extern char *elfcore_write_ppc_linux_prpsinfo32
   (bfd *, char *, int *, const struct elf_internal_linux_prpsinfo *);
 
 extern bfd *_bfd_elf32_bfd_from_remote_memory
-  (bfd *templ, bfd_vma ehdr_vma, bfd_vma *loadbasep,
+  (bfd *templ, bfd_vma ehdr_vma, size_t size, bfd_vma *loadbasep,
    int (*target_read_memory) (bfd_vma, bfd_byte *, bfd_size_type));
 extern bfd *_bfd_elf64_bfd_from_remote_memory
-  (bfd *templ, bfd_vma ehdr_vma, bfd_vma *loadbasep,
+  (bfd *templ, bfd_vma ehdr_vma, size_t size, bfd_vma *loadbasep,
    int (*target_read_memory) (bfd_vma, bfd_byte *, bfd_size_type));
 
 extern bfd_vma bfd_elf_obj_attr_size (bfd *);
diff --git a/bfd/elf.c b/bfd/elf.c
index 3ded683..d67b917 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -9908,11 +9908,12 @@ bfd *
 bfd_elf_bfd_from_remote_memory
   (bfd *templ,
    bfd_vma ehdr_vma,
+   size_t size,
    bfd_vma *loadbasep,
    int (*target_read_memory) (bfd_vma, bfd_byte *, bfd_size_type))
 {
   return (*get_elf_backend_data (templ)->elf_backend_bfd_from_remote_memory)
-    (templ, ehdr_vma, loadbasep, target_read_memory);
+    (templ, ehdr_vma, size, loadbasep, target_read_memory);
 }
 \f
 long
diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index 20101be..31f67a8 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -1587,22 +1587,25 @@ elf_debug_file (Elf_Internal_Ehdr *ehdrp)
 #endif
 \f
 /* Create a new BFD as if by bfd_openr.  Rather than opening a file,
-   reconstruct an ELF file by reading the segments out of remote memory
-   based on the ELF file header at EHDR_VMA and the ELF program headers it
-   points to.  If not null, *LOADBASEP is filled in with the difference
-   between the VMAs from which the segments were read, and the VMAs the
-   file headers (and hence BFD's idea of each section's VMA) put them at.
-
-   The function TARGET_READ_MEMORY is called to copy LEN bytes from the
-   remote memory at target address VMA into the local buffer at MYADDR; it
-   should return zero on success or an `errno' code on failure.  TEMPL must
-   be a BFD for a target with the word size and byte order found in the
-   remote memory.  */
+   reconstruct an ELF file by reading the segments out of remote
+   memory based on the ELF file header at EHDR_VMA and the ELF program
+   headers it points to.  If non-zero, SIZE is the known extent of the
+   object.  If not null, *LOADBASEP is filled in with the difference
+   between the VMAs from which the segments were read, and the VMAs
+   the file headers (and hence BFD's idea of each section's VMA) put
+   them at.
+
+   The function TARGET_READ_MEMORY is called to copy LEN bytes from
+   the remote memory at target address VMA into the local buffer at
+   MYADDR; it should return zero on success or an `errno' code on
+   failure.  TEMPL must be a BFD for a target with the word size and
+   byte order found in the remote memory.  */
 
 bfd *
 NAME(_bfd_elf,bfd_from_remote_memory)
   (bfd *templ,
    bfd_vma ehdr_vma,
+   size_t size,
    bfd_vma *loadbasep,
    int (*target_read_memory) (bfd_vma, bfd_byte *, bfd_size_type))
 {
@@ -1612,10 +1615,11 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   Elf_Internal_Phdr *i_phdrs, *last_phdr;
   bfd *nbfd;
   struct bfd_in_memory *bim;
-  int contents_size;
   bfd_byte *contents;
   int err;
   unsigned int i;
+  bfd_vma high_offset;
+  bfd_vma shdr_end;
   bfd_vma loadbase;
   bfd_boolean loadbase_set;
 
@@ -1677,10 +1681,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   x_phdrs = (Elf_External_Phdr *)
       bfd_malloc (i_ehdr.e_phnum * (sizeof *x_phdrs + sizeof *i_phdrs));
   if (x_phdrs == NULL)
-    {
-      bfd_set_error (bfd_error_no_memory);
-      return NULL;
-    }
+    return NULL;
   err = target_read_memory (ehdr_vma + i_ehdr.e_phoff, (bfd_byte *) x_phdrs,
 			    i_ehdr.e_phnum * sizeof x_phdrs[0]);
   if (err)
@@ -1692,34 +1693,44 @@ NAME(_bfd_elf,bfd_from_remote_memory)
     }
   i_phdrs = (Elf_Internal_Phdr *) &x_phdrs[i_ehdr.e_phnum];
 
-  contents_size = 0;
+  high_offset = 0;
   last_phdr = NULL;
-  loadbase = ehdr_vma;
+  loadbase = 0;
   loadbase_set = FALSE;
   for (i = 0; i < i_ehdr.e_phnum; ++i)
     {
       elf_swap_phdr_in (templ, &x_phdrs[i], &i_phdrs[i]);
       if (i_phdrs[i].p_type == PT_LOAD)
 	{
-	  bfd_vma segment_end;
-	  segment_end = (i_phdrs[i].p_offset + i_phdrs[i].p_filesz
-			 + i_phdrs[i].p_align - 1) & -i_phdrs[i].p_align;
-	  if (segment_end > (bfd_vma) contents_size)
-	    contents_size = segment_end;
-
-	  /* LOADADDR is the `Base address' from the gELF specification:
-	     `lowest p_vaddr value for a PT_LOAD segment' is P_VADDR from the
-	     first PT_LOAD as PT_LOADs are ordered by P_VADDR.  */
-	  if (!loadbase_set && (i_phdrs[i].p_offset & -i_phdrs[i].p_align) == 0)
+	  bfd_vma segment_end = i_phdrs[i].p_offset + i_phdrs[i].p_filesz;
+
+	  if (segment_end > high_offset)
 	    {
-	      loadbase = ehdr_vma - (i_phdrs[i].p_vaddr & -i_phdrs[i].p_align);
-	      loadbase_set = TRUE;
+	      high_offset = segment_end;
+	      last_phdr = &i_phdrs[i];
 	    }
 
-	  last_phdr = &i_phdrs[i];
+	  /* If this program header covers offset zero, where the file
+	     header sits, then we can figure out the loadbase.  */
+	  if (!loadbase_set)
+	    {
+	      bfd_vma p_offset = i_phdrs[i].p_offset;
+	      bfd_vma p_vaddr = i_phdrs[i].p_vaddr;
+
+	      if (i_phdrs[i].p_align > 1)
+		{
+		  p_offset &= -i_phdrs[i].p_align;
+		  p_vaddr &= -i_phdrs[i].p_align;
+		}
+	      if (p_offset == 0)
+		{
+		  loadbase = ehdr_vma - p_vaddr;
+		  loadbase_set = TRUE;
+		}
+	    }
 	}
     }
-  if (last_phdr == NULL)
+  if (high_offset == 0)
     {
       /* There were no PT_LOAD segments, so we don't have anything to read.  */
       free (x_phdrs);
@@ -1727,40 +1738,61 @@ NAME(_bfd_elf,bfd_from_remote_memory)
       return NULL;
     }
 
-  /* Trim the last segment so we don't bother with zeros in the last page
-     that are off the end of the file.  However, if the extra bit in that
-     page includes the section headers, keep them.  */
-  if ((bfd_vma) contents_size > last_phdr->p_offset + last_phdr->p_filesz
-      && (bfd_vma) contents_size >= (i_ehdr.e_shoff
-				     + i_ehdr.e_shnum * i_ehdr.e_shentsize))
+  shdr_end = 0;
+  if (i_ehdr.e_shoff != 0 && i_ehdr.e_shnum != 0 && i_ehdr.e_shentsize != 0)
     {
-      contents_size = last_phdr->p_offset + last_phdr->p_filesz;
-      if ((bfd_vma) contents_size < (i_ehdr.e_shoff
-				     + i_ehdr.e_shnum * i_ehdr.e_shentsize))
-	contents_size = i_ehdr.e_shoff + i_ehdr.e_shnum * i_ehdr.e_shentsize;
+      shdr_end = i_ehdr.e_shoff + i_ehdr.e_shnum * i_ehdr.e_shentsize;
+
+      if (last_phdr->p_filesz != last_phdr->p_memsz)
+	{
+	  /* If the last PT_LOAD header has a bss area then ld.so will
+	     have cleared anything past p_filesz, zapping the section
+	     headers.  */
+	}
+      else if (size >= shdr_end)
+	high_offset = shdr_end;
+      else
+	{
+	  bfd_vma page_size = get_elf_backend_data (templ)->minpagesize;
+	  bfd_vma segment_end = last_phdr->p_offset + last_phdr->p_filesz;
+
+	  /* Assume we loaded full pages, allowing us to sometimes see
+	     section headers.  */
+	  if (page_size > 1 && shdr_end > segment_end)
+	    {
+	      bfd_vma page_end = (segment_end + page_size - 1) & -page_size;
+
+	      if (page_end >= shdr_end)
+		/* Whee, section headers covered.  */
+		high_offset = shdr_end;
+	    }
+	}
     }
-  else
-    contents_size = last_phdr->p_offset + last_phdr->p_filesz;
 
   /* Now we know the size of the whole image we want read in.  */
-  contents = (bfd_byte *) bfd_zmalloc (contents_size);
+  contents = (bfd_byte *) bfd_zmalloc (high_offset);
   if (contents == NULL)
     {
       free (x_phdrs);
-      bfd_set_error (bfd_error_no_memory);
       return NULL;
     }
 
   for (i = 0; i < i_ehdr.e_phnum; ++i)
     if (i_phdrs[i].p_type == PT_LOAD)
       {
-	bfd_vma start = i_phdrs[i].p_offset & -i_phdrs[i].p_align;
-	bfd_vma end = (i_phdrs[i].p_offset + i_phdrs[i].p_filesz
-		       + i_phdrs[i].p_align - 1) & -i_phdrs[i].p_align;
-	if (end > (bfd_vma) contents_size)
-	  end = contents_size;
-	err = target_read_memory ((loadbase + i_phdrs[i].p_vaddr)
-				  & -i_phdrs[i].p_align,
+	bfd_vma start = i_phdrs[i].p_offset;
+	bfd_vma end = start + i_phdrs[i].p_filesz;
+	bfd_vma vaddr = i_phdrs[i].p_vaddr;
+
+	if (i_phdrs[i].p_align > 1)
+	  {
+	    start &= -i_phdrs[i].p_align;
+	    end = (end + i_phdrs[i].p_align - 1) & -i_phdrs[i].p_align;
+	    vaddr &= -i_phdrs[i].p_align;
+	  }
+	if (end > high_offset)
+	  end = high_offset;
+	err = target_read_memory (loadbase + vaddr,
 				  contents + start, end - start);
 	if (err)
 	  {
@@ -1775,8 +1807,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
 
   /* If the segments visible in memory didn't include the section headers,
      then clear them from the file header.  */
-  if ((bfd_vma) contents_size < (i_ehdr.e_shoff
-				 + i_ehdr.e_shnum * i_ehdr.e_shentsize))
+  if (high_offset < shdr_end)
     {
       memset (&x_ehdr.e_shoff, 0, sizeof x_ehdr.e_shoff);
       memset (&x_ehdr.e_shnum, 0, sizeof x_ehdr.e_shnum);
@@ -1792,7 +1823,6 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   if (bim == NULL)
     {
       free (contents);
-      bfd_set_error (bfd_error_no_memory);
       return NULL;
     }
   nbfd = _bfd_new_bfd ();
@@ -1800,12 +1830,11 @@ NAME(_bfd_elf,bfd_from_remote_memory)
     {
       free (bim);
       free (contents);
-      bfd_set_error (bfd_error_no_memory);
       return NULL;
     }
   nbfd->filename = xstrdup ("<in-memory>");
   nbfd->xvec = templ->xvec;
-  bim->size = contents_size;
+  bim->size = high_offset;
   bim->buffer = contents;
   nbfd->iostream = bim;
   nbfd->flags = BFD_IN_MEMORY;
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index e3230de..98764cd 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -76,13 +76,14 @@ target_read_memory_bfd (bfd_vma memaddr, bfd_byte *myaddr, bfd_size_type len)
 }
 
 /* Read inferior memory at ADDR to find the header of a loaded object file
-   and read its in-core symbols out of inferior memory.  TEMPL is a bfd
+   and read its in-core symbols out of inferior memory.  SIZE, if
+   non-zero, is the known size of the object.  TEMPL is a bfd
    representing the target's format.  NAME is the name to use for this
    symbol file in messages; it can be NULL or a malloc-allocated string
    which will be attached to the BFD.  */
 static struct objfile *
-symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr, char *name,
-			     int from_tty)
+symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr,
+			     size_t size, char *name, int from_tty)
 {
   struct objfile *objf;
   struct bfd *nbfd;
@@ -95,7 +96,7 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr, char *name,
   if (bfd_get_flavour (templ) != bfd_target_elf_flavour)
     error (_("add-symbol-file-from-memory not supported for this target"));
 
-  nbfd = bfd_elf_bfd_from_remote_memory (templ, addr, &loadbase,
+  nbfd = bfd_elf_bfd_from_remote_memory (templ, addr, size, &loadbase,
 					 target_read_memory_bfd);
   if (nbfd == NULL)
     error (_("Failed to read a valid object file image from memory."));
@@ -158,7 +159,7 @@ add_symbol_file_from_memory_command (char *args, int from_tty)
     error (_("Must use symbol-file or exec-file "
 	     "before add-symbol-file-from-memory."));
 
-  symbol_file_add_from_memory (templ, addr, NULL, from_tty);
+  symbol_file_add_from_memory (templ, addr, 0, NULL, from_tty);
 }
 
 /* Arguments for symbol_file_add_from_memory_wrapper.  */
@@ -167,6 +168,7 @@ struct symbol_file_add_from_memory_args
 {
   struct bfd *bfd;
   CORE_ADDR sysinfo_ehdr;
+  size_t size;
   char *name;
   int from_tty;
 };
@@ -179,8 +181,26 @@ symbol_file_add_from_memory_wrapper (struct ui_out *uiout, void *data)
 {
   struct symbol_file_add_from_memory_args *args = data;
 
-  symbol_file_add_from_memory (args->bfd, args->sysinfo_ehdr, args->name,
-			       args->from_tty);
+  symbol_file_add_from_memory (args->bfd, args->sysinfo_ehdr, args->size,
+			       args->name, args->from_tty);
+  return 0;
+}
+
+/* Rummage through mappings to find the vsyscall page size.  */
+
+static int
+find_vdso_size (CORE_ADDR vaddr, unsigned long size,
+		int read ATTRIBUTE_UNUSED, int write ATTRIBUTE_UNUSED,
+		int exec ATTRIBUTE_UNUSED, int modified ATTRIBUTE_UNUSED,
+		void *data)
+{
+  struct symbol_file_add_from_memory_args *args = data;
+
+  if (vaddr == args->sysinfo_ehdr)
+    {
+      args->size = size;
+      return 1;
+    }
   return 0;
 }
 
@@ -217,6 +237,11 @@ add_vsyscall_page (struct target_ops *target, int from_tty)
 	}
       args.bfd = bfd;
       args.sysinfo_ehdr = sysinfo_ehdr;
+      args.size = 0;
+      if (gdbarch_find_memory_regions_p (target_gdbarch ()))
+	(void) gdbarch_find_memory_regions (target_gdbarch (),
+					    find_vdso_size, &args);
+
       args.name = xstrprintf ("system-supplied DSO at %s",
 			      paddress (target_gdbarch (), sysinfo_ehdr));
       /* Pass zero for FROM_TTY, because the action of loading the


-- 
Alan Modra
Australia Development Lab, IBM

Re: vdso handling

[Pedro Alves]

On 03/28/2014 06:13 AM, Alan Modra wrote:
> On Fri, Mar 21, 2014 at 03:48:48PM +0000, Pedro Alves wrote:
>> I just tried pointing add-symbol-file-from-memory at an already
>> mapped DSO's elf header, but it doesn't work as is unfortunately:
>>
>>  (gdb) info shared curses
>>  0x000000324d006d20  0x000000324d01df58  Yes         /lib64/libncurses.so.5
>>  (gdb) x /4b 0x000000324d000000
>>  0x324d000000:   127     69      76      70
>>  (gdb) add-symbol-file-from-memory 0x000000324d000000
>>  Failed to read a valid object file image from memory.
>>
>> I single stepped a little through
>> bfd_elf_bfd_from_remote_memory - something goes wrong with the
>> reading of the load segment contents, probably something wrong
>> with the address computations.
> 
> readelf -a --wide on my x86_64 libncurses.so.5 shows
> 
> [snip]
>   Start of section headers:          132144 (bytes into file)
> [snip]
>   [25] .shstrtab         STRTAB          0000000000000000 02034c 0000de 00      0   0  1
> [snip]
>   LOAD           0x000000 0x0000000000000000 0x0000000000000000 0x01efe4 0x01efe4 R E 0x200000
>   LOAD           0x01fd50 0x000000000021fd50 0x000000000021fd50 0x0005e4 0x000770 RW  0x200000
> 
> So .shstrtab and the section headers might have been loaded by the
> second PT_LOAD header, *but* the second PT_LOAD has a bss area.
> Anything past 0x220334 will be cleared out by ld.so.  No chance of
> getting at section headers then, and this will be true for most
> in-memory images.

Indeed.

> bfd_from_remote_memory should take note of p_memsz..  Hmm, and there
> are quite a few other issues there too, most notably that p_align
> on x86_64 these days tends to be *much* larger than the page size used
> by ld.so.

Hmm.  Indeed.  With current mainline, and with your patch as is,
the command still fails for me.  In fact, it turns out
exactly related to p_align vs page size.

$ cat /proc/30669/maps | grep ncurses
324d000000-324d023000 r-xp 00000000 fd:01 315662                         /usr/lib64/libncurses.so.5.9
324d023000-324d222000 ---p 00023000 fd:01 315662                         /usr/lib64/libncurses.so.5.9
324d222000-324d223000 r--p 00022000 fd:01 315662                         /usr/lib64/libncurses.so.5.9
324d223000-324d224000 rw-p 00023000 fd:01 315662                         /usr/lib64/libncurses.so.5.9

So when trying to read the second PT_LOAD with p_vmaddr 324d222cf8
and p_vmaddr+p_filesz 324d2236b4, (the 3rd and 4th region above),
we'd end up reading from 324d200000 to 324d2236b4:

(top-gdb) p /x loadbase + vaddr
$5 = 0x324d200000
(top-gdb) p /x end
$6 = 0x236b4
(top-gdb) p /x loadbase + vaddr + end
$8 = 0x324d2236b4

which fails as it hits the (324d023000-324d222000) region,
which has no permissions.

This patch on top of yours makes things work for me:

---
 bfd/elfcode.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index 31f67a8..974c8b4 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -1622,6 +1622,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   bfd_vma shdr_end;
   bfd_vma loadbase;
   bfd_boolean loadbase_set;
+  bfd_vma page_size;

   /* Read in the ELF header in external format.  */
   err = target_read_memory (ehdr_vma, (bfd_byte *) &x_ehdr, sizeof x_ehdr);
@@ -1693,6 +1694,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
     }
   i_phdrs = (Elf_Internal_Phdr *) &x_phdrs[i_ehdr.e_phnum];

+  page_size = get_elf_backend_data (templ)->minpagesize;
   high_offset = 0;
   last_phdr = NULL;
   loadbase = 0;
@@ -1753,7 +1755,6 @@ NAME(_bfd_elf,bfd_from_remote_memory)
 	high_offset = shdr_end;
       else
 	{
-	  bfd_vma page_size = get_elf_backend_data (templ)->minpagesize;
 	  bfd_vma segment_end = last_phdr->p_offset + last_phdr->p_filesz;

 	  /* Assume we loaded full pages, allowing us to sometimes see
@@ -1781,15 +1782,14 @@ NAME(_bfd_elf,bfd_from_remote_memory)
     if (i_phdrs[i].p_type == PT_LOAD)
       {
 	bfd_vma start = i_phdrs[i].p_offset;
-	bfd_vma end = start + i_phdrs[i].p_filesz;
 	bfd_vma vaddr = i_phdrs[i].p_vaddr;
+	bfd_vma end = start + i_phdrs[i].p_filesz;

-	if (i_phdrs[i].p_align > 1)
-	  {
-	    start &= -i_phdrs[i].p_align;
-	    end = (end + i_phdrs[i].p_align - 1) & -i_phdrs[i].p_align;
-	    vaddr &= -i_phdrs[i].p_align;
-	  }
+	/* Assume we loaded full pages, allowing us to sometimes see
+	   section headers.  */
+	start &= -page_size;
+	vaddr &= -page_size;
+	end = (end + page_size - 1) & -page_size;
 	if (end > high_offset)
 	  end = high_offset;
 	err = target_read_memory (loadbase + vaddr,
-- 
1.7.11.7


> Gah, I've been sucked into looking at this long enough that I may as
> well fix it.  Does this look OK?

It does to me.  Thanks!

-- 
Pedro Alves

Re: vdso handling

[Alan Modra]

On Fri, Mar 28, 2014 at 01:37:52PM +0000, Pedro Alves wrote:
> Hmm.  Indeed.  With current mainline, and with your patch as is,
> the command still fails for me.  In fact, it turns out
> exactly related to p_align vs page size.
> 
> $ cat /proc/30669/maps | grep ncurses
> 324d000000-324d023000 r-xp 00000000 fd:01 315662                         /usr/lib64/libncurses.so.5.9
> 324d023000-324d222000 ---p 00023000 fd:01 315662                         /usr/lib64/libncurses.so.5.9
> 324d222000-324d223000 r--p 00022000 fd:01 315662                         /usr/lib64/libncurses.so.5.9
> 324d223000-324d224000 rw-p 00023000 fd:01 315662                         /usr/lib64/libncurses.so.5.9
> 
> So when trying to read the second PT_LOAD with p_vmaddr 324d222cf8
> and p_vmaddr+p_filesz 324d2236b4, (the 3rd and 4th region above),
> we'd end up reading from 324d200000 to 324d2236b4:
> 
> (top-gdb) p /x loadbase + vaddr
> $5 = 0x324d200000
> (top-gdb) p /x end
> $6 = 0x236b4
> (top-gdb) p /x loadbase + vaddr + end
> $8 = 0x324d2236b4
> 
> which fails as it hits the (324d023000-324d222000) region,
> which has no permissions.

Ah ha!  What's more, if the read did happen to succeed you'd overwrite
contents written when processing the first PT_LOAD.  I believe that
will still happen with your fixup patch.  Not that it's a problem,
since ld.so reads the page holding the end of the first PT_LOAD and
the beginning of the second PT_LOAD twice, but I think it would be
better if BFD didn't rely on this ld.so behaviour (and an exact match
between BFD and ld.so's page size).

I believe the intent of rounding to a page was to pick up the file
and program headers at the start of a file and section headers at the
end, so let's do just that.  On top of my last patch:

diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index 31f67a8..a005948 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -1612,7 +1612,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   Elf_External_Ehdr x_ehdr;	/* Elf file header, external form */
   Elf_Internal_Ehdr i_ehdr;	/* Elf file header, internal form */
   Elf_External_Phdr *x_phdrs;
-  Elf_Internal_Phdr *i_phdrs, *last_phdr;
+  Elf_Internal_Phdr *i_phdrs, *last_phdr, *first_phdr;
   bfd *nbfd;
   struct bfd_in_memory *bim;
   bfd_byte *contents;
@@ -1621,7 +1621,6 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   bfd_vma high_offset;
   bfd_vma shdr_end;
   bfd_vma loadbase;
-  bfd_boolean loadbase_set;
 
   /* Read in the ELF header in external format.  */
   err = target_read_memory (ehdr_vma, (bfd_byte *) &x_ehdr, sizeof x_ehdr);
@@ -1694,9 +1693,9 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   i_phdrs = (Elf_Internal_Phdr *) &x_phdrs[i_ehdr.e_phnum];
 
   high_offset = 0;
-  last_phdr = NULL;
   loadbase = 0;
-  loadbase_set = FALSE;
+  first_phdr = NULL;
+  last_phdr = NULL;
   for (i = 0; i < i_ehdr.e_phnum; ++i)
     {
       elf_swap_phdr_in (templ, &x_phdrs[i], &i_phdrs[i]);
@@ -1712,7 +1711,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
 
 	  /* If this program header covers offset zero, where the file
 	     header sits, then we can figure out the loadbase.  */
-	  if (!loadbase_set)
+	  if (first_phdr == NULL)
 	    {
 	      bfd_vma p_offset = i_phdrs[i].p_offset;
 	      bfd_vma p_vaddr = i_phdrs[i].p_vaddr;
@@ -1725,7 +1724,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
 	      if (p_offset == 0)
 		{
 		  loadbase = ehdr_vma - p_vaddr;
-		  loadbase_set = TRUE;
+		  first_phdr = &i_phdrs[i];
 		}
 	    }
 	}
@@ -1784,13 +1783,15 @@ NAME(_bfd_elf,bfd_from_remote_memory)
 	bfd_vma end = start + i_phdrs[i].p_filesz;
 	bfd_vma vaddr = i_phdrs[i].p_vaddr;
 
-	if (i_phdrs[i].p_align > 1)
+	/* Extend the beginning of the first pt_load to cover file
+	   header and program headers.  */
+	if (first_phdr == &i_phdrs[i])
 	  {
-	    start &= -i_phdrs[i].p_align;
-	    end = (end + i_phdrs[i].p_align - 1) & -i_phdrs[i].p_align;
-	    vaddr &= -i_phdrs[i].p_align;
+	    vaddr -= start;
+	    start = 0;
 	  }
-	if (end > high_offset)
+	/* Extend the end of the last pt_load to cover section headers.  */
+	if (last_phdr == &i_phdrs[i])
 	  end = high_offset;
 	err = target_read_memory (loadbase + vaddr,
 				  contents + start, end - start);

-- 
Alan Modra
Australia Development Lab, IBM

Re: vdso handling

[Pedro Alves]

On 03/28/2014 11:00 PM, Alan Modra wrote:

> I believe the intent of rounding to a page was to pick up the file
> and program headers at the start of a file and section headers at the
> end, so let's do just that.  On top of my last patch:

Agreed.  This works for me.  Thanks!

> -	if (i_phdrs[i].p_align > 1)
> +	/* Extend the beginning of the first pt_load to cover file
> +	   header and program headers.  */
> +	if (first_phdr == &i_phdrs[i])

Minor nit:  Perhaps the comment could say "first pt_load
if it covers offset 0"?  The computation below confused me a little
until I scrolled up and realized that first_phdr is only set if
the first segment covers offset 0, not whatever the first segment
is.  (I'd even consider renaming it to zero_phdr or
zero_offset_phdr, but with the comment I'd already be
super happy).


On the GDB patch, sorry for not noticing earlier, but:

> +static int
> +find_vdso_size (CORE_ADDR vaddr, unsigned long size,
> +		int read ATTRIBUTE_UNUSED, int write ATTRIBUTE_UNUSED,
> +		int exec ATTRIBUTE_UNUSED, int modified ATTRIBUTE_UNUSED,
> +		void *data)
> +{

Please don't use ATTRIBUTE_UNUSED under gdb/, it'd be flagged by the
ARI as a regression:

gdb/contrib/gdb_ari.sh:

BEGIN { doc["ATTRIBUTE_UNUSED"] = "\
Do not use ATTRIBUTE_UNUSED, do not bother (GDB is compiled with -Werror and, \
consequently, is not able to tolerate false warnings.  Since -Wunused-param \
produces such warnings, neither that warning flag nor ATTRIBUTE_UNUSED \
are used by GDB"
    category["ATTRIBUTE_UNUSED"] = ari_regression
}
/(^|[^_[:alnum:]])ATTRIBUTE_UNUSED([^_[:alnum:]]|$)/ {
    fail("ATTRIBUTE_UNUSED")
}

-- 
Pedro Alves

Re: vdso handling

[Alan Modra]

On Tue, Apr 01, 2014 at 02:46:38PM +0100, Pedro Alves wrote:
> On 03/28/2014 11:00 PM, Alan Modra wrote:
> 
> > I believe the intent of rounding to a page was to pick up the file
> > and program headers at the start of a file and section headers at the
> > end, so let's do just that.  On top of my last patch:
> 
> Agreed.  This works for me.  Thanks!
> 
> > -	if (i_phdrs[i].p_align > 1)
> > +	/* Extend the beginning of the first pt_load to cover file
> > +	   header and program headers.  */
> > +	if (first_phdr == &i_phdrs[i])
> 
> Minor nit:  Perhaps the comment could say "first pt_load
> if it covers offset 0"?

Fixed.

	/* Extend the beginning of the first pt_load to cover file
	   header and program headers, if we proved earlier that its
	   aligned offset is 0.  */

> Please don't use ATTRIBUTE_UNUSED under gdb/, it'd be flagged by the
> ARI as a regression:

Hmm, OK.  Fixed and pushed.

-- 
Alan Modra
Australia Development Lab, IBM

RE: vdso handling

[Metzger, Markus T]

> -----Original Message-----
> From: Alan Modra [mailto:amodra@gmail.com]
> Sent: Wednesday, April 02, 2014 3:50 AM


> Hmm, OK.  Fixed and pushed.

Thanks,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: vdso handling

[Hans-Peter Nilsson]

On Fri, 28 Mar 2014, Alan Modra wrote:
>
> bfd_from_remote_memory should take note of p_memsz..  Hmm, and there
> are quite a few other issues there too, most notably that p_align
> on x86_64 these days tends to be *much* larger than the page size used
> by ld.so.
>
> Gah, I've been sucked into looking at this long enough that I may as
> well fix it.  Does this look OK?

The new size parameter uses size_t in bfd headers, breaking some
simulators like cris-elf, frv-elf, h8300-elf, iq2000-elf,
m32r-elf, mips-elf, mn10300-elf.

The obvious change is to instead use bfd_size_type, like
everything else in BFD headers.  Any reason not to do that here?

> 	* elfcode.h (bfd_from_remote_memory): Add "size" parameter.

brgds, H-P

Re: vdso handling

[Alan Modra]

On Wed, Apr 02, 2014 at 04:04:24AM -0400, Hans-Peter Nilsson wrote:
> The new size parameter uses size_t in bfd headers, breaking some
> simulators like cris-elf, frv-elf, h8300-elf, iq2000-elf,
> m32r-elf, mips-elf, mn10300-elf.
> 
> The obvious change is to instead use bfd_size_type, like
> everything else in BFD headers.  Any reason not to do that here?

I originally had bfd_size_type and wondered if we could use size_t,
which is the natural type to use here.  (bfd_size_type might be 64-bit
and it doesn't really make sense to use that when describing a memory
area on a 32-bit host.)  gdb and binutils built without trouble on a
few targets, so I went with size_t.  Sorry about the sim breakage.

	* elf-bfd.h (struct elf_backend_data
	<elf_backend_bfd_from_remote_memory>): Replace "size_t size"
	with "bfd_size_type size".
	(_bfd_elf32_bfd_from_remote_memory): Likewise.
	(_bfd_elf64_bfd_from_remote_memory): Likewise.
	* elf.c (bfd_elf_bfd_from_remote_memory): Likewise.
	* elfcode.h (bfd_from_remote_memory): Likewise.

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 6d07303..f58a3b7 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1191,7 +1191,7 @@ struct elf_backend_data
   /* This function implements `bfd_elf_bfd_from_remote_memory';
      see elf.c, elfcode.h.  */
   bfd *(*elf_backend_bfd_from_remote_memory)
-    (bfd *templ, bfd_vma ehdr_vma, size_t size, bfd_vma *loadbasep,
+    (bfd *templ, bfd_vma ehdr_vma, bfd_size_type size, bfd_vma *loadbasep,
      int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr,
 				bfd_size_type len));
 
@@ -2334,10 +2334,10 @@ extern char *elfcore_write_ppc_linux_prpsinfo32
   (bfd *, char *, int *, const struct elf_internal_linux_prpsinfo *);
 
 extern bfd *_bfd_elf32_bfd_from_remote_memory
-  (bfd *templ, bfd_vma ehdr_vma, size_t size, bfd_vma *loadbasep,
+  (bfd *templ, bfd_vma ehdr_vma, bfd_size_type size, bfd_vma *loadbasep,
    int (*target_read_memory) (bfd_vma, bfd_byte *, bfd_size_type));
 extern bfd *_bfd_elf64_bfd_from_remote_memory
-  (bfd *templ, bfd_vma ehdr_vma, size_t size, bfd_vma *loadbasep,
+  (bfd *templ, bfd_vma ehdr_vma, bfd_size_type size, bfd_vma *loadbasep,
    int (*target_read_memory) (bfd_vma, bfd_byte *, bfd_size_type));
 
 extern bfd_vma bfd_elf_obj_attr_size (bfd *);
diff --git a/bfd/elf.c b/bfd/elf.c
index d67b917..9e46f7c 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -9908,7 +9908,7 @@ bfd *
 bfd_elf_bfd_from_remote_memory
   (bfd *templ,
    bfd_vma ehdr_vma,
-   size_t size,
+   bfd_size_type size,
    bfd_vma *loadbasep,
    int (*target_read_memory) (bfd_vma, bfd_byte *, bfd_size_type))
 {
diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index f840065..a49a708 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -1605,7 +1605,7 @@ bfd *
 NAME(_bfd_elf,bfd_from_remote_memory)
   (bfd *templ,
    bfd_vma ehdr_vma,
-   size_t size,
+   bfd_size_type size,
    bfd_vma *loadbasep,
    int (*target_read_memory) (bfd_vma, bfd_byte *, bfd_size_type))
 {


-- 
Alan Modra
Australia Development Lab, IBM

Re: vdso handling

[Alan Modra]

On Thu, Apr 03, 2014 at 11:36:44AM +1030, Alan Modra wrote:
> 	* elf-bfd.h (struct elf_backend_data
> 	<elf_backend_bfd_from_remote_memory>): Replace "size_t size"
> 	with "bfd_size_type size".
> 	(_bfd_elf32_bfd_from_remote_memory): Likewise.
> 	(_bfd_elf64_bfd_from_remote_memory): Likewise.
> 	* elf.c (bfd_elf_bfd_from_remote_memory): Likewise.
> 	* elfcode.h (bfd_from_remote_memory): Likewise.

Sigh.  Here too.

	* bfd-in.h (bfd_elf_bfd_from_remote_memory): Likewise.
	* bfd-in2.h: Regenerate.

-- 
Alan Modra
Australia Development Lab, IBM