diff mbox

[BZ,#19490] Add unwind descriptors for x86_64 _mcount and __fentry__

Message ID CALoOobMRss18H48kOngTdyXWO1tJf=YahxqJavLyuZcXQVzW8g@mail.gmail.com
State New, archived
Headers

Commit Message

Paul Pluzhnikov March 3, 2016, 5:01 p.m. UTC 
  On Thu, Mar 3, 2016 at 12:30 AM, Andreas Schwab <schwab@suse.de> wrote:
> Paul Pluzhnikov <ppluzhnikov@google.com> writes:
>
>> Using CALL_MCOUNT in implementation of _mcount isn't going to end well.
>
> _mcount needs to be arranged to be compiled without PROF, like the
> noprof routines in gmon/Makefile.

Done by HJ in commit 87a07a437656aede6f303688b55ae1834962bee2.

Updated patch attached. Tested on Linux/x86_64.

Thanks,

2016-03-03  Paul Pluzhnikov  <ppluzhnikov@google.com>

        [BZ #19490]
        * sysdeps/x86_64/_mcount.S (_mcount): Add unwind descriptor.
        (__fentry__): Likewise

Comments

H.J. Lu March 3, 2016, 5:30 p.m. UTC | #1 
On Thu, Mar 3, 2016 at 9:01 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> On Thu, Mar 3, 2016 at 12:30 AM, Andreas Schwab <schwab@suse.de> wrote:
>> Paul Pluzhnikov <ppluzhnikov@google.com> writes:
>>
>>> Using CALL_MCOUNT in implementation of _mcount isn't going to end well.
>>
>> _mcount needs to be arranged to be compiled without PROF, like the
>> noprof routines in gmon/Makefile.
>
> Done by HJ in commit 87a07a437656aede6f303688b55ae1834962bee2.
>
> Updated patch attached. Tested on Linux/x86_64.
>
> Thanks,
>
> 2016-03-03  Paul Pluzhnikov  <ppluzhnikov@google.com>
>
>         [BZ #19490]
>         * sysdeps/x86_64/_mcount.S (_mcount): Add unwind descriptor.
>         (__fentry__): Likewise
>

Hi Andi,

Why do we need __fentry__ different from _mcount in glibc? Can we
make __fentry__  an alias of _mcount?
Paul Pluzhnikov March 3, 2016, 5:35 p.m. UTC | #2 
On Thu, Mar 3, 2016 at 9:30 AM, H.J. Lu <hjl.tools@gmail.com> wrote:

> Why do we need __fentry__ different from _mcount in glibc? Can we
> make __fentry__  an alias of _mcount?

For one thing, they have different stack alignment requirements, as I
already said on this thread:
https://sourceware.org/ml/libc-alpha/2016-02/msg00823.html
H.J. Lu March 3, 2016, 5:41 p.m. UTC | #3 
On Thu, Mar 3, 2016 at 9:35 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> On Thu, Mar 3, 2016 at 9:30 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
>> Why do we need __fentry__ different from _mcount in glibc? Can we
>> make __fentry__  an alias of _mcount?
>
> For one thing, they have different stack alignment requirements, as I
> already said on this thread:
> https://sourceware.org/ml/libc-alpha/2016-02/msg00823.html

Please update _mcount.S with your comments above.  OK with
that change.

Thanks for your patience.
Paul Pluzhnikov March 3, 2016, 5:54 p.m. UTC | #4 
On Thu, Mar 3, 2016 at 9:41 AM, H.J. Lu <hjl.tools@gmail.com> wrote:

> Please update _mcount.S with your comments above.  OK with
> that change.

Done and committed.

Thanks!
Andi Kleen March 3, 2016, 6:29 p.m. UTC | #5 
> Why do we need __fentry__ different from _mcount in glibc? Can we
> make __fentry__  an alias of _mcount?

The stack frame is different, and mcount needs to look at the
stack frame. mcount is after the frame pointer, __fentry__ before.

-Andi
diff mbox

Patch

diff --git a/sysdeps/x86_64/_mcount.S b/sysdeps/x86_64/_mcount.S
index ba13643..69c4b84 100644
--- a/sysdeps/x86_64/_mcount.S
+++ b/sysdeps/x86_64/_mcount.S
@@ -24,19 +24,24 @@ 
 
 #include <sysdep.h>
 
-	.globl C_SYMBOL_NAME(_mcount)
-	.type C_SYMBOL_NAME(_mcount), @function
-	.align ALIGNARG(4)
-C_LABEL(_mcount)
+ENTRY(_mcount)
 	/* Allocate space for 7 registers.  */
 	subq	$56,%rsp
+	cfi_adjust_cfa_offset (56)
 	movq	%rax,(%rsp)
+	cfi_rel_offset (rax, 0)
 	movq	%rcx,8(%rsp)
+	cfi_rel_offset (rcx, 8)
 	movq	%rdx,16(%rsp)
+	cfi_rel_offset (rdx, 16)
 	movq	%rsi,24(%rsp)
+	cfi_rel_offset (rsi, 24)
 	movq	%rdi,32(%rsp)
+	cfi_rel_offset (rdi, 32)
 	movq	%r8,40(%rsp)
+	cfi_rel_offset (r8, 40)
 	movq	%r9,48(%rsp)
+	cfi_rel_offset (r9, 48)
 
 	/* Setup parameter for __mcount_internal.  */
 	/* selfpc is the return address on the stack.  */
@@ -47,33 +52,46 @@  C_LABEL(_mcount)
 	/* Pop the saved registers.  Please note that `mcount' has no
 	   return value.  */
 	movq	48(%rsp),%r9
+	cfi_restore (r9)
 	movq	40(%rsp),%r8
+	cfi_restore (r8)
 	movq	32(%rsp),%rdi
+	cfi_restore (rdi)
 	movq	24(%rsp),%rsi
+	cfi_restore (rsi)
 	movq	16(%rsp),%rdx
+	cfi_restore (rdx)
 	movq	8(%rsp),%rcx
+	cfi_restore (rcx)
 	movq	(%rsp),%rax
+	cfi_restore (rax)
 	addq	$56,%rsp
+	cfi_adjust_cfa_offset (-56)
 	ret
-
-	ASM_SIZE_DIRECTIVE(C_SYMBOL_NAME(_mcount))
+END(_mcount)
 
 #undef mcount
 weak_alias (_mcount, mcount)
 
-	.globl C_SYMBOL_NAME(__fentry__)
-	.type C_SYMBOL_NAME(__fentry__), @function
-	.align ALIGNARG(4)
-C_LABEL(__fentry__)
-	/* Allocate space for 7 registers.  */
+ENTRY(__fentry__)
+	/* Allocate space for 7 registers
+	   (+8 bytes for proper stack alignment).  */
 	subq	$64,%rsp
+	cfi_adjust_cfa_offset (64)
 	movq	%rax,(%rsp)
+	cfi_rel_offset (rax, 0)
 	movq	%rcx,8(%rsp)
+	cfi_rel_offset (rcx, 8)
 	movq	%rdx,16(%rsp)
+	cfi_rel_offset (rdx, 16)
 	movq	%rsi,24(%rsp)
+	cfi_rel_offset (rsi, 24)
 	movq	%rdi,32(%rsp)
+	cfi_rel_offset (rdi, 32)
 	movq	%r8,40(%rsp)
+	cfi_rel_offset (r8, 40)
 	movq	%r9,48(%rsp)
+	cfi_rel_offset (r9, 48)
 
 	/* Setup parameter for __mcount_internal.  */
 	/* selfpc is the return address on the stack.  */
@@ -84,13 +102,20 @@  C_LABEL(__fentry__)
 	/* Pop the saved registers.  Please note that `__fentry__' has no
 	   return value.  */
 	movq	48(%rsp),%r9
+	cfi_restore (r9)
 	movq	40(%rsp),%r8
+	cfi_restore (r8)
 	movq	32(%rsp),%rdi
+	cfi_restore (rdi)
 	movq	24(%rsp),%rsi
+	cfi_restore (rsi)
 	movq	16(%rsp),%rdx
+	cfi_restore (rdx)
 	movq	8(%rsp),%rcx
+	cfi_restore (rcx)
 	movq	(%rsp),%rax
+	cfi_restore (rax)
 	addq	$64,%rsp
+	cfi_adjust_cfa_offset (-64)
 	ret
-
-	ASM_SIZE_DIRECTIVE(C_SYMBOL_NAME(__fentry__))
+END(__fentry__)