[PATCH] Fix AMD64 backtrace

[PATCH] Fix AMD64 backtrace

[Jakub Jelinek]

Hi!

The cfi_startproc hidden in x86_64's ENTRY and cfi_endproc in END macros
causes several routines to have incorrect unwind info.
I went through them and the problems are in at least:
libc/linuxthreads/sysdeps/unix/sysv/linux/x86_64/vfork.S
libc/sysdeps/unix/sysv/linux/x86_64/__start_context.S
libc/sysdeps/unix/sysv/linux/x86_64/clone.S
libc/sysdeps/unix/sysv/linux/x86_64/getcontext.S
libc/sysdeps/unix/sysv/linux/x86_64/setcontext.S
libc/sysdeps/unix/sysv/linux/x86_64/swapcontext.S
libc/sysdeps/unix/sysv/linux/x86_64/sysdep.S
libc/sysdeps/unix/sysv/linux/x86_64/vfork.S
libc/sysdeps/unix/x86_64/sysdep.S
libc/sysdeps/x86_64/__longjmp.S
libc/sysdeps/x86_64/strcspn.S
libc/sysdeps/x86_64/strspn.S
(that's all .S x86_64 files which use ENTRY/END, don't use any cfi_*
directives, and don't maintain constant %rsp over its lifetime or
clobber call saved registers).

The testcase below segfaults on AMD64, because thread_start part of __clone
has incorrect unwind info.
I don't think there is any frame info termination on AMD64
(e.g. when context->ra is 0 libgcc segfaults), so I'd say it is better to
avoid the unwind info in that case altogether, which will cause e.g.
backtrace to stop.  I've terminated the FDE already before syscall,
because then the unwind info would need to differentiate between
%rax == 0 (terminate unwind info chain; how?) and %rax != 0 (the current
DW_CFA_nop should be sufficient).

For the remaining of the above failes, either they should start using
ENTRY_NOCFI/END_NOCFI, or, IMHO better given that GCC defaults to
-fasynchronous-unwind-tables on AMD64, cfi_* directives should be added.

2004-01-08  Jakub Jelinek  <jakub@redhat.com>

	* sysdeps/unix/sysv/linux/x86_64/clone.S (__clone): Add cfi_endproc
	right before syscall and cfi_startproc before PSEUDO_END.
	Clear %rbp in thread_start.
nptl/
	* Makefile (tests): Add tst-backtrace1.
	* tst-backtrace1.c: New test.

--- libc/sysdeps/unix/sysv/linux/x86_64/clone.S.jj	2003-03-04 20:45:35.000000000 +0100
+++ libc/sysdeps/unix/sysv/linux/x86_64/clone.S	2004-01-08 16:20:37.000000000 +0100
@@ -1,4 +1,4 @@
-/* Copyright (C) 2001, 2002, 2003 Free Software Foundation, Inc.
+/* Copyright (C) 2001, 2002, 2003, 2004 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -72,6 +72,10 @@ ENTRY (BP_SYM (__clone))
 	movq	%r9, %r8
 	movq	8(%rsp), %r10
 	movq	$SYS_ify(clone),%rax
+
+	/* End FDE now, because in the child the unwind info will be
+	   wrong.  */
+	cfi_endproc;
 	syscall
 
 	testq	%rax,%rax
@@ -82,6 +86,10 @@ L(pseudo_end):
 	ret
 
 thread_start:
+	/* Clear the frame pointer.  The ABI suggests this be done, to mark
+	   the outermost frame obviously.  */
+	xorq	%rbp, %rbp
+
 	/* Set up arguments for the function call.  */
 	popq	%rax		/* Function to call.  */
 	popq	%rdi		/* Argument.  */
@@ -90,6 +98,7 @@ thread_start:
 	movq	%rax, %rdi
 	call	HIDDEN_JUMPTARGET (_exit)
 
+	cfi_startproc;
 PSEUDO_END (BP_SYM (__clone))
 
 weak_alias (BP_SYM (__clone), BP_SYM (clone))
--- libc/nptl/Makefile.jj	2003-12-29 15:01:27.000000000 +0100
+++ libc/nptl/Makefile	2004-01-08 16:25:24.000000000 +0100
@@ -1,4 +1,4 @@
-# Copyright (C) 2002, 2003 Free Software Foundation, Inc.
+# Copyright (C) 2002, 2003, 2004 Free Software Foundation, Inc.
 # This file is part of the GNU C Library.
 
 # The GNU C Library is free software; you can redistribute it and/or
@@ -235,6 +235,7 @@ tests = tst-attr1 tst-attr2 tst-attr3 \
 	tst-clock1 tst-clock2 \
 	tst-context1 \
 	tst-sched1 \
+	tst-backtrace1 \
 	tst-oddstacklimit
 
 # This sets the stack resource limit to 1023kb, which is not a multiple
