This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Re: Unwinding CFI gcc practice of assumed `same value' regs


On Wed, 13 Dec 2006 23:11:11 +0100, Jan Kratochvil wrote:
> On Wed, 13 Dec 2006 22:52:52 +0100, Mark Kettenis wrote:
> ...
> > No, sorry, it is me who is confused.  I didn't actually look at Jan's patch
> > and assumed it did the the right thing of marking the return address as
> > undefined.
> 
> Therefore what should the patch do? Currently `.cfi_undefined' looks as too
> radical.

Read all the past threads and I hope this patch complies with all the parties.

I would like if the `.cfi_undefined' patch
	http://sourceware.org/ml/libc-alpha/2006-11/msg00082.html
could stay in the development glibc with the unwinder patched by Jakub Jelinek
	http://sources.redhat.com/ml/gdb/2006-12/msg00100.html
as with such CFI patch gdb does not show any bogus 0x0 unwind of `clone'.

On the other hand as a legacy compatibility I wrote the attached patch to fix
the GDB excessive non-CFI amd64 `clone' unwind as 0x0.  Other platforms (and
functions?) could be carbon-copied from this one.

I consider it the same approach as the way GDB currently on amd64 detects
signal frames by checking the instructions - the CFI for signal frames has been
also recently checked to the development glibc as was the `clone' CFI.

Tested for regressions by the gdb testsuite, it is the most conservative way so
it should not have any sideeffects.  The 0x0 unwinding of any non-`clone'
functions remains the same so Mark should have no objections, I hope.



Regards,
Jan
2006-12-16  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb/amd64-linux-tdep.c (linux_clone_code, LINUX_CLONE_LEN,
	amd64_linux_clone_running, amd64_linux_outermost_frame): Detect the
	caller of the thread `start_routing' as the outermost frame to unwind.
	(amd64_linux_init_abi): New hooking for `amd64_linux_outermost_frame'.
	* gdb/i386-tdep.h (gdbarch_tdep): New hook `outermost_frame_p'.
	* gdb/i386-tdep.c (i386_gdbarch_init): Likewise.
	* gdb/amd64-tdep.c (amd64_frame_this_id): Call `outermost_frame_p'.

2006-12-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.threads/bt-clone-stop.exp, gdb.threads/bt-clone-stop.c:
	Backtraced `clone' must not have `PC == 0' as its previous frame.


