This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Unwinding CFI for x86_64 signal frame (__restore_rt)
On Wed, Nov 29, 2006 at 12:04:10AM +0100, Jan Kratochvil wrote:
> 2006-11-28 Daniel Jacobowitz <dan@codesourcery.com>
> Jakub Jelinek <jakub@redhat.com>
> Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * sysdeps/unix/sysv/linux/x86_64/sigaction.c (restore_rt): Add correct
> unwind information.
> * sysdeps/unix/sysv/linux/x86_64/Makefile: Prvide symbols for
> `restore_rt' even in the `signal' directory.
> * sysdeps/unix/sysv/linux/x86_64/ucontext_i.sym: Extend the regs list.
Looks good, only minor nits (below).
Not sure, but perhaps it would be more readable if the .eh_frame asm
wasn't in the macro, but just an extra asm () below the RESTORE.
There is just one RESTORE, you'd just add the nop and .LSTART_restore_rt
and .LEND_restore_rt symbols to the RESTORE macro, and you could get rid of
all the \'s at the end of the lines, #name uglification etc.
Perhaps the RESTORE macro could be dropped altogether, all we need
is something like CFI_STRINGIFY which would be defined unconditionally
and use it for the syscall number.
Guess Ulrich should decide how he wants it to look like.
> +asm \
> + ( \
> + /* `nop' for debuggers assuming `call' should not disalign the code. */ \
> + " nop\n" \
> + ".align 16\n" \
> + ".LSTART_" #name ":\n" \
> + " nop\n" \
This nop is unneeded.
> + " .type __" #name ",@function\n" \
> + "__" #name ":\n" \
> + " movq $" #syscall ", %rax\n" \
> + " syscall\n" \
> + ".LEND_" #name ":\n" \
> + ".section .eh_frame,\"a\",@progbits\n" \
> + ".LSTARTFRAME_" #name ":\n" \
> + " .long .LENDCIE_" #name "-.LSTARTCIE_" #name "\n" \
> + ".LSTARTCIE_" #name ":\n" \
> + " .long 0\n" /* CIE ID */ \
> + " .byte 1\n" /* Version number */ \
> + " .string \"zRS\"\n" /* NUL-terminated augmentation string */ \
> + " .uleb128 1\n" /* Code alignment factor */ \
> + " .sleb128 -8\n" /* Data alignment factor */ \
> + " .byte 0x10\n" /* Return address register column */ \
> + " .uleb128 1\n" /* Augmentation value length */ \
> + " .byte 0x1b\n" /* DW_EH_PE_pcrel|DW_EH_PE_sdata4. */ \
> + " .byte 0\n" /* DW_CFA_nop */ \
Why is this DW_CFA_nop needed? No insns are correct initial CIE
insns as well (and, .align 8 adds some anyway).
> + " .align 8\n" \
> + ".LENDCIE_" #name ":\n" \
> + " .long .LENDFDE_" #name "-.LSTARTFDE_" #name "\n" /* FDE len */ \
> + ".LSTARTFDE_" #name ":\n" \
> + " .long .LSTARTFDE_" #name "-.LSTARTFRAME_" #name "\n" /* CIE */ \
> + /* `LSTART_' is subtracted 1 as debuggers assume a `call' here. */ \
> + " .long (.LSTART_" #name "-1)-.\n" /* PC-relative start addr. */ \
> + " .long .LEND_" #name "-(.LSTART_" #name "-1)\n" \
> + " .uleb128 0\n" /* Augmentation */ \
> + do_cfa_expr \
> + do_expr (8 /* r8 */, oR8) \
> + do_expr (9 /* r9 */, oR9) \
> + do_expr (10 /* r10 */, oR10) \
> + do_expr (11 /* r11 */, oR11) \
> + do_expr (12 /* r12 */, oR12) \
> + do_expr (13 /* r13 */, oR13) \
> + do_expr (14 /* r14 */, oR14) \
> + do_expr (15 /* r15 */, oR15) \
> + do_expr (5 /* rdi */, oRDI) \
> + do_expr (4 /* rsi */, oRSI) \
> + do_expr (6 /* rbp */, oRBP) \
> + do_expr (3 /* rbx */, oRBX) \
> + do_expr (1 /* rdx */, oRDX) \
> + do_expr (0 /* rax */, oRAX) \
> + do_expr (2 /* rcx */, oRCX) \
> + do_expr (7 /* rsp */, oRSP) \
> + do_expr (16 /* rip */, oRIP) \
> + /* Older gas(1) does not support `rflags'. */ \
This comment is not needed when .eh_frame is hand written.
> + do_expr (49 /* rflags */, oEFL) \
> + /* `cs'/`ds'/`fs' are unaligned and a different size. */ \
> + /* gas: Error: register save offset not a multiple of 8 */ \
> + " .align 4\n" \
You want .align 8 here.
Jakub