Re: [patch] bfd_elf_bfd_from_remote_memory() workaround for the ELF misalignment [Re: loadbase alignment - ld.so/prelink/kernel or bfd_elf_bfd_from_remote_memory() bug?]

[Daniel Jacobowitz <drow at false dot org>]

On Tue, Aug 21, 2007 at 01:43:53PM -0700, Roland McGrath wrote:
> The only case where this function is used today is the Linux vDSO.  That
> is a DSO with one segment, or two segments that are both read-only and
> map the same part of the file (IA64).  The part of the file after the
> p_vaddr+p_filesz (=p_memsz) of the last segment will be in the memory
> image, up to the next page boundary.  This is where the section headers
> and .shstrtab reside (or other nonallocated sections for an unstripped
> file, but this is not the case with the vDSO).  Of course, the zeros seen
> in memory for the tail of the page past the end of the file are not part
> of the file, so it misrepresents the original file to just always include
> the whole last page.  Hence the function has the special heuristic to
> truncate to the shorter of the file portion available in memory and the
> offset at the end of the section headers as indicated by the ELF header.

So what we are doing here is guessing from the program headers where
to find other bits that would have been in the disk file, but are not
part of the ELF object's memory image.  Then we try to decide whether
what we have in memory is likely to include them or not.  Is that
about right?

> It is pleasant to have the section headers and names when they are
> available.  At least at some point in the past, I think it may have been
> required for gdb to find everything it should, because it didn't find
> everything from PT_GNU_EH_FRAME, PT_DYNAMIC, DT_*, etc.  (I don't know if
> that is an issue with today's code.)

If it is still an issue, IMHO we should fix that instead of attempting
to recover the section headers.  The corner cases are too cornered.

> This is the only case used by gdb today.  There is another case relevant
> to my thinking, that I do support in the equivalent code in the elfutils
> libraries.  A normal DSO usually has a data segment with some bss,
> i.e. p_memsz > p_filesz.  In that case, the end of the file is never
> available in memory.  The part of the file you can get intact (or close)
> from memory is truncated at the p_filesz of the data segment.  But, you
> can usually see the whole text segment, which includes .eh_frame and for
> DSOs includes the dynamic symbol table.  So, it really is worthwhile to
> get as much as you can get, and you do need to know the page size to
> reliably know what you can get.  (This is probably only of interest when
> dealing with core dumps when using the option to dump all segments in
> their entirety rather than suppressing text segments as usual.)

What I'm missing - I'm sure it's there somewhere - is how you got from
"can usually see the whole text segment, including [some loaded
sections]" to "need to know the page size".  .eh_frame will be found
via the program headers, .dynsym will be indicated by .dynamic, and
all three are included in both p_filesz and p_memsz.  I don't see why
we need any heurestics for this case.

I think we can effectively ignore p_align.  We know the actual load
address of the program headers.  We can rely on at least one PT_LOAD
segment including the file offset of the program headers - this is not
the case on some targets but we won't be able to do what we're trying
in that case anyway.  So we know the file's link time virtual address
for the program headers.  The difference is the base address, no
truncation at all.  Obviously incomplete patch:

Index: elfcode.h
===================================================================
RCS file: /cvs/src/src/bfd/elfcode.h,v
retrieving revision 1.86
diff -u -p -r1.86 elfcode.h
--- elfcode.h	14 Aug 2007 08:04:47 -0000	1.86
+++ elfcode.h	21 Aug 2007 21:27:16 -0000
@@ -1627,7 +1627,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, *phdrs_phdr;
   bfd *nbfd;
   struct bfd_in_memory *bim;
   int contents_size;
@@ -1635,7 +1635,6 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   int err;
   unsigned int i;
   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);
@@ -1711,8 +1710,8 @@ NAME(_bfd_elf,bfd_from_remote_memory)
 
   contents_size = 0;
   last_phdr = NULL;
+  phdrs_phdr = NULL;
   loadbase = ehdr_vma;
-  loadbase_set = FALSE;
   for (i = 0; i < i_ehdr.e_phnum; ++i)
     {
       elf_swap_phdr_in (templ, &x_phdrs[i], &i_phdrs[i]);
@@ -1724,14 +1723,11 @@ NAME(_bfd_elf,bfd_from_remote_memory)
 	  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)
-	    {
-	      loadbase = ehdr_vma - (i_phdrs[i].p_vaddr & -i_phdrs[i].p_align);
-	      loadbase_set = TRUE;
-	    }
+	  if (i_phdrs[i].p_offset < i_ehdr.e_phoff
+	      && i_phdrs[i].p_offset + i_phdrs[i].p_filesz > i_ehdr.e_phoff
+	      && (phdrs_phdr == NULL
+		  || i_phdrs[i].p_vaddr < phdrs_phdr->p_vaddr))
+	    phdrs_phdr = &i_phdrs[i];
 
 	  last_phdr = &i_phdrs[i];
 	}
@@ -1744,6 +1740,13 @@ NAME(_bfd_elf,bfd_from_remote_memory)
       return NULL;
     }
 
+  /* LOADADDR is the `Base address' from the gELF specification.  */
+  if (phdrs_phdr != NULL)
+    {
+      loadbase = ehdr_vma + i_ehdr.e_phoff;
+      loadbase -= phdrs_phdr->p_vaddr - phdrs_phdr->p_offset + i_ehdr.e_phoff;
+    }
+
   /* 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.  */


-- 
Daniel Jacobowitz
CodeSourcery