[patch 14/15] PIE: Fix back valgrind --db-attach=yes [fixup]

Jan Kratochvil jan.kratochvil@redhat.com
Sun Jan 10 21:43:00 GMT 2010


Hi,

small fix-up of unstable results of the testcase:
	+gdb_test "" "" "eat first prompt"

------------------------------------------------------------------------------

`valgrind --db-attach=yes' will run GDB on the crashed PID which is only
a virtual emulated program inside a virtual machine running at that PID.
The /proc/PID/exe program is a valgrind tool (such as `memcheck').  valgrind
spawns GDB using
	gdb /proc/PID/fd/FD PID

where it provides virtual ELF image corresponding to the emulated process (and
not `memcheck' itself).

It worked OK so far but with the PIE support GDB still reads /proc/PID/auxv
which corresponds to `memcheck' itself instead of the virtual ELF image.
This way it finds incorrect PIE displacement and breaks the debugging.

As discussed with Roland McGrath valgrind already provides even the virtual
AUXV vector with properly provided AT_ENTRY value.  AUXV vector can be besides
/proc/PID/auxv found also in the initial `_start' function parameters in the
order (argc, argv, envp array, auxv array).  ld.so stores this auxv array into
its `_dl_auxv' variable.

One possibility is to backtrace to `_start' and find AUXV there.
Unfortunately when the initial thread already pthread_exit()s the program can
be still perfectly running but GDB can no longer backtrace to reach `_start'.

Using the `_dl_auxv' variable has a problem it is located in ld.so and not the
main executable.  This brings in a chicken-and-egg problem (the primary issue
of the former Red Hat PIE patch) one needs a relocated executable to find its
shared libraries where it finds the relocation displacement (this is not so
true as one can find the shared libraries even without the relocation but the
code has many code paths where some of them require the relocation).

As a best compromise after various attempts found to just use `_dl_auxv'
during attachment and not during since-`_start' inferior startup.  This means
GDB would have problems only in the case of `valgrind --db-attach=yes' to
inferior in a startup pre-main() phase before even before filling in `_dl_auxv'.

svr4_solib_create_inferior_hook conditional is only some simplification
/ performance optimization to avoid relocation to invalid displacement.
svr4_special_symbol_handling has no conditional as that time all the solib
stuff is initialized and that time it is most safe to query the displacement.
It is safe to adjust the main executable displacement multiple times (which
should never happen) and it is cheap to try to adjust the displacement to an
already set value.

The testcase could possibly try some PIE/prelinking/debuginfos combinations.
It does not.


Thanks,
Jan

    
gdb/
	Support Valgrind attachments broken by the PIE support.
	* auxv.c: Include gdbcore.h.
	(procfs_xfer_auxv): Make static.  Reduce its comment.  Drop its
	parameters ops, object and annex.  Remove their assertions.
	(ld_so_xfer_auxv, memory_xfer_auxv): New function.
	* auxv.h (procfs_xfer_auxv): Remove comment.  Rename to ...
	(memory_xfer_auxv): ... here.
	* linux-nat.c (linux_xfer_partial): Rename procfs_xfer_auxv to
	memory_xfer_auxv.
	* procfs.c (procfs_xfer_partial): Likewise.
	* solib-svr4.c (svr4_relocate_main_executable): New prototype.
	(svr4_special_symbol_handling): Call svr4_relocate_main_executable.
	(svr4_solib_create_inferior_hook): Conditionalize the
	svr4_relocate_main_executable call.
    
gdb/testsuite/
	* gdb.base/valgrind-db-attach.exp, gdb.base/valgrind-db-attach.c: New.

--- a/gdb/auxv.c
+++ b/gdb/auxv.c
@@ -25,6 +25,7 @@
 #include "inferior.h"
 #include "valprint.h"
 #include "gdb_assert.h"
+#include "gdbcore.h"
 
 #include "auxv.h"
 #include "elf/common.h"
@@ -33,15 +34,11 @@
 #include <fcntl.h>
 
 
-/* This function is called like a to_xfer_partial hook, but must be
-   called with TARGET_OBJECT_AUXV.  It handles access via
-   /proc/PID/auxv, which is a common method for native targets.  */
+/* This function handles access via /proc/PID/auxv, which is a common method
+   for native targets.  */
 
-LONGEST
-procfs_xfer_auxv (struct target_ops *ops,
-		  enum target_object object,
-		  const char *annex,
-		  gdb_byte *readbuf,
+static LONGEST
+procfs_xfer_auxv (gdb_byte *readbuf,
 		  const gdb_byte *writebuf,
 		  ULONGEST offset,
 		  LONGEST len)
@@ -50,9 +47,6 @@ procfs_xfer_auxv (struct target_ops *ops,
   int fd;
   LONGEST n;
 
-  gdb_assert (object == TARGET_OBJECT_AUXV);
-  gdb_assert (readbuf || writebuf);
-
   pathname = xstrprintf ("/proc/%d/auxv", PIDGET (inferior_ptid));
   fd = open (pathname, writebuf != NULL ? O_WRONLY : O_RDONLY);
   xfree (pathname);
@@ -72,6 +66,143 @@ procfs_xfer_auxv (struct target_ops *ops,
   return n;
 }
 
+/* This function handles access via ld.so's symbol `_dl_auxv'.  */
+
+static LONGEST
+ld_so_xfer_auxv (gdb_byte *readbuf,
+		 const gdb_byte *writebuf,
+		 ULONGEST offset,
+		 LONGEST len)
+{
+  struct minimal_symbol *msym;
+  CORE_ADDR data_address, pointer_address;
+  struct type *ptr_type = builtin_type (target_gdbarch)->builtin_data_ptr;
+  size_t ptr_size = TYPE_LENGTH (ptr_type);
+  size_t auxv_pair_size = 2 * ptr_size;
+  gdb_byte *ptr_buf = alloca (ptr_size);
+  LONGEST retval;
+  size_t block;
+
+  msym = lookup_minimal_symbol ("_dl_auxv", NULL, NULL);
+  if (msym == NULL)
+    return -1;
+
+  if (MSYMBOL_SIZE (msym) != ptr_size)
+    return -1;
+
+  /* POINTER_ADDRESS is a location where the `_dl_auxv' variable resides.
+     DATA_ADDRESS is the inferior value present in `_dl_auxv', therefore the
+     real inferior AUXV address.  */
+
+  pointer_address = SYMBOL_VALUE_ADDRESS (msym);
+
+  data_address = read_memory_typed_address (pointer_address, ptr_type);
+
+  /* Possibly still not initialized such as during an inferior startup.  */
+  if (data_address == 0)
+    return -1;
+
+  data_address += offset;
+
+  if (writebuf != NULL)
+    {
+      if (target_write_memory (data_address, writebuf, len) == 0)
+	return len;
+      else
+	return -1;
+    }
+
+  /* Stop if trying to read past the existing AUXV block.  The final AT_NULL
+     was already returned before.  */
+
+  if (offset >= auxv_pair_size)
+    {
+      if (target_read_memory (data_address - auxv_pair_size, ptr_buf,
+			      ptr_size) != 0)
+	return -1;
+
+      if (extract_typed_address (ptr_buf, ptr_type) == AT_NULL)
+	return 0;
+    }
+
+  retval = 0;
+  block = 0x400;
+  gdb_assert (block % auxv_pair_size == 0);
+
+  while (len > 0)
+    {
+      if (block > len)
+	block = len;
+
+      /* Reading sizes smaller than AUXV_PAIR_SIZE is not supported.  Tails
+	 unaligned to AUXV_PAIR_SIZE will not be read during a call (they
+	 should be completed during next read with new/extended buffer).  */
+
+      block &= -auxv_pair_size;
+      if (block == 0)
+	return retval;
+
+      if (target_read_memory (data_address, readbuf, block) != 0)
+	{
+	  if (block <= auxv_pair_size)
+	    return retval;
+
+	  block = auxv_pair_size;
+	  continue;
+	}
+
+      data_address += block;
+      len -= block;
+
+      /* Check terminal AT_NULL.  This function is being called indefinitely
+         being extended its READBUF until it returns EOF (0).  */
+
+      while (block >= auxv_pair_size)
+	{
+	  retval += auxv_pair_size;
+
+	  if (extract_typed_address (readbuf, ptr_type) == AT_NULL)
+	    return retval;
+
+	  readbuf += auxv_pair_size;
+	  block -= auxv_pair_size;
+	}
+    }
+
+  return retval;
+}
+
+/* This function is called like a to_xfer_partial hook, but must be
+   called with TARGET_OBJECT_AUXV.  It handles access to AUXV.  */
+
+LONGEST
+memory_xfer_auxv (struct target_ops *ops,
+		  enum target_object object,
+		  const char *annex,
+		  gdb_byte *readbuf,
+		  const gdb_byte *writebuf,
+		  ULONGEST offset,
+		  LONGEST len)
+{
+  gdb_assert (object == TARGET_OBJECT_AUXV);
+  gdb_assert (readbuf || writebuf);
+
+   /* ld_so_xfer_auxv is the only function safe for virtual executables being
+      executed by valgrind's memcheck.  As using ld_so_xfer_auxv is problematic
+      during inferior startup GDB does call it only for attached processes.  */
+
+  if (current_inferior ()->attach_flag != 0)
+    {
+      LONGEST retval;
+
+      retval = ld_so_xfer_auxv (readbuf, writebuf, offset, len);
+      if (retval != -1)
+	return retval;
+    }
+
+  return procfs_xfer_auxv (readbuf, writebuf, offset, len);
+}
+
 /* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
    Return 0 if *READPTR is already at the end of the buffer.
    Return -1 if there is insufficient buffer for a whole entry.
--- a/gdb/auxv.h
+++ b/gdb/auxv.h
@@ -43,11 +43,7 @@ extern int target_auxv_search (struct target_ops *ops,
 /* Print the contents of the target's AUXV on the specified file. */
 extern int fprint_target_auxv (struct ui_file *file, struct target_ops *ops);
 
-/* This function is called like a to_xfer_partial hook, but must be
-   called with TARGET_OBJECT_AUXV.  It handles access via
-   /proc/PID/auxv, which is a common method for native targets.  */
-
-extern LONGEST procfs_xfer_auxv (struct target_ops *ops,
+extern LONGEST memory_xfer_auxv (struct target_ops *ops,
 				 enum target_object object,
 				 const char *annex,
 				 gdb_byte *readbuf,
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4928,7 +4928,7 @@ linux_xfer_partial (struct target_ops *ops, enum target_object object,
   LONGEST xfer;
 
   if (object == TARGET_OBJECT_AUXV)
-    return procfs_xfer_auxv (ops, object, annex, readbuf, writebuf,
+    return memory_xfer_auxv (ops, object, annex, readbuf, writebuf,
 			     offset, len);
 
   if (object == TARGET_OBJECT_OSDATA)
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -4377,7 +4377,7 @@ procfs_xfer_partial (struct target_ops *ops, enum target_object object,
 
 #ifdef NEW_PROC_API
     case TARGET_OBJECT_AUXV:
-      return procfs_xfer_auxv (ops, object, annex, readbuf, writebuf,
+      return memory_xfer_auxv (ops, object, annex, readbuf, writebuf,
 			       offset, len);
 #endif
 
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -50,6 +50,7 @@
 
 static struct link_map_offsets *svr4_fetch_link_map_offsets (void);
 static int svr4_have_link_map_offsets (void);
+static void svr4_relocate_main_executable (void);
 
 /* Link map info to include in an allocated so_list entry */
 
@@ -1483,6 +1484,7 @@ enable_break (struct svr4_info *info)
 static void
 svr4_special_symbol_handling (void)
 {
+  svr4_relocate_main_executable ();
 }
 
 /* Relocate the main executable.  This function should be called upon
@@ -1655,7 +1657,8 @@ svr4_solib_create_inferior_hook (void)
   info = get_svr4_info ();
 
   /* Relocate the main executable if necessary.  */
-  svr4_relocate_main_executable ();
+  if (current_inferior ()->attach_flag == 0)
+    svr4_relocate_main_executable ();
 
   if (!svr4_have_link_map_offsets ())
     return;
--- /dev/null
+++ b/gdb/testsuite/gdb.base/valgrind-db-attach.c
@@ -0,0 +1,30 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2009 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+
+int main()
+{
+  void *p;
+
+  p = malloc (1);
+  if (p == NULL)
+    return 1;
+  free (p);
+  free (p);	/* double-free */
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/valgrind-db-attach.exp
@@ -0,0 +1,76 @@
+# Copyright 2009 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set test valgrind-db-attach
+set srcfile $test.c
+set executable $test
+set binfile ${objdir}/${subdir}/${executable}
+if {[build_executable $test.exp $executable $srcfile {debug}] == -1} {
+    return -1
+}
+
+gdb_exit
+
+# remote_spawn breaks the command on each whitespace despite possible quoting.
+# Use backslash-escaped whitespace there instead:
+
+set db_command "--db-command=$GDB $INTERNAL_GDBFLAGS $GDBFLAGS [host_info gdb_opts] %f %p"
+regsub -all " " $db_command "\\ " db_command
+
+set test "spawn valgrind"
+set cmd "valgrind --db-attach=yes $db_command $binfile"
+set res [remote_spawn host $cmd];
+if { $res < 0 || $res == "" } {
+    verbose -log "Spawning $cmd failed."
+    setup_xfail *-*-*
+    fail $test
+    return -1
+}
+pass $test
+# Declare GDB now as running.
+set gdb_spawn_id -1
+
+set test "valgrind started"
+# The trailing '.' differs for different memcheck versions.
+gdb_test_multiple "" $test {
+    -re "Memcheck, a memory error detector\\.?\r\n" {
+	pass $test
+    }
+    -re "valgrind: failed to start tool 'memcheck' for platform '.*': No such file or directory" {
+	setup_xfail *-*-*
+	fail $test
+	return -1
+    }
+}
+
+set double_free [gdb_get_line_number "double-free"]
+
+gdb_test_multiple "" $test {
+    -re "Invalid free\\(\\) / delete / delete\\\[\\\]\r\n.*: main \\(${srcfile}:$double_free\\)\r\n.*---- Attach to debugger \\? --- \[^\r\n\]* ---- " {
+	send_gdb "y\r"
+    }
+    -re "---- Attach to debugger \\? --- \[^\r\n\]* ---- " {
+	send_gdb "n\r"
+	exp_continue
+    }
+}
+
+gdb_test "" "" "eat first prompt"
+
+# Initialization from default_gdb_start.
+gdb_test "set height 0"
+gdb_test "set width 0"
+
+gdb_test "bt" "in main \\(.*\\) at .*${srcfile}:$double_free"



More information about the Gdb-patches mailing list