--- libc/nptl/tst-backtrace1.c.jj	2004-01-08 16:24:13.000000000 +0100
+++ libc/nptl/tst-backtrace1.c	2004-01-08 16:24:06.000000000 +0100
@@ -0,0 +1,86 @@
+/* Copyright (C) 2004 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Jakub Jelinek <jakub@redhat.com>, 2004.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+#include <execinfo.h>
+#include <pthread.h>
+#include <stdio.h>
+
+#define BT_SIZE 64
+void *bt_array[BT_SIZE];
+int bt_cnt;
+
+int
+do_bt (void)
+{
+  bt_cnt = backtrace (bt_array, BT_SIZE);
+  return 56;
+}
+
+int
+call_do_bt (void)
+{
+  return do_bt () + 1;
+}
+
+void *
+tf (void *arg)
+{
+  if (call_do_bt () != 57)
+    return (void *) 1L;
+  return NULL;
+}
+
+int
+do_test (void)
+{
+  pthread_t th;
+  if (pthread_create (&th, NULL, tf, NULL))
+    {
+      puts ("create failed");
+      return 1;
+    }
+
+  void *res;
+  if (pthread_join (th, &res))
+    {
+      puts ("join failed");
+      return 1;
+    }
+
+  if (res != NULL)
+    {
+      puts ("thread failed");
+      return 1;
+    }
+
+  char **text = backtrace_symbols (bt_array, bt_cnt);
+  if (text == NULL)
+    {
+      puts ("backtrace_symbols failed");
+      return 1;
+    }
+
+  for (int i = 0; i < bt_cnt; ++i)
+    puts (text[i]);
+
+  return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"

	Jakub

Re: [PATCH] Fix AMD64 backtrace

[Ulrich Drepper]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jakub Jelinek wrote:

> 	* sysdeps/unix/sysv/linux/x86_64/clone.S (__clone): Add cfi_endproc
> 	right before syscall and cfi_startproc before PSEUDO_END.
> 	Clear %rbp in thread_start.
> nptl/
> 	* Makefile (tests): Add tst-backtrace1.
> 	* tst-backtrace1.c: New test.

I've applied the patch.  Thanks,

- -- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (GNU/Linux)

iD8DBQE//Z/X2ijCOnn/RHQRAgc9AKC2g9fd39r19XMsu1FkVSwiOCox0ACfTq9G
oRQUNIEDwjIaqdg990OXN+A=
=8mpq
-----END PGP SIGNATURE-----

Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 8462 bytes --]

Jakub Jelinek <jakub@redhat.com> writes:

> Hi!
>
> The cfi_startproc hidden in x86_64's ENTRY and cfi_endproc in END macros
> causes several routines to have incorrect unwind info.
> I went through them and the problems are in at least:
> libc/linuxthreads/sysdeps/unix/sysv/linux/x86_64/vfork.S

Fixed.

> libc/sysdeps/unix/sysv/linux/x86_64/__start_context.S

Fixed.

> libc/sysdeps/unix/sysv/linux/x86_64/clone.S

Done by yourself.

> libc/sysdeps/unix/sysv/linux/x86_64/getcontext.S

I don't see a problem here directly.  Am I missing something?

> libc/sysdeps/unix/sysv/linux/x86_64/setcontext.S

Adding right cfi directives here is not easy.  What do you think of my
patch?  It's just the minimal solution or do we need to do more?
Should we mark all call-clobbered registers with "cfi_undefined"?

> libc/sysdeps/unix/sysv/linux/x86_64/swapcontext.S

Let's get setcontext and getcontext fixed first...

> libc/sysdeps/unix/sysv/linux/x86_64/sysdep.S

Fixed (includes sysdeps/unix/x86_64/sysdep.S which is the problem).

> libc/sysdeps/unix/sysv/linux/x86_64/vfork.S

Fixed.

> libc/sysdeps/unix/x86_64/sysdep.S

Fixed.

> libc/sysdeps/x86_64/__longjmp.S

What's the best way to handle the destroying of the registers with
CFI?  Should we just mark the registers with "cfi_undefined"?

> libc/sysdeps/x86_64/strcspn.S
> libc/sysdeps/x86_64/strspn.S

And those two are also fixed.

I'm appending a patch.  Can you give it a quick review, please?

> (that's all .S x86_64 files which use ENTRY/END, don't use any cfi_*
> directives, and don't maintain constant %rsp over its lifetime or
> clobber call saved registers).
>
> The testcase below segfaults on AMD64, because thread_start part of __clone
> has incorrect unwind info.
> I don't think there is any frame info termination on AMD64
> (e.g. when context->ra is 0 libgcc segfaults), so I'd say it is better to
> avoid the unwind info in that case altogether, which will cause e.g.
> backtrace to stop.  I've terminated the FDE already before syscall,
> because then the unwind info would need to differentiate between
> %rax == 0 (terminate unwind info chain; how?) and %rax != 0 (the current
> DW_CFA_nop should be sufficient).
>
> For the remaining of the above failes, either they should start using
> ENTRY_NOCFI/END_NOCFI, or, IMHO better given that GCC defaults to
> -fasynchronous-unwind-tables on AMD64, cfi_* directives should be added.

I agree.  Thanks for reminding me about this and for your patch!

Andreas

2004-01-10  Andreas Jaeger  <aj@suse.de>

	* sysdeps/unix/sysv/linux/x86_64/__start_context.S: Add cfi
	directives.
	* sysdeps/unix/x86_64/sysdep.S (__syscall_error): Likewise.
	* sysdeps/unix/sysv/linux/x86_64/vfork.S: Likewise.
	* sysdeps/x86_64/strcspn.S: Likewise.
	* sysdeps/x86_64/strspn.S: Likewise.

For linuxthreads:
	* sysdeps/unix/sysv/linux/x86_64/vfork.S: Add cfi
	directives.

============================================================
Index: sysdeps/unix/sysv/linux/x86_64/__start_context.S
--- sysdeps/unix/sysv/linux/x86_64/__start_context.S	27 Aug 2003 23:03:41 -0000	1.2
+++ sysdeps/unix/sysv/linux/x86_64/__start_context.S	10 Jan 2004 16:09:40 -0000
@@ -1,4 +1,4 @@
-/* Copyright (C) 2002, 2003 Free Software Foundation, Inc.
+/* Copyright (C) 2002, 2003, 2004 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Andreas Jaeger <aj@suse.de>, 2002.
 
@@ -33,6 +33,7 @@ ENTRY(__start_context)
 	movq	%rbx, %rsp
 
 	popq	%rdi			/* This is the next context.  */
+	cfi_adjust_cfa_offset(-8)
 	testq	%rdi, %rdi
 	je	2f			/* If it is zero exit.  */
 
============================================================
Index: sysdeps/unix/sysv/linux/x86_64/vfork.S
--- sysdeps/unix/sysv/linux/x86_64/vfork.S	31 Dec 2002 20:37:32 -0000	1.5
+++ sysdeps/unix/sysv/linux/x86_64/vfork.S	10 Jan 2004 16:09:41 -0000
@@ -1,4 +1,4 @@
-/* Copyright (C) 2001, 2002 Free Software Foundation, Inc.
+/* Copyright (C) 2001, 2002, 2004 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -30,6 +30,7 @@ ENTRY (__vfork)
 	/* Pop the return PC value into RDI.  We need a register that
 	   is preserved by the syscall and that we're allowed to destroy. */
 	popq	%rdi
+	cfi_adjust_cfa_offset(-8)
 
 	/* Stuff the syscall number in RAX and enter into the kernel.  */
 	movl	$SYS_ify (vfork), %eax
@@ -37,6 +38,7 @@ ENTRY (__vfork)
 
 	/* Push back the return PC.  */
 	pushq	%rdi
+	cfi_adjust_cfa_offset(8)
 
 	cmpl	$-4095, %eax
 	jae SYSCALL_ERROR_LABEL		/* Branch forward if it failed.  */
