RE: vdso handling

Metzger, Markus T
2014-03-18T15:14:04Z

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