[BZ,#19490] Add unwind descriptors for x86_64 _mcount and __fentry__
Commit Message
On Mon, Feb 22, 2016 at 1:14 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On 23 Jan 2016 14:52, Paul Pluzhnikov wrote:
...
>> + 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 ?
I don't believe we are very consistent here. E.g. in sysdeps/i386/submul_1.S:
ENTRY (__mpn_submul_1)
pushl %edi
cfi_adjust_cfa_offset (4)
pushl %esi
cfi_adjust_cfa_offset (4)
pushl %ebp
cfi_adjust_cfa_offset (4)
pushl %ebx
cfi_adjust_cfa_offset (4)
cfi_rel_offset (edi, 12)
cfi_rel_offset (esi, 8)
cfi_rel_offset (ebp, 4)
cfi_rel_offset (ebx, 0)
But x86_64 does seem to interleave, so I've updated the patch to do the same.
>> .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 ?
No: the difference between _mcount and __fentry__ is that the former
is called after function prologue. That is, on entry into _mcount
(%RSP & 0xF) == 8 (as is usual for x86_64), but on entry into
__fentry__ (%RSP & 0xF) == 0 (which is special).
Note that I am only changing comment here, not the actual alignment.
Thanks,
2016-02-26 Paul Pluzhnikov <ppluzhnikov@google.com>
[BZ #19490]
* sysdeps/x86_64/_mcount.S (_mcount): Add unwind descriptor.
(__fentry__): Likewise
Comments
i'm not an x86_64 maintainer, but patch looks ok to me
-mike
On Thu, Feb 25, 2016 at 7:40 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> i'm not an x86_64 maintainer, but patch looks ok to me
Can I have an approval for this patch?
https://sourceware.org/ml/libc-alpha/2016-02/msg00823.html
Thanks,
On Wed, Mar 2, 2016 at 6:12 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> On Thu, Feb 25, 2016 at 7:40 PM, Mike Frysinger <vapier@gentoo.org> wrote:
>> i'm not an x86_64 maintainer, but patch looks ok to me
>
> Can I have an approval for this patch?
> https://sourceware.org/ml/libc-alpha/2016-02/msg00823.html
Can you also update it to use ENTRY/END?
On Wed, Mar 2, 2016 at 7:44 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> Can you also update it to use ENTRY/END?
In sysdeps/x86_64/sysdep.h:
/* Define an entry point visible from C. */
#define ENTRY(name) \
.globl C_SYMBOL_NAME(name); \
.type C_SYMBOL_NAME(name),@function; \
.align ALIGNARG(4); \
C_LABEL(name) \
cfi_startproc; \
CALL_MCOUNT
Using CALL_MCOUNT in implementation of _mcount isn't going to end well.
I guess I could '#undef PROF', but that seems fragile and likely to
cause further problems.
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.
Andreas.
@@ -28,15 +28,24 @@
.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)
+ 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. */
@@ -51,14 +60,23 @@ 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
+ cfi_endproc
ASM_SIZE_DIRECTIVE(C_SYMBOL_NAME(_mcount))
@@ -69,15 +87,25 @@ 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 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. */
@@ -92,13 +120,22 @@ 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
+ cfi_endproc
ASM_SIZE_DIRECTIVE(C_SYMBOL_NAME(__fentry__))