RFC: Verify AT_ENTRY before using it

RFC: Verify AT_ENTRY before using it

[Daniel Jacobowitz]

I have a lot of occasions to run programs with a non-default dynamic
loader.  Typically, I do this by taking advantage of gdbserver.
In one window, "gdbserver :PORT /path/to/this/loader /path/to/binary".
In the other, "gdb /path/to/binary" and "target remote :PORT".

Starting with the merge of PIE support, this blows up.  GDB tries to
relocate BINARY by the auxv AT_ENTRY value, but AT_ENTRY describes the
loader, not the binary.

I chose to fix this by a program header comparison.  We can fetch
program headers from AT_PHDRS / AT_PHENT / AT_PHNUM, or from the BFD.
If the program headers in both places are the same, the odds are very
good that AT_ENTRY (which will match AT_PHDRS - the kernel handles
that) belongs to the binary we think it does.  If not, give up on
auxv-based relocation.

There's a short-circuit check so that the extra memory read can
be skipped for non-PIE.

I also have a local executable that acts as a proxy dynamic loader.
That triggers the same scenario.

I've tested this on x86_64, where it unbreaks the ld.so scenario
described above.  I'll test it with our proxy loader on ARM tomorrow;
that machine is currently down for maintenance.

Any comments, or shall I commit this?

-- 
Daniel Jacobowitz
CodeSourcery

2010-02-24  Daniel Jacobowitz  <dan@codesourcery.com>

	* solib-svr4.c (read_program_header): Support type == -1 to read
	all program headers.
	(bfd_read_program_headers): New function.
	(svr4_exec_displacement): Verify that AT_ENTRY is associated with
	exec_bfd.

Index: solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.125
diff -u -p -r1.125 solib-svr4.c
--- solib-svr4.c	24 Feb 2010 00:29:02 -0000	1.125
+++ solib-svr4.c	24 Feb 2010 22:41:35 -0000
@@ -451,6 +451,9 @@ bfd_lookup_symbol (bfd *abfd, char *symn
 /* Read program header TYPE from inferior memory.  The header is found
    by scanning the OS auxillary vector.
 
+   If TYPE == -1, return the program headers instead of the contents of
+   one program header.
+
    Return a pointer to allocated memory holding the program header contents,
    or NULL on failure.  If sucessful, and unless P_SECT_SIZE is NULL, the
    size of those contents is returned to P_SECT_SIZE.  Likewise, the target
@@ -483,8 +486,13 @@ read_program_header (int type, int *p_se
   else
     return 0;
 
-  /* Find .dynamic section via the PT_DYNAMIC PHDR.  */
-  if (arch_size == 32)
+  /* Find the requested segment.  */
+  if (type == -1)
+    {
+      sect_addr = at_phdr;
+      sect_size = at_phent * at_phnum;
+    }
+  else if (arch_size == 32)
     {
       Elf32_External_Phdr phdr;
       int i;
@@ -1669,6 +1677,29 @@ svr4_static_exec_displacement (void)
   return 0;
 }
 
+static gdb_byte *
+bfd_read_program_headers (bfd *abfd, int *phdrs_size)
+{
+  Elf_Internal_Ehdr *ehdr;
+  gdb_byte *buf;
+
+  ehdr = elf_elfheader (abfd);
+
+  *phdrs_size = ehdr->e_phnum * ehdr->e_phentsize;
+  if (*phdrs_size == 0)
+    return NULL;
+
+  buf = xmalloc (*phdrs_size);
+  if (bfd_seek (abfd, ehdr->e_phoff, SEEK_SET) != 0
+      || bfd_bread (buf, *phdrs_size, abfd) != *phdrs_size)
+    {
+      xfree (buf);
+      return NULL;
+    }
+
+  return buf;
+}
+
 /* We relocate all of the sections by the same amount.  This
    behavior is mandated by recent editions of the System V ABI. 
    According to the System V Application Binary Interface,
@@ -1702,7 +1733,36 @@ svr4_exec_displacement (void)
     return 0;
 
   if (target_auxv_search (&current_target, AT_ENTRY, &entry_point) == 1)
-    return entry_point - bfd_get_start_address (exec_bfd);
+    {
+      /* Verify that the auxilliary vector describes the same file as
+	 exec_bfd, by comparing their program headers.  If the program
+	 headers in the auxilliary vector do not match the program
+	 headers in the executable, then we are looking at a different
+	 file than the one used by the kernel - for instance, "gdb
+	 program" connected to "gdbserver :PORT ld.so program".  */
+      int phdrs_size, phdrs2_size, ok = 0;
+      gdb_byte *buf, *buf2;
+
+      /* Take a shortcut for the common case.  If the entry addresses
+	 match, then it is incredibly unlikely that anything
+	 complicated has happened.  It's not impossible, if the loader
+	 and executable are both PIE, but it would still require a
+	 rare conjunction of load addresses.  */
+      if (entry_point == bfd_get_start_address (exec_bfd))
+	return 0;
+
+      buf = read_program_header (-1, &phdrs_size, NULL);
+      buf2 = bfd_read_program_headers (exec_bfd, &phdrs2_size);
+      if (buf != NULL && buf2 != NULL
+	  && phdrs_size == phdrs2_size
+	  && memcmp (buf, buf2, phdrs_size) == 0)
+	ok = 1;
+      xfree (buf);
+      xfree (buf2);
+
+      if (ok)
+	return entry_point - bfd_get_start_address (exec_bfd);
+    }
 
   return svr4_static_exec_displacement ();
 }

Re: RFC: Verify AT_ENTRY before using it

[Jan Kratochvil]

On Wed, 24 Feb 2010 23:49:19 +0100, Daniel Jacobowitz wrote:
> In one window, "gdbserver :PORT /path/to/this/loader /path/to/binary".
> In the other, "gdb /path/to/binary" and "target remote :PORT".

In such case the symbol file ("/path/to/binary") does not correspond to the
target memory ("/path/to/this/loader").  In real world it works and
I understand my PIE patch has brought in a regression.


I would find more correct to use "gdb /path/to/this/loader" instead.  This
would be mostly equivalent to local:
	./gdb -nx -ex 'set breakpoint pending on' -ex 'b main' -ex r --args /lib64/ld-linux-x86-64.so.2 ./gdb -nx
which currently does not work, though.  GDB does not find out it should load
"./gdb" after ld.so loads the "./gdb" binary.

It is because _r_debug->r_map[0].l_name is forced to be "" by ld.so instead of
the real executable name.

The real executable name can be found from _dl_argv[0]; -> a different patch.


> Starting with the merge of PIE support, this blows up.  GDB tries to
> relocate BINARY by the auxv AT_ENTRY value, but AT_ENTRY describes the
> loader, not the binary.

This has been address by this pending patch ...
	Re: [patch] Sanity check PIE displacement #2
	http://sourceware.org/ml/gdb-patches/2010-02/msg00346.html


> I chose to fix this by a program header comparison.

... although I understand the patch of yours is more reliable than just the
displacement page size alignment.


> Any comments, or shall I commit this?

I agree with the patch, I will rebase the "displacement #2" on top of it with
some additional one to make "gdb /path/to/this/loader" working.


> +static gdb_byte *
> +bfd_read_program_headers (bfd *abfd, int *phdrs_size)

(besides missing comment)
/* Return newly allocated memory with ELF program headers content.  Store the
   allocated size in *PHDRS_SIZE, PHDRS_SIZE must not be NULL.  */

Should GDB ever use symbols starting "bfd_*"?  They may clash with bfd/ files.


> +{
> +  Elf_Internal_Ehdr *ehdr;
> +  gdb_byte *buf;
> +
> +  ehdr = elf_elfheader (abfd);

I miss here
  if (bfd_get_flavour (abfd) != bfd_target_elf_flavour)
    return NULL;


> +      int phdrs_size, phdrs2_size, ok = 0;
...
> +      buf = read_program_header (-1, &phdrs_size, NULL);
> +      buf2 = bfd_read_program_headers (exec_bfd, &phdrs2_size);
> +      if (buf != NULL && buf2 != NULL
> +	  && phdrs_size == phdrs2_size
> +	  && memcmp (buf, buf2, phdrs_size) == 0)
> +	ok = 1;
> +      xfree (buf);
> +      xfree (buf2);
> +
> +      if (ok)
> +	return entry_point - bfd_get_start_address (exec_bfd);
> +    }
>  
>    return svr4_static_exec_displacement ();

If the comparison cannot be made (such as on non-ELF files - are they really
sometimes used with solib-svr4.c?) the code is pessimistic and rather does NOT
use the PIE displacement.  Just a statement, unaware of non-PIE targets.


Thanks,
Jan

Re: RFC: Verify AT_ENTRY before using it

[Daniel Jacobowitz]

On Thu, Feb 25, 2010 at 11:16:20PM +0100, Jan Kratochvil wrote:
> > Any comments, or shall I commit this?
> 
> I agree with the patch, I will rebase the "displacement #2" on top of it with
> some additional one to make "gdb /path/to/this/loader" working.

Thanks!

> > +static gdb_byte *
> > +bfd_read_program_headers (bfd *abfd, int *phdrs_size)
> 
> (besides missing comment)
> /* Return newly allocated memory with ELF program headers content.  Store the
>    allocated size in *PHDRS_SIZE, PHDRS_SIZE must not be NULL.  */
> 
> Should GDB ever use symbols starting "bfd_*"?  They may clash with bfd/ files.

Yeah, good point.

