[patch] svr4_exec_displacement success indicator [Re: PIE question]

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

[repost to <gdb-patches@sourceware.org>]

On Sun, 07 Mar 2010 01:53:29 +0100, Daniel Jacobowitz wrote:
> Jan, could you explain a little how the situation in this comment can
> happen?
> 
> static void
> svr4_relocate_main_executable (void)
> {
>   CORE_ADDR 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
>      relocate the executable at this function to improper address before.  */

OK, no longer valid with current state of sources, going to post a separate
cleanup patch (later after some verifications) so that
svr4_relocate_main_executable gets called only once.

Some history:
------------------------------------------------------------------------------
svr4_exec_displacement behavior was considered a bit magic as it could
(a) depend on svr4_static_exec_displacement returning something I did not
    understand; to be removed by pending:
      Re: RFC: Verify AT_ENTRY before using it
      http://sourceware.org/ml/gdb-patches/2010-03/msg00030.html
(b) auxv read by ld_so_xfer_auxv resolving relocatable symbol "_dl_auxv" which
    brings some chicken-and-egg problems.
    But ld_so_xfer_auxv is used only if ATTACH_FLAG and in such case
    svr4_relocate_main_executable was called only once.
(c) I was trying to get valgrind-executed-PIE working and thought enough steps
    of investigating inferior would fix it.  There is currently no way to get
    valgrind-executed-PIE working (in general case), filed RFE for valgrind:
      valgrind: --db-command should support %{auxv address}
      http://bugs.kde.org/show_bug.cgi?id=223702
------------------------------------------------------------------------------


> I came across this because our local ARM uClinux incorrectly links in
> solib-svr4.c.  The remote target sends qOffsets, uses the result to
> relocate the objfile, and then this code overrides that.

I did not expect symfile_objfile can be already relocated before.

Attached these changes:

* svr4_exec_displacement calling convention should have success indicator.

* Preserving now section_offsets if they are already set, inspired by
  init_objfile_sect_indices.

I believe either of parts would be sufficient for this problem.


> I don't think this is related to our other discussion about executable
> relocation; I haven't forgotten, I'll get back to you as soon as I can.

I agree but technically these two new patches depend on those previous ones.
      Re: RFC: Verify AT_ENTRY before using it
      http://sourceware.org/ml/gdb-patches/2010-03/msg00030.html
      [patch-testcase] Re: RFC: Verify AT_ENTRY before using it
      http://sourceware.org/ml/gdb-patches/2010-03/msg00033.html


No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu although with
/-fPIE/-pie board it is not clear to me.  Debugging some more PIE
incompletenesses (such as unrelocated ei.entry_point in some cases).

OK to check-in?


Thanks,
Jan


2010-03-07  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* solib-svr4.c (svr4_exec_displacement): Return now success, new
	parameter displacementp.  Update comment.
	(svr4_relocate_main_executable): Return if non-zero SECTION_OFFSETS
	element exists.  Return if svr4_exec_displacement was not successful.
	Update comment.

--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1652,7 +1651,10 @@ read_program_headers_from_bfd (bfd *abfd, int *phdrs_size)
   return buf;
 }
 
-/* We relocate all of the sections by the same amount.  This
+/* Return 1 and fill *DISPLACEMENTP with detected PIE offset of inferior
+   exec_bfd.  Otherwise return 0.
+
+   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,
    Edition 4.1, page 5-5:
@@ -1692,8 +1694,8 @@ read_program_headers_from_bfd (bfd *abfd, int *phdrs_size)
      should either be removed or modified to accomodate the new file
      type.  - Kevin, Nov 2000. ]  */
 
-static CORE_ADDR
-svr4_exec_displacement (void)
+static int
+svr4_exec_displacement (CORE_ADDR *displacementp)
 {
   /* ENTRY_POINT is a possible function descriptor - before
      a call to gdbarch_convert_from_func_ptr_addr.  */
@@ -1785,7 +1787,8 @@ svr4_exec_displacement (void)
 	       bfd_get_filename (exec_bfd));
     }
 
-  return displacement;
+  *displacementp = displacement;
+  return 1;
 }
 
 /* Relocate the main executable.  This function should be called upon
@@ -1796,11 +1799,25 @@ svr4_exec_displacement (void)
 static void
 svr4_relocate_main_executable (void)
 {
-  CORE_ADDR displacement = svr4_exec_displacement ();
+  CORE_ADDR displacement;
+
+  if (symfile_objfile)
+    {
+      int i;
+
+      /* Remote target may have already set specific offsets by `qOffsets'
+	 which should be preferred.  */
+
+      for (i = 0; i < symfile_objfile->num_sections; i++)
+	if (ANOFFSET (symfile_objfile->section_offsets, i) != 0)
+	  return;
+    }
+
+  if (! svr4_exec_displacement (&displacement))
+    return;
 
-  /* 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
-     relocate the executable at this function to improper address before.  */
+  /* Even DISPLACEMENT 0 is a valid new difference of in-memory vs. in-file
+     addresses.  */
 
   if (symfile_objfile)
     {