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

Jan Kratochvil jan.kratochvil@redhat.com
Wed Dec 13 20:46:00 GMT 2006


Hi,

On Tue, 12 Dec 2006 16:52:33 +0100, Jakub Jelinek wrote:
...
> Here is something that would handle by default same_value retaddr_column:
[ http://sources.redhat.com/ml/gdb/2006-12/msg00100.html ]

Thanks for this backward compatible glibc unwinder patch.  I wish to have it
accepted as a step in preparing the environment to use `.cfi_undefined PC'
sometimes in the future.

Attaching patch for current glibc CVS which removes the `.cfi_undefined PC'
unwinder handling requirement but which provides explicit return address 0 from
the `__clone' function.  Currently the 0 is already present there but it is
uninitialized value out of some TLS or 'struct pthread' area (did not check).

Attaching patch for current gdb CVS to properly terminate on return address 0.
The check was already present there but it got applied one backward step later.

I hope these three patches are be 100% reliable and also backward compatible.


Regards,
Jan
-------------- next part --------------
2006-12-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* sysdeps/unix/sysv/linux/i386/clone.S: CFI `clone' unwinding
	outermost frame indicator replaced by more unwinders compatible
	termination indication of `PC == 0'.
	* sysdeps/unix/sysv/linux/x86_64/clone.S: Likewise.

--- libc/sysdeps/unix/sysv/linux/i386/clone.S	3 Dec 2006 23:12:36 -0000	1.27
+++ libc/sysdeps/unix/sysv/linux/i386/clone.S	13 Dec 2006 11:20:55 -0000
@@ -68,6 +68,8 @@ ENTRY (BP_SYM (__clone))
 	   thread is started with an alignment of (mod 16).  */
 	andl	$0xfffffff0, %ecx
 	subl	$28,%ecx
+	/* Terminate the stack frame by pretended return address 0.  */
+	movl	$0,16(%ecx)
 	movl	ARG(%esp),%eax		/* no negative argument counts */
 	movl	%eax,12(%ecx)
 
@@ -121,10 +123,15 @@ L(pseudo_end):
 
 L(thread_start):
 	cfi_startproc;
-	/* Clearing frame pointer is insufficient, use CFI.  */
-	cfi_undefined (eip);
-	/* Note: %esi is zero.  */
-	movl	%esi,%ebp	/* terminate the stack frame */
+	/* This CFI recommended way of unwindable function is incompatible
+	   across unwinders incl. the libgcc_s one.
+	   cfi_undefined (eip);
+	   */
+	/* Frame pointer 0 was considered as the stack frame termination
+	   before but it is no longer valid for -fomit-frame-pointer code.
+	   Still keep the backward compatibility and clear the register.
+	   Note: %esi is zero.  */
+	movl	%esi,%ebp
 #ifdef RESET_PID
 	testl	$CLONE_THREAD, %edi
 	je	L(newpid)
--- libc/sysdeps/unix/sysv/linux/x86_64/clone.S	3 Dec 2006 23:12:36 -0000	1.7
+++ libc/sysdeps/unix/sysv/linux/x86_64/clone.S	13 Dec 2006 11:20:55 -0000
@@ -61,8 +61,12 @@ ENTRY (BP_SYM (__clone))
 	testq	%rsi,%rsi		/* no NULL stack pointers */
 	jz	SYSCALL_ERROR_LABEL
 
+	/* Prepare the data located at %rsp after `syscall' below.
+	   Used only 3*8 bytes but the stack is 16 bytes aligned.  */
+	subq	$32,%rsi
+	/* Terminate the stack frame by pretended return address 0.  */
+	movq	$0,16(%rsi)
 	/* Insert the argument onto the new stack.  */
-	subq	$16,%rsi
 	movq	%rcx,8(%rsi)
 
 	/* Save the function pointer.  It will be popped off in the
@@ -90,10 +94,15 @@ L(pseudo_end):
 
 L(thread_start):
 	cfi_startproc;
-	/* Clearing frame pointer is insufficient, use CFI.  */
-	cfi_undefined (rip);
-	/* Clear the frame pointer.  The ABI suggests this be done, to mark
-	   the outermost frame obviously.  */
+	/* This CFI recommended way of unwindable function is incompatible
+	   across unwinders incl. the libgcc_s one.
+	   cfi_undefined (rip);
+	   */
+	/* Frame pointer 0 was considered as the stack frame termination
+	   before but it is no longer valid for -fomit-frame-pointer code.
+	   Still keep the backward compatibility and clear the register,
+	   the ABI suggests this be done, to mark the outermost frame
+	   obviously.  */
 	xorl	%ebp, %ebp
 
 #ifdef RESET_PID
-------------- next part --------------
2006-12-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb/frame.c (get_prev_frame): Already the first `PC == 0' stack frame
	is declared invalid, not the second one as before.

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/frame.c	10 Nov 2006 20:11:35 -0000	1.215
+++ ./gdb/frame.c	13 Dec 2006 19:06:19 -0000
@@ -1390,19 +1390,28 @@ get_prev_frame (struct frame_info *this_
       return NULL;
     }
 
+  prev_frame = get_prev_frame_1 (this_frame);
+  if (!prev_frame)
+    return NULL;
+
   /* Assume that the only way to get a zero PC is through something
      like a SIGSEGV or a dummy frame, and hence that NORMAL frames
-     will never unwind a zero PC.  */
-  if (this_frame->level > 0
+     will never unwind a zero PC.
+     This check must be done after retrieving the previous frame as we
+     would report the last PC == 0 frame:
+     	#4  0x00000036dd0c68c3 in clone () from /lib64/tls/libc.so.6
+     	#5  0x0000000000000000 in ?? ()
+     */
+  if (prev_frame->level > 0
+      && get_frame_type (prev_frame) == NORMAL_FRAME
       && get_frame_type (this_frame) == NORMAL_FRAME
-      && get_frame_type (get_next_frame (this_frame)) == NORMAL_FRAME
-      && get_frame_pc (this_frame) == 0)
+      && get_frame_pc (prev_frame) == 0)
     {
-      frame_debug_got_null_frame (gdb_stdlog, this_frame, "zero PC");
+      frame_debug_got_null_frame (gdb_stdlog, prev_frame, "zero PC");
       return NULL;
     }
 
-  return get_prev_frame_1 (this_frame);
+  return prev_frame;
 }
 
 CORE_ADDR
--- /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"
+    }
+}


More information about the Gdb-patches mailing list