============================================================
Index: sysdeps/unix/x86_64/sysdep.S
--- sysdeps/unix/x86_64/sysdep.S	11 Oct 2002 10:52:03 -0000	1.4
+++ sysdeps/unix/x86_64/sysdep.S	10 Jan 2004 16:09:41 -0000
@@ -1,4 +1,4 @@
-/* Copyright (C) 2001, 2002 Free Software Foundation, Inc.
+/* Copyright (C) 2001, 2002, 2004 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -57,10 +57,12 @@ notb:
 	movl %eax, C_SYMBOL_NAME(errno)
 # else
 	pushq %rax
+	cfi_adjust_cfa_offset(8)
 	PUSH_ERRNO_LOCATION_RETURN
 	call BP_SYM (__errno_location)
 	POP_ERRNO_LOCATION_RETURN
 	popq %rcx
+	cfi_adjust_cfa_offset(-8)
 	movl %ecx, (%rax)
 # endif
 #else
@@ -72,10 +74,12 @@ notb:
 	movl %eax, (%rcx)
 # else
 	pushq %rax
+	cfi_adjust_cfa_offset(8)
 	PUSH_ERRNO_LOCATION_RETURN
 	call C_SYMBOL_NAME (BP_SYM (__errno_location)@PLT)
 	POP_ERRNO_LOCATION_RETURN
 	popq %rcx
+	cfi_adjust_cfa_offset(-8)
 	movl %ecx, (%rax)
 # endif
 #endif
============================================================
Index: sysdeps/x86_64/strcspn.S
--- sysdeps/x86_64/strcspn.S	29 Apr 2003 22:47:18 -0000	1.2
+++ sysdeps/x86_64/strcspn.S	10 Jan 2004 16:09:41 -0000
@@ -1,7 +1,7 @@
 /* strcspn (str, ss) -- Return the length of the initial segment of STR
 			which contains no characters from SS.
    For AMD x86-64.
-   Copyright (C) 1994-1997, 2000, 2002, 2003 Free Software Foundation, Inc.
+   Copyright (C) 1994-1997, 2000, 2002, 2003, 2004 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@gnu.ai.mit.edu>.
    Bug fixes by Alan Modra <Alan@SPRI.Levels.UniSA.Edu.Au>.
@@ -40,6 +40,7 @@ ENTRY (strcspn)
 	   table.  */
 	movq %rdi, %r8			/* Save value.  */
 	subq $256, %rsp			/* Make space for 256 bytes.  */
+	cfi_adjust_cfa_offset(-256)
 	movq $32,  %rcx			/* 32*8 bytes = 256 bytes.  */
 	movq %rsp, %rdi
 	xorq %rax, %rax			/* We store 0s.  */
@@ -110,6 +111,7 @@ L(6):	incq %rax
 L(5):	incq %rax
 
 L(4):	addq $256, %rsp		/* remove skipset */
+	cfi_adjust_cfa_offset(-256)
 #if STRPBRK_P
 	xorq %rdx,%rdx
 	orb %cl, %cl		/* was last character NUL? */
============================================================
Index: sysdeps/x86_64/strspn.S
--- sysdeps/x86_64/strspn.S	29 Apr 2003 22:47:17 -0000	1.2
+++ sysdeps/x86_64/strspn.S	10 Jan 2004 16:09:41 -0000
@@ -1,7 +1,7 @@
 /* strspn (str, ss) -- Return the length of the initial segment of STR
 			which contains only characters from SS.
    For AMD x86-64.
-   Copyright (C) 1994-1997, 2000, 2002, 2003 Free Software Foundation, Inc.
+   Copyright (C) 1994-1997, 2000,2002,2003,2004 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@gnu.ai.mit.edu>.
    Bug fixes by Alan Modra <Alan@SPRI.Levels.UniSA.Edu.Au>.
@@ -36,6 +36,7 @@ ENTRY (strspn)
 	   table.  */
 	movq %rdi, %r8			/* Save value.  */
 	subq $256, %rsp			/* Make space for 256 bytes.  */
+	cfi_adjust_cfa_offset(256)
 	movq $32,  %rcx			/* 32*8 bytes = 256 bytes.  */
 	movq %rsp, %rdi
 	xorq %rax, %rax			/* We store 0s.  */
@@ -106,6 +107,7 @@ L(6):	incq %rax
 L(5):	incq %rax
 
 L(4):	addq $256, %rsp		/* remove stopset */
+	cfi_adjust_cfa_offset(-256)
 	subq %rdx, %rax		/* we have to return the number of valid
 				   characters, so compute distance to first
 				   non-valid character */

-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] Fix AMD64 backtrace

[Ulrich Drepper]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I have added the patch bu

~ there is no LinuxThreads patch

~ if you send mails signed (or any MIME mail) don't include the patches
inlined.

- -- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (GNU/Linux)

iD8DBQFAAD7A2ijCOnn/RHQRAlVqAKCg8Su0dQEmbsqJRXPuGRx5z/OehgCfZdXD
tgpbIrtepbkwyP+wtxwZB2c=
=UHEO
-----END PGP SIGNATURE-----

Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger]

Ulrich Drepper <drepper@redhat.com> writes:

> I have added the patch bu
>
> ~ there is no LinuxThreads patch

Appended - I'll commit it myself now.

> ~ if you send mails signed (or any MIME mail) don't include the patches
> inlined.

What exactly is the problem?

Andreas

============================================================
Index: linuxthreads/sysdeps/unix/sysv/linux/x86_64/vfork.S
--- linuxthreads/sysdeps/unix/sysv/linux/x86_64/vfork.S	11 Feb 2003 06:27:54 -0000	1.4
+++ linuxthreads/sysdeps/unix/sysv/linux/x86_64/vfork.S	10 Jan 2004 19:21:58 -0000
@@ -1,4 +1,4 @@
-/* Copyright (C) 2001, 2002, 2003 Free Software Foundation, Inc.
+/* Copyright (C) 2001, 2002, 2003, 2004 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -39,6 +39,7 @@ ENTRY (__vfork)
 	/* Pop the return PC value into RDI.  We need a register that
 	   is preserved by the syscall and that we're allowed to destroy. */
 	popq	%rdi
+	cfi_adjust_cfa_offset(-8)
 
 	/* Stuff the syscall number in RAX and enter into the kernel.  */
 	movl	$SYS_ify (vfork), %eax
@@ -46,6 +47,7 @@ ENTRY (__vfork)
 
 	/* Push back the return PC.  */
 	pushq	%rdi
+	cfi_adjust_cfa_offset(-7)
 
 	cmpl	$-4095, %eax
 	jae SYSCALL_ERROR_LABEL		/* Branch forward if it failed.  */

-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

Re: [PATCH] Fix AMD64 backtrace

[Jakub Jelinek]

On Sat, Jan 10, 2004 at 08:22:51PM +0100, Andreas Jaeger wrote:
> @@ -46,6 +47,7 @@ ENTRY (__vfork)
>  
>  	/* Push back the return PC.  */
>  	pushq	%rdi
> +	cfi_adjust_cfa_offset(-7)

You mean -8, right?

	Jakub

Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 519 bytes --]

Jakub Jelinek <jakub@redhat.com> writes:

> On Sat, Jan 10, 2004 at 08:22:51PM +0100, Andreas Jaeger wrote:
>> @@ -46,6 +47,7 @@ ENTRY (__vfork)
>>  
>>  	/* Push back the return PC.  */
>>  	pushq	%rdi
>> +	cfi_adjust_cfa_offset(-7)
>
> You mean -8, right?