> > +{
> > +  Elf_Internal_Ehdr *ehdr;
> > +  gdb_byte *buf;
> > +
> > +  ehdr = elf_elfheader (abfd);
> 
> I miss here
>   if (bfd_get_flavour (abfd) != bfd_target_elf_flavour)
>     return NULL;

Can we get here without ELF?  I guess we can, since other places in
the file check.  We'll never get this far, since auxilliary vectors
are ELF-specific, so I'll put the check earlier.

> 
> 
> > +      int phdrs_size, phdrs2_size, ok = 0;
> ...
> > +      buf = read_program_header (-1, &phdrs_size, NULL);
> > +      buf2 = bfd_read_program_headers (exec_bfd, &phdrs2_size);
> > +      if (buf != NULL && buf2 != NULL
> > +	  && phdrs_size == phdrs2_size
> > +	  && memcmp (buf, buf2, phdrs_size) == 0)
> > +	ok = 1;
> > +      xfree (buf);
> > +      xfree (buf2);
> > +
> > +      if (ok)
> > +	return entry_point - bfd_get_start_address (exec_bfd);
> > +    }
> >  
> >    return svr4_static_exec_displacement ();
> 
> If the comparison cannot be made (such as on non-ELF files - are they really
> sometimes used with solib-svr4.c?) the code is pessimistic and rather does NOT
> use the PIE displacement.  Just a statement, unaware of non-PIE targets.

I meant to be conservative and do it the other way, but I botched the
check.  Fixed in this version.

-- 
Daniel Jacobowitz
CodeSourcery

2010-02-26  Daniel Jacobowitz  <dan@codesourcery.com>

	* solib-svr4.c (read_program_header): Support type == -1 to read
	all program headers.
	(read_program_headers_from_bfd): New function.
	(svr4_exec_displacement): Verify that AT_ENTRY is associated with
	exec_bfd.

Index: solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.125
diff -u -p -r1.125 solib-svr4.c
--- solib-svr4.c	24 Feb 2010 00:29:02 -0000	1.125
+++ solib-svr4.c	26 Feb 2010 21:10:28 -0000
@@ -451,6 +451,9 @@ bfd_lookup_symbol (bfd *abfd, char *symn
 /* Read program header TYPE from inferior memory.  The header is found
    by scanning the OS auxillary vector.
 
+   If TYPE == -1, return the program headers instead of the contents of
+   one program header.
+
    Return a pointer to allocated memory holding the program header contents,
    or NULL on failure.  If sucessful, and unless P_SECT_SIZE is NULL, the
    size of those contents is returned to P_SECT_SIZE.  Likewise, the target
@@ -483,8 +486,13 @@ read_program_header (int type, int *p_se
   else
     return 0;
 
-  /* Find .dynamic section via the PT_DYNAMIC PHDR.  */
-  if (arch_size == 32)
+  /* Find the requested segment.  */
+  if (type == -1)
+    {
+      sect_addr = at_phdr;
+      sect_size = at_phent * at_phnum;
+    }
+  else if (arch_size == 32)
     {
       Elf32_External_Phdr phdr;
       int i;
@@ -1669,6 +1677,32 @@ svr4_static_exec_displacement (void)
   return 0;
 }
 
+/* Read the ELF program headers from ABFD.  Return the contents and
+   set *PHDRS_SIZE to the size of the program headers.  */
+
+static gdb_byte *
+read_program_headers_from_bfd (bfd *abfd, int *phdrs_size)
+{
+  Elf_Internal_Ehdr *ehdr;
+  gdb_byte *buf;
+
+  ehdr = elf_elfheader (abfd);
+
+  *phdrs_size = ehdr->e_phnum * ehdr->e_phentsize;
+  if (*phdrs_size == 0)
+    return NULL;
+
+  buf = xmalloc (*phdrs_size);
+  if (bfd_seek (abfd, ehdr->e_phoff, SEEK_SET) != 0
+      || bfd_bread (buf, *phdrs_size, abfd) != *phdrs_size)
+    {
+      xfree (buf);
+      return NULL;
+    }
+
+  return buf;
+}
+
 /* We relocate all of the sections by the same amount.  This
    behavior is mandated by recent editions of the System V ABI. 
    According to the System V Application Binary Interface,
@@ -1702,7 +1736,40 @@ svr4_exec_displacement (void)
     return 0;
 
   if (target_auxv_search (&current_target, AT_ENTRY, &entry_point) == 1)
-    return entry_point - bfd_get_start_address (exec_bfd);
+    {
+      /* Verify that the auxilliary vector describes the same file as
+	 exec_bfd, by comparing their program headers.  If the program
+	 headers in the auxilliary vector do not match the program
+	 headers in the executable, then we are looking at a different
+	 file than the one used by the kernel - for instance, "gdb
+	 program" connected to "gdbserver :PORT ld.so program".  */
+      int phdrs_size, phdrs2_size, ok = 1;
+      gdb_byte *buf, *buf2;
+
+      /* Take a shortcut for the common case.  If the entry addresses
+	 match, then it is incredibly unlikely that anything
+	 complicated has happened.  It's not impossible, if the loader
+	 and executable are both PIE, but it would still require a
+	 rare conjunction of load addresses.  */
+      if (entry_point == bfd_get_start_address (exec_bfd))
+	return 0;
+
+      if (bfd_get_flavour (abfd) == bfd_target_elf_flavour)
+	{
+	  buf = read_program_header (-1, &phdrs_size, NULL);
+	  buf2 = read_program_headers_from_bfd (exec_bfd, &phdrs2_size);
+	  if (buf != NULL && buf2 != NULL
+	      && (phdrs_size != phdrs2_size
+		  || memcmp (buf, buf2, phdrs_size) != 0))
+	    ok = 0;
+	}
+
+      xfree (buf);
+      xfree (buf2);
+
+      if (ok)
+	return entry_point - bfd_get_start_address (exec_bfd);
+    }
 
   return svr4_static_exec_displacement ();
 }

Re: RFC: Verify AT_ENTRY before using it

[Jan Kratochvil]

On Fri, 26 Feb 2010 22:12:16 +0100, Daniel Jacobowitz wrote:
>    if (target_auxv_search (&current_target, AT_ENTRY, &entry_point) == 1)
> -    return entry_point - bfd_get_start_address (exec_bfd);
> +    {
> +      /* Verify that the auxilliary vector describes the same file as
> +	 exec_bfd, by comparing their program headers.  If the program
> +	 headers in the auxilliary vector do not match the program
> +	 headers in the executable, then we are looking at a different
> +	 file than the one used by the kernel - for instance, "gdb
> +	 program" connected to "gdbserver :PORT ld.so program".  */
> +      int phdrs_size, phdrs2_size, ok = 1;
> +      gdb_byte *buf, *buf2;
> +
> +      /* Take a shortcut for the common case.  If the entry addresses
> +	 match, then it is incredibly unlikely that anything
> +	 complicated has happened.  It's not impossible, if the loader
> +	 and executable are both PIE, but it would still require a
> +	 rare conjunction of load addresses.  */
> +      if (entry_point == bfd_get_start_address (exec_bfd))
> +	return 0;
> +
> +      if (bfd_get_flavour (abfd) == bfd_target_elf_flavour)

solib-svr4.c:1757: error: ‘abfd’ undeclared (first use in this function)
solib-svr4.c:1757: error: (Each undeclared identifier is reported only once
solib-svr4.c:1757: error: for each function it appears in.)


> +	{
> +	  buf = read_program_header (-1, &phdrs_size, NULL);
> +	  buf2 = read_program_headers_from_bfd (exec_bfd, &phdrs2_size);
> +	  if (buf != NULL && buf2 != NULL
> +	      && (phdrs_size != phdrs2_size
> +		  || memcmp (buf, buf2, phdrs_size) != 0))
> +	    ok = 0;
> +	}
> +
> +      xfree (buf);
> +      xfree (buf2);

solib-svr4.c:1747: error: ‘buf’ may be used uninitialized in this function
solib-svr4.c:1747: note: ‘buf’ was declared here
solib-svr4.c:1747: error: ‘buf2’ may be used uninitialized in this function
solib-svr4.c:1747: note: ‘buf2’ was declared here


> +      if (ok)
> +	return entry_point - bfd_get_start_address (exec_bfd);
> +    }
>  
>    return svr4_static_exec_displacement ();
>  }

The previous alignment-based variant
	Re: [patch] Sanity check PIE displacement #2
	http://sourceware.org/ml/gdb-patches/2010-02/msg00346.html

was returning 0 if the PIE check failed.  It has been done intentionally as
svr4_static_exec_displacement is buggy for the case:

* auxv AT_ENTRY is not found
  (it is present in a live spawned or attached process or even in a core file)
  [ therefore svr4_static_exec_displacement is executed at all ]

* exec_bfd is static (it has no .interp)

* exec_bfd is PIE (=ET_DYN).

* GDB does a process attachment.

Reproducer of the svr4_static_exec_displacement bug was posted as:
	Re: [patch 08/15] PIE: Base functionality
	http://sourceware.org/ml/gdb-patches/2010-01/msg00383.html

I had various wrong assumptions about svr4_static_exec_displacement before
(such as it is for some embedded targets) and so I wanted to keep it in place.
But found now the function svr4_static_exec_displacement comes from:
	[PATCH RFA] solib-svr4.c patch for dynamic executables
	http://sourceware.org/ml/gdb-patches/2000-11/msg00040.html
	af21058b0321e272e6d0ad2877f402b286b0fb18

And its goal was ld.so on GNU/Linux so this functionality has been already
superseded by the current checked-in PIE patchset and
svr4_static_exec_displacement can be safely dropped.

Its advantage was the idea of DYNAMIC (ET_DYN) check - imported it now which
in fact removes any regression possibilities as ET_DYN executables have not
worked in GDB at all before anyway.

Its disadvantage was that it was using regcache_read_pc instead of auxv
AT_ENTRY which did not work for attached processes.


The Daniel J.'s problem of wrong PIE offset has multiple solutions:
	./gdbserver/gdbserver :2222 /lib64/ld-linux-x86-64.so.2 ./gdb -nx
+
	./gdb -nx -ex 'target remote localhost:2222' -ex c ./gdb

(a) The most safe + cheap is the DYNAMIC (ET_DYN) check in this patch taken
    from former variant from year 2000.

(b) Mostly reliable (but being silent in the case of non-matching executable)
    PHDRs checking patch:
    RFC: Verify AT_ENTRY before using it
    http://sourceware.org/ml/gdb-patches/2010-02/msg00612.html

