From ede5fce07daf479c97050a8de4dd328e3959dff0 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Wed, 18 Sep 2024 10:34:44 +0100 Subject: [PATCH 1/4] Fix inline assembly labels on x86 This avoids errors due to a new lint added to rustc. --- src/arch/x86.rs | 10 +++++----- src/arch/x86_64.rs | 10 +++++----- src/arch/x86_64_windows.rs | 14 +++++++------- src/arch/x86_windows.rs | 11 +++++------ 4 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/arch/x86.rs b/src/arch/x86.rs index 87f3c64..234db4e 100644 --- a/src/arch/x86.rs +++ b/src/arch/x86.rs @@ -236,7 +236,7 @@ pub unsafe fn switch_and_link( // Push a return address onto our stack and then jump to the return // address at the top of the coroutine stack. // - // From here on execution continues in stack_init_trampoline or the 0: + // From here on execution continues in stack_init_trampoline or the 3: // label in switch_yield. "call [eax]", @@ -291,7 +291,7 @@ pub unsafe fn switch_yield(arg: EncodedValue, parent_link: *mut StackPointer) -> // point to "0". We use an intermediate constant here to work around a // limitation of LLVM's Intel syntax parser which doesn't support 2 // symbols in an expression. - ".equ .Loffset_yield, 0f - 2b", + ".equ .Loffset_yield, 3f - 2b", "add dword ptr [esp], offset .Loffset_yield", // Save our stack pointer to EDX, which is then returned out of @@ -325,7 +325,7 @@ pub unsafe fn switch_yield(arg: EncodedValue, parent_link: *mut StackPointer) -> // - EAX points to the top of our stack. // - EDX points to the base of our stack. // - ECX contains the argument passed from switch_and_link. - "0:", + "3:", // Save the EBP of the parent context to the parent stack. "push ebp", @@ -402,7 +402,7 @@ pub unsafe fn switch_and_throw( // about how this code works. "call 2f", "2:", - ".equ .Loffset_throw, 0f - 2b", + ".equ .Loffset_throw, 3f - 2b", "add dword ptr [esp], offset .Loffset_throw", // Save EBP of the parent context. @@ -437,7 +437,7 @@ pub unsafe fn switch_and_throw( // Upon returning, our register state is just like a normal return into // switch_and_link(). - "0:", + "3:", // Restore registers just like the second half of switch_and_link. "pop esi", diff --git a/src/arch/x86_64.rs b/src/arch/x86_64.rs index ca7f696..fa08a50 100644 --- a/src/arch/x86_64.rs +++ b/src/arch/x86_64.rs @@ -345,7 +345,7 @@ pub unsafe fn switch_and_link( // Push a return address onto our stack and then jump to the return // address at the top of the coroutine stack. // - // From here on execution continues in stack_init_trampoline or the 0: + // From here on execution continues in stack_init_trampoline or the 2: // label in switch_yield. "call [rdx]", @@ -423,7 +423,7 @@ pub unsafe fn switch_yield(arg: EncodedValue, parent_link: *mut StackPointer) -> // Push a return address on the stack. This is the address that will be // called by switch_and_link() the next time this context is resumed. - "lea rax, [rip + 0f]", + "lea rax, [rip + 2f]", "push rax", // Save our stack pointer to RSI, which is then returned out of @@ -458,7 +458,7 @@ pub unsafe fn switch_yield(arg: EncodedValue, parent_link: *mut StackPointer) -> // - RDX points to the top of our stack, including the return address. // - RSI points to the base of our stack. // - RDI contains the argument passed from switch_and_link. - "0:", + "2:", // Save the RBP of the parent context to the parent stack. When combined // with the return address this forms a valid frame record (RBP & RIP) @@ -554,7 +554,7 @@ pub unsafe fn switch_and_throw( "push rbx", // Push a return address to the stack. - "lea rax, [rip + 0f]", + "lea rax, [rip + 2f]", "push rax", // Save RBP of the parent context. @@ -589,7 +589,7 @@ pub unsafe fn switch_and_throw( // Upon returning, our register state is just like a normal return into // switch_and_link(). - "0:", + "2:", // Restore registers just like the second half of switch_and_link. "pop rbx", diff --git a/src/arch/x86_64_windows.rs b/src/arch/x86_64_windows.rs index 8cda824..53d4226 100644 --- a/src/arch/x86_64_windows.rs +++ b/src/arch/x86_64_windows.rs @@ -330,7 +330,7 @@ pub unsafe fn switch_and_link( asm_may_unwind_root!( // Set up a secondary copy of the return address. This is only used by // the unwinder, not by actual returns. - "lea rax, [rip + 0f]", + "lea rax, [rip + 2f]", "push rax", // Save the TEB fields to the stack. @@ -345,7 +345,7 @@ pub unsafe fn switch_and_link( // Push a return address onto our stack and then jump to the return // address at the top of the coroutine stack. // - // From here on execution continues in stack_init_trampoline or the 0: + // From here on execution continues in stack_init_trampoline or the 2: // label in switch_yield. "call [rdx]", @@ -354,7 +354,7 @@ pub unsafe fn switch_and_link( // - RSI: The top of the coroutine stack, or 0 if coming from // switch_and_reset. // - RDI: The argument passed from the coroutine. - "0:", + "2:", "pop rbx", @@ -405,7 +405,7 @@ pub unsafe fn switch_yield(arg: EncodedValue, parent_link: *mut StackPointer) -> // Push a return address on the stack. This is the address that will be // called by switch_and_link() the next time this context is resumed. - "lea rax, [rip + 0f]", + "lea rax, [rip + 2f]", "push rax", // Save our stack pointer to RSI, which is then returned out of @@ -429,7 +429,7 @@ pub unsafe fn switch_yield(arg: EncodedValue, parent_link: *mut StackPointer) -> // - RDX points to the top of our stack, including the return address. // - RSI points to the base of our stack. // - RDI contains the argument passed from switch_and_link. - "0:", + "2:", // Save RBP from the parent context last to create a valid frame record. "push rbp", @@ -513,7 +513,7 @@ pub unsafe fn switch_and_throw( asm_may_unwind_root!( // Save state just like the first half of switch_and_link(). - "lea rax, [rip + 0f]", + "lea rax, [rip + 2f]", "push rax", "push qword ptr gs:[0x1748]", // GuaranteedStackBytes "push qword ptr gs:[0x1478]", // DeallocationStack @@ -556,7 +556,7 @@ pub unsafe fn switch_and_throw( // Upon returning, our register state is just like a normal return into // switch_and_link(). - "0:", + "2", // This is copied from the second half of switch_and_link(). "pop rbx", diff --git a/src/arch/x86_windows.rs b/src/arch/x86_windows.rs index e5d35b4..fc736d6 100644 --- a/src/arch/x86_windows.rs +++ b/src/arch/x86_windows.rs @@ -411,7 +411,7 @@ pub unsafe fn switch_and_link( // Push a return address onto our stack and then jump to the return // address at the top of the coroutine stack. // - // From here on execution continues in stack_init_trampoline or the 0: + // From here on execution continues in stack_init_trampoline or the 3: // label in switch_yield. "call [eax]", @@ -420,7 +420,6 @@ pub unsafe fn switch_and_link( // - EDX: The top of the coroutine stack, or 0 if coming from // switch_and_reset. // - ECX: The argument passed from the coroutine. - "0:", "pop esi", @@ -480,7 +479,7 @@ pub unsafe fn switch_yield(arg: EncodedValue, parent_link: *mut StackPointer) -> // point to "0". We use an intermediate constant here to work around a // limitation of LLVM's Intel syntax parser which doesn't support 2 // symbols in an expression. - ".equ .Loffset_yield, 0f - 2b", + ".equ .Loffset_yield, 3f - 2b", "add dword ptr [esp], offset .Loffset_yield", // Save our stack pointer to EDX, which is then returned out of @@ -514,7 +513,7 @@ pub unsafe fn switch_yield(arg: EncodedValue, parent_link: *mut StackPointer) -> // - EAX points to the top of our stack. // - EDX points to the base of our stack. // - ECX contains the argument passed from switch_and_link. - "0:", + "3:", // Save the EBP of the parent context to the parent stack. "push ebp", @@ -611,7 +610,7 @@ pub unsafe fn switch_and_throw( // about how this code works. "call 2f", "2:", - ".equ .Loffset_throw, 0f - 2b", + ".equ .Loffset_throw, 3f - 2b", "add dword ptr [esp], offset .Loffset_throw", // Save EBP of the parent context. @@ -645,7 +644,7 @@ pub unsafe fn switch_and_throw( // Upon returning, our register state is just like a normal return into // switch_and_link(). - "0:", + "3:", // This is copied from the second half of switch_and_link(). "pop esi", From 714fac369d310df654f7df142331c8710f07c565 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Wed, 18 Sep 2024 10:50:18 +0100 Subject: [PATCH 2/4] Fix warnings in tests --- src/tests/coroutine.rs | 5 +++-- src/tests/on_stack.rs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/tests/coroutine.rs b/src/tests/coroutine.rs index 0059d1f..99dac0d 100644 --- a/src/tests/coroutine.rs +++ b/src/tests/coroutine.rs @@ -95,7 +95,7 @@ fn panics_propagated() { let a = Rc::new(Cell::new(false)); let b = SetOnDrop(a.clone()); let mut coroutine = Coroutine::<(), (), ()>::new(move |_, ()| { - drop(&b); + drop(b); panic!("foobar"); }); let result = panic::catch_unwind(AssertUnwindSafe(|| coroutine.resume(()))); @@ -121,7 +121,7 @@ fn panics_propagated_via_parent() { let a = Rc::new(Cell::new(false)); let b = SetOnDrop(a.clone()); let mut coroutine = Coroutine::<(), (), ()>::new(move |y, ()| { - drop(&b); + drop(b); y.on_parent_stack(|| { panic!("foobar"); }); @@ -157,6 +157,7 @@ fn suspend_and_resume_values() { #[test] fn stateful() { #[repr(align(128))] + #[allow(dead_code)] struct Aligned(u8); let state = [41, 42, 43, 44, 45]; let aligned = Aligned(100); diff --git a/src/tests/on_stack.rs b/src/tests/on_stack.rs index 6936b47..15cd2f4 100644 --- a/src/tests/on_stack.rs +++ b/src/tests/on_stack.rs @@ -59,7 +59,7 @@ fn panics_propagated() { let b = SetOnDrop(a.clone()); let result = panic::catch_unwind(AssertUnwindSafe(move || { on_stack(DefaultStack::default(), move || { - drop(&b); + drop(b); panic!("foobar"); }) })); From 82b9581c35c30d9e913798841dd3889bf0396df2 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Fri, 4 Oct 2024 22:33:16 +0200 Subject: [PATCH 3/4] x86 Windows: reset SEH chain before any Rust code is executed This was previously causing issues when the exception catcher was inlined into the root handler function. --- src/arch/x86_windows.rs | 51 +++++++++++++++++++++++++++++------------ src/tests/coroutine.rs | 4 +++- src/trap.rs | 6 ----- 3 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/arch/x86_windows.rs b/src/arch/x86_windows.rs index fc736d6..fc120ac 100644 --- a/src/arch/x86_windows.rs +++ b/src/arch/x86_windows.rs @@ -282,6 +282,30 @@ global_asm!( asm_function_end!("stack_call_trampoline"), ); +// Special trampoline to reset the SEH exception chain before calling +// trap_handler. +global_asm!( + // See stack_init_trampoline for an explanation of the assembler directives + // used here. + ".balign 16", + asm_function_begin!("trap_handler_trampoline"), + // At this point our register state contains the following: + // - ESP points to the coroutine stack and holds a return address pointing + // to stack_init_trampoline_return. + // - EBP points to parent_link on the coroutine stack, forming a valid frame + // pointer chain. + // - EAX points to the initial SEH exception chain entry. + // - EBX holds the address of the trap_handler function. + // - EDX and ECX hold the arguments to trap_handler. + // + // Reset the SEH exception chain to just the initial entry pointing to + // FinalExceptionHandler. + "mov fs:[0x0], eax", + // Jump to trap_handler. + "jmp ebx", + asm_function_end!("trap_handler_trampoline"), +); + // These trampolines use a custom calling convention and should only be called // with inline assembly. extern "C" { @@ -289,6 +313,7 @@ extern "C" { fn stack_init_trampoline_return(); #[allow(dead_code)] fn stack_call_trampoline(arg: *mut u8, sp: StackPointer, f: StackCallFunc); + fn trap_handler_trampoline(); } /// The end of the exception handler chain is marked with 0xffffffff. @@ -752,6 +777,8 @@ pub struct TrapHandlerRegs { pub eip: u32, pub esp: u32, pub ebp: u32, + pub eax: u32, + pub ebx: u32, pub ecx: u32, pub edx: u32, } @@ -775,10 +802,18 @@ pub unsafe fn setup_trap_trampoline( // Set up a return address which returns to stack_init_trampoline. push(&mut sp, Some(stack_init_trampoline_return as StackWord)); + // Since we reset the stack offset back to the base of the coroutine stack, + // we also need to reset the SEH ExceptionList chain in the TIB to just the + // initial FinalExceptionHandler. This is done by pointing to a small + // trampoline that runs before any other code is executed. + let initial_seh_handler = parent_link - 8; + // Set up registers for entry into the function. TrapHandlerRegs { - eip: handler as u32, + eip: trap_handler_trampoline as u32, esp: sp as u32, + eax: initial_seh_handler as u32, + ebx: handler as u32, ecx: val_ptr as u32, edx: parent_link as u32, ebp: parent_link as u32, @@ -823,17 +858,3 @@ pub unsafe fn on_stack(arg: *mut u8, stack: S, f: StackCallFunc) { clobber_abi("fastcall"), ); } - -/// The trap handler will have reset our stack offset back to the base of the -/// coroutine stack, but it won't have reset the SEH ExceptionList chain in the -/// TIB. We need to manually reset it here before executing any user code which -/// might raise an exception. -pub unsafe fn reset_seh_handler(parent_link: *mut StackPointer) { - // The initial exception record is conveniently located just below the - // parent link. - let exception_record = parent_link as usize - 8; - - // Write to the ExceptionList field in the TIB, just like on entry to the - // coroutine. - asm!("mov fs:[0x0], {}", in(reg) exception_record, options(nostack, preserves_flags)); -} diff --git a/src/tests/coroutine.rs b/src/tests/coroutine.rs index 99dac0d..16d14f1 100644 --- a/src/tests/coroutine.rs +++ b/src/tests/coroutine.rs @@ -642,10 +642,12 @@ mod trap_handler { (*(*exception_info).ContextRecord).Rdi = rdi; (*(*exception_info).ContextRecord).Rsi = rsi; } else if #[cfg(target_arch = "x86")] { - let TrapHandlerRegs { eip, esp, ebp, ecx, edx } = regs; + let TrapHandlerRegs { eip, esp, ebp,eax, ebx, ecx, edx } = regs; (*(*exception_info).ContextRecord).Eip = eip; (*(*exception_info).ContextRecord).Esp = esp; (*(*exception_info).ContextRecord).Ebp = ebp; + (*(*exception_info).ContextRecord).Eax = eax; + (*(*exception_info).ContextRecord).Ebx = ebx; (*(*exception_info).ContextRecord).Ecx = ecx; (*(*exception_info).ContextRecord).Edx = edx; } else { diff --git a/src/trap.rs b/src/trap.rs index b4eaf47..ead9988 100644 --- a/src/trap.rs +++ b/src/trap.rs @@ -80,12 +80,6 @@ impl CoroutineTrapHandler { f: *mut F, parent_link: *mut StackPointer ) -> ! { - // After returning from the exception handler we may have an - // invalid SEH exception chain. We need to reset it to the - // exception record at the root of the stack. - #[cfg(all(windows, target_arch = "x86"))] - arch::reset_seh_handler(parent_link); - // This must be called after a stack overflow exception, but it // doesn't hurt to call it for other exception types as well. #[cfg(windows)] From d1d9d4676ff57d387392fd971ec12db696b91b19 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Fri, 4 Oct 2024 22:38:54 +0200 Subject: [PATCH 4/4] Disable asm-unwind in CI This is currently broken due to LLVM generating incorrect code. --- ci/run.sh | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ci/run.sh b/ci/run.sh index afa3ffd..9984059 100755 --- a/ci/run.sh +++ b/ci/run.sh @@ -32,7 +32,8 @@ export RUST_TEST_THREADS=1 "${CARGO}" test $CARGO_TEST_FLAGS --target "${TARGET}" --all-targets --release # asm-unwind -if [ "${CHANNEL}" = "nightly" ]; then - "${CARGO}" test $CARGO_TEST_FLAGS --target "${TARGET}" --all-targets --features asm-unwind - "${CARGO}" test $CARGO_TEST_FLAGS --target "${TARGET}" --all-targets --features asm-unwind --release -fi +# Currently disabled because of LLVM issues. +#if [ "${CHANNEL}" = "nightly" ]; then +# "${CARGO}" test $CARGO_TEST_FLAGS --target "${TARGET}" --all-targets --features asm-unwind +# "${CARGO}" test $CARGO_TEST_FLAGS --target "${TARGET}" --all-targets --features asm-unwind --release +#fi