Yes, definitly.  Thanks, will fix it directly...

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 485 bytes --]

Jakub Jelinek <jakub@redhat.com> writes:

> On Sat, Jan 10, 2004 at 08:22:51PM +0100, Andreas Jaeger wrote:
>> @@ -46,6 +47,7 @@ ENTRY (__vfork)
>>  
>>  	/* Push back the return PC.  */
>>  	pushq	%rdi
>> +	cfi_adjust_cfa_offset(-7)
>
> You mean -8, right?

+8 in fact ;-(

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] Fix AMD64 backtrace

[Ulrich Drepper]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andreas Jaeger wrote:

>>~ if you send mails signed (or any MIME mail) don't include the patches
>>inlined.
> 
> 
> What exactly is the problem?

The MIME encoding of course.  The body contains all these ugly =3D etc.
 If you use MIME, as signing enforces, use attachments.

- -- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (GNU/Linux)

iD8DBQFAAFIp2ijCOnn/RHQRAoQUAJ4lvk0MfRb3IzE8KjaYqioCc3zmGwCglMpr
J6Ue0EHq9ybR/NpUNHDEm2o=
=FfA6
-----END PGP SIGNATURE-----

Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]

Andreas Jaeger <aj@suse.de> writes:

> Jakub Jelinek <jakub@redhat.com> writes:
> [...]
>>> libc/sysdeps/x86_64/__longjmp.S
>
> What's the best way to handle the destroying of the registers with
> CFI?  Should we just mark the registers with "cfi_undefined"?

I'm appending a proposed patch for __longjmp.S.  Or can anybody
envision a better way doing this?

Andreas

2004-01-10  Andreas Jaeger  <aj@suse.de>

	* sysdeps/x86_64/__longjmp.S: Add cfi directives.

============================================================
Index: sysdeps/x86_64/__longjmp.S
--- sysdeps/x86_64/__longjmp.S	19 Sep 2001 10:12:07 -0000	1.1
+++ sysdeps/x86_64/__longjmp.S	10 Jan 2004 19:47:01 -0000
@@ -1,4 +1,4 @@
-/* Copyright (C) 2001 Free Software Foundation, Inc.
+/* Copyright (C) 2001, 2004 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -28,11 +28,17 @@
 ENTRY(__longjmp)
 	/* Restore registers.  */
 	movq (JB_RBX*8)(%rdi),%rbx
+	cfi_undefined(%rbx)
 	movq (JB_RBP*8)(%rdi),%rbp
+	cfi_undefined(%rbp)
 	movq (JB_R12*8)(%rdi),%r12
+	cfi_undefined(%r12)
 	movq (JB_R13*8)(%rdi),%r13
+	cfi_undefined(%r13)
 	movq (JB_R14*8)(%rdi),%r14
+	cfi_undefined(%r14)
 	movq (JB_R15*8)(%rdi),%r15
+	cfi_undefined(%r15)
 	/* Set return value for setjmp.  */
 	test %esi,%esi
 	mov $01,%eax
@@ -40,5 +46,6 @@ ENTRY(__longjmp)
 	mov %esi, %eax
 	movq (JB_PC*8)(%rdi),%rdx
 	movq (JB_RSP*8)(%rdi),%rsp
+	cfi_undefined(%rsp)
 	jmpq *%rdx
 END (BP_SYM (__longjmp))

-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] Fix AMD64 backtrace

[Jakub Jelinek]

On Sat, Jan 10, 2004 at 08:49:49PM +0100, Andreas Jaeger wrote:
> Andreas Jaeger <aj@suse.de> writes:
> 
> > Jakub Jelinek <jakub@redhat.com> writes:
> > [...]
> >>> libc/sysdeps/x86_64/__longjmp.S
> >
> > What's the best way to handle the destroying of the registers with
> > CFI?  Should we just mark the registers with "cfi_undefined"?
> 
> I'm appending a proposed patch for __longjmp.S.  Or can anybody
> envision a better way doing this?

Cannot you from the first movq ***,%rbx pretend you're
already at the jmpq %*rdx instruction, ie. unwind to the
longjmp target?
Write in the unwind info where to fetch %rbp, %r12..%r15,
ra etc. from?

	Jakub

Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 1123 bytes --]

Jakub Jelinek <jakub@redhat.com> writes:

> On Sat, Jan 10, 2004 at 08:49:49PM +0100, Andreas Jaeger wrote:
>> Andreas Jaeger <aj@suse.de> writes:
>> 
>> > Jakub Jelinek <jakub@redhat.com> writes:
>> > [...]
>> >>> libc/sysdeps/x86_64/__longjmp.S
>> >
>> > What's the best way to handle the destroying of the registers with
>> > CFI?  Should we just mark the registers with "cfi_undefined"?
>> 
>> I'm appending a proposed patch for __longjmp.S.  Or can anybody
>> envision a better way doing this?
>
> Cannot you from the first movq ***,%rbx pretend you're
> already at the jmpq %*rdx instruction, ie. unwind to the
> longjmp target?
> Write in the unwind info where to fetch %rbp, %r12..%r15,
> ra etc. from?

You mean something like the following?

cfi_def_cfa(%rdi,0)
cfi_offset(%rbx,JB_RBX*8)
cfi_offset(%r12,JB_R12*8)
...

I'm not sure what the right way is to handle this here at all ...

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 2009 bytes --]

Andreas Jaeger <aj@suse.de> writes:

> Jakub Jelinek <jakub@redhat.com> writes:
>
>> On Sat, Jan 10, 2004 at 08:49:49PM +0100, Andreas Jaeger wrote:
>>> Andreas Jaeger <aj@suse.de> writes:
>>> 
>>> > Jakub Jelinek <jakub@redhat.com> writes:
>>> > [...]
>>> >>> libc/sysdeps/x86_64/__longjmp.S
>>> >
>>> > What's the best way to handle the destroying of the registers with
>>> > CFI?  Should we just mark the registers with "cfi_undefined"?
>>> 
>>> I'm appending a proposed patch for __longjmp.S.  Or can anybody
>>> envision a better way doing this?
>>
>> Cannot you from the first movq ***,%rbx pretend you're
>> already at the jmpq %*rdx instruction, ie. unwind to the
>> longjmp target?
>> Write in the unwind info where to fetch %rbp, %r12..%r15,
>> ra etc. from?
>
> You mean something like the following?
>
> cfi_def_cfa(%rdi,0)
> cfi_offset(%rbx,JB_RBX*8)
> cfi_offset(%r12,JB_R12*8)
> ...
>
> I'm not sure what the right way is to handle this here at all ...

I've done it now completely:

