Re: [PATCH] Fix AMD64 backtrace

[Andreas Jaeger <aj@suse.de>]

[-- 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 --]