diff mbox

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

Message ID CALoOobPzA=QaRyua2KVLR5CtKoks_Tz=c45SwWBg18sY0X4kWg@mail.gmail.com
State New
Headers show

Commit Message

Paul Pluzhnikov Jan. 23, 2016, 10:52 p.m. UTC
Greetings,

Attached patch adds unwind descriptors for x86_64 _mcount and __fentry__.
Tested on Linux/x86_64, no failures.

Assuming this is OK, should I wait for lifting of the freeze?

Thanks,

2016-01-23  Paul Pluzhnikov  <ppluzhnikov@google.com>

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

Comments

Mike Frysinger Feb. 22, 2016, 9:14 a.m. UTC | #1
On 23 Jan 2016 14:52, Paul Pluzhnikov wrote:
> --- a/sysdeps/x86_64/_mcount.S
> +++ b/sysdeps/x86_64/_mcount.S
> @@ -28,8 +28,10 @@
>  	.type C_SYMBOL_NAME(_mcount), @function
>  	.align ALIGNARG(4)
>  C_LABEL(_mcount)
> +	cfi_startproc
>  	/* Allocate space for 7 registers.  */
>  	subq	$56,%rsp
> +	cfi_adjust_cfa_offset (56)
>  	movq	%rax,(%rsp)
>  	movq	%rcx,8(%rsp)
>  	movq	%rdx,16(%rsp)
> @@ -37,6 +39,13 @@ C_LABEL(_mcount)
>  	movq	%rdi,32(%rsp)
>  	movq	%r8,40(%rsp)
>  	movq	%r9,48(%rsp)
> +	cfi_rel_offset (rax, 0)
> +	cfi_rel_offset (rcx, 8)
> +	cfi_rel_offset (rdx, 16)
> +	cfi_rel_offset (rsi, 24)
> +	cfi_rel_offset (rdi, 32)
> +	cfi_rel_offset (r8, 40)
> +	cfi_rel_offset (r9, 48)

don't we usually interleave the insns & cfi calls so that it's harder
for them to get out of sync ?

C_LABEL(_mcount)
	cfi_startproc
	/* 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)
...

ignoring that, the actual patch looks fine

>  	.type C_SYMBOL_NAME(__fentry__), @function
>  	.align ALIGNARG(4)
>  C_LABEL(__fentry__)
> -	/* Allocate space for 7 registers.  */
> +	cfi_startproc
> +	/* Allocate space for 7 registers (+8 for proper stack alignment).  */
>  	subq	$64,%rsp

mmm, 56 is used above w/_mcount and is 8 byte aligned.  are you saying
we need 16 byte alignment and thus _mcount should be fixed ?
-mike
diff mbox

Patch

diff --git a/sysdeps/x86_64/_mcount.S b/sysdeps/x86_64/_mcount.S
index 5d7edd2..5f186be 100644
--- a/sysdeps/x86_64/_mcount.S
+++ b/sysdeps/x86_64/_mcount.S
@@ -28,8 +28,10 @@ 
 	.type C_SYMBOL_NAME(_mcount), @function
 	.align ALIGNARG(4)
 C_LABEL(_mcount)
+	cfi_startproc
 	/* Allocate space for 7 registers.  */
 	subq	$56,%rsp
+	cfi_adjust_cfa_offset (56)
 	movq	%rax,(%rsp)
 	movq	%rcx,8(%rsp)
 	movq	%rdx,16(%rsp)
@@ -37,6 +39,13 @@  C_LABEL(_mcount)
 	movq	%rdi,32(%rsp)
 	movq	%r8,40(%rsp)
 	movq	%r9,48(%rsp)
+	cfi_rel_offset (rax, 0)
+	cfi_rel_offset (rcx, 8)
+	cfi_rel_offset (rdx, 16)
+	cfi_rel_offset (rsi, 24)
+	cfi_rel_offset (rdi, 32)
+	cfi_rel_offset (r8, 40)
+	cfi_rel_offset (r9, 48)
 
 	/* Setup parameter for __mcount_internal.  */
 	/* selfpc is the return address on the stack.  */
@@ -58,7 +67,16 @@  C_LABEL(_mcount)
 	movq	8(%rsp),%rcx
 	movq	(%rsp),%rax
 	addq	$56,%rsp
+	cfi_restore (r9)
+	cfi_restore (r8)
+	cfi_restore (rdi)
+	cfi_restore (rsi)
+	cfi_restore (rdx)
+	cfi_restore (rcx)
+	cfi_restore (rax)
+	cfi_adjust_cfa_offset (-56)
 	ret
+	cfi_endproc
 
 	ASM_SIZE_DIRECTIVE(C_SYMBOL_NAME(_mcount))
 
@@ -69,8 +87,10 @@  weak_alias (_mcount, mcount)
 	.type C_SYMBOL_NAME(__fentry__), @function
 	.align ALIGNARG(4)
 C_LABEL(__fentry__)
-	/* Allocate space for 7 registers.  */
+	cfi_startproc
+	/* Allocate space for 7 registers (+8 for proper stack alignment).  */
 	subq	$64,%rsp
+	cfi_adjust_cfa_offset (64)
 	movq	%rax,(%rsp)
 	movq	%rcx,8(%rsp)
 	movq	%rdx,16(%rsp)
@@ -78,6 +98,13 @@  C_LABEL(__fentry__)
 	movq	%rdi,32(%rsp)
 	movq	%r8,40(%rsp)
 	movq	%r9,48(%rsp)
+	cfi_rel_offset (rax, 0)
+	cfi_rel_offset (rcx, 8)
+	cfi_rel_offset (rdx, 16)
+	cfi_rel_offset (rsi, 24)
+	cfi_rel_offset (rdi, 32)
+	cfi_rel_offset (r8, 40)
+	cfi_rel_offset (r9, 48)
 
 	/* Setup parameter for __mcount_internal.  */
 	/* selfpc is the return address on the stack.  */
@@ -98,7 +125,16 @@  C_LABEL(__fentry__)
 	movq	16(%rsp),%rdx
 	movq	8(%rsp),%rcx
 	movq	(%rsp),%rax
+	cfi_restore (r9)
+	cfi_restore (r8)
+	cfi_restore (rdi)
+	cfi_restore (rsi)
+	cfi_restore (rdx)
+	cfi_restore (rcx)
+	cfi_restore (rax)
 	addq	$64,%rsp
+	cfi_adjust_cfa_offset (-64)
 	ret
+	cfi_endproc
 
 	ASM_SIZE_DIRECTIVE(C_SYMBOL_NAME(__fentry__))