Implement task unwinding on stack overflow #9834

alexcrichton · 2013-10-13T05:20:40Z

This commit re-introduces the functionality of __morestack in a way that it was
not originally anticipated. Rust does not currently have segmented stacks,
rather just large stack segments. We do not detect when these stack segments are
overrun currently, but this commit leverages __morestack in order to check this.

This commit purges a lot of the old __morestack and stack limit C++
functionality, migrating the necessary chunks to rust. The stack limit is now
entirely maintained in rust, and the "main logic bits" of __morestack are now
also implemented in rust as well.

I put my best effort into validating that this currently builds and runs successfully on osx and linux 32/64 bit, but I was unable to get this working on windows. We never did have unwinding through __morestack frames, and although I tried poking at it for a bit, I was unable to understand why we don't get unwinding right now.

A focus of this commit is to implement as much of the logic in rust as possible. This involved some liberal usage of no_split_stack in various locations, along with some use of the asm! macro (scary). I modified a bit of C++ to stop calling record_sp_limit because this is no longer defined in C++, rather in rust.

Another consequence of this commit is that thread_local_storage::{get, set} must both be flagged with #[rust_stack]. I've briefly looked at the implementations on osx/linux/windows to ensure that they're pretty small stacks, and I'm pretty sure that they're definitely less than 20K stacks, so we probably don't have a lot to worry about.

Other things worthy of note:

The default stack size is now 4MB instead of 2MB. This is so that when we request 2MB to call a C function you don't immediately overflow because you have consumed any stack at all.
asm! is actually pretty cool, maybe we could actually define context switching with it?
I wanted to add links to the internet about all this jazz of storing information in TLS, but I was only able to find a link for the windows implementation. Otherwise my suggestion is just "disassemble on that arch and see what happens"
I put my best effort forward on arm/mips to tweak __morestack correctly, we have no ability to test this so an extra set of eyes would be useful on these spots.
This is all really tricky stuff, so I tried to put as many comments as I thought were necessary, but if anything is still unclear (or I completely forgot to take something into account), I'm willing to write more!

huonw · 2013-10-13T07:03:10Z

src/libstd/rt/context.rs

@@ -125,6 +182,8 @@ fn initialize_call_frame(regs: &mut Registers, fptr: *c_void, arg: *c_void,
    regs.ebp = 0;
 }

+// windows requires saving more registers (both general and XXM), so the windows


Should this be XMM registers?

alexcrichton · 2013-10-13T17:56:32Z

Comments fixed (wheee typos!), and now using the correct extern "C" fn instead of just a vanilla unsafe fn. I modified trans to output no_split_stack on both the rust fn and the c-abi fn.

brson · 2013-10-14T19:08:05Z

Well this is a pretty terrifying patch due to all the assembly changes, but I'm glad you did it and not me! It took a long time to get all those fiddly bits right so I'm sad to lose it all, but that's progress.

If it all works, then let's merge it. Bravo.

Can you open an issue for the windows unwinding and nominate it?

thestinger · 2013-10-15T02:56:38Z

I don't think this is actually going to be memory safe with the current standard library. It would raise the bar for low-level code, due to any function call potentially failing.

alexcrichton · 2013-10-16T16:29:45Z

With the concerns raised by @thestinger, as well as @brson's concerns about leaking arguments, I'm not convinced that this is the right approach. I imagine that we don't expect this to be a very common occurrence, so I'm starting to lean more towards killing the entire process (via the new abort intrinsic or something similar) instead of attempting to unwind at all.

One piece we would be missing right now is the ability to specify your stack size when you spawn a task, but I imagine that it is desired regardless.

alexcrichton · 2013-10-17T08:44:40Z

I've updated this to abort on stack overflow instead of unwind. I wrote up a comment in the relevant location about all the current concerns about unwinding (so hopefully they're not forgotten in the future).

Waiting on a snapshot to push this through (it uses the abort intrinsic), but other than that it should be good-to-go

alexcrichton · 2013-10-17T22:56:17Z

src/rt/arch/x86_64/morestack.S

+#else
+#define SOME_RUSTRT_SYMBOL  upcall_rust_personality
+#endif
+.globl SOME_RUSTRT_SYMBOL


I managed to track down the segfaults on linux to this. I have no idea why this causes a segfault if it's omitted, this code is never even run. I think this is getting to the point where this needs to land to make much more forward progress, so I'm just going to leave this in for now. I can open an issue about this as well (will do so after landing, assuming that it lands).

alexcrichton · 2013-10-18T17:44:32Z

