This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: vdso handling
- From: Alan Modra <amodra at gmail dot com>
- To: "Metzger, Markus T" <markus dot t dot metzger at intel dot com>
- Cc: Pedro Alves <palves at redhat dot com>, Mark Wielaard <mjw at redhat dot com>, Cary Coutant <ccoutant at google dot com>, Doug Evans <dje at google dot com>, "gdb at sourceware dot org" <gdb at sourceware dot org>, "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Wed, 19 Mar 2014 09:39:39 +1030
- Subject: Re: vdso handling
- Authentication-results: sourceware.org; auth=none
- References: <CADPb22SAmK5JB3muW_nCvuHN5L-aOcdyzYNR+OtnM3bA1x_OJg at mail dot gmail dot com> <CAHACq4o=HmdCo1FPFL-96raf2UN805jvM=VZM-9dbKrmzJFJTw at mail dot gmail dot com> <20140313010147 dot GZ26922 at bubble dot grove dot modra dot org> <1394704336 dot 11818 dot 115 dot camel at bordewijk dot wildebeest dot org> <20140313130322 dot GA3384 at bubble dot grove dot modra dot org> <5321C7C8 dot 6000707 at redhat dot com> <5321C8FA dot 40708 at gmail dot com> <5321CE1A dot 20509 at redhat dot com> <20140313235347 dot GD3384 at bubble dot grove dot modra dot org> <A78C989F6D9628469189715575E55B230AAB6B17 at IRSMSX103 dot ger dot corp dot intel dot com>
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