Merge pull request #33 from Amanieu/fix-labels

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

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",

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",

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",

src/arch/x86_windows.rs

@@ -282,13 +282,38 @@ 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" {
     fn stack_init_trampoline(arg: EncodedValue, stack_base: StackPointer, stack_ptr: StackPointer);
     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.
@@ -411,7 +436,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 +445,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 +504,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 +538,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 +635,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 +669,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",
@@ -753,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,
 }
@@ -776,10 +802,18 @@ pub unsafe fn setup_trap_trampoline<T>(
     // 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,
@@ -824,17 +858,3 @@ pub unsafe fn on_stack<S: 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));
-}

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);
@@ -641,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 {

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");
         })
     }));

src/trap.rs

@@ -80,12 +80,6 @@ impl<Return> CoroutineTrapHandler<Return> {
                 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)]