ENTRY(__longjmp)
	/* Restore registers.  */
	/* We add unwind information for the target here.  */
	cfi_def_cfa(%rdi, 0)
	cfi_offset(%rbx,JB_RBX*8)
	cfi_offset(%r12,JB_R12*8)
	cfi_offset(%r13,JB_R13*8)
	cfi_offset(%r14,JB_R14*8)
	cfi_offset(%r15,JB_R15*8)
	cfi_offset(%rsp,JB_RSP*8)
	movq (JB_RBX*8)(%rdi),%rbx
	movq (JB_RBP*8)(%rdi),%rbp
	movq (JB_R12*8)(%rdi),%r12
	movq (JB_R13*8)(%rdi),%r13
	movq (JB_R14*8)(%rdi),%r14
	movq (JB_R15*8)(%rdi),%r15
	/* Set return value for setjmp.  */
	test %esi,%esi
	mov $01,%eax
	cmove %eax,%esi
	mov %esi, %eax
	movq (JB_PC*8)(%rdi),%rdx
	movq (JB_RSP*8)(%rdi),%rsp
	jmpq *%rdx
END (BP_SYM (__longjmp))

Does this look ok? In that case I'll commit it (and will fix
setcontext/swapcontext in a similar way)...

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 738 bytes --]

Richard Henderson <rth@redhat.com> writes:

> On Sun, Jan 11, 2004 at 11:40:06AM +0100, Andreas Jaeger wrote:
>> 	/* We add unwind information for the target here.  */
>> 	cfi_def_cfa(%rdi, 0)
>> 	cfi_offset(%rbx,JB_RBX*8)
>> 	cfi_offset(%r12,JB_R12*8)
>> 	cfi_offset(%r13,JB_R13*8)
>> 	cfi_offset(%r14,JB_R14*8)
>> 	cfi_offset(%r15,JB_R15*8)
>> 	cfi_offset(%rsp,JB_RSP*8)
>
> You're missing
>
>> 	movq (JB_RBP*8)(%rdi),%rbp

Right.

>> 	movq (JB_PC*8)(%rdi),%rdx

Are you sure?  Do I have to do this for the PC also?

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 558 bytes --]

Richard Henderson <rth@redhat.com> writes:

> On Sun, Jan 11, 2004 at 11:50:12AM +0100, Andreas Jaeger wrote:
>> >> 	movq (JB_PC*8)(%rdi),%rdx
>> 
>> Are you sure?  Do I have to do this for the PC also?
>
> Yes.

Added.

We're not doing it anywhere for the program counter AFAIK.  Or is this
done implicitly and we have to do it here explicitly?

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 985 bytes --]


Ok, here's the complete function now:
ENTRY(__longjmp)
	/* Restore registers.  */
	/* We add unwind information for the target here.  */
	cfi_def_cfa(%rdi, 0)
	cfi_offset(%rbx,JB_RBX*8)
	cfi_offset(%rbx,JB_RBP*8)
	cfi_offset(%r12,JB_R12*8)
	cfi_offset(%r13,JB_R13*8)
	cfi_offset(%r14,JB_R14*8)
	cfi_offset(%r15,JB_R15*8)
	cfi_offset(%rsp,JB_RSP*8)
	cfi_offset(%rip,JB_PC*8)
	movq (JB_RBX*8)(%rdi),%rbx
	movq (JB_RBP*8)(%rdi),%rbp
	movq (JB_R12*8)(%rdi),%r12
	movq (JB_R13*8)(%rdi),%r13
	movq (JB_R14*8)(%rdi),%r14
	movq (JB_R15*8)(%rdi),%r15
	/* Set return value for setjmp.  */
	test %esi,%esi
	mov $01,%eax
	cmove %eax,%esi
	mov %esi, %eax
	movq (JB_PC*8)(%rdi),%rdx
	movq (JB_RSP*8)(%rdi),%rsp
	jmpq *%rdx
END (BP_SYM (__longjmp))

What do you think?

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]

Richard Henderson <rth@redhat.com> writes:

> On Sun, Jan 11, 2004 at 11:57:36AM +0100, Andreas Jaeger wrote:
>> 	cfi_offset(%rbx,JB_RBP*8)
>
> Typo                  ^

Pasto ;-)

Thanks, I'll commit the change later and will then do also
swapcontext/setcontext,

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger]

