Re: [patch] Fix bfd_elf_bfd_from_remote_memory() loadbase

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

On Mon, 13 Aug 2007 05:56:54 +0200, Alan Modra wrote:
...
> - remove the nonsense PF_R tests.
> - set loadbase using the first header with (p_offset & -p_align) == 0

On Mon, 13 Aug 2007 06:14:30 +0200, Roland McGrath wrote:
...
> It is probably also appropriate to change how the later part of the function
> works (the actual data reading).  Removing the PF_R check from the second
> loop in the existing code

[attached]
GDB testsuite ran on x86_64; vDSO manually tested on IA-64.


On Mon, 13 Aug 2007 03:15:44 +0200, Roland McGrath wrote:
...
> Jan's new comment explaining the PF_R check is incorrect.  It may once
> have been the case in Linux that the debugger was unable to read memory
> mapped without read permission, but that has certainly not been true
> recently.

While it is Linux kernel dependent (kernel-2.6.21-1.3228.fc7.x86_64) I have
made a test [attached] of this behavior with the results you describe.

Still some segments / segments parts may be missing if using a core file
backend but it is the responsibility of the TARGET_READ_MEMORY callback to
never report error - or possibly a fix out of scope of this patch.


Regards,
Jan


prot: @address self-Read(requested)->(permitted) self-Write self-eXecute ptrace-Read(permitted) ptrace-Write
00: @0x2aaaaaaae000 R(-)->(-) W(-)->(-) X(-)->(-) pR(+) pW(+)
01: @0x2aaaaaab0000 R(-)->(+) W(-)->(-) X(+)->(-) pR(+) pW(+)
02: @0x2aaaaaab2000 R(-)->(+) W(+)->(+) X(-)->(-) pR(+) pW(+)
03: @0x2aaaaaab4000 R(-)->(+) W(+)->(+) X(+)->(-) pR(+) pW(+)
04: @0x2aaaaaab6000 R(+)->(+) W(-)->(-) X(-)->(+) pR(+) pW(+)
05: @0x2aaaaaab8000 R(+)->(+) W(-)->(-) X(+)->(+) pR(+) pW(+)
06: @0x2aaaaaaba000 R(+)->(+) W(+)->(+) X(-)->(+) pR(+) pW(+)
07: @0x2aaaaaabc000 R(+)->(+) W(+)->(+) X(+)->(+) pR(+) pW(+)
/proc/PID/maps:
2aaaaaaae000-2aaaaaaaf000 ---p 2aaaaaaae000 00:00 0
2aaaaaab0000-2aaaaaab1000 r--p 2aaaaaab0000 00:00 0
2aaaaaab2000-2aaaaaab3000 -w-p 2aaaaaab2000 00:00 0
2aaaaaab4000-2aaaaaab5000 rw-p 2aaaaaab4000 00:00 0
2aaaaaab6000-2aaaaaab7000 --xp 2aaaaaab6000 00:00 0
2aaaaaab8000-2aaaaaab9000 r-xp 2aaaaaab8000 00:00 0
2aaaaaaba000-2aaaaaabb000 -wxp 2aaaaaaba000 00:00 0
2aaaaaabc000-2aaaaaabd000 rwxp 2aaaaaabc000 00:00 0
core:
Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  LOAD           0x0000000000010000 0x00002aaaaaaae000 0x0000000000000000
                 0x0000000000001000 0x0000000000001000         1000
  LOAD           0x0000000000011000 0x00002aaaaaab0000 0x0000000000000000
                 0x0000000000001000 0x0000000000001000  R      1000
  LOAD           0x0000000000012000 0x00002aaaaaab2000 0x0000000000000000
                 0x0000000000001000 0x0000000000001000   W     1000
  LOAD           0x0000000000013000 0x00002aaaaaab4000 0x0000000000000000
                 0x0000000000001000 0x0000000000001000  RW     1000
  LOAD           0x0000000000014000 0x00002aaaaaab6000 0x0000000000000000
                 0x0000000000001000 0x0000000000001000    E    1000
  LOAD           0x0000000000015000 0x00002aaaaaab8000 0x0000000000000000
                 0x0000000000001000 0x0000000000001000  R E    1000
  LOAD           0x0000000000016000 0x00002aaaaaaba000 0x0000000000000000
                 0x0000000000001000 0x0000000000001000   WE    1000
  LOAD           0x0000000000017000 0x00002aaaaaabc000 0x0000000000000000
                 0x0000000000001000 0x0000000000001000  RWE    1000

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

	* elfcode.h (NAME(_bfd_elf,bfd_from_remote_memory)): LOADBASE is now
	initialized only on the first PT_LOAD.  New variable LOADBASE_SET.
	Removed PF_R checking for IA-64 vDSOs as redundant now.
	Code advisory: Roland McGrath

--- bfd/elfcode.h	4 Aug 2007 16:31:00 -0000	1.85
+++ bfd/elfcode.h	13 Aug 2007 20:52:25 -0000
@@ -1635,6 +1635,7 @@ 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,13 +1712,11 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   contents_size = 0;
   last_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]);
-      /* IA-64 vDSO may have two mappings for one segment, where one mapping
-	 is executable only, and one is read only.  We must not use the
-	 executable one.  */
-      if (i_phdrs[i].p_type == PT_LOAD && (i_phdrs[i].p_flags & PF_R))
+      if (i_phdrs[i].p_type == PT_LOAD)
 	{
 	  bfd_vma segment_end;
 	  segment_end = (i_phdrs[i].p_offset + i_phdrs[i].p_filesz
@@ -1725,8 +1724,14 @@ NAME(_bfd_elf,bfd_from_remote_memory)
 	  if (segment_end > (bfd_vma) contents_size)
 	    contents_size = segment_end;
 
-	  if ((i_phdrs[i].p_offset & -i_phdrs[i].p_align) == 0)
-	    loadbase = ehdr_vma - (i_phdrs[i].p_vaddr & -i_phdrs[i].p_align);
+	  /* 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;
+	    }
 
 	  last_phdr = &i_phdrs[i];
 	}
@@ -1764,10 +1769,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
     }
 
   for (i = 0; i < i_ehdr.e_phnum; ++i)
-    /* IA-64 vDSO may have two mappings for one segment, where one mapping
-       is executable only, and one is read only.  We must not use the
-       executable one.  */
-    if (i_phdrs[i].p_type == PT_LOAD && (i_phdrs[i].p_flags & PF_R))
+    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

Attachment: mmapprot.c
Description: Text document