(c) non-matching executable check based on alignment,
    less reliable variant of (b)
    Re: [patch] Sanity check PIE displacement #2
    http://sourceware.org/ml/gdb-patches/2010-02/msg00346.html

(d) Using /lib64/ld-linux-x86-64.so.2 as the argument to the client gdb which
    IMO makes it just whole more correct:
    [patch] Support gdb --args ld.so progname
    http://sourceware.org/ml/gdb-patches/2010-02/msg00647.html

The patch below combines (a)+(c)+(b) (in this order).  It also implements
a warning on non-matching exec_bfd vs. target memory.  (d) is still left as
applicable independent patch for approval.

This patch or some its subset should got for gdb_7_1-branch.

No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.


Thanks,
Jan


gdb/
2010-03-01  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Daniel Jacobowitz  <dan@codesourcery.com>

	* solib-svr4.c (read_program_header): Support type == -1 to read
	all program headers.
	(read_program_headers_from_bfd): New function.
	(svr4_static_exec_displacement): Remove and move the comment ...
	(svr4_exec_displacement): ... here.  Remove variable found.  New
	variable displacement.  Check also DYNAMIC.  Verify DISPLACEMENT
	alignment for ELF targets.  Compare target vs. exec_bfd PHDRs for ELF
	targets using read_program_headers_from_bfd.  Remove the call of
	svr4_static_exec_displacement.

--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -451,6 +451,9 @@ bfd_lookup_symbol (bfd *abfd, char *symname)
 /* Read program header TYPE from inferior memory.  The header is found
    by scanning the OS auxillary vector.
 
+   If TYPE == -1, return the program headers instead of the contents of
+   one program header.
+
    Return a pointer to allocated memory holding the program header contents,
    or NULL on failure.  If sucessful, and unless P_SECT_SIZE is NULL, the
    size of those contents is returned to P_SECT_SIZE.  Likewise, the target
@@ -483,8 +486,13 @@ read_program_header (int type, int *p_sect_size, int *p_arch_size)
   else
     return 0;
 
-  /* Find .dynamic section via the PT_DYNAMIC PHDR.  */
-  if (arch_size == 32)
+  /* Find the requested segment.  */
+  if (type == -1)
+    {
+      sect_addr = at_phdr;
+      sect_size = at_phent * at_phnum;
+    }
+  else if (arch_size == 32)
     {
       Elf32_External_Phdr phdr;
       int i;
@@ -1618,55 +1626,30 @@ svr4_special_symbol_handling (void)
   svr4_relocate_main_executable ();
 }
 
-/* Decide if the objfile needs to be relocated.  As indicated above,
-   we will only be here when execution is stopped at the beginning
-   of the program.  Relocation is necessary if the address at which
-   we are presently stopped differs from the start address stored in
-   the executable AND there's no interpreter section.  The condition
-   regarding the interpreter section is very important because if
-   there *is* an interpreter section, execution will begin there
-   instead.  When there is an interpreter section, the start address
-   is (presumably) used by the interpreter at some point to start
-   execution of the program.
-
-   If there is an interpreter, it is normal for it to be set to an
-   arbitrary address at the outset.  The job of finding it is
-   handled in enable_break().
-
-   So, to summarize, relocations are necessary when there is no
-   interpreter section and the start address obtained from the
-   executable is different from the address at which GDB is
-   currently stopped.
-   
-   [ The astute reader will note that we also test to make sure that
-     the executable in question has the DYNAMIC flag set.  It is my
-     opinion that this test is unnecessary (undesirable even).  It
-     was added to avoid inadvertent relocation of an executable
-     whose e_type member in the ELF header is not ET_DYN.  There may
-     be a time in the future when it is desirable to do relocations
-     on other types of files as well in which case this condition
-     should either be removed or modified to accomodate the new file
-     type.  (E.g, an ET_EXEC executable which has been built to be
-     position-independent could safely be relocated by the OS if
-     desired.  It is true that this violates the ABI, but the ABI
-     has been known to be bent from time to time.)  - Kevin, Nov 2000. ]
-   */
+/* Read the ELF program headers from ABFD.  Return the contents and
+   set *PHDRS_SIZE to the size of the program headers.  */
 
-static CORE_ADDR
-svr4_static_exec_displacement (void)
+static gdb_byte *
+read_program_headers_from_bfd (bfd *abfd, int *phdrs_size)
 {
-  asection *interp_sect;
-  struct regcache *regcache
-    = get_thread_arch_regcache (inferior_ptid, target_gdbarch);
-  CORE_ADDR pc = regcache_read_pc (regcache);
+  Elf_Internal_Ehdr *ehdr;
+  gdb_byte *buf;
 
-  interp_sect = bfd_get_section_by_name (exec_bfd, ".interp");
-  if (interp_sect == NULL 
-      && (bfd_get_file_flags (exec_bfd) & DYNAMIC) != 0
-      && (exec_entry_point (exec_bfd, &exec_ops) != pc))
-    return pc - exec_entry_point (exec_bfd, &exec_ops);
+  ehdr = elf_elfheader (abfd);
 
-  return 0;
+  *phdrs_size = ehdr->e_phnum * ehdr->e_phentsize;
+  if (*phdrs_size == 0)
+    return NULL;
+
+  buf = xmalloc (*phdrs_size);
+  if (bfd_seek (abfd, ehdr->e_phoff, SEEK_SET) != 0
+      || bfd_bread (buf, *phdrs_size, abfd) != *phdrs_size)
+    {
+      xfree (buf);
+      return NULL;
+    }
+
+  return buf;
 }
 
 /* We relocate all of the sections by the same amount.  This
@@ -1688,23 +1671,109 @@ svr4_static_exec_displacement (void)
      memory image of the program during dynamic linking.
 
    The same language also appears in Edition 4.0 of the System V
-   ABI and is left unspecified in some of the earlier editions.  */
+   ABI and is left unspecified in some of the earlier editions.
+
+   Decide if the objfile needs to be relocated.  As indicated above, we will
+   only be here when execution is stopped.  But during attachment PC can be at
+   arbitrary address therefore regcache_read_pc can be misleading (contrary to
+   the auxv AT_ENTRY value).  Moreover for executable with interpreter section
+   regcache_read_pc would point to the interpreter and not the main executable.
+
+   So, to summarize, relocations are necessary when the start address obtained
+   from the executable is different from the address in auxv AT_ENTRY entry.
+   
+   [ The astute reader will note that we also test to make sure that
+     the executable in question has the DYNAMIC flag set.  It is my
+     opinion that this test is unnecessary (undesirable even).  It
+     was added to avoid inadvertent relocation of an executable
+     whose e_type member in the ELF header is not ET_DYN.  There may
+     be a time in the future when it is desirable to do relocations
+     on other types of files as well in which case this condition
+     should either be removed or modified to accomodate the new file
+     type.  - Kevin, Nov 2000. ]  */
 
 static CORE_ADDR
 svr4_exec_displacement (void)
 {
-  int found;
   /* ENTRY_POINT is a possible function descriptor - before
      a call to gdbarch_convert_from_func_ptr_addr.  */
-  CORE_ADDR entry_point;
+  CORE_ADDR entry_point, displacement;
 
   if (exec_bfd == NULL)
     return 0;
 
-  if (target_auxv_search (&current_target, AT_ENTRY, &entry_point) == 1)
-    return entry_point - bfd_get_start_address (exec_bfd);
+  /* Therefore for ELF it is ET_EXEC and not ET_DYN.  Both shared libraries
+     being executed themselves and PIE (Position Independent Executable)
+     executables are ET_DYN.  */
+
+  if ((bfd_get_file_flags (exec_bfd) & DYNAMIC) == 0)
+    return 0;
+
+  if (target_auxv_search (&current_target, AT_ENTRY, &entry_point) <= 0)
+    return 0;
+
+  displacement = entry_point - bfd_get_start_address (exec_bfd);
+
+  /* Verify the DISPLACEMENT candidate complies with the required page
+     alignment.  It is cheaper than the program headers comparison below.  */
+
+  if (bfd_get_flavour (exec_bfd) == bfd_target_elf_flavour)
+    {
+      const struct elf_backend_data *elf = get_elf_backend_data (exec_bfd);
+
+      /* p_align of PT_LOAD segments does not specify any alignment but
+	 only congruency of addresses:
+	   p_offset % p_align == p_vaddr % p_align
+	 Kernel is free to load the executable with lower alignment.  */
+
+      if ((displacement & (elf->minpagesize - 1)) != 0)
+	{
+	  warning (_("PIE (Position Independent Executable) displacement "
+		     "%s is not aligned to the minimal page size %s "
+		     "for \"%s\" (wrong executable or version mismatch?)"),
+		   paddress (target_gdbarch, displacement),
+		   paddress (target_gdbarch, elf->minpagesize),
+		   bfd_get_filename (exec_bfd));
+	  return 0;
+	}
+    }
+
+  /* Verify that the auxilliary vector describes the same file as exec_bfd, by
+     comparing their program headers.  If the program headers in the auxilliary
+     vector do not match the program headers in the executable, then we are
+     looking at a different file than the one used by the kernel - for
+     instance, "gdb program" connected to "gdbserver :PORT ld.so program".  */
+
+  if (bfd_get_flavour (exec_bfd) == bfd_target_elf_flavour)
+    {
+      /* Be optimistic and clear OK only if GDB was able to verify the headers
+	 really do not match.  */
+      int phdrs_size, phdrs2_size, ok = 1;
+      gdb_byte *buf, *buf2;
+
+      buf = read_program_header (-1, &phdrs_size, NULL);
+      buf2 = read_program_headers_from_bfd (exec_bfd, &phdrs2_size);
+      if (buf != NULL && buf2 != NULL
+	  && (phdrs_size != phdrs2_size
+	      || memcmp (buf, buf2, phdrs_size) != 0))
+	ok = 0;
+
+      xfree (buf);
+      xfree (buf2);
+
+      if (!ok)
+	{
+	  warning (_("PIE (Position Independent Executable) displacement "
+		     "%s is ingored as invalid as the target program headers "
+		     "do not match those in file \"%s\" (wrong executable or "
+		     "version mismatch?)"),
+		   paddress (target_gdbarch, displacement),
+		   bfd_get_filename (exec_bfd));
+	  return 0;
+	}
+    }
 
-  return svr4_static_exec_displacement ();
+  return displacement;
 }
 
 /* Relocate the main executable.  This function should be called upon

[patch-testcase] Re: RFC: Verify AT_ENTRY before using it

[Jan Kratochvil]

Hi,

just a patch on top of the in-reply-to one for more sanity checking.

It provides `set verbose' message which already exists for a long time in
LM_ADDR_CHECK (not info_verbose-conditioned until recently):
              if (info_verbose)
                {
                  warning (_(".dynamic section for \"%s\" "
                             "is not at the expected address"), so->so_name);
                  warning (_("difference appears to be caused by prelink, "
                             "adjusting expectations"));
                }

No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.

That quadruplication of `-re "Using PIE ...' block (4x2 similar blocks) is
ugly but AFAIK trying to make it more common faces TCL "uplevel" issues
already being solved complicated way in lib/gdb.exp proc gdb_test_multiple.


Thanks,
Jan


gdb/
2010-03-01  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* solib-svr4.c (svr4_exec_displacement):  Print DISPLACEMENT if
	INFO_VERBOSE.

gdb/testsuite/
2010-03-01  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/break-interp.exp: Create new displacement parameter value
	for the test_ld calls.
	(reach): New parameter displacement, verify its content.  New push of
	pf_prefix "reach-$func:".
	(test_core): New parameter displacement, verify its content.  New push
	of pf_prefix "core:".  New command "set verbose on".
	(test_attach): New parameter displacement, verify its content.  New
	push of pf_prefix "attach:".  New command "set verbose on".
	(test_ld): New parameter displacement, pass it to the reach, test_core
	and test_attach calls and verify its content in the "ld.so exit" test.

--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1773,6 +1773,18 @@ svr4_exec_displacement (void)
 	}
     }
 
+  if (info_verbose)
+    {
+      /* It can be printed repeatedly as there is no easy way to check
+	 the executable symbols/file has been already relocated to
+	 displacement.  */
+
+      warning (_("Using PIE (Position Independent Executable) "
+		 "displacement %s for \"%s\""),
+	       paddress (target_gdbarch, displacement),
+	       bfd_get_filename (exec_bfd));
+    }
+
   return displacement;
 }
 