[-- Attachment #1.1: Type: text/plain, Size: 2086 bytes --]


And here comes setcontext:


ENTRY(__setcontext)
	/* Save argument since syscall will destroy it.  */
	pushq	%rdi
	cfi_adjust_cfa_offset(8)

	/* Set the signal mask with
	   rt_sigprocmask (SIG_SETMASK, mask, NULL, _NSIG/8).  */
	leaq	oSIGMASK(%rdi), %rsi
	xorq	%rdx, %rdx
	movq	$SIG_SETMASK, %rdi
	movq	$_NSIG8,%r10
	movq	$__NR_rt_sigprocmask, %rax
	syscall
	popq	%rdi			/* Reload %rdi, adjust stack.  */
	cfi_adjust_cfa_offset(-8)
	cmpq	$-4095, %rax		/* Check %rax for error.  */
	jae	SYSCALL_ERROR_LABEL	/* Jump to error handler if error.  */

	/* Restore the floating-point context.  Not the registers, only the
	   rest.  */
	movq	oFPREGS(%rdi), %rcx
	fldenv	(%rcx)
	ldmxcsr oMXCSR(%rdi)


	/* Load the new stack pointer, the preserved registers and
	   registers used for passing args.  */
	cfi_def_cfa(%rdi, 0)
	cfi_offset(%rbx,oRBX)
	cfi_offset(%rbp,oRBP)
	cfi_offset(%r12,oR12)
	cfi_offset(%r13,oR13)
	cfi_offset(%r14,oR14)
	cfi_offset(%r15,oR15)
	cfi_offset(%rsp,oRSP)
	cfi_offset(%rip,oRIP)
	
	movq	oRSP(%rdi), %rsp
	movq	oRBX(%rdi), %rbx
	movq	oRBP(%rdi), %rbp
	movq	oR12(%rdi), %r12
	movq	oR13(%rdi), %r13
	movq	oR14(%rdi), %r14
	movq	oR15(%rdi), %r15

	/* The following ret should return to the address set with
	getcontext.  Therefore push the address on the stack.  */
	movq	oRIP(%rdi), %rcx
	pushq	%rcx
	cfi_adjust_cfa_offset(8)

	movq	oRSI(%rdi), %rsi
	movq	oRDX(%rdi), %rdx
	movq	oRCX(%rdi), %rcx
	movq	oR8(%rdi), %r8
	movq	oR9(%rdi), %r9

	/* Setup finally  %rdi.  */
	movq	oRDI(%rdi), %rdi

	/* We changed %rdi, fix cfi.  */
	cfi_def_cfa(%rsp, 0)
	cfi_same_value(%rbx)
	cfi_same_value(%rbp)
	cfi_same_value(%r12)
	cfi_same_value(%r13)
	cfi_same_value(%r14)
	cfi_same_value(%r15)

	/* Clear rax to indicate success.  */
	xorq	%rax, %rax

L(pseudo_end):
	ret
PSEUDO_END(__setcontext)

Does it look sane?  

Btw. cfi_same_value is not yet defined in sysdeps.h and needs to be
defined with the appended patch.  Is it ok to commit that one?

Andreas

2004-01-11  Andreas Jaeger  <aj@suse.de>

	* sysdeps/generic/sysdep.h (CFI_SAME_VALUE): New.
	(cfi_same_value): New.


[-- Attachment #1.2: libc-diff --]
[-- Type: text/plain, Size: 1469 bytes --]

============================================================
Index: sysdeps/generic/sysdep.h
--- sysdeps/generic/sysdep.h	31 Aug 2003 17:21:03 -0000	1.13
+++ sysdeps/generic/sysdep.h	11 Jan 2004 12:16:42 -0000
@@ -1,5 +1,5 @@
 /* Generic asm macros used on many machines.
-   Copyright (C) 1991,92,93,96,98,2002,2003 Free Software Foundation, Inc.
+   Copyright (C) 1991,92,93,96,98,2002,2003,2004 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -60,6 +60,7 @@
 #  define cfi_register(r1, r2)		.cfi_register r1, r2
 #  define cfi_return_column(reg)	.cfi_return_column reg
 #  define cfi_restore(reg)		.cfi_restore reg
+#  define cfi_same_value(reg)		.cfi_same_value reg
 #  define cfi_undefined(reg)		.cfi_undefined reg
 #  define cfi_remember_state		.cfi_remember_state
 #  define cfi_restore_state		.cfi_restore_state
@@ -76,6 +77,7 @@
 #  define cfi_register(r1, r2)
 #  define cfi_return_column(reg)
 #  define cfi_restore(reg)
+#  define cfi_same_value(reg)
 #  define cfi_undefined(reg)
 #  define cfi_remember_state
 #  define cfi_restore_state
@@ -106,6 +108,8 @@
    ".cfi_return_column " CFI_STRINGIFY(reg)
 #  define CFI_RESTORE(reg) \
    ".cfi_restore " CFI_STRINGIFY(reg)
+#  define CFI_SAME_VALUE(reg) \
+   ".cfi_same_value " CFI_STRINGIFY(reg)
 #  define CFI_UNDEFINED(reg) \
    ".cfi_undefined " CFI_STRINGIFY(reg)
 #  define CFI_REMEMBER_STATE \

[-- Attachment #1.3: Type: text/plain, Size: 190 bytes --]


-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] Fix AMD64 backtrace

[Jakub Jelinek]

On Sun, Jan 11, 2004 at 01:17:52PM +0100, Andreas Jaeger wrote:
> 	/* Load the new stack pointer, the preserved registers and
> 	   registers used for passing args.  */
> 	cfi_def_cfa(%rdi, 0)
> 	cfi_offset(%rbx,oRBX)
> 	cfi_offset(%rbp,oRBP)
> 	cfi_offset(%r12,oR12)
> 	cfi_offset(%r13,oR13)
> 	cfi_offset(%r14,oR14)
> 	cfi_offset(%r15,oR15)
> 	cfi_offset(%rsp,oRSP)
> 	cfi_offset(%rip,oRIP)
> 	
> 	movq	oRSP(%rdi), %rsp
> 	movq	oRBX(%rdi), %rbx
> 	movq	oRBP(%rdi), %rbp
> 	movq	oR12(%rdi), %r12
> 	movq	oR13(%rdi), %r13
> 	movq	oR14(%rdi), %r14
> 	movq	oR15(%rdi), %r15
> 
> 	/* The following ret should return to the address set with
> 	getcontext.  Therefore push the address on the stack.  */
> 	movq	oRIP(%rdi), %rcx
> 	pushq	%rcx
> 	cfi_adjust_cfa_offset(8)

pushq %rcx modifies %rsp, but current CFI here is %rdi
which is unchanged.

	Jakub

Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 1250 bytes --]

Jakub Jelinek <jakub@redhat.com> writes:

> On Sun, Jan 11, 2004 at 01:17:52PM +0100, Andreas Jaeger wrote:
>> 	/* Load the new stack pointer, the preserved registers and
>> 	   registers used for passing args.  */
>> 	cfi_def_cfa(%rdi, 0)
>> 	cfi_offset(%rbx,oRBX)
>> 	cfi_offset(%rbp,oRBP)
>> 	cfi_offset(%r12,oR12)
>> 	cfi_offset(%r13,oR13)
>> 	cfi_offset(%r14,oR14)
>> 	cfi_offset(%r15,oR15)
>> 	cfi_offset(%rsp,oRSP)
>> 	cfi_offset(%rip,oRIP)
>> 	
>> 	movq	oRSP(%rdi), %rsp
>> 	movq	oRBX(%rdi), %rbx
>> 	movq	oRBP(%rdi), %rbp
>> 	movq	oR12(%rdi), %r12
>> 	movq	oR13(%rdi), %r13
>> 	movq	oR14(%rdi), %r14
>> 	movq	oR15(%rdi), %r15
>> 
>> 	/* The following ret should return to the address set with
>> 	getcontext.  Therefore push the address on the stack.  */
>> 	movq	oRIP(%rdi), %rcx
>> 	pushq	%rcx
>> 	cfi_adjust_cfa_offset(8)
>
> pushq %rcx modifies %rsp, but current CFI here is %rdi
> which is unchanged.

Thanks!  I'll commit with that change once I get approval for the
sysdeps/generic/sysdep.h change...

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] Fix AMD64 backtrace

[Jakub Jelinek]

On Sun, Jan 11, 2004 at 08:28:37PM +0100, Andreas Jaeger wrote:
> Thanks!  I'll commit with that change once I get approval for the
> sysdeps/generic/sysdep.h change...

Have you tested it?
E.g. for longjmp program with 2 threads, one pthread_kill'ing the other
one with some signal randomly and the other doing setjmp/longjmp in a tight
loop and checking in the signal handler if the address is inside of
__longjmp and if so, trying to do e.g. backtrace or some other _Unwind_*
function.

	Jakub

Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 803 bytes --]

Jakub Jelinek <jakub@redhat.com> writes:

> On Sun, Jan 11, 2004 at 08:28:37PM +0100, Andreas Jaeger wrote:
>> Thanks!  I'll commit with that change once I get approval for the
>> sysdeps/generic/sysdep.h change...
>
> Have you tested it?
> E.g. for longjmp program with 2 threads, one pthread_kill'ing the other
> one with some signal randomly and the other doing setjmp/longjmp in a tight
> loop and checking in the signal handler if the address is inside of
> __longjmp and if so, trying to do e.g. backtrace or some other _Unwind_*
> function.

