Re: [patch] bfd_elf_bfd_from_remote_memory() workaround for the ELF misalignment

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Tue, 21 Aug 2007 23:27:35 +0200, Daniel Jacobowitz wrote:
...
> > 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.

Attaching the the patch I had prepared before.

You are tight it would work if GDB (IMO BFD) has to be fixed to parse PHDRs
instead.  Attaching IA64 VDSO + its variant as it gets cut+cleared by
bfd_elf_bfd_from_remote_memory().

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

The section headers table itself is behind the last loaded segment.


Regards,
Jan

2007-08-20  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* elfcode.h (NAME(_bfd_elf,bfd_from_remote_memory)): Removed all the
	P_ALIGN alignments.  New P_ALIGN validity check.  Fixed LOADBASE
	computation for segments without P_ALIGN alignment.  Explicitly read
	PHDRs into the returned BUFFER.

--- bfd/elfcode.h	14 Aug 2007 08:04:47 -0000	1.86
+++ bfd/elfcode.h	20 Aug 2007 16:56:49 -0000
@@ -1709,7 +1709,8 @@ NAME(_bfd_elf,bfd_from_remote_memory)
     }
   i_phdrs = (Elf_Internal_Phdr *) &x_phdrs[i_ehdr.e_phnum];
 
-  contents_size = 0;
+  /* Also cover `sizeof x_ehdr'.  */
+  contents_size = i_ehdr.e_phoff + i_ehdr.e_phnum * sizeof x_phdrs[0];
   last_phdr = NULL;
   loadbase = ehdr_vma;
   loadbase_set = FALSE;
@@ -1719,17 +1720,27 @@ NAME(_bfd_elf,bfd_from_remote_memory)
       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;
+	  segment_end = i_phdrs[i].p_offset + i_phdrs[i].p_filesz;
 	  if (segment_end > (bfd_vma) contents_size)
 	    contents_size = segment_end;
 
+	  /* ELF file sanity check for valid ELF-compliant P_ALIGN.  */
+	  if ((i_phdrs[i].p_vaddr - i_phdrs[i].p_offset) % i_phdrs[i].p_align
+	      != 0)
+	    {
+	      free (x_phdrs);
+	      bfd_set_error (bfd_error_wrong_format);
+	      return NULL;
+	    }
+
 	  /* 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 may not be P_ALIGN aligned.  It violates the gELF
+		 standard but it is a common practice.  */
+	      loadbase = ehdr_vma - (i_phdrs[i].p_vaddr - i_phdrs[i].p_offset);
 	      loadbase_set = TRUE;
 	    }
 
@@ -1771,13 +1782,11 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   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;
+	bfd_vma start = i_phdrs[i].p_offset;
+	bfd_vma end = i_phdrs[i].p_offset + i_phdrs[i].p_filesz;
 	if (end > (bfd_vma) contents_size)
 	  end = contents_size;
-	err = target_read_memory ((loadbase + i_phdrs[i].p_vaddr)
-				  & -i_phdrs[i].p_align,
+	err = target_read_memory (loadbase + i_phdrs[i].p_vaddr,
 				  contents + start, end - start);
 	if (err)
 	  {
@@ -1788,6 +1797,9 @@ NAME(_bfd_elf,bfd_from_remote_memory)
 	    return NULL;
 	  }
       }
+  /* This will normally have been in the first PT_LOAD segment.  */
+  memcpy (contents + i_ehdr.e_phoff, x_phdrs,
+	  i_ehdr.e_phnum * sizeof x_phdrs[0]);
   free (x_phdrs);
 
   /* If the segments visible in memory didn't include the section headers,

Attachment: vdso-ia64-orig.so.gz
Description: GNU Zip compressed data

Attachment: vdso-ia64-by-bfd_elf_bfd_from_remote_memory.so.gz
Description: GNU Zip compressed data