diff mbox

vdso handling

Message ID 20140328061321.GU18201@bubble.grove.modra.org
State Committed
Headers show

Commit Message

Alan Modra March 28, 2014, 6:13 a.m. UTC
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.

Comments

Hans-Peter Nilsson April 2, 2014, 8:04 a.m. UTC | #1
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
diff mbox

Patch

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);
 }
 
 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
 
 /* 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