No, I haven't done anything like that...

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 2506 bytes --]

Richard Henderson <rth@redhat.com> writes:

> On Sun, Jan 11, 2004 at 01:17:52PM +0100, Andreas Jaeger wrote:
>> 	pushq	%rcx
>> 	cfi_adjust_cfa_offset(8)
>
> Jakub already pointed out that this is wrong.
>
>> 	/* We changed %rdi, fix cfi.  */
>> 	cfi_def_cfa(%rsp, 0)
>
> But the push means this is wrong too -- the CFA is rsp+8.
>
> Also wrong at this point is the return address.  The value
> is now stored at CFA-8, where we pushed it above.

This would give the following routine:

ENTRY(__setcontext)
	/* Save argument since syscall will destroy it.  */
	pushq	%rdi
	cfi_adjust_cfa_offset(8)

	/* Set the signal mask with
	   rt_sigprocmask (SIG_SETMASK, mask, NULL, _NSIG/8).  */
	leaq	oSIGMASK(%rdi), %rsi
	xorq	%rdx, %rdx
	movq	$SIG_SETMASK, %rdi
	movq	$_NSIG8,%r10
	movq	$__NR_rt_sigprocmask, %rax
	syscall
	popq	%rdi			/* Reload %rdi, adjust stack.  */
	cfi_adjust_cfa_offset(-8)
	cmpq	$-4095, %rax		/* Check %rax for error.  */
	jae	SYSCALL_ERROR_LABEL	/* Jump to error handler if error.  */

	/* Restore the floating-point context.  Not the registers, only the
	   rest.  */
	movq	oFPREGS(%rdi), %rcx
	fldenv	(%rcx)
	ldmxcsr oMXCSR(%rdi)


	/* Load the new stack pointer, the preserved registers and
	   registers used for passing args.  */
	cfi_def_cfa(%rdi, 0)
	cfi_offset(%rbx,oRBX)
	cfi_offset(%rbp,oRBP)
	cfi_offset(%r12,oR12)
	cfi_offset(%r13,oR13)
	cfi_offset(%r14,oR14)
	cfi_offset(%r15,oR15)
	cfi_offset(%rsp,oRSP)
	cfi_offset(%rip,oRIP)
	
	movq	oRSP(%rdi), %rsp
	movq	oRBX(%rdi), %rbx
	movq	oRBP(%rdi), %rbp
	movq	oR12(%rdi), %r12
	movq	oR13(%rdi), %r13
	movq	oR14(%rdi), %r14
	movq	oR15(%rdi), %r15

	/* The following ret should return to the address set with
	getcontext.  Therefore push the address on the stack.  */
	movq	oRIP(%rdi), %rcx
	pushq	%rcx

	movq	oRSI(%rdi), %rsi
	movq	oRDX(%rdi), %rdx
	movq	oRCX(%rdi), %rcx
	movq	oR8(%rdi), %r8
	movq	oR9(%rdi), %r9

	/* Setup finally  %rdi.  */
	movq	oRDI(%rdi), %rdi

	/* We changed %rdi, fix cfi.  */
	cfi_def_cfa(%rsp, 8)
	cfi_same_value(%rbx)
	cfi_same_value(%rbp)
	cfi_same_value(%r12)
	cfi_same_value(%r13)
	cfi_same_value(%r14)
	cfi_same_value(%r15)
	cfi_offset(%rip,-8)

	/* Clear rax to indicate success.  */
	xorq	%rax, %rax

Thanks,
Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] Fix AMD64 backtrace

[Jakub Jelinek]

On Sat, Jan 10, 2004 at 05:10:18PM +0100, Andreas Jaeger wrote:
> --- sysdeps/unix/sysv/linux/x86_64/__start_context.S	27 Aug 2003 23:03:41 -0000	1.2
> +++ sysdeps/unix/sysv/linux/x86_64/__start_context.S	10 Jan 2004 16:09:40 -0000
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 2002, 2003 Free Software Foundation, Inc.
> +/* Copyright (C) 2002, 2003, 2004 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>     Contributed by Andreas Jaeger <aj@suse.de>, 2002.
>  
> @@ -33,6 +33,7 @@ ENTRY(__start_context)
>  	movq	%rbx, %rsp

This is not reflected in the unwind info.
Unless %rbx points to a populated stack with correct caller
something needs to be done about it.

> --- sysdeps/x86_64/strcspn.S	29 Apr 2003 22:47:18 -0000	1.2
> +++ sysdeps/x86_64/strcspn.S	10 Jan 2004 16:09:41 -0000
> @@ -1,7 +1,7 @@
>  /* strcspn (str, ss) -- Return the length of the initial segment of STR
>  			which contains no characters from SS.
>     For AMD x86-64.
> -   Copyright (C) 1994-1997, 2000, 2002, 2003 Free Software Foundation, Inc.
> +   Copyright (C) 1994-1997, 2000, 2002, 2003, 2004 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>     Contributed by Ulrich Drepper <drepper@gnu.ai.mit.edu>.
>     Bug fixes by Alan Modra <Alan@SPRI.Levels.UniSA.Edu.Au>.
> @@ -40,6 +40,7 @@ ENTRY (strcspn)
>  	   table.  */
>  	movq %rdi, %r8			/* Save value.  */
>  	subq $256, %rsp			/* Make space for 256 bytes.  */
> +	cfi_adjust_cfa_offset(-256)

Typo/pasto.

	Jakub

Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 2123 bytes --]

Jakub Jelinek <jakub@redhat.com> writes:

> On Sat, Jan 10, 2004 at 05:10:18PM +0100, Andreas Jaeger wrote:
>> --- sysdeps/unix/sysv/linux/x86_64/__start_context.S	27 Aug 2003 23:03:41 -0000	1.2
>> +++ sysdeps/unix/sysv/linux/x86_64/__start_context.S	10 Jan 2004 16:09:40 -0000
>> @@ -1,4 +1,4 @@
>> -/* Copyright (C) 2002, 2003 Free Software Foundation, Inc.
>> +/* Copyright (C) 2002, 2003, 2004 Free Software Foundation, Inc.
>>     This file is part of the GNU C Library.
>>     Contributed by Andreas Jaeger <aj@suse.de>, 2002.
>>  
>> @@ -33,6 +33,7 @@ ENTRY(__start_context)
>>  	movq	%rbx, %rsp
>
> This is not reflected in the unwind info.
> Unless %rbx points to a populated stack with correct caller
> something needs to be done about it.

It should point to a populated stack:
	/* This removes the parameters passed to the function given to
	   'makecontext' from the stack.  RBX contains the address
	   on the stack pointer for the next context.  */

But yeah, we destroy %rsp - so what can we do here?