--- a/gdb/testsuite/gdb.base/break-interp.exp
+++ b/gdb/testsuite/gdb.base/break-interp.exp
@@ -221,12 +221,40 @@ proc strip_debug {dest} {
 }
 
 # `runto' does not check we stopped really at the function we specified.
-proc reach {func command} {
+# DISPLACEMENT can be "NONE", "ZERO" or "NONZERO"
+proc reach {func command displacement} {
     global gdb_prompt
 
+    global pf_prefix
+    set old_ldprefix $pf_prefix
+    lappend pf_prefix "reach-$func:"
+
     if [gdb_breakpoint $func allow-pending] {
-	set test "reach $func"
+	set test "reach"
+	set test_displacement "seen displacement message as $displacement"
 	gdb_test_multiple $command $test {
+	    -re "Using PIE \\(Position Independent Executable\\) displacement 0x0 " {
+		# Missing "$gdb_prompt $" is intentional.
+		if {$displacement == "ZERO"} {
+		    pass $test_displacement
+		    # Permit multiple such messages.
+		    set displacement "FOUND-$displacement"
+		} elseif {$displacement != "FOUND-ZERO"} {
+		    fail $test_displacement
+		}
+		exp_continue
+	    }
+	    -re "Using PIE \\(Position Independent Executable\\) displacement" {
+		# Missing "$gdb_prompt $" is intentional.
+		if {$displacement == "NONZERO"} {
+		    pass $test_displacement
+		    # Permit multiple such messages.
+		    set displacement "FOUND-$displacement"
+		} elseif {$displacement != "FOUND-NONZERO"} {
+		    fail $test_displacement
+		}
+		exp_continue
+	    }
 	    -re "Breakpoint \[0-9\]+, $func \\(.*\\) at .*:\[0-9\]+\r\n.*$gdb_prompt $" {
 		pass $test
 	    }
@@ -234,10 +262,15 @@ proc reach {func command} {
 		pass $test
 	    }
 	}
+	if ![regexp {^(NONE|FOUND-.*)$} $displacement] {
+	    fail $test_displacement
+	}
     }
+
+    set pf_prefix $old_ldprefix
 }
 
-proc test_core {file} {
+proc test_core {file displacement} {
     global srcdir subdir gdb_prompt
 
     set corefile [core_find $file {} "segv"]
@@ -245,6 +278,10 @@ proc test_core {file} {
 	return
     }
 
+    global pf_prefix
+    set old_ldprefix $pf_prefix
+    lappend pf_prefix "core:"
+
     gdb_exit
     gdb_start
     # Clear it to never find any separate debug infos in $debug_root.
@@ -252,14 +289,50 @@ proc test_core {file} {
     gdb_reinitialize_dir $srcdir/$subdir
     gdb_load $file
 
-    # Do not check the binary filename as it may be truncated.
-    gdb_test "core-file $corefile" "Core was generated by .*\r\n#0 .*" "core loaded"
+    # Print the "PIE (Position Independent Executable) displacement" message.
+    gdb_test "set verbose on"
+
+    set test "core loaded"
+    set test_displacement "seen displacement message"
+    gdb_test_multiple "core-file $corefile" $test {
+	-re "Using PIE \\(Position Independent Executable\\) displacement 0x0 " {
+	    # Missing "$gdb_prompt $" is intentional.
+	    if {$displacement == "ZERO"} {
+		pass $test_displacement
+		# Permit multiple such messages.
+		set displacement "FOUND-$displacement"
+	    } elseif {$displacement != "FOUND-ZERO"} {
+		fail $test_displacement
+	    }
+	    exp_continue
+	}
+	-re "Using PIE \\(Position Independent Executable\\) displacement" {
+	    # Missing "$gdb_prompt $" is intentional.
+	    if {$displacement == "NONZERO"} {
+		pass $test_displacement
+		# Permit multiple such messages.
+		set displacement "FOUND-$displacement"
+	    } elseif {$displacement != "FOUND-NONZERO"} {
+		fail $test_displacement
+	    }
+	    exp_continue
+	}
+	-re "Core was generated by .*\r\n#0 .*$gdb_prompt $" {
+	    # Do not check the binary filename as it may be truncated.
+	    pass $test
+	}
+    }
+    if ![regexp {^(NONE|FOUND-.*)$} $displacement] {
+	fail $test_displacement
+    }
 
     gdb_test "bt" "#\[0-9\]+ +\[^\r\n\]*\\mlibfunc\\M\[^\r\n\]*\r\n#\[0-9\]+ +\[^\r\n\]*\\mmain\\M.*" "core main bt"
+
+    set pf_prefix $old_ldprefix
 }
 
-proc test_attach {file} {
-    global board_info
+proc test_attach {file displacement} {
+    global board_info gdb_prompt
 
     gdb_exit
 
@@ -287,16 +360,66 @@ proc test_attach {file} {
 	}
     }
 
+    global pf_prefix
+    set old_ldprefix $pf_prefix
+    lappend pf_prefix "attach:"
+
     gdb_exit
     gdb_start
-    gdb_test "attach $pid" "Attaching to process $pid\r\n.*" "attach"
+
+    # Print the "PIE (Position Independent Executable) displacement" message.
+    gdb_test "set verbose on"
+
+    set test "attach"
+    gdb_test_multiple "attach $pid" $test {
+	-re "Attaching to process $pid\r\n" {
+	    # Missing "$gdb_prompt $" is intentional.
+	    pass $test
+	}
+    }
+
+    set test "attach final prompt"
+    set test_displacement "seen displacement message"
+    gdb_test_multiple "" $test {
+	-re "Using PIE \\(Position Independent Executable\\) displacement 0x0 " {
+	    # Missing "$gdb_prompt $" is intentional.
+	    if {$displacement == "ZERO"} {
+		pass $test_displacement
+		# Permit multiple such messages.
+		set displacement "FOUND-$displacement"
+	    } elseif {$displacement != "FOUND-ZERO"} {
+		fail $test_displacement
+	    }
+	    exp_continue
+	}
+	-re "Using PIE \\(Position Independent Executable\\) displacement" {
+	    # Missing "$gdb_prompt $" is intentional.
+	    if {$displacement == "NONZERO"} {
+		pass $test_displacement
+		# Permit multiple such messages.
+		set displacement "FOUND-$displacement"
+	    } elseif {$displacement != "FOUND-NONZERO"} {
+		fail $test_displacement
+	    }
+	    exp_continue
+	}
+	-re "$gdb_prompt $" {
+	    pass $test
+	}
+    }
+    if ![regexp {^(NONE|FOUND-.*)$} $displacement] {
+	fail $test_displacement
+    }
+
     gdb_test "bt" "#\[0-9\]+ +\[^\r\n\]*\\mlibfunc\\M\[^\r\n\]*\r\n#\[0-9\]+ +\[^\r\n\]*\\mmain\\M.*" "attach main bt"
     gdb_exit
 
     remote_exec host "kill -9 $pid"
+
+    set pf_prefix $old_ldprefix
 }
 
-proc test_ld {file ifmain trynosym} {
+proc test_ld {file ifmain trynosym displacement} {
     global srcdir subdir gdb_prompt
 
     # First test normal `file'-command loaded $FILE with symbols.
@@ -308,20 +431,25 @@ proc test_ld {file ifmain trynosym} {
     gdb_reinitialize_dir $srcdir/$subdir
     gdb_load $file
 
-    reach "dl_main" "run segv"
+    # Print the "PIE (Position Independent Executable) displacement" message.
+    gdb_test "set verbose on"
+
+    reach "dl_main" "run segv" $displacement
 
     gdb_test "bt" "#0 +\[^\r\n\]*\\mdl_main\\M.*" "dl bt"
 
     if $ifmain {
-	reach "main" continue
+	# Displacement message will be printed the second time on initializing
+	# the linker from svr4_special_symbol_handling.
+	reach "main" continue $displacement
 
-	reach "libfunc" continue
+	reach "libfunc" continue "NONE"
 
 	gdb_test "bt" "#0 +\[^\r\n\]*\\mlibfunc\\M\[^\r\n\]*\r\n#1 +\[^\r\n\]*\\mmain\\M.*" "main bt"
 
-	test_core $file
+	test_core $file $displacement
 
-	test_attach $file
+	test_attach $file $displacement
     }
 
     if !$trynosym {
@@ -341,12 +469,15 @@ proc test_ld {file ifmain trynosym} {
     gdb_test "set debug-file-directory"
     gdb_reinitialize_dir $srcdir/$subdir
 
+    # Print the "PIE (Position Independent Executable) displacement" message.
+    gdb_test "set verbose on"
+
     # Test no (error) message has been printed by `exec-file'.
     set escapedfile [string_to_regexp $file]
     gdb_test "exec-file $file" "exec-file $escapedfile" "load"
 
     if $ifmain {
-	reach "dl_main" run
+	reach "dl_main" run $displacement
 
 	set test "info files"
 	set entrynohex ""
@@ -363,7 +494,40 @@ proc test_ld {file ifmain trynosym} {
     } else {
 	# There is no symbol to break at ld.so.  Moreover it can exit with an
 	# error code.
-	gdb_test "run" "Program exited (normally|with code \[0-9\]+)\\." "ld.so exit"
+
+	set test "ld.so exit"
+	set test_displacement "seen displacement message"
+	gdb_test_multiple "run" $test {
+	    -re "Using PIE \\(Position Independent Executable\\) displacement 0x0 " {
+		# Missing "$gdb_prompt $" is intentional.
+		if {$displacement == "ZERO"} {
+		    pass $test_displacement
+		    # Permit multiple such messages.
+		    set displacement "FOUND-$displacement"
+		} elseif {$displacement != "FOUND-ZERO"} {
+		    fail $test_displacement
+		}
+		exp_continue
+	    }
+	    -re "Using PIE \\(Position Independent Executable\\) displacement" {
+		# Missing "$gdb_prompt $" is intentional.
+		if {$displacement == "NONZERO"} {
+		    pass $test_displacement
+		    # Permit multiple such messages.
+		    set displacement "FOUND-$displacement"
+		} elseif {$displacement != "FOUND-NONZERO"} {
+		    fail $test_displacement
+		}
+		exp_continue
+	    }
+	    -re "Program exited (normally|with code \[0-9\]+)\\.\r\n$gdb_prompt $" {
+		# Do not check the binary filename as it may be truncated.
+		pass $test
+	    }
+	}
+	if ![regexp {^(NONE|FOUND-.*)$} $displacement] {
+	    fail $test_displacement
+	}
     }
 
     set pf_prefix $old_ldprefix
@@ -450,7 +614,12 @@ foreach ldprelink {NO YES} {
 	if ![prelink$ldprelink $interp] {
 	    continue
 	}
-	test_ld $interp 0 [expr {$ldsepdebug == "NO"}]
+	if {$ldprelink == "NO"} {
+	    set displacement "NONZERO"
+	} else {
+	    set displacement "ZERO"
+	}
+	test_ld $interp 0 [expr {$ldsepdebug == "NO"}] $displacement
 
 	if ![copy $interp $interp_saved] {
 	    continue
@@ -531,7 +700,14 @@ foreach ldprelink {NO YES} {
 
 		    if {[prelink$binprelink "--dynamic-linker=$interp --ld-library-path=$dir $exec $interp [concat $dests]" [file tail $exec]]
 		        && [copy $interp_saved $interp]} {
-			test_ld $exec 1 [expr {$binsepdebug == "NO"}]
+			if {$binpie == "NO"} {
+			    set displacement "NONE"
+			} elseif {$binprelink == "NO"} {
+			    set displacement "NONZERO"
+			} else {
+			    set displacement "ZERO"
+			}
+			test_ld $exec 1 [expr {$binsepdebug == "NO"}] $displacement
 		    }
 		}
 	    }

Re: RFC: Verify AT_ENTRY before using it

[Daniel Jacobowitz]

On Mon, Mar 01, 2010 at 09:04:28PM +0100, Jan Kratochvil wrote:
> The patch below combines (a)+(c)+(b) (in this order).  It also implements
> a warning on non-matching exec_bfd vs. target memory.  (d) is still left as
> applicable independent patch for approval.

Thanks for working on this.  I agree with all of it except the
warnings.  Since there's no other working way to accomplish (d)
today, and I have a use case for that which won't go away, I don't
think it's appropriate to warn.

Note, if the program headers matched but the alignment was not page
aligned, that would be a different case: that seems worth warning
about.  I don't know what that would mean.  Is it a case you've
encountered?

Without the warnings, this is OK to check in.  If you want the
warnings, we should discuss it a little more.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch-testcase] Re: RFC: Verify AT_ENTRY before using it

[Daniel Jacobowitz]

On Mon, Mar 01, 2010 at 10:22:16PM +0100, Jan Kratochvil wrote:
> +  if (info_verbose)
> +    {
> +      /* It can be printed repeatedly as there is no easy way to check
> +	 the executable symbols/file has been already relocated to
> +	 displacement.  */
> +
> +      warning (_("Using PIE (Position Independent Executable) "
> +		 "displacement %s for \"%s\""),
> +	       paddress (target_gdbarch, displacement),
> +	       bfd_get_filename (exec_bfd));
> +    }
> +
>    return displacement;
>  }

This isn't a warning; just use printf_unfiltered.

> +	    -re "Using PIE \\(Position Independent Executable\\) displacement 0x0 " {
> +		# Missing "$gdb_prompt $" is intentional.
> +		if {$displacement == "ZERO"} {
> +		    pass $test_displacement
> +		    # Permit multiple such messages.
> +		    set displacement "FOUND-$displacement"
> +		} elseif {$displacement != "FOUND-ZERO"} {
> +		    fail $test_displacement
> +		}
> +		exp_continue
> +	    }
> +	    -re "Using PIE \\(Position Independent Executable\\) displacement" {
> +		# Missing "$gdb_prompt $" is intentional.
> +		if {$displacement == "NONZERO"} {
> +		    pass $test_displacement
> +		    # Permit multiple such messages.
> +		    set displacement "FOUND-$displacement"
> +		} elseif {$displacement != "FOUND-NONZERO"} {
> +		    fail $test_displacement
> +		}
> +		exp_continue
> +	    }

This isn't a safe way to use expect, unfortunately.  If you have two
patterns, and the second one matches a subset of the first, either
might match; it depends where the OS and C library break up GDB's call
to 'write'.  You have two choices: combine the patterns, and check the
displacement inside, or make the second pattern not match the first.
For instance, " 0x[0-9a-f]*[1-9a-f]*[0-9a-f]* " won't match 0x0.

Otherwise OK.

-- 
Daniel Jacobowitz
CodeSourcery

Re: RFC: Verify AT_ENTRY before using it

[Jan Kratochvil]

On Mon, 08 Mar 2010 23:06:35 +0100, Daniel Jacobowitz wrote:
> On Mon, Mar 01, 2010 at 09:04:28PM +0100, Jan Kratochvil wrote:
> > The patch below combines (a)+(c)+(b) (in this order).  It also implements
> > a warning on non-matching exec_bfd vs. target memory.  (d) is still left as
> > applicable independent patch for approval.
> 
> Thanks for working on this.  I agree with all of it except the
> warnings.

It has been checked-in without the warning() calls.


> Since there's no other working way to accomplish (d)
[ (d) refers to:
	[patch] Support gdb --args ld.so progname
	http://sourceware.org/ml/gdb-patches/2010-02/msg00647.html
]
> today, and I have a use case for that which won't go away, I don't
> think it's appropriate to warn.

Factually I do not understand how the warnings could currently ever be printed
with the DYNAMIC/ET_DYN check there (or do you run PIEs with gdbserver?).


> Note, if the program headers matched but the alignment was not page
> aligned, that would be a different case: that seems worth warning
> about.  I don't know what that would mean.  Is it a case you've
> encountered?

No.  I cannot imagine such a rare case would happen in real world.


> Without the warnings, this is OK to check in.  If you want the
> warnings, we should discuss it a little more.

As I (currently) do not use gdbserver where such exec_bfd vs. target memory
difference could only happen I am only curious why it did not work.

Or do you at least approve simple `if (info_verbose) warning()' patch?

I do not have intention to push for the warning() calls more (now).


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2010-03/msg00096.html

--- src/gdb/ChangeLog	2010/03/10 18:41:37	1.11466
+++ src/gdb/ChangeLog	2010/03/10 20:50:48	1.11467
@@ -1,3 +1,16 @@
+2010-03-10  Jan Kratochvil  <jan.kratochvil@redhat.com>
+	    Daniel Jacobowitz  <dan@codesourcery.com>
+
+	* solib-svr4.c (read_program_header): Support type == -1 to read
+	all program headers.
+	(read_program_headers_from_bfd): New function.
+	(svr4_static_exec_displacement): Remove and move the comment ...
+	(svr4_exec_displacement): ... here.  Remove variable found.  New
+	variable displacement.  Check also DYNAMIC.  Verify DISPLACEMENT
+	alignment for ELF targets.  Compare target vs. exec_bfd PHDRs for ELF
+	targets using read_program_headers_from_bfd.  Remove the call of
+	svr4_static_exec_displacement.
+
 2010-03-10  Tom Tromey  <tromey@redhat.com>
 
 	* dwarf2read.c (struct pubnames_header): Remove.
--- src/gdb/solib-svr4.c	2010/03/09 18:09:07	1.127
+++ src/gdb/solib-svr4.c	2010/03/10 20:50:55	1.128
@@ -451,6 +451,9 @@
 /* Read program header TYPE from inferior memory.  The header is found
    by scanning the OS auxillary vector.
 
+   If TYPE == -1, return the program headers instead of the contents of
+   one program header.
+
    Return a pointer to allocated memory holding the program header contents,
    or NULL on failure.  If sucessful, and unless P_SECT_SIZE is NULL, the
    size of those contents is returned to P_SECT_SIZE.  Likewise, the target
@@ -483,8 +486,13 @@
   else
     return 0;
 
-  /* Find .dynamic section via the PT_DYNAMIC PHDR.  */
-  if (arch_size == 32)
+  /* Find the requested segment.  */
+  if (type == -1)
+    {
+      sect_addr = at_phdr;
+      sect_size = at_phent * at_phnum;
+    }
+  else if (arch_size == 32)
     {
       Elf32_External_Phdr phdr;
       int i;
@@ -1625,55 +1633,30 @@
   svr4_relocate_main_executable ();
 }
 
-/* Decide if the objfile needs to be relocated.  As indicated above,
-   we will only be here when execution is stopped at the beginning
-   of the program.  Relocation is necessary if the address at which
-   we are presently stopped differs from the start address stored in
-   the executable AND there's no interpreter section.  The condition
-   regarding the interpreter section is very important because if
-   there *is* an interpreter section, execution will begin there
-   instead.  When there is an interpreter section, the start address
-   is (presumably) used by the interpreter at some point to start
-   execution of the program.
-
-   If there is an interpreter, it is normal for it to be set to an
-   arbitrary address at the outset.  The job of finding it is
-   handled in enable_break().
-
-   So, to summarize, relocations are necessary when there is no
-   interpreter section and the start address obtained from the
-   executable is different from the address at which GDB is
-   currently stopped.
-   
-   [ The astute reader will note that we also test to make sure that
-     the executable in question has the DYNAMIC flag set.  It is my
-     opinion that this test is unnecessary (undesirable even).  It
-     was added to avoid inadvertent relocation of an executable
-     whose e_type member in the ELF header is not ET_DYN.  There may
-     be a time in the future when it is desirable to do relocations
-     on other types of files as well in which case this condition
-     should either be removed or modified to accomodate the new file
-     type.  (E.g, an ET_EXEC executable which has been built to be
-     position-independent could safely be relocated by the OS if
-     desired.  It is true that this violates the ABI, but the ABI
-     has been known to be bent from time to time.)  - Kevin, Nov 2000. ]
-   */
+/* Read the ELF program headers from ABFD.  Return the contents and
+   set *PHDRS_SIZE to the size of the program headers.  */
 
-static CORE_ADDR
-svr4_static_exec_displacement (void)
+static gdb_byte *
+read_program_headers_from_bfd (bfd *abfd, int *phdrs_size)
 {
-  asection *interp_sect;
-  struct regcache *regcache
-    = get_thread_arch_regcache (inferior_ptid, target_gdbarch);
-  CORE_ADDR pc = regcache_read_pc (regcache);
-
-  interp_sect = bfd_get_section_by_name (exec_bfd, ".interp");
-  if (interp_sect == NULL 
-      && (bfd_get_file_flags (exec_bfd) & DYNAMIC) != 0
-      && (exec_entry_point (exec_bfd, &exec_ops) != pc))
-    return pc - exec_entry_point (exec_bfd, &exec_ops);
+  Elf_Internal_Ehdr *ehdr;
+  gdb_byte *buf;
 
-  return 0;
+  ehdr = elf_elfheader (abfd);
+
+  *phdrs_size = ehdr->e_phnum * ehdr->e_phentsize;
+  if (*phdrs_size == 0)
+    return NULL;
+
+  buf = xmalloc (*phdrs_size);
+  if (bfd_seek (abfd, ehdr->e_phoff, SEEK_SET) != 0
+      || bfd_bread (buf, *phdrs_size, abfd) != *phdrs_size)
+    {
+      xfree (buf);
+      return NULL;
+    }
+
+  return buf;
 }
 
 /* We relocate all of the sections by the same amount.  This
@@ -1695,23 +1678,93 @@
      memory image of the program during dynamic linking.
 
    The same language also appears in Edition 4.0 of the System V
-   ABI and is left unspecified in some of the earlier editions.  */
+   ABI and is left unspecified in some of the earlier editions.
+
+   Decide if the objfile needs to be relocated.  As indicated above, we will
+   only be here when execution is stopped.  But during attachment PC can be at
+   arbitrary address therefore regcache_read_pc can be misleading (contrary to
+   the auxv AT_ENTRY value).  Moreover for executable with interpreter section
+   regcache_read_pc would point to the interpreter and not the main executable.
+
+   So, to summarize, relocations are necessary when the start address obtained
+   from the executable is different from the address in auxv AT_ENTRY entry.
+   
+   [ The astute reader will note that we also test to make sure that
+     the executable in question has the DYNAMIC flag set.  It is my
+     opinion that this test is unnecessary (undesirable even).  It
+     was added to avoid inadvertent relocation of an executable
+     whose e_type member in the ELF header is not ET_DYN.  There may
+     be a time in the future when it is desirable to do relocations
+     on other types of files as well in which case this condition
+     should either be removed or modified to accomodate the new file
+     type.  - Kevin, Nov 2000. ]  */
 
 static CORE_ADDR
 svr4_exec_displacement (void)
 {
-  int found;
   /* ENTRY_POINT is a possible function descriptor - before
      a call to gdbarch_convert_from_func_ptr_addr.  */
-  CORE_ADDR entry_point;
+  CORE_ADDR entry_point, displacement;
 
   if (exec_bfd == NULL)
     return 0;
 
-  if (target_auxv_search (&current_target, AT_ENTRY, &entry_point) == 1)
-    return entry_point - bfd_get_start_address (exec_bfd);
+  /* Therefore for ELF it is ET_EXEC and not ET_DYN.  Both shared libraries
+     being executed themselves and PIE (Position Independent Executable)
+     executables are ET_DYN.  */
+
+  if ((bfd_get_file_flags (exec_bfd) & DYNAMIC) == 0)
+    return 0;
+
+  if (target_auxv_search (&current_target, AT_ENTRY, &entry_point) <= 0)
+    return 0;
+
+  displacement = entry_point - bfd_get_start_address (exec_bfd);
+
+  /* Verify the DISPLACEMENT candidate complies with the required page
+     alignment.  It is cheaper than the program headers comparison below.  */
+
+  if (bfd_get_flavour (exec_bfd) == bfd_target_elf_flavour)
+    {
+      const struct elf_backend_data *elf = get_elf_backend_data (exec_bfd);
+
+      /* p_align of PT_LOAD segments does not specify any alignment but
+	 only congruency of addresses:
+	   p_offset % p_align == p_vaddr % p_align
+	 Kernel is free to load the executable with lower alignment.  */
+
+      if ((displacement & (elf->minpagesize - 1)) != 0)
+	return 0;
+    }
+
+  /* Verify that the auxilliary vector describes the same file as exec_bfd, by
+     comparing their program headers.  If the program headers in the auxilliary
+     vector do not match the program headers in the executable, then we are
+     looking at a different file than the one used by the kernel - for
+     instance, "gdb program" connected to "gdbserver :PORT ld.so program".  */
+
+  if (bfd_get_flavour (exec_bfd) == bfd_target_elf_flavour)
+    {
+      /* Be optimistic and clear OK only if GDB was able to verify the headers
+	 really do not match.  */
+      int phdrs_size, phdrs2_size, ok = 1;
+      gdb_byte *buf, *buf2;
+
+      buf = read_program_header (-1, &phdrs_size, NULL);
+      buf2 = read_program_headers_from_bfd (exec_bfd, &phdrs2_size);
+      if (buf != NULL && buf2 != NULL
+	  && (phdrs_size != phdrs2_size
+	      || memcmp (buf, buf2, phdrs_size) != 0))
+	ok = 0;
+
+      xfree (buf);
+      xfree (buf2);
+
+      if (!ok)
+	return 0;
+    }
 
-  return svr4_static_exec_displacement ();
+  return displacement;
 }
 
 /* Relocate the main executable.  This function should be called upon

Re: RFC: Verify AT_ENTRY before using it

[Daniel Jacobowitz]

On Wed, Mar 10, 2010 at 10:11:38PM +0100, Jan Kratochvil wrote:
> Factually I do not understand how the warnings could currently ever be printed
> with the DYNAMIC/ET_DYN check there (or do you run PIEs with gdbserver?).

I haven't tried it myself, but it should work fine to do so.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch-testcase] Re: RFC: Verify AT_ENTRY before using it

[Jan Kratochvil]

On Mon, 08 Mar 2010 23:13:50 +0100, Daniel Jacobowitz wrote:
> On Mon, Mar 01, 2010 at 10:22:16PM +0100, Jan Kratochvil wrote:
> > +  if (info_verbose)
> > +    {
> > +      /* It can be printed repeatedly as there is no easy way to check
> > +	 the executable symbols/file has been already relocated to
> > +	 displacement.  */
> > +
> > +      warning (_("Using PIE (Position Independent Executable) "
> > +		 "displacement %s for \"%s\""),
> > +	       paddress (target_gdbarch, displacement),
> > +	       bfd_get_filename (exec_bfd));
> > +    }
> > +
> >    return displacement;
> >  }
> 
> This isn't a warning; just use printf_unfiltered.

While I agree I wanted to keep this message in sync with its existing PIC
counterpart.  Therefore changed even the PIC message now in this patch.

The PIC message printing is already changed by gdb-7.0->7.1 only if
(info_verbose) therefore possible expectations on GDB output have been already
broken.


> > +	    -re "Using PIE \\(Position Independent Executable\\) displacement 0x0 " {
> > +	    -re "Using PIE \\(Position Independent Executable\\) displacement" {
> 
> This isn't a safe way to use expect, unfortunately.  If you have two
> patterns, and the second one matches a subset of the first, either
> might match; it depends where the OS and C library break up GDB's call
> to 'write'.

Missed that, thanks.


This patch is no longer a pre-requisite for anything and while it would be
fine I do not find it too important myself.  There will be changes in this
code anyway on some planned unposted PIE cleanup/fixup patches.

No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.


Thanks,
Jan


gdb/
2010-03-11  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* solib-svr4.c (LM_ADDR_CHECK) <info_verbose>: Use printf_unfiltered
	for the PIC displacement, print also the displacement value.
	(svr4_exec_displacement):  Print DISPLACEMENT if INFO_VERBOSE.

gdb/testsuite/
2010-03-11  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/break-interp.exp: Create new displacement parameter value
	for the test_ld calls.
	(reach): New parameter displacement, verify its content.  New push of
	pf_prefix "reach-$func:".  Import global expect_out.
	(test_core): New parameter displacement, verify its content.  New push
	of pf_prefix "core:".  New command "set verbose on".  Import global
	expect_out.
	(test_attach): New parameter displacement, verify its content.  New
	push of pf_prefix "attach:".  New command "set verbose on".  Import
	global expect_out.
	(test_ld): New parameter displacement, pass it to the reach, test_core
	and test_attach calls and verify its content in the "ld.so exit" test.
	* gdb.base/prelink.exp: Remove gdb_exit and final return.
	(prelink): Update expected text, use gdb_test.

--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -243,12 +243,10 @@ LM_ADDR_CHECK (struct so_list *so, bfd *abfd)
 	      l_addr = l_dynaddr - dynaddr;
 
 	      if (info_verbose)
-		{
-		  warning (_(".dynamic section for \"%s\" "
-			     "is not at the expected address"), so->so_name);
-		  warning (_("difference appears to be caused by prelink, "
-			     "adjusting expectations"));
-		}
+		printf_unfiltered (_("Using PIC (Position Independent Code) "
+				     "prelink displacement %s for \"%s\".\n"),
+				   paddress (target_gdbarch, l_addr),
+				   so->so_name);
 	    }
 	  else
 	    warning (_(".dynamic section for \"%s\" "
@@ -1767,6 +1765,18 @@ svr4_exec_displacement (CORE_ADDR *displacementp)
 	return 0;
     }
 
+  if (info_verbose)
+    {
+      /* It can be printed repeatedly as there is no easy way to check
+	 the executable symbols/file has been already relocated to
+	 displacement.  */
+
+      printf_unfiltered (_("Using PIE (Position Independent Executable) "
+			   "displacement %s for \"%s\".\n"),
+			 paddress (target_gdbarch, displacement),
+			 bfd_get_filename (exec_bfd));
+    }
+
   *displacementp = displacement;
   return 1;
 }
--- a/gdb/testsuite/gdb.base/break-interp.exp
+++ b/gdb/testsuite/gdb.base/break-interp.exp
@@ -221,30 +221,61 @@ proc strip_debug {dest} {
 }
 
 # `runto' does not check we stopped really at the function we specified.
-proc reach {func command} {
-    global gdb_prompt
+# DISPLACEMENT can be "NONE", "ZERO" or "NONZERO"
+proc reach {func command displacement} {
+    global gdb_prompt expect_out
+
+    global pf_prefix
+    set old_ldprefix $pf_prefix
+    lappend pf_prefix "reach-$func:"
 
     if [gdb_breakpoint $func allow-pending] {
-	set test "reach $func"
+	set test "reach"
+	set test_displacement "seen displacement message as $displacement"
 	gdb_test_multiple $command $test {
+	    -re "Using PIE \\(Position Independent Executable\\) displacement (0x\[0-9a-f\]+) " {
+		# Missing "$gdb_prompt $" is intentional.
+		if {$expect_out(1,string) == "0x0"} {
+		    set case "ZERO"
+		} else {
+		    set case "NONZERO"
+		}
+		if {$displacement == $case} {
+		    pass $test_displacement
+		    # Permit multiple such messages.
+		    set displacement "FOUND-$displacement"
+		} elseif {$displacement != "FOUND-$case"} {
+		    fail $test_displacement
+		}
+		exp_continue
+	    }
 	    -re "Breakpoint \[0-9\]+, $func \\(.*\\) at .*:\[0-9\]+\r\n.*$gdb_prompt $" {
 		pass $test
 	    }
-	    -re "Breakpoint \[0-9\]+, \[0-9xa-f\]+ in $func \\(\\)( from .*)?\r\n$gdb_prompt $" { 
+	    -re "Breakpoint \[0-9\]+, \[0-9xa-f\]+ in $func \\(\\)( from .*)?\r\n$gdb_prompt $" {
 		pass $test
 	    }
 	}
+	if ![regexp {^(NONE|FOUND-.*)$} $displacement] {
+	    fail $test_displacement
+	}
     }
+
+    set pf_prefix $old_ldprefix
 }
 
-proc test_core {file} {
-    global srcdir subdir gdb_prompt
+proc test_core {file displacement} {
+    global srcdir subdir gdb_prompt expect_out
 
     set corefile [core_find $file {} "segv"]
     if {$corefile == ""} {
 	return
     }
 
+    global pf_prefix
+    set old_ldprefix $pf_prefix
+    lappend pf_prefix "core:"
+
     gdb_exit
     gdb_start
     # Clear it to never find any separate debug infos in $debug_root.
@@ -252,14 +283,44 @@ proc test_core {file} {
     gdb_reinitialize_dir $srcdir/$subdir
     gdb_load $file
 
-    # Do not check the binary filename as it may be truncated.
-    gdb_test "core-file $corefile" "Core was generated by .*\r\n#0 .*" "core loaded"
+    # Print the "PIE (Position Independent Executable) displacement" message.
+    gdb_test "set verbose on"
+
+    set test "core loaded"
+    set test_displacement "seen displacement message"
+    gdb_test_multiple "core-file $corefile" $test {
+	-re "Using PIE \\(Position Independent Executable\\) displacement (0x\[0-9a-f\]+) " {
+	    # Missing "$gdb_prompt $" is intentional.
+	    if {$expect_out(1,string) == "0x0"} {
+		set case "ZERO"
+	    } else {
+		set case "NONZERO"
+	    }
+	    if {$displacement == $case} {
+		pass $test_displacement
+		# Permit multiple such messages.
+		set displacement "FOUND-$displacement"
+	    } elseif {$displacement != "FOUND-$case"} {
+		fail $test_displacement
+	    }
+	    exp_continue
+	}
+	-re "Core was generated by .*\r\n#0 .*$gdb_prompt $" {
+	    # Do not check the binary filename as it may be truncated.
+	    pass $test
+	}
+    }
+    if ![regexp {^(NONE|FOUND-.*)$} $displacement] {
+	fail $test_displacement
+    }
 
     gdb_test "bt" "#\[0-9\]+ +\[^\r\n\]*\\mlibfunc\\M\[^\r\n\]*\r\n#\[0-9\]+ +\[^\r\n\]*\\mmain\\M.*" "core main bt"
+
+    set pf_prefix $old_ldprefix
 }
 
-proc test_attach {file} {
-    global board_info
+proc test_attach {file displacement} {
+    global board_info gdb_prompt expect_out
 
     gdb_exit
 
@@ -287,17 +348,61 @@ proc test_attach {file} {
 	}
     }
 
+    global pf_prefix
+    set old_ldprefix $pf_prefix
+    lappend pf_prefix "attach:"
+
     gdb_exit
     gdb_start
-    gdb_test "attach $pid" "Attaching to process $pid\r\n.*" "attach"
+
+    # Print the "PIE (Position Independent Executable) displacement" message.
+    gdb_test "set verbose on"
+
+    set test "attach"
+    gdb_test_multiple "attach $pid" $test {
+	-re "Attaching to process $pid\r\n" {
+	    # Missing "$gdb_prompt $" is intentional.
+	    pass $test
+	}
+    }
+
+    set test "attach final prompt"
+    set test_displacement "seen displacement message"
+    gdb_test_multiple "" $test {
+	-re "Using PIE \\(Position Independent Executable\\) displacement (0x\[0-9a-f\]+) " {
+	    # Missing "$gdb_prompt $" is intentional.
+	    if {$expect_out(1,string) == "0x0"} {
+		set case "ZERO"
+	    } else {
+		set case "NONZERO"
+	    }
+	    if {$displacement == $case} {
+		pass $test_displacement
+		# Permit multiple such messages.
+		set displacement "FOUND-$displacement"
+	    } elseif {$displacement != "FOUND-$case"} {
+		fail $test_displacement
+	    }
+	    exp_continue
+	}
+	-re "$gdb_prompt $" {
+	    pass $test
+	}
+    }
+    if ![regexp {^(NONE|FOUND-.*)$} $displacement] {
+	fail $test_displacement
+    }
+
     gdb_test "bt" "#\[0-9\]+ +\[^\r\n\]*\\mlibfunc\\M\[^\r\n\]*\r\n#\[0-9\]+ +\[^\r\n\]*\\mmain\\M.*" "attach main bt"
     gdb_exit
 
     remote_exec host "kill -9 $pid"
+
+    set pf_prefix $old_ldprefix
 }
 
-proc test_ld {file ifmain trynosym} {
-    global srcdir subdir gdb_prompt
+proc test_ld {file ifmain trynosym displacement} {
+    global srcdir subdir gdb_prompt expect_out
 
     # First test normal `file'-command loaded $FILE with symbols.
 
@@ -308,20 +413,31 @@ proc test_ld {file ifmain trynosym} {
     gdb_reinitialize_dir $srcdir/$subdir
     gdb_load $file
 
-    reach "dl_main" "run segv"
+    # Print the "PIE (Position Independent Executable) displacement" message.
+    gdb_test "set verbose on"
+
+    reach "dl_main" "run segv" $displacement
 
     gdb_test "bt" "#0 +\[^\r\n\]*\\mdl_main\\M.*" "dl bt"
 
     if $ifmain {
-	reach "main" continue
+	# Displacement message will be printed the second time on initializing
+	# the linker from svr4_special_symbol_handling.  If any ANOFFSET has
+	# been already set as non-zero the detection will no longer be run.
+	if {$displacement == "NONZERO"} {
+	    set displacement_main "NONE"
+	} else {
+	    set displacement_main $displacement
+	}
+	reach "main" continue $displacement_main
 
-	reach "libfunc" continue
+	reach "libfunc" continue "NONE"
 
 	gdb_test "bt" "#0 +\[^\r\n\]*\\mlibfunc\\M\[^\r\n\]*\r\n#1 +\[^\r\n\]*\\mmain\\M.*" "main bt"
 
-	test_core $file
+	test_core $file $displacement
 
-	test_attach $file
+	test_attach $file $displacement
     }
 
     if !$trynosym {
@@ -341,18 +457,21 @@ proc test_ld {file ifmain trynosym} {
     gdb_test "set debug-file-directory"
     gdb_reinitialize_dir $srcdir/$subdir
 
+    # Print the "PIE (Position Independent Executable) displacement" message.
+    gdb_test "set verbose on"
+
     # Test no (error) message has been printed by `exec-file'.
     set escapedfile [string_to_regexp $file]
     gdb_test "exec-file $file" "exec-file $escapedfile" "load"
 
     if $ifmain {
-	reach "dl_main" run
+	reach "dl_main" run $displacement
 
 	set test "info files"
 	set entrynohex ""
 	gdb_test_multiple $test $test {
 	    -re "\r\n\[\t \]*Entry point:\[\t \]*0x(\[0-9a-f\]+)\r\n.*$gdb_prompt $" {
-		set entrynohex $expect_out(1,string) 
+		set entrynohex $expect_out(1,string)
 		pass $test
 	    }
 	}
@@ -363,7 +482,34 @@ proc test_ld {file ifmain trynosym} {
     } else {
 	# There is no symbol to break at ld.so.  Moreover it can exit with an
 	# error code.
-	gdb_test "run" "Program exited (normally|with code \[0-9\]+)\\." "ld.so exit"
+
+	set test "ld.so exit"
+	set test_displacement "seen displacement message"
+	gdb_test_multiple "run" $test {
+	    -re "Using PIE \\(Position Independent Executable\\) displacement (0x\[0-9a-f\]+) " {
+		# Missing "$gdb_prompt $" is intentional.
+		if {$expect_out(1,string) == "0x0"} {
+		    set case "ZERO"
+		} else {
+		    set case "NONZERO"
+		}
+		if {$displacement == $case} {
+		    pass $test_displacement
+		    # Permit multiple such messages.
+		    set displacement "FOUND-$displacement"
+		} elseif {$displacement != "FOUND-$case"} {
+		    fail $test_displacement
+		}
+		exp_continue
+	    }
+	    -re "Program exited (normally|with code \[0-9\]+)\\.\r\n$gdb_prompt $" {
+		# Do not check the binary filename as it may be truncated.
+		pass $test
+	    }
+	}
+	if ![regexp {^(NONE|FOUND-.*)$} $displacement] {
+	    fail $test_displacement
+	}
     }
 
     set pf_prefix $old_ldprefix
@@ -450,7 +596,12 @@ foreach ldprelink {NO YES} {
 	if ![prelink$ldprelink $interp] {
 	    continue
 	}
-	test_ld $interp 0 [expr {$ldsepdebug == "NO"}]
+	if {$ldprelink == "NO"} {
+	    set displacement "NONZERO"
+	} else {
+	    set displacement "ZERO"
+	}
+	test_ld $interp 0 [expr {$ldsepdebug == "NO"}] $displacement
 
 	if ![copy $interp $interp_saved] {
 	    continue
@@ -531,7 +682,14 @@ foreach ldprelink {NO YES} {
 
 		    if {[prelink$binprelink "--dynamic-linker=$interp --ld-library-path=$dir $exec $interp [concat $dests]" [file tail $exec]]
 		        && [copy $interp_saved $interp]} {
-			test_ld $exec 1 [expr {$binsepdebug == "NO"}]
+			if {$binpie == "NO"} {
+			    set displacement "NONE"
+			} elseif {$binprelink == "NO"} {
+			    set displacement "NONZERO"
+			} else {
+			    set displacement "ZERO"
+			}
+			test_ld $exec 1 [expr {$binsepdebug == "NO"}] $displacement
 		    }
 		}
 	    }
--- a/gdb/testsuite/gdb.base/prelink.exp
+++ b/gdb/testsuite/gdb.base/prelink.exp
@@ -112,15 +112,4 @@ gdb_load ${binfile}
 # Print the "adjusting expectations" message.
 gdb_test "set verbose on"
 
-set test "prelink"
-global gdb_prompt
-gdb_test_multiple "core-file $objdir/$subdir/prelink.core" "$test" {
-    -re "warning: \.dynamic section.*not at the expected address.*warning: difference.*caused by prelink, adjusting expectations.*$gdb_prompt $" {
-	pass "$test"
-    }
-}
-
-gdb_exit
-
-return 0
-
+gdb_test "core-file $objdir/$subdir/prelink.core" {Using PIC \(Position Independent Code\) prelink displacement.*} "prelink"

Re: [patch-testcase] Re: RFC: Verify AT_ENTRY before using it

[Daniel Jacobowitz]

On Thu, Mar 11, 2010 at 10:54:16PM +0100, Jan Kratochvil wrote:
> This patch is no longer a pre-requisite for anything and while it would be
> fine I do not find it too important myself.  There will be changes in this
> code anyway on some planned unposted PIE cleanup/fixup patches.

Looks fine to me if you want to check this in.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch-testcase] Re: RFC: Verify AT_ENTRY before using it

[Jan Kratochvil]

On Thu, 11 Mar 2010 22:57:42 +0100, Daniel Jacobowitz wrote:
> On Thu, Mar 11, 2010 at 10:54:16PM +0100, Jan Kratochvil wrote:
> > This patch is no longer a pre-requisite for anything and while it would be
> > fine I do not find it too important myself.  There will be changes in this
> > code anyway on some planned unposted PIE cleanup/fixup patches.
> 
> Looks fine to me if you want to check this in.

Checked-in:
	http://sourceware.org/ml/gdb-cvs/2010-03/msg00104.html


Thanks,
Jan

Re: RFC: Verify AT_ENTRY before using it

[Jan Kratochvil]

On Wed, 10 Mar 2010 22:11:38 +0100, Jan Kratochvil wrote:
> It has been checked-in without the warning() calls.
[...]
> http://sourceware.org/ml/gdb-cvs/2010-03/msg00096.html
> 
> --- src/gdb/ChangeLog	2010/03/10 18:41:37	1.11466
> +++ src/gdb/ChangeLog	2010/03/10 20:50:48	1.11467
> @@ -1,3 +1,16 @@
> +2010-03-10  Jan Kratochvil  <jan.kratochvil@redhat.com>
> +	    Daniel Jacobowitz  <dan@codesourcery.com>
> +
> +	* solib-svr4.c (read_program_header): Support type == -1 to read
> +	all program headers.
> +	(read_program_headers_from_bfd): New function.
> +	(svr4_static_exec_displacement): Remove and move the comment ...
> +	(svr4_exec_displacement): ... here.  Remove variable found.  New
> +	variable displacement.  Check also DYNAMIC.  Verify DISPLACEMENT
> +	alignment for ELF targets.  Compare target vs. exec_bfd PHDRs for ELF
> +	targets using read_program_headers_from_bfd.  Remove the call of
> +	svr4_static_exec_displacement.

Is it OK also for gdb_7_1-branch?

Otherwise the minimized attached patch also works for me.  Still I would
prefer the full patch from the master branch.

No regressions on {x86_64,x86_64-m32,i686}-fedora13-linux-gnu.


Thanks,
Jan


gdb/
2010-03-12  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* solib-svr4.c (svr4_relocate_main_executable): Delay the
	svr4_exec_displacement call.  Return on non-DYNAMIC exec_bfd.

--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1719,7 +1719,15 @@ svr4_exec_displacement (void)
 static void
 svr4_relocate_main_executable (void)
 {
-  CORE_ADDR displacement = svr4_exec_displacement ();
+  CORE_ADDR displacement;
+
+  /* Therefore for ELF it is ET_EXEC and not ET_DYN.  Both shared libraries
+     being executed themselves and PIE (Position Independent Executable)
+     executables are ET_DYN.  */
+  if (exec_bfd && (bfd_get_file_flags (exec_bfd) & DYNAMIC) == 0)
+    return;
+
+  displacement = svr4_exec_displacement ();
 
   /* Even if DISPLACEMENT is 0 still try to relocate it as this is a new
      difference of in-memory vs. in-file addresses and we could already

Re: RFC: Verify AT_ENTRY before using it

[Daniel Jacobowitz]

On Fri, Mar 12, 2010 at 04:28:31PM +0100, Jan Kratochvil wrote:
> Is it OK also for gdb_7_1-branch?

IMO yes.  But I don't know the state of that branch right now; Joel?

-- 
Daniel Jacobowitz
CodeSourcery

Re: RFC: Verify AT_ENTRY before using it

[Jan Kratochvil]

On Fri, 12 Mar 2010 16:38:52 +0100, Daniel Jacobowitz wrote:
> On Fri, Mar 12, 2010 at 04:28:31PM +0100, Jan Kratochvil wrote:
> > Is it OK also for gdb_7_1-branch?
> 
> IMO yes.

Checked-in for gdb_7_1-branch:
	http://sourceware.org/ml/gdb-cvs/2010-03/msg00123.html

2010-03-14  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Daniel Jacobowitz  <dan@codesourcery.com>

	* solib-svr4.c (read_program_header): Support type == -1 to read
	all program headers.
	(read_program_headers_from_bfd): New function.
	(svr4_static_exec_displacement): Remove and move the comment ...
	(svr4_exec_displacement): ... here.  Remove variable found.  New
	variable displacement.  Check also DYNAMIC.  Verify DISPLACEMENT
	alignment for ELF targets.  Compare target vs. exec_bfd PHDRs for ELF
	targets using read_program_headers_from_bfd.  Remove the call of
	svr4_static_exec_displacement.


Thanks,
Jan