This is still not ready for merging, I'm in the process of testing this on top of removing jemalloc and ensuring that everything is valgrind-clean

alexcrichton · 2013-10-18T20:23:48Z

Valgrind is clean, just waiting for a resolution of the jemalloc troubles.

This commit resumes management of the stack boundaries and limits when switching between tasks. This additionally leverages the __morestack function to run code on "stack overflow". The current behavior is to abort the process, but this is probably not the best behavior in the long term (for deails, see the comment I wrote up in the stack exhaustion routine).

This commit re-introduces the functionality of __morestack in a way that it was not originally anticipated. Rust does not currently have segmented stacks, rather just large stack segments. We do not detect when these stack segments are overrun currently, but this commit leverages __morestack in order to check this. This commit purges a lot of the old __morestack and stack limit C++ functionality, migrating the necessary chunks to rust. The stack limit is now entirely maintained in rust, and the "main logic bits" of __morestack are now also implemented in rust as well. I put my best effort into validating that this currently builds and runs successfully on osx and linux 32/64 bit, but I was unable to get this working on windows. We never did have unwinding through __morestack frames, and although I tried poking at it for a bit, I was unable to understand why we don't get unwinding right now. A focus of this commit is to implement as much of the logic in rust as possible. This involved some liberal usage of `no_split_stack` in various locations, along with some use of the `asm!` macro (scary). I modified a bit of C++ to stop calling `record_sp_limit` because this is no longer defined in C++, rather in rust. Another consequence of this commit is that `thread_local_storage::{get, set}` must both be flagged with `#[rust_stack]`. I've briefly looked at the implementations on osx/linux/windows to ensure that they're pretty small stacks, and I'm pretty sure that they're definitely less than 20K stacks, so we probably don't have a lot to worry about. Other things worthy of note: * The default stack size is now 4MB instead of 2MB. This is so that when we request 2MB to call a C function you don't immediately overflow because you have consumed any stack at all. * `asm!` is actually pretty cool, maybe we could actually define context switching with it? * I wanted to add links to the internet about all this jazz of storing information in TLS, but I was only able to find a link for the windows implementation. Otherwise my suggestion is just "disassemble on that arch and see what happens" * I put my best effort forward on arm/mips to tweak __morestack correctly, we have no ability to test this so an extra set of eyes would be useful on these spots. * This is all really tricky stuff, so I tried to put as many comments as I thought were necessary, but if anything is still unclear (or I completely forgot to take something into account), I'm willing to write more!

…=xFrednet refac: remove unnecessary mutability refactor `manual_is_ascii_check` lint. * remove unnecessary mutability * fix typo changelog: none r? `@xFrednet`

huonw reviewed Oct 13, 2013
View reviewed changes

alexcrichton mentioned this pull request Oct 13, 2013

#[no_std] no longer works #9839

Closed

alexcrichton mentioned this pull request Oct 14, 2013

Unwinding through __morestack on windows does not work #9854

Closed

alexcrichton reviewed Oct 17, 2013
View reviewed changes

alexcrichton mentioned this pull request Oct 18, 2013

Remove jemalloc from the runtime #9933

Merged

bors closed this Oct 19, 2013

bors merged commit 6d8330a into rust-lang:master Oct 19, 2013

alexcrichton deleted the morestack branch October 19, 2013 18:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement task unwinding on stack overflow #9834

Implement task unwinding on stack overflow #9834

alexcrichton commented Oct 13, 2013

huonw Oct 13, 2013

alexcrichton commented Oct 13, 2013

brson commented Oct 14, 2013

thestinger commented Oct 15, 2013

alexcrichton commented Oct 16, 2013

alexcrichton commented Oct 17, 2013

alexcrichton Oct 17, 2013

alexcrichton Oct 17, 2013

alexcrichton commented Oct 18, 2013

alexcrichton commented Oct 18, 2013

Implement task unwinding on stack overflow #9834

Implement task unwinding on stack overflow #9834

Conversation

alexcrichton commented Oct 13, 2013

huonw Oct 13, 2013

Choose a reason for hiding this comment

alexcrichton commented Oct 13, 2013

brson commented Oct 14, 2013

thestinger commented Oct 15, 2013

alexcrichton commented Oct 16, 2013

alexcrichton commented Oct 17, 2013

alexcrichton Oct 17, 2013

Choose a reason for hiding this comment

alexcrichton Oct 17, 2013

Choose a reason for hiding this comment

alexcrichton commented Oct 18, 2013

alexcrichton commented Oct 18, 2013