>
>> --- sysdeps/x86_64/strcspn.S	29 Apr 2003 22:47:18 -0000	1.2
>> +++ sysdeps/x86_64/strcspn.S	10 Jan 2004 16:09:41 -0000
>> @@ -1,7 +1,7 @@
>>  /* strcspn (str, ss) -- Return the length of the initial segment of STR
>>  			which contains no characters from SS.
>>     For AMD x86-64.
>> -   Copyright (C) 1994-1997, 2000, 2002, 2003 Free Software Foundation, Inc.
>> +   Copyright (C) 1994-1997, 2000, 2002, 2003, 2004 Free Software Foundation, Inc.
>>     This file is part of the GNU C Library.
>>     Contributed by Ulrich Drepper <drepper@gnu.ai.mit.edu>.
>>     Bug fixes by Alan Modra <Alan@SPRI.Levels.UniSA.Edu.Au>.
>> @@ -40,6 +40,7 @@ ENTRY (strcspn)
>>  	   table.  */
>>  	movq %rdi, %r8			/* Save value.  */
>>  	subq $256, %rsp			/* Make space for 256 bytes.  */
>> +	cfi_adjust_cfa_offset(-256)
>
> Typo/pasto.

Fixed and committed - thanks,

Andreas
-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [rfc] dwarf2 backtrace from setcontext

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 2372 bytes --]

Richard Henderson <rth@redhat.com> writes:

> On Mon, Jan 12, 2004 at 06:19:19PM -0500, Daniel Jacobowitz wrote:
>>   It seems to me that the best solution would be a new rule, either
>>   val_expression(E) or the simpler val_cfa_offset(N).  Then the unwind
>>   information can accurately describe the stack pointer.
>
> That would certainly work for me.
>
>> That would probably work for this example - I can't see any reason that
>> GDB would care.  And for setcontext it makes a certain sense, since you
>> do seem to have "fallen through" into a different backtrace.
>
> I'll recommend we do that for glibc then.

Like the following?

Andreas

============================================================
Index: sysdeps/unix/sysv/linux/x86_64/setcontext.S
--- x86_64/setcontext.S	31 Aug 2002 08:05:51 -0000	1.2
+++ x86_64/setcontext.S	22 Jan 2004 07:24:29 -0000
@@ -1,5 +1,5 @@
 /* Install given context.
-   Copyright (C) 2002 Free Software Foundation, Inc.
+   Copyright (C) 2002, 2004 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Andreas Jaeger <aj@suse.de>, 2002.
 
@@ -35,6 +35,7 @@
 ENTRY(__setcontext)
 	/* Save argument since syscall will destroy it.  */
 	pushq	%rdi
+	cfi_adjust_cfa_offset(8)
 
 	/* Set the signal mask with
 	   rt_sigprocmask (SIG_SETMASK, mask, NULL, _NSIG/8).  */
@@ -45,6 +46,7 @@ ENTRY(__setcontext)
 	movq	$__NR_rt_sigprocmask, %rax
 	syscall
 	popq	%rdi			/* Reload %rdi, adjust stack.  */
+	cfi_adjust_cfa_offset(-8)
 	cmpq	$-4095, %rax		/* Check %rax for error.  */
 	jae	SYSCALL_ERROR_LABEL	/* Jump to error handler if error.  */
 
@@ -57,6 +59,9 @@ ENTRY(__setcontext)
 
 	/* Load the new stack pointer, the preserved registers and
 	   registers used for passing args.  */
+	/* End FDE here, we fall into another context.  */
+	cfi_endproc
+	
 	movq	oRSP(%rdi), %rsp
 	movq	oRBX(%rdi), %rbx
 	movq	oRBP(%rdi), %rbp
@@ -78,6 +83,9 @@ ENTRY(__setcontext)
 
 	/* Setup finally  %rdi.  */
 	movq	oRDI(%rdi), %rdi
+
+	/* We changed %rdi, fix cfi.  */
+	cfi_startproc
 
 	/* Clear rax to indicate success.  */
 	xorq	%rax, %rax

-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [rfc] dwarf2 backtrace from setcontext

[Andreas Jaeger]

[-- Attachment #1: Type: text/plain, Size: 2015 bytes --]


Thanks.  Here's the version that I'm going to commit now,

Andreas

============================================================
Index: sysdeps/unix/sysv/linux/x86_64/setcontext.S
--- sysdeps/unix/sysv/linux/x86_64/setcontext.S	31 Aug 2002 08:05:51 -0000	1.2
+++ sysdeps/unix/sysv/linux/x86_64/setcontext.S	22 Jan 2004 08:16:49 -0000
@@ -1,5 +1,5 @@
 /* Install given context.
-   Copyright (C) 2002 Free Software Foundation, Inc.
+   Copyright (C) 2002, 2004 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Andreas Jaeger <aj@suse.de>, 2002.
 
@@ -35,6 +35,7 @@
 ENTRY(__setcontext)
 	/* Save argument since syscall will destroy it.  */
 	pushq	%rdi
+	cfi_adjust_cfa_offset(8)
 
 	/* Set the signal mask with
 	   rt_sigprocmask (SIG_SETMASK, mask, NULL, _NSIG/8).  */
@@ -45,6 +46,7 @@ ENTRY(__setcontext)
 	movq	$__NR_rt_sigprocmask, %rax
 	syscall
 	popq	%rdi			/* Reload %rdi, adjust stack.  */
+	cfi_adjust_cfa_offset(-8)
 	cmpq	$-4095, %rax		/* Check %rax for error.  */
 	jae	SYSCALL_ERROR_LABEL	/* Jump to error handler if error.  */
 
@@ -57,6 +59,16 @@ ENTRY(__setcontext)
 
 	/* Load the new stack pointer, the preserved registers and
 	   registers used for passing args.  */
+	cfi_def_cfa(%rdi, 0)
+	cfi_offset(%rbx,oRBX)
+	cfi_offset(%rbp,oRBP)
+	cfi_offset(%r12,oR12)
+	cfi_offset(%r13,oR13)
+	cfi_offset(%r14,oR14)
+	cfi_offset(%r15,oR15)
+	cfi_offset(%rsp,oRSP)
+	cfi_offset(%rip,oRIP)
+
 	movq	oRSP(%rdi), %rsp
 	movq	oRBX(%rdi), %rbx
 	movq	oRBP(%rdi), %rbp
@@ -78,6 +90,10 @@ ENTRY(__setcontext)
 
 	/* Setup finally  %rdi.  */
 	movq	oRDI(%rdi), %rdi
+
+	/* End FDE here, we fall into another context.  */
+	cfi_endproc
+	cfi_startproc
 
 	/* Clear rax to indicate success.  */
 	xorq	%rax, %rax

-- 
 Andreas Jaeger, aj@suse.de, http://www.suse.de/~aj
  SuSE Linux AG, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]