| Message ID | 1401046909-25821-9-git-send-email-rth@twiddle.net |
|---|---|
| State | New |
| Headers | show |
On 25 May 2014 20:41, Richard Henderson <rth@twiddle.net> wrote: > From: Richard Henderson <rth@redhat.com> > > At the same time, rely on non-clobbered registers across syscall > so that we eliminate the stack frame that we previously ignored > in the unwind info. > --- > sysdeps/unix/sysv/linux/aarch64/clone.S | 56 ++++++++++++---------------- > sysdeps/unix/sysv/linux/aarch64/nptl/clone.S | 21 ----------- > 2 files changed, 24 insertions(+), 53 deletions(-) > delete mode 100644 sysdeps/unix/sysv/linux/aarch64/nptl/clone.S > > diff --git a/sysdeps/unix/sysv/linux/aarch64/clone.S b/sysdeps/unix/sysv/linux/aarch64/clone.S > + beq thread_start I notice that some of the targets make thread_start local (x86_64, i386, hppa, tile, mips) while others do not (alpha, m68k, s390). Which seems odd, any idea why? /Marcus
Marcus Shawcroft <marcus.shawcroft@gmail.com> writes: > I notice that some of the targets make thread_start local (x86_64, > i386, hppa, tile, mips) while others do not (alpha, m68k, s390). An assembler label is always local by default. Andreas.
On 05/29/2014 01:15 AM, Andreas Schwab wrote: > Marcus Shawcroft <marcus.shawcroft@gmail.com> writes: > >> I notice that some of the targets make thread_start local (x86_64, >> i386, hppa, tile, mips) while others do not (alpha, m68k, s390). > > An assembler label is always local by default. Indeed, but what Marcus meant was "really" local, i.e. ".L" or "1:" type symbols, vs "static" symbols that are local in the elf sense but preserved by the assembler. My guess is that with the pre-dwarf alpha unwind info one had to use a different ".ent" directive, which implied that it was impossible to put the unwind info for thread_start into the same function as clone. And then others copied how Alpha had set it up. Personally, I like having "thread_start" at the top of the backtrace. But I can see how others might prefer "clone", as some indication of how that happened. r~
On 29 May 2014 16:34, Richard Henderson <rth@twiddle.net> wrote: > On 05/29/2014 01:15 AM, Andreas Schwab wrote: >> Marcus Shawcroft <marcus.shawcroft@gmail.com> writes: >> >>> I notice that some of the targets make thread_start local (x86_64, >>> i386, hppa, tile, mips) while others do not (alpha, m68k, s390). >> >> An assembler label is always local by default. > > Indeed, but what Marcus meant was "really" local, i.e. ".L" or "1:" > type symbols, vs "static" symbols that are local in the elf sense but preserved > by the assembler. > > My guess is that with the pre-dwarf alpha unwind info one had to use a > different ".ent" directive, which implied that it was impossible to put the > unwind info for thread_start into the same function as clone. And then others > copied how Alpha had set it up. > > Personally, I like having "thread_start" at the top of the backtrace. But I > can see how others might prefer "clone", as some indication of how that happened. I don't have a strong opinion either way, but I am slightly surprised that we (glibc) don't appear to have a preference for new ports. I'm happy to go with your proposal. /Marcus
diff --git a/sysdeps/unix/sysv/linux/aarch64/clone.S b/sysdeps/unix/sysv/linux/aarch64/clone.S index f2964f4..a2b5a2b 100644 --- a/sysdeps/unix/sysv/linux/aarch64/clone.S +++ b/sysdeps/unix/sysv/linux/aarch64/clone.S @@ -39,47 +39,42 @@ */ .text ENTRY(__clone) + /* Save args for the child. */ + mov x10, x0 + mov x11, x2 + mov x12, x3 + /* Sanity check args. */ - cbz x0, 1f - cbz x1, 1f - /* Insert the args onto the new stack. */ - stp x0, x3, [x1, #-16]! /* Fn, arg. */ + mov x0, #-EINVAL + cbz x10, .Lsyscall_error + cbz x1, .Lsyscall_error /* Do the system call. */ + /* X0:flags, x1:newsp, x2:parenttidptr, x3:newtls, x4:childtid. */ mov x0, x2 /* flags */ - /* New sp is already in x1. */ mov x2, x4 /* ptid */ mov x3, x5 /* tls */ mov x4, x6 /* ctid */ - -#ifdef RESET_PID - /* We rely on the kernel preserving the argument regsiters across a - each system call so that we can inspect the flags against after - the clone call. */ - mov x5, x0 -#endif - mov x8, #SYS_ify(clone) - /* X0:flags, x1:newsp, x2:parenttidptr, x3:newtls, x4:childtid. */ svc 0x0 - cfi_endproc + cmp x0, #0 - beq 2f - blt 3f + beq thread_start + blt .Lsyscall_error RET -1: mov x0, #-EINVAL -3: - b syscall_error +PSEUDO_END (__clone) -2: + .align 4 + .type thread_start, %function +thread_start: cfi_startproc cfi_undefined (x30) mov x29, 0 -#ifdef RESET_PID - tbnz x5, #CLONE_THREAD_BIT, 3f + + tbnz x11, #CLONE_THREAD_BIT, 3f mov x0, #-1 - tbnz x5, #CLONE_VM_BIT, 2f + tbnz x11, #CLONE_VM_BIT, 2f mov x8, #SYS_ify(getpid) svc 0x0 2: @@ -87,18 +82,15 @@ ENTRY(__clone) sub x1, x1, #PTHREAD_SIZEOF str w0, [x1, #PTHREAD_PID_OFFSET] str w0, [x1, #PTHREAD_TID_OFFSET] - 3: -#endif - /* Pick the function arg and call address from the stack and - execute. */ - ldp x1, x0, [sp], #16 - blr x1 + + /* Pick the function arg and execute. */ + mov x0, x12 + blr x10 /* We are done, pass the return value through x0. */ b HIDDEN_JUMPTARGET(_exit) cfi_endproc - cfi_startproc -PSEUDO_END (__clone) + .size thread_start, .-thread_start weak_alias (__clone, clone) diff --git a/sysdeps/unix/sysv/linux/aarch64/nptl/clone.S b/sysdeps/unix/sysv/linux/aarch64/nptl/clone.S deleted file mode 100644 index 281be3b..0000000 --- a/sysdeps/unix/sysv/linux/aarch64/nptl/clone.S +++ /dev/null @@ -1,21 +0,0 @@ -/* Copyright (C) 2009-2014 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 - 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, see - <http://www.gnu.org/licenses/>. */ - -#define RESET_PID -#include <tcb-offsets.h> -#include "../clone.S"
From: Richard Henderson <rth@redhat.com> At the same time, rely on non-clobbered registers across syscall so that we eliminate the stack frame that we previously ignored in the unwind info. --- sysdeps/unix/sysv/linux/aarch64/clone.S | 56 ++++++++++++---------------- sysdeps/unix/sysv/linux/aarch64/nptl/clone.S | 21 ----------- 2 files changed, 24 insertions(+), 53 deletions(-) delete mode 100644 sysdeps/unix/sysv/linux/aarch64/nptl/clone.S