[PATCH v7 0/1] MR1065: ntdll: Add CFI unwind info to __wine_syscall_dispatcher (x86_64)
This change is adding DWARF (CFI) unwind information to the hand-written assembly of the `__wine_syscall_dispatcher` function. This enables unwinding through the dispatcher from the Linux stack into (and through) the Windows stack. The general idea is that the `syscall_frame` struct contains the content of the callee-save registers before the function call (in particular the stack pointer and the return address). At any point of the execution, we have a pointer into the `syscall_frame` in $rcx, $rbp or $rsp. For the CFI codes the general idea is that we are defining the computations of the callee-save registers based on the `syscall_frame` using DWARF’s `breg` instruction, rather than relative to CFA. This change adds a bunch of convenience macros, to (hopefully) improve readability of the CFI instructions. Note: Those change was used with great success for unwinding through the dispatcher using a modified LLDB shown in the [“how-wine-works-101”](https://werat.dev/blog/how-wine-works-101/) blog post as well as for in the [Orbit profiler](https://github.com/google/orbit), that has mixed-callstack unwinding support. Test: Inspect callstacks reported by the Orbit profiler while running some Windows targets using the modified wine, as well as verify debugging reports correct callstacks when stepping with our modified LLDB through the dispatcher itself (so that we are able to unwind through the dispatcher at any instruction). -- v7: ntdll: Add CFI unwind info to __wine_syscall_dispatcher (x86_64) https://gitlab.winehq.org/wine/wine/-/merge_requests/1065
Okay, while `.cfi_same_value` in the clean-up part technically worked for the tested unwinders, it was actually still wrong to use. The content of the register can be computed by looking at the register itself and not at the value previously computed by the previous frame (which does not exist at that point in time as we are in leaf-code). Regarding the XMM registers. I added the instructions, to be the register content during the execution of the function and being the "same value" as the previous computation (i.e. the callee, i.e. the syscall) during the call. In general, this is all mostly just doing the right thing because it is right. Windows unwinding codes (runtime_functions) must only make use of very few registers for their computation (IIRC, rbp and rsp only). But, as said, let's do it right, which I hope, should be the case now. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1065#note_11267
From: Florian Kübler <kuebler(a)google.com> This change is adding DWARF (CFI) unwind information to the hand-written assembly of the `__wine_syscall_dispatcher` function. This enables unwinding through the dispatcher from the Linux stack into (and through) the Windows stack. The general idea is that the `syscall_frame` struct contains the content of the callee-save registers before the function call (in particular the stack pointer and the return address). At any point of the execution, we have a pointer into the `syscall_frame` in $rcx, $rbp or $rsp. For the CFI codes the general idea is that we are defining the computations of the callee-save registers based on the `syscall_frame` using DWARF’s `breg` instruction, rather than relative to CFA. This change adds a bunch of convenience macros, to (hopefully) improve readability of the CFI instructions. Note: Those change was used with great success for unwinding through the dispatcher using a modified LLDB shown in the “how-wine-works-101” (https://werat.dev/blog/how-wine-works-101/) blog post as well as for in the Orbit profiler (https://github.com/google/orbit), that has mixed-callstack unwinding support. Test: Inspect callstacks reported by the Orbit profiler while running some Windows targets using the modified wine, as well as verify debugging reports correct callstacks when stepping with our modified LLDB through the dispatcher itself (so that we are able to unwind through the dispatcher at any instruction). --- dlls/ntdll/unix/signal_x86_64.c | 486 +++++++++++++++++++------------- 1 file changed, 295 insertions(+), 191 deletions(-) diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 88c517daecf..1b6ace3a877 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -494,196 +494,190 @@ static USHORT fs32_sel; /* selector for %fs in 32-bit mode */ * Definitions for Dwarf unwind tables */ -enum dwarf_call_frame_info -{ - DW_CFA_advance_loc = 0x40, - DW_CFA_offset = 0x80, - DW_CFA_restore = 0xc0, - DW_CFA_nop = 0x00, - DW_CFA_set_loc = 0x01, - DW_CFA_advance_loc1 = 0x02, - DW_CFA_advance_loc2 = 0x03, - DW_CFA_advance_loc4 = 0x04, - DW_CFA_offset_extended = 0x05, - DW_CFA_restore_extended = 0x06, - DW_CFA_undefined = 0x07, - DW_CFA_same_value = 0x08, - DW_CFA_register = 0x09, - DW_CFA_remember_state = 0x0a, - DW_CFA_restore_state = 0x0b, - DW_CFA_def_cfa = 0x0c, - DW_CFA_def_cfa_register = 0x0d, - DW_CFA_def_cfa_offset = 0x0e, - DW_CFA_def_cfa_expression = 0x0f, - DW_CFA_expression = 0x10, - DW_CFA_offset_extended_sf = 0x11, - DW_CFA_def_cfa_sf = 0x12, - DW_CFA_def_cfa_offset_sf = 0x13, - DW_CFA_val_offset = 0x14, - DW_CFA_val_offset_sf = 0x15, - DW_CFA_val_expression = 0x16, -}; - -enum dwarf_operation -{ - DW_OP_addr = 0x03, - DW_OP_deref = 0x06, - DW_OP_const1u = 0x08, - DW_OP_const1s = 0x09, - DW_OP_const2u = 0x0a, - DW_OP_const2s = 0x0b, - DW_OP_const4u = 0x0c, - DW_OP_const4s = 0x0d, - DW_OP_const8u = 0x0e, - DW_OP_const8s = 0x0f, - DW_OP_constu = 0x10, - DW_OP_consts = 0x11, - DW_OP_dup = 0x12, - DW_OP_drop = 0x13, - DW_OP_over = 0x14, - DW_OP_pick = 0x15, - DW_OP_swap = 0x16, - DW_OP_rot = 0x17, - DW_OP_xderef = 0x18, - DW_OP_abs = 0x19, - DW_OP_and = 0x1a, - DW_OP_div = 0x1b, - DW_OP_minus = 0x1c, - DW_OP_mod = 0x1d, - DW_OP_mul = 0x1e, - DW_OP_neg = 0x1f, - DW_OP_not = 0x20, - DW_OP_or = 0x21, - DW_OP_plus = 0x22, - DW_OP_plus_uconst = 0x23, - DW_OP_shl = 0x24, - DW_OP_shr = 0x25, - DW_OP_shra = 0x26, - DW_OP_xor = 0x27, - DW_OP_bra = 0x28, - DW_OP_eq = 0x29, - DW_OP_ge = 0x2a, - DW_OP_gt = 0x2b, - DW_OP_le = 0x2c, - DW_OP_lt = 0x2d, - DW_OP_ne = 0x2e, - DW_OP_skip = 0x2f, - DW_OP_lit0 = 0x30, - DW_OP_lit1 = 0x31, - DW_OP_lit2 = 0x32, - DW_OP_lit3 = 0x33, - DW_OP_lit4 = 0x34, - DW_OP_lit5 = 0x35, - DW_OP_lit6 = 0x36, - DW_OP_lit7 = 0x37, - DW_OP_lit8 = 0x38, - DW_OP_lit9 = 0x39, - DW_OP_lit10 = 0x3a, - DW_OP_lit11 = 0x3b, - DW_OP_lit12 = 0x3c, - DW_OP_lit13 = 0x3d, - DW_OP_lit14 = 0x3e, - DW_OP_lit15 = 0x3f, - DW_OP_lit16 = 0x40, - DW_OP_lit17 = 0x41, - DW_OP_lit18 = 0x42, - DW_OP_lit19 = 0x43, - DW_OP_lit20 = 0x44, - DW_OP_lit21 = 0x45, - DW_OP_lit22 = 0x46, - DW_OP_lit23 = 0x47, - DW_OP_lit24 = 0x48, - DW_OP_lit25 = 0x49, - DW_OP_lit26 = 0x4a, - DW_OP_lit27 = 0x4b, - DW_OP_lit28 = 0x4c, - DW_OP_lit29 = 0x4d, - DW_OP_lit30 = 0x4e, - DW_OP_lit31 = 0x4f, - DW_OP_reg0 = 0x50, - DW_OP_reg1 = 0x51, - DW_OP_reg2 = 0x52, - DW_OP_reg3 = 0x53, - DW_OP_reg4 = 0x54, - DW_OP_reg5 = 0x55, - DW_OP_reg6 = 0x56, - DW_OP_reg7 = 0x57, - DW_OP_reg8 = 0x58, - DW_OP_reg9 = 0x59, - DW_OP_reg10 = 0x5a, - DW_OP_reg11 = 0x5b, - DW_OP_reg12 = 0x5c, - DW_OP_reg13 = 0x5d, - DW_OP_reg14 = 0x5e, - DW_OP_reg15 = 0x5f, - DW_OP_reg16 = 0x60, - DW_OP_reg17 = 0x61, - DW_OP_reg18 = 0x62, - DW_OP_reg19 = 0x63, - DW_OP_reg20 = 0x64, - DW_OP_reg21 = 0x65, - DW_OP_reg22 = 0x66, - DW_OP_reg23 = 0x67, - DW_OP_reg24 = 0x68, - DW_OP_reg25 = 0x69, - DW_OP_reg26 = 0x6a, - DW_OP_reg27 = 0x6b, - DW_OP_reg28 = 0x6c, - DW_OP_reg29 = 0x6d, - DW_OP_reg30 = 0x6e, - DW_OP_reg31 = 0x6f, - DW_OP_breg0 = 0x70, - DW_OP_breg1 = 0x71, - DW_OP_breg2 = 0x72, - DW_OP_breg3 = 0x73, - DW_OP_breg4 = 0x74, - DW_OP_breg5 = 0x75, - DW_OP_breg6 = 0x76, - DW_OP_breg7 = 0x77, - DW_OP_breg8 = 0x78, - DW_OP_breg9 = 0x79, - DW_OP_breg10 = 0x7a, - DW_OP_breg11 = 0x7b, - DW_OP_breg12 = 0x7c, - DW_OP_breg13 = 0x7d, - DW_OP_breg14 = 0x7e, - DW_OP_breg15 = 0x7f, - DW_OP_breg16 = 0x80, - DW_OP_breg17 = 0x81, - DW_OP_breg18 = 0x82, - DW_OP_breg19 = 0x83, - DW_OP_breg20 = 0x84, - DW_OP_breg21 = 0x85, - DW_OP_breg22 = 0x86, - DW_OP_breg23 = 0x87, - DW_OP_breg24 = 0x88, - DW_OP_breg25 = 0x89, - DW_OP_breg26 = 0x8a, - DW_OP_breg27 = 0x8b, - DW_OP_breg28 = 0x8c, - DW_OP_breg29 = 0x8d, - DW_OP_breg30 = 0x8e, - DW_OP_breg31 = 0x8f, - DW_OP_regx = 0x90, - DW_OP_fbreg = 0x91, - DW_OP_bregx = 0x92, - DW_OP_piece = 0x93, - DW_OP_deref_size = 0x94, - DW_OP_xderef_size = 0x95, - DW_OP_nop = 0x96, - DW_OP_push_object_address = 0x97, - DW_OP_call2 = 0x98, - DW_OP_call4 = 0x99, - DW_OP_call_ref = 0x9a, - DW_OP_form_tls_address = 0x9b, - DW_OP_call_frame_cfa = 0x9c, - DW_OP_bit_piece = 0x9d, - DW_OP_lo_user = 0xe0, - DW_OP_hi_user = 0xff, - DW_OP_GNU_push_tls_address = 0xe0, - DW_OP_GNU_uninit = 0xf0, - DW_OP_GNU_encoded_addr = 0xf1, -}; +#define DW_CFA_advance_loc 0x40 +#define DW_CFA_offset 0x80 +#define DW_CFA_restore 0xc0 +#define DW_CFA_nop 0x00 +#define DW_CFA_set_loc 0x01 +#define DW_CFA_advance_loc1 0x02 +#define DW_CFA_advance_loc2 0x03 +#define DW_CFA_advance_loc4 0x04 +#define DW_CFA_offset_extended 0x05 +#define DW_CFA_restore_extended 0x06 +#define DW_CFA_undefined 0x07 +#define DW_CFA_same_value 0x08 +#define DW_CFA_register 0x09 +#define DW_CFA_remember_state 0x0a +#define DW_CFA_restore_state 0x0b +#define DW_CFA_def_cfa 0x0c +#define DW_CFA_def_cfa_register 0x0d +#define DW_CFA_def_cfa_offset 0x0e +#define DW_CFA_def_cfa_expression 0x0f +#define DW_CFA_expression 0x10 +#define DW_CFA_offset_extended_sf 0x11 +#define DW_CFA_def_cfa_sf 0x12 +#define DW_CFA_def_cfa_offset_sf 0x13 +#define DW_CFA_val_offset 0x14 +#define DW_CFA_val_offset_sf 0x15 +#define DW_CFA_val_expression 0x16 + +#define DW_OP_addr 0x03 +#define DW_OP_deref 0x06 +#define DW_OP_const1u 0x08 +#define DW_OP_const1s 0x09 +#define DW_OP_const2u 0x0a +#define DW_OP_const2s 0x0b +#define DW_OP_const4u 0x0c +#define DW_OP_const4s 0x0d +#define DW_OP_const8u 0x0e +#define DW_OP_const8s 0x0f +#define DW_OP_constu 0x10 +#define DW_OP_consts 0x11 +#define DW_OP_dup 0x12 +#define DW_OP_drop 0x13 +#define DW_OP_over 0x14 +#define DW_OP_pick 0x15 +#define DW_OP_swap 0x16 +#define DW_OP_rot 0x17 +#define DW_OP_xderef 0x18 +#define DW_OP_abs 0x19 +#define DW_OP_and 0x1a +#define DW_OP_div 0x1b +#define DW_OP_minus 0x1c +#define DW_OP_mod 0x1d +#define DW_OP_mul 0x1e +#define DW_OP_neg 0x1f +#define DW_OP_not 0x20 +#define DW_OP_or 0x21 +#define DW_OP_plus 0x22 +#define DW_OP_plus_uconst 0x23 +#define DW_OP_shl 0x24 +#define DW_OP_shr 0x25 +#define DW_OP_shra 0x26 +#define DW_OP_xor 0x27 +#define DW_OP_bra 0x28 +#define DW_OP_eq 0x29 +#define DW_OP_ge 0x2a +#define DW_OP_gt 0x2b +#define DW_OP_le 0x2c +#define DW_OP_lt 0x2d +#define DW_OP_ne 0x2e +#define DW_OP_skip 0x2f +#define DW_OP_lit0 0x30 +#define DW_OP_lit1 0x31 +#define DW_OP_lit2 0x32 +#define DW_OP_lit3 0x33 +#define DW_OP_lit4 0x34 +#define DW_OP_lit5 0x35 +#define DW_OP_lit6 0x36 +#define DW_OP_lit7 0x37 +#define DW_OP_lit8 0x38 +#define DW_OP_lit9 0x39 +#define DW_OP_lit10 0x3a +#define DW_OP_lit11 0x3b +#define DW_OP_lit12 0x3c +#define DW_OP_lit13 0x3d +#define DW_OP_lit14 0x3e +#define DW_OP_lit15 0x3f +#define DW_OP_lit16 0x40 +#define DW_OP_lit17 0x41 +#define DW_OP_lit18 0x42 +#define DW_OP_lit19 0x43 +#define DW_OP_lit20 0x44 +#define DW_OP_lit21 0x45 +#define DW_OP_lit22 0x46 +#define DW_OP_lit23 0x47 +#define DW_OP_lit24 0x48 +#define DW_OP_lit25 0x49 +#define DW_OP_lit26 0x4a +#define DW_OP_lit27 0x4b +#define DW_OP_lit28 0x4c +#define DW_OP_lit29 0x4d +#define DW_OP_lit30 0x4e +#define DW_OP_lit31 0x4f +#define DW_OP_reg0 0x50 +#define DW_OP_reg1 0x51 +#define DW_OP_reg2 0x52 +#define DW_OP_reg3 0x53 +#define DW_OP_reg4 0x54 +#define DW_OP_reg5 0x55 +#define DW_OP_reg6 0x56 +#define DW_OP_reg7 0x57 +#define DW_OP_reg8 0x58 +#define DW_OP_reg9 0x59 +#define DW_OP_reg10 0x5a +#define DW_OP_reg11 0x5b +#define DW_OP_reg12 0x5c +#define DW_OP_reg13 0x5d +#define DW_OP_reg14 0x5e +#define DW_OP_reg15 0x5f +#define DW_OP_reg16 0x60 +#define DW_OP_reg17 0x61 +#define DW_OP_reg18 0x62 +#define DW_OP_reg19 0x63 +#define DW_OP_reg20 0x64 +#define DW_OP_reg21 0x65 +#define DW_OP_reg22 0x66 +#define DW_OP_reg23 0x67 +#define DW_OP_reg24 0x68 +#define DW_OP_reg25 0x69 +#define DW_OP_reg26 0x6a +#define DW_OP_reg27 0x6b +#define DW_OP_reg28 0x6c +#define DW_OP_reg29 0x6d +#define DW_OP_reg30 0x6e +#define DW_OP_reg31 0x6f +#define DW_OP_breg0 0x70 +#define DW_OP_breg1 0x71 +#define DW_OP_breg2 0x72 +#define DW_OP_breg3 0x73 +#define DW_OP_breg4 0x74 +#define DW_OP_breg5 0x75 +#define DW_OP_breg6 0x76 +#define DW_OP_breg7 0x77 +#define DW_OP_breg8 0x78 +#define DW_OP_breg9 0x79 +#define DW_OP_breg10 0x7a +#define DW_OP_breg11 0x7b +#define DW_OP_breg12 0x7c +#define DW_OP_breg13 0x7d +#define DW_OP_breg14 0x7e +#define DW_OP_breg15 0x7f +#define DW_OP_breg16 0x80 +#define DW_OP_breg17 0x81 +#define DW_OP_breg18 0x82 +#define DW_OP_breg19 0x83 +#define DW_OP_breg20 0x84 +#define DW_OP_breg21 0x85 +#define DW_OP_breg22 0x86 +#define DW_OP_breg23 0x87 +#define DW_OP_breg24 0x88 +#define DW_OP_breg25 0x89 +#define DW_OP_breg26 0x8a +#define DW_OP_breg27 0x8b +#define DW_OP_breg28 0x8c +#define DW_OP_breg29 0x8d +#define DW_OP_breg30 0x8e +#define DW_OP_breg31 0x8f +#define DW_OP_regx 0x90 +#define DW_OP_fbreg 0x91 +#define DW_OP_bregx 0x92 +#define DW_OP_piece 0x93 +#define DW_OP_deref_size 0x94 +#define DW_OP_xderef_size 0x95 +#define DW_OP_nop 0x96 +#define DW_OP_push_object_address 0x97 +#define DW_OP_call2 0x98 +#define DW_OP_call4 0x99 +#define DW_OP_call_ref 0x9a +#define DW_OP_form_tls_address 0x9b +#define DW_OP_call_frame_cfa 0x9c +#define DW_OP_bit_piece 0x9d +#define DW_OP_lo_user 0xe0 +#define DW_OP_hi_user 0xff +#define DW_OP_GNU_push_tls_address 0xe0 +#define DW_OP_GNU_uninit 0xf0 +#define DW_OP_GNU_encoded_addr 0xf1 #define DW_EH_PE_native 0x00 #define DW_EH_PE_uleb128 0x01 @@ -894,7 +888,7 @@ static void execute_cfa_instructions( const unsigned char *ptr, const unsigned c { while (ptr < end && info->ip < last_ip + info->signal_frame) { - enum dwarf_call_frame_info op = *ptr++; + const unsigned char op = *ptr++; if (op & 0xc0) { @@ -3345,35 +3339,86 @@ __ASM_GLOBAL_FUNC( signal_exit_thread, __ASM_CFI(".cfi_rel_offset %r15,8\n\t") "call *%rsi" ) +#define DW_OP_rcx DW_OP_breg2 +#define DW_OP_rbp DW_OP_breg6 +#define DW_OP_rsp DW_OP_breg7 + +#define DW_REG_rbx 0x03 +#define DW_REG_rsi 0x04 +#define DW_REG_rdi 0x05 +#define DW_REG_rbp 0x06 +#define DW_REG_rsp 0x07 +#define DW_REG_r12 0x0c +#define DW_REG_r13 0x0d +#define DW_REG_r14 0x0e +#define DW_REG_r15 0x0f +#define DW_REG_rip 0x10 + +#define __ASM_CFI_STR(...) #__VA_ARGS__ +#define __ASM_CFI_ESC(...) \ + __ASM_CFI(".cfi_escape " __ASM_CFI_STR(__VA_ARGS__) "\n\t") +#define __ASM_CFI_CFA_IS_AT1(base, offset) \ + __ASM_CFI_ESC(DW_CFA_def_cfa_expression, 0x03, DW_OP_ ## base, offset, DW_OP_deref) +#define __ASM_CFI_REG_IS_AT1(reg, base, offset) \ + __ASM_CFI_ESC(DW_CFA_expression, DW_REG_ ## reg, 0x02, DW_OP_ ## base, offset) +#define __ASM_CFI_CFA_IS_AT2(base, lo, hi) \ + __ASM_CFI_ESC(DW_CFA_def_cfa_expression, 0x04, DW_OP_ ## base, lo, hi, DW_OP_deref) +#define __ASM_CFI_REG_IS_AT2(reg, base, lo, hi) \ + __ASM_CFI_ESC(DW_CFA_expression, DW_REG_ ## reg, 0x03, DW_OP_ ## base, lo, hi) + /*********************************************************************** * __wine_syscall_dispatcher */ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, + __ASM_CFI(".cfi_register xmm6, xmm6\n\t") + __ASM_CFI(".cfi_register xmm7, xmm7\n\t") + __ASM_CFI(".cfi_register xmm8, xmm8\n\t") + __ASM_CFI(".cfi_register xmm9, xmm9\n\t") + __ASM_CFI(".cfi_register xmm10, xmm10\n\t") + __ASM_CFI(".cfi_register xmm11, xmm11\n\t") + __ASM_CFI(".cfi_register xmm12, xmm12\n\t") + __ASM_CFI(".cfi_register xmm13, xmm13\n\t") + __ASM_CFI(".cfi_register xmm14, xmm14\n\t") + __ASM_CFI(".cfi_register xmm15, xmm15\n\t") "movq %gs:0x30,%rcx\n\t" "movq 0x328(%rcx),%rcx\n\t" /* amd64_thread_data()->syscall_frame */ "popq 0x70(%rcx)\n\t" /* frame->rip */ + __ASM_CFI(".cfi_adjust_cfa_offset -8\n\t") + __ASM_CFI_REG_IS_AT2(rip, rcx, 0xf0,0x00) "pushfq\n\t" + __ASM_CFI(".cfi_adjust_cfa_offset 8\n\t") "popq 0x80(%rcx)\n\t" + __ASM_CFI(".cfi_adjust_cfa_offset -8\n\t") "movl $0,0x94(%rcx)\n\t" /* frame->restore_flags */ ".globl " __ASM_NAME("__wine_syscall_dispatcher_prolog_end") "\n" __ASM_NAME("__wine_syscall_dispatcher_prolog_end") ":\n\t" "movq %rax,0x00(%rcx)\n\t" "movq %rbx,0x08(%rcx)\n\t" + __ASM_CFI_REG_IS_AT1(rbx, rcx, 0x08) "movq %rdx,0x18(%rcx)\n\t" "movq %rsi,0x20(%rcx)\n\t" + __ASM_CFI_REG_IS_AT1(rsi, rcx, 0x20) "movq %rdi,0x28(%rcx)\n\t" + __ASM_CFI_REG_IS_AT1(rdi, rcx, 0x28) "movq %r12,0x50(%rcx)\n\t" + __ASM_CFI_REG_IS_AT2(r12, rcx, 0xd0, 0x00) "movq %r13,0x58(%rcx)\n\t" + __ASM_CFI_REG_IS_AT2(r13, rcx, 0xd8, 0x00) "movq %r14,0x60(%rcx)\n\t" + __ASM_CFI_REG_IS_AT2(r14, rcx, 0xe0, 0x00) "movq %r15,0x68(%rcx)\n\t" + __ASM_CFI_REG_IS_AT2(r15, rcx, 0xe8, 0x00) "movw %cs,0x78(%rcx)\n\t" "movw %ds,0x7a(%rcx)\n\t" "movw %es,0x7c(%rcx)\n\t" "movw %fs,0x7e(%rcx)\n\t" "movq %rsp,0x88(%rcx)\n\t" + __ASM_CFI_CFA_IS_AT2(rcx, 0x88, 0x01) + __ASM_CFI_REG_IS_AT2(rsp, rcx, 0x88, 0x01) "movw %ss,0x90(%rcx)\n\t" "movw %gs,0x92(%rcx)\n\t" "movq %rbp,0x98(%rcx)\n\t" + __ASM_CFI_REG_IS_AT2(rbp, rcx, 0x98, 0x01) /* Legends of Runeterra hooks the first system call return instruction, and * depends on us returning to it. Adjust the return address accordingly. */ "subq $0xb,0x70(%rcx)\n\t" @@ -3397,7 +3442,20 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "1:\txsave64 0xc0(%rcx)\n\t" "jmp 3f\n" "2:\tfxsave64 0xc0(%rcx)\n" + /* remember state when $rcx is pointing to "frame" */ + __ASM_CFI(".cfi_remember_state\n\t") "3:\tleaq 0x98(%rcx),%rbp\n\t" + __ASM_CFI_CFA_IS_AT1(rbp, 0x70) + __ASM_CFI_REG_IS_AT1(rsp, rbp, 0x70) + __ASM_CFI_REG_IS_AT1(rip, rbp, 0x58) + __ASM_CFI_REG_IS_AT2(rbx, rbp, 0xf0, 0x7e) + __ASM_CFI_REG_IS_AT2(rsi, rbp, 0x88, 0x7f) + __ASM_CFI_REG_IS_AT2(rdi, rbp, 0x90, 0x7f) + __ASM_CFI_REG_IS_AT2(r12, rbp, 0xb8, 0x7f) + __ASM_CFI_REG_IS_AT1(r13, rbp, 0x40) + __ASM_CFI_REG_IS_AT1(r14, rbp, 0x48) + __ASM_CFI_REG_IS_AT1(r15, rbp, 0x50) + __ASM_CFI_REG_IS_AT1(rbp, rbp, 0x00) #ifdef __linux__ "testl $12,%r14d\n\t" /* SYSCALL_HAVE_PTHREAD_TEB | SYSCALL_HAVE_WRFSGSBASE */ "jz 2f\n\t" @@ -3437,8 +3495,20 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "1:\tmovq %r10,%rcx\n\t" "subq $0x20,%rsp\n\t" "movq (%rbx),%r10\n\t" /* table->ServiceTable */ + __ASM_CFI(".cfi_same_value xmm6\n\t") + __ASM_CFI(".cfi_same_value xmm7\n\t") + __ASM_CFI(".cfi_same_value xmm8\n\t") + __ASM_CFI(".cfi_same_value xmm9\n\t") + __ASM_CFI(".cfi_same_value xmm10\n\t") + __ASM_CFI(".cfi_same_value xmm11\n\t") + __ASM_CFI(".cfi_same_value xmm12\n\t") + __ASM_CFI(".cfi_same_value xmm13\n\t") + __ASM_CFI(".cfi_same_value xmm14\n\t") + __ASM_CFI(".cfi_same_value xmm15\n\t") "callq *(%r10,%rax,8)\n\t" "leaq -0x98(%rbp),%rcx\n" + /* $rcx is now pointing to "frame" again */ + __ASM_CFI(".cfi_restore_state\n\t") "2:\tmovl 0x94(%rcx),%edx\n\t" /* frame->restore_flags */ #ifdef __linux__ "testl $12,%r14d\n\t" /* SYSCALL_HAVE_PTHREAD_TEB | SYSCALL_HAVE_WRFSGSBASE */ @@ -3459,27 +3529,48 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "jmp 4f\n" "3:\tfxrstor64 0xc0(%rcx)\n" "4:\tmovq 0x98(%rcx),%rbp\n\t" + __ASM_CFI(".cfi_register rbp, rbp\n\t") "movq 0x68(%rcx),%r15\n\t" + __ASM_CFI(".cfi_register r15, r15\n\t") "movq 0x60(%rcx),%r14\n\t" + __ASM_CFI(".cfi_register r14, r14\n\t") "movq 0x58(%rcx),%r13\n\t" + __ASM_CFI(".cfi_register r13, r13\n\t") "movq 0x50(%rcx),%r12\n\t" + __ASM_CFI(".cfi_register r12, r12\n\t") "movq 0x28(%rcx),%rdi\n\t" + __ASM_CFI(".cfi_register rdi, rdi\n\t") "movq 0x20(%rcx),%rsi\n\t" + __ASM_CFI(".cfi_register rsi, rsi\n\t") "movq 0x08(%rcx),%rbx\n\t" + __ASM_CFI(".cfi_register rbx, rbx\n\t") "testl $0x3,%edx\n\t" /* CONTEXT_CONTROL | CONTEXT_INTEGER */ "jnz 1f\n\t" + __ASM_CFI(".cfi_remember_state\n\t") "movq 0x80(%rcx),%r11\n\t" /* frame->eflags */ "pushq %r11\n\t" "popfq\n\t" "movq 0x88(%rcx),%rsp\n\t" + __ASM_CFI(".cfi_def_cfa rsp, 0\n\t") + __ASM_CFI(".cfi_register rsp, rsp\n\t") "movq 0x70(%rcx),%rcx\n\t" /* frame->rip */ + __ASM_CFI(".cfi_register rip, rcx\n\t") "jmpq *%rcx\n\t" + /* $rcx is now pointing to "frame" again */ + __ASM_CFI(".cfi_restore_state\n\t") + /* remember state when $rcx is pointing to "frame" */ + __ASM_CFI(".cfi_remember_state\n\t") "1:\tleaq 0x70(%rcx),%rsp\n\t" + __ASM_CFI_CFA_IS_AT1(rsp, 0x18) + __ASM_CFI_REG_IS_AT1(rsp, rsp, 0x18) + __ASM_CFI_REG_IS_AT1(rip, rsp, 0x00) "testl $0x2,%edx\n\t" /* CONTEXT_INTEGER */ "jnz 1f\n\t" "movq 0x10(%rsp),%r11\n\t" /* frame->eflags */ "movq (%rsp),%rcx\n\t" /* frame->rip */ + __ASM_CFI(".cfi_register rip, rcx\n\t") "iretq\n" + __ASM_CFI_REG_IS_AT1(rip, rsp, 0x00) "1:\tmovq 0x00(%rcx),%rax\n\t" "movq 0x18(%rcx),%rdx\n\t" "movq 0x30(%rcx),%r8\n\t" @@ -3488,8 +3579,21 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movq 0x48(%rcx),%r11\n\t" "movq 0x10(%rcx),%rcx\n" "iretq\n" + __ASM_CFI_CFA_IS_AT1(rbp, 0x70) + __ASM_CFI_REG_IS_AT1(rsp, rbp, 0x70) + __ASM_CFI_REG_IS_AT1(rip, rbp, 0x58) + __ASM_CFI_REG_IS_AT2(rbx, rbp, 0xf0, 0x7e) + __ASM_CFI_REG_IS_AT2(rsi, rbp, 0x88, 0x7f) + __ASM_CFI_REG_IS_AT2(rdi, rbp, 0x90, 0x7f) + __ASM_CFI_REG_IS_AT2(r12, rbp, 0xb8, 0x7f) + __ASM_CFI_REG_IS_AT1(r13, rbp, 0x40) + __ASM_CFI_REG_IS_AT1(r14, rbp, 0x48) + __ASM_CFI_REG_IS_AT1(r15, rbp, 0x50) + __ASM_CFI_REG_IS_AT1(rbp, rbp, 0x00) "5:\tmovl $0xc000000d,%edx\n\t" /* STATUS_INVALID_PARAMETER */ "movq %rsp,%rcx\n\t" + /* $rcx is now pointing to "frame" again */ + __ASM_CFI(".cfi_restore_state\n\t") ".globl " __ASM_NAME("__wine_syscall_dispatcher_return") "\n" __ASM_NAME("__wine_syscall_dispatcher_return") ":\n\t" "movl 0xb0(%rcx),%r14d\n\t" /* frame->syscall_flags */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1065
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=125127 Your paranoid android. === debian11 (32 bit report) === ddraw: ddraw7.c:15663: Test failed: Expected unsynchronised map for flags 0x1000. ddraw7.c:15663: Test failed: Expected unsynchronised map for flags 0x3000. === debian11 (build log) === Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24872. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24872. Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24872.
Also, while I *might* find time to look into i386, I think this should be a different merge request. This change is already beneficial on its own. Also, I will be out for two weeks soon, and will likely not be able to finish that beforehand. So having the first part in, would be a nice win, I guess. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1065#note_11268
Okay, while `.cfi_same_value` in the clean-up part technically worked for the tested unwinders, it was actually still wrong to use. The content of the register can be computed by looking at the register itself and not at the value previously computed by the previous frame (which does not exist at that point in time as we are in leaf-code).
I'm not completely sure to follow you here. Doesn't `.cfi_same_value x` mean "at this point the value of the register is the same as the returning frame, and doesn't need restoration (ie: it has been restored already)". Isn't that, in effect, the exact same thing as `.cfi_register x, x`, even though it's encoded differently?
Regarding the XMM registers. I added the instructions, to be the register content during the execution of the function and being the "same value" as the previous computation (i.e. the callee, i.e. the syscall) during the call.
I was mostly asking out of curiosity, maybe it's not worth adding them. Now, the `.cfi_register xmmX, xmmX` at the top of the procedure seem a bit weird to me. Isn't that the default already, do we need to make it explicit? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1065#note_11288
On Wed Oct 19 09:46:05 2022 +0000, Florian Kübler wrote:
Also, while I *might* find time to look into i386, I think this should be a different merge request. This change is already beneficial on its own. Also, I will be out for two weeks soon, and will likely not be able to finish that beforehand. So having the first part in, would be a nice win, I guess. Sure, and I'd approve that MR with only x86_64 anyway, I was only trying to squeeze a bit more juice :) Thanks already for spending time on that.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1065#note_11289
On Wed Oct 19 09:44:30 2022 +0000, Rémi Bernon wrote:
Okay, while `.cfi_same_value` in the clean-up part technically worked for the tested unwinders, it was actually still wrong to use. The content of the register can be computed by looking at the register itself and not at the value previously computed by the previous frame (which does not exist at that point in time as we are in leaf-code). I'm not completely sure to follow you here. Doesn't `.cfi_same_value x` mean "at this point the value of the register is the same as the returning frame, and doesn't need restoration (ie: it has been restored already)". Isn't that, in effect, the exact same thing as `.cfi_register x, x`, even though it's encoded differently? Regarding the XMM registers. I added the instructions, to be the register content during the execution of the function and being the "same value" as the previous computation (i.e. the callee, i.e. the syscall) during the call. I was mostly asking out of curiosity, maybe it's not worth adding them. Now, the `.cfi_register xmmX, xmmX` at the top of the procedure seem a bit weird to me. Isn't that the default already, do we need to make it explicit? Looking at GDB source I believe my understanding is correct, and so `.cfi_same_value` was right (and as we're using it already elsewhere, probably better):
```c case DWARF2_FRAME_REG_SAVED_REG: realnum = dwarf_reg_to_regnum_or_error (gdbarch, cache->reg[regnum].loc.reg); return frame_unwind_got_register (this_frame, regnum, realnum); /* ... */ case DWARF2_FRAME_REG_UNSPECIFIED: /* GCC, in its infinite wisdom decided to not provide unwind information for registers that are "same value". Since DWARF2 (3 draft 7) doesn't define such behavior, said registers are actually undefined (which is different to CFI "undefined"). Code above issues a complaint about this. Here just fudge the books, assume GCC, and that the value is more inner on the stack. */ return frame_unwind_got_register (this_frame, regnum, regnum); case DWARF2_FRAME_REG_SAME_VALUE: return frame_unwind_got_register (this_frame, regnum, regnum); ``` As you can see, it implements `.cfi_same_value x` the same as `.cfi_register x, x` here. It also assumes unspecified registers have a default of `.cfi_same_value` too, so I'd say it's not required to specify that for xmm register at the top of the function. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1065#note_11372
participants (3)
-
Florian Kübler -
Marvin -
Rémi Bernon