--- ./gdb/amd64-linux-tdep.c	19 Aug 2006 15:15:18 -0000	1.12
+++ ./gdb/amd64-linux-tdep.c	16 Dec 2006 20:15:16 -0000
@@ -235,6 +235,72 @@ amd64_linux_register_reggroup_p (struct 
 
 /* Set the program counter for process PTID to PC.  */
 
+/* Detect the outermost frame as of:
+   #5  0x000000305cec68c3 in clone () from /lib64/tls/libc.so.6
+   We compare the `linux_clone_code' block to be _before_ the unwound PC.  */
+
+static const unsigned char linux_clone_code[] =
+{
+/* libc/sysdeps/unix/sysv/linux/x86_64/clone.S */
+/* #ifdef RESET_PID */
+/* ... */
+/* 	movl	$SYS_ify(getpid), %eax */
+  0x48, 0xc7, 0xc0, 0x27, 0x00, 0x00, 0x00,
+/* 	syscall */
+  0x0f, 0x05,
+/* 	movl	%eax, %fs:PID */
+  0x64, 0x89, 0x04, 0x25, 0x94, 0x00, 0x00, 0x00,
+/* 	movl	%eax, %fs:TID */
+  0x64, 0x89, 0x04, 0x25, 0x90, 0x00, 0x00, 0x00,
+/* #endif */
+/* 	|* Set up arguments for the function call.  *| */
+/* 	popq	%rax		|* Function to call.  *| */
+  0x58,
+/* 	popq	%rdi		|* Argument.  *| */
+  0x5f,
+/* 	call	*%rax$   */
+  0xff, 0xd0
+};
+
+#define LINUX_CLONE_LEN (sizeof linux_clone_code)
+
+static int
+amd64_linux_clone_running (struct frame_info *next_frame)
+{
+  CORE_ADDR pc = frame_pc_unwind (next_frame);
+  unsigned char buf[LINUX_CLONE_LEN];
+
+  if (!safe_frame_unwind_memory (next_frame, pc - LINUX_CLONE_LEN, buf,
+				 LINUX_CLONE_LEN))
+    return 0;
+
+  if (memcmp (buf, linux_clone_code, LINUX_CLONE_LEN) != 0)
+    return 0;
+
+  return 1;
+}
+
+static int
+amd64_linux_outermost_frame (struct frame_info *next_frame)
+{
+  CORE_ADDR pc = frame_pc_unwind (next_frame);
+  char *name;
+
+  find_pc_partial_function (pc, &name, NULL, NULL);
+
+  /* If we have NAME, we can optimize the search.
+     Check the code even in the case of the unambigious `__clone'
+     as before the `clone' syscall we still have a valid stack there.
+     FIXME: The whole part after the successful syscall there no longer has the
+     valid stack but we still try to unwind it, fortunately it applies only for
+     the case of tracing through `__clone'.  */
+  if (name == NULL || strcmp (name, "clone") == 0
+      || strcmp ("__clone", name) == 0)
+    return (amd64_linux_clone_running (next_frame) != 0);
+
+  return 0;
+}
+
 static void
 amd64_linux_write_pc (CORE_ADDR pc, ptid_t ptid)
 {
@@ -273,6 +339,8 @@ amd64_linux_init_abi (struct gdbarch_inf
   tdep->sc_reg_offset = amd64_linux_sc_reg_offset;
   tdep->sc_num_regs = ARRAY_SIZE (amd64_linux_sc_reg_offset);
 
+  tdep->outermost_frame_p = amd64_linux_outermost_frame;
+
   /* GNU/Linux uses SVR4-style shared libraries.  */
   set_solib_svr4_fetch_link_map_offsets
     (gdbarch, svr4_lp64_fetch_link_map_offsets);
--- ./gdb/amd64-tdep.c	19 Aug 2006 15:15:18 -0000	1.32
+++ ./gdb/amd64-tdep.c	16 Dec 2006 20:15:16 -0000
@@ -879,11 +879,16 @@ amd64_frame_this_id (struct frame_info *
 {
   struct amd64_frame_cache *cache =
     amd64_frame_cache (next_frame, this_cache);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
 
   /* This marks the outermost frame.  */
   if (cache->base == 0)
     return;
 
+  /* Detect OS dependent outermost frames; such as `clone'.  */
+  if (tdep->outermost_frame_p && tdep->outermost_frame_p (next_frame))
+    return;
+
   (*this_id) = frame_id_build (cache->base + 16, cache->pc);
 }
 
--- ./gdb/i386-tdep.h	21 Jan 2006 20:59:50 -0000	1.46
+++ ./gdb/i386-tdep.h	16 Dec 2006 20:15:16 -0000
@@ -104,6 +104,9 @@ struct gdbarch_tdep
      is deprecated, please use `sc_reg_offset' instead.  */
   int sc_pc_offset;
   int sc_sp_offset;
+
+  /* Detect OS dependent outermost frames; such as `clone'.  */
+  int (*outermost_frame_p) (struct frame_info *next_frame);
 };
 
 /* Floating-point registers.  */
--- ./gdb/i386-tdep.c	8 Aug 2006 21:36:46 -0000	1.225
+++ ./gdb/i386-tdep.c	16 Dec 2006 21:13:19 -0000
@@ -2313,6 +2313,9 @@ i386_gdbarch_init (struct gdbarch_info i
   tdep->sc_pc_offset = -1;
   tdep->sc_sp_offset = -1;
 
+  /* Unwinding stops on i386 automatically.  */
+  tdep->outermost_frame_p = NULL;
+
   /* The format used for `long double' on almost all i386 targets is
      the i387 extended floating-point format.  In fact, of all targets
      in the GCC 2.95 tree, only OSF/1 does it different, and insists
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ./gdb/testsuite/gdb.threads/bt-clone-stop.c	13 Dec 2006 19:06:19 -0000
@@ -0,0 +1,39 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2006 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 2 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, write to the Free Software
+   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+   MA 02110-1301, USA.  */
+
+
+#include <pthread.h>
+#include <unistd.h>
+#include <assert.h>
+
+
+void *threader(void *arg)
+{
+	assert(0);
+	return NULL;
+}
+
+int main()
+{
+	pthread_t t1;
+
+	pthread_create(&t1,NULL,threader,(void *)NULL);
+	for (;;)
+		pause();
+}
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ./gdb/testsuite/gdb.threads/bt-clone-stop.exp	13 Dec 2006 19:06:19 -0000
@@ -0,0 +1,56 @@
+# Copyright 2006 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 2 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, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set testfile bt-clone-stop
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if  { [gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested "Couldn't compile test program"
+    return -1
+}
+
+# Get things started.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# threader: threader.c:8: threader: Assertion `0' failed.
+# Program received signal SIGABRT, Aborted.
+
+gdb_test "run" \
+     "Program received signal SIGABRT.*" \
+     "run"
+
+# #5  0x0000003421ecd62d in ?? () from /lib64/libc.so.6
+# #6  0x0000000000000000 in ?? ()
+# (gdb)
+# Line `#6' must not be present
+# Keep those two cases in this order!
+# Fallback is some invalid output (FAIL).
+gdb_test_multiple "bt" "0x0 entry output invalid" {
+    -re "in threader \\(.*\n#\[0-9\]* *0x0* in .*$gdb_prompt $" {
+    	fail "0x0 entry found there"
+    }
+    -re "in threader \\(.*$gdb_prompt $" {
+    	pass "0x0 entry not found there"
+    }
+}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]