Remove morestack support #27338

alexcrichton · 2015-07-27T23:09:00Z

This commit removes all morestack support from the compiler which entails:

Segmented stacks are no longer emitted in codegen.
We no longer build or distribute libmorestack.a
The stack_exhausted lang item is no longer required

The only current use of the segmented stack support in LLVM is to detect stack
overflow. This is no longer really required, however, because we already have
guard pages for all threads and registered signal handlers watching for a
segfault on those pages (to print out a stack overflow message). Additionally,
major platforms (aka Windows) already don't use morestack.

This means that Rust is by default less likely to catch stack overflows because
if a function takes up more than one page of stack space it won't hit the guard
page. This is what the purpose of morestack was (to catch this case), but it's
better served with stack probes which have more cross platform support and no
runtime support necessary. Until LLVM supports this for all platform it looks
like morestack isn't really buying us much.

cc #16012 (still need stack probes)
Closes #26458 (a drive-by fix to help diagnostics on stack overflow)

r? @brson

alexcrichton · 2015-07-27T23:09:24Z

cc @Zoxc, @geofft

steveklabnik · 2015-07-27T23:10:43Z

🎊 amaze!

brson · 2015-07-27T23:36:18Z

Since this will open a fairly big hole in Rust's memory safety until stack probes work (which could be a long time) I'd like this to have some visibility before pulling the trigger. cc @nrc @pnkfelix @eddyb.

brson · 2015-07-27T23:37:54Z

I'm also a bit wary that this makes it much harder for anybody to add true split stacks back to a custom build of Rust (since it removes everything), but maybe it's time to let that dream die.

alexcrichton · 2015-07-27T23:41:41Z

Since this will open a fairly big hole in Rust's memory safety until stack probes work

I feel like our story has been worn down here quite a bit over time, so I don't personally feel that our usage of morestack is really gaining us much memory safety today. One of our tier 1 platforms, Windows, hasn't been using morestack for months now, but I also wouldn't consider ourselves memory unsafe on Windows.

eefriedman · 2015-07-28T00:46:03Z

One of our tier 1 platforms, Windows, hasn't been using morestack for months now, but I also wouldn't consider ourselves memory unsafe on Windows.

morestack is irrelevant on Windows because stack probes are already implemented there. That isn't a very strong argument for allowing arbitrary wild memory accesses on Linux.

eddyb · 2015-07-28T04:25:03Z

I'm also a bit wary that this makes it much harder for anybody to add true split stacks back to a custom build of Rust (since it removes everything), but maybe it's time to let that dream die.

Amen to that 👍.

Aatch · 2015-07-28T04:56:24Z

Eh, the danger on Linux is fairly small, you need the following conditions for it to actually cause problems:

A frame using more than page of stack space
The frame starting at a location where it completely covers the guard page
The function not actually interacting with the >4Kb data it allocated on the stack before making a function call
The space after the end of the stack being mapped

That's quite a significant amount of requirements, to the point that hitting all of them almost certainly requires doing so deliberately. Obviously having stack probes is better than not, but I'm not going to be kept awake by the removal of morestack.

oli-obk · 2015-07-28T07:03:10Z

how about also adding a lint that checks the stack size of functions? It would create some false positives since optimizations might lower the stack usage, but I don't think that's too important.

nagisa · 2015-07-28T09:07:32Z

src/libstd/sys/unix/stack_overflow.rs

        let guard = thread_info::stack_guard().unwrap_or(0);
        let addr = (*info).si_addr as usize;

+        let _ = ::sys::stdio::Stderr::new().and_then(|mut s| {
+            use io::Write;
+            s.write_fmt(format_args!("{:?} {:?} {:?}", guard, addr, PAGE_SIZE))


left-over debug print?

nagisa · 2015-07-28T09:43:17Z

Given morestack doesn’t work on non-linux/non-apple anyway, and all of the platforms have ASLR I’m not convinced “hole” is that much bigger than it was before. Sure, it exists, but to exploit it you pretty much have to be doing it maliciously, and for malicious purposes there’s many much much easier ways to do that via unsafe.

So overall 👍 from me.

Zoxc · 2015-07-28T09:51:03Z

src/libstd/sys/unix/stack_overflow.rs

-                                     _data: *mut libc::c_void) {
-
-        // We can not return from a SIGSEGV or SIGBUS signal.
-        // See: https://www.gnu.org/software/libc/manual/html_node/Handler-Returns.html


Don't remove this comment. Add one stating why we can return on linux, macos, bitrig, netbsd and openbsd. (Should probably be tested on all those platforms as well)

And FreeBSD. I think we're good on all the UNIXes that Rust currently supports; if not, the re-raise code needs to be kept. (I ran out of motivation to go categorize this and send in a PR for #26458 but I could go do that now I guess.)

Ah sorry, I forgot that I meant to reword this, and I've now done so.

pythonesque · 2015-07-28T15:56:53Z

I'm also a bit wary that this makes it much harder for anybody to add true split stacks back to a custom build of Rust (since it removes everything), but maybe it's time to let that dream die.

I think that's okay, given that other languages (Go) that used segmented stacks have had to move away from them anyway.

alexcrichton · 2015-07-28T16:36:18Z

@nagisa, @Zoxc ah I forgot to go through and do a final cleanup, but I've added comments and addressed issues now, thanks for taking a look!

geofft · 2015-07-28T19:22:06Z

Big 👍 to both the idea and the code. The continued existence of __morestack confused me a lot, and it was never clear how to implement the stack_exhausted lang-item.

That said, I do agree with the wariness about removing this from Linux while stack probes aren't present. (I'm still not totally sure what's the blocker, as in why MIPS can't use generic stack probing, but let's have that conversation on #16012.) I am concerned about whether this makes it easy to write "safe" code (that is, code that doesn't use unsafe) that messes with memory: see my writeup of doing this in well-defined C, and in particular CVE-2010-2240, which uses attacker-controlled recursion in the X server, plus MIT-SHM, to gain local root on Linux. (It did rely on coaxing the kernel into not generating a guard page, since each function happened to use less than a page of stack space.) If there's no current protection on Windows, meh, but if stack probes are present on Windows but not Linux, then I think removing explicit stack checks on Linux would be unfortunate. If we can quickly get generic stack probes in on all architectures, that sounds like the right approach.

That said, if there's already a way for malicious code to deliberately violate memory safety without unsafe in pure Rust, which I think @nagisa said there is, I don't think I can complain about there being another way to do it.

I don't think custom split stacks are a compelling reason to keep this code around; I agree this is the right long-term thing to do.

geofft · 2015-07-28T19:30:17Z

src/librustc_back/target/apple_ios_base.rs

@@ -94,7 +94,6 @@ pub fn opts(arch: Arch) -> TargetOptions {
        //
        // SS might be also enabled on Arm64 as it has builtin support in LLVM
        // but I haven't tested it through yet
-        morestack: false,


The comments above this line are now irrelevant.

nagisa · 2015-07-28T19:56:45Z

That said, if there's already a way for malicious code to deliberately violate memory safety without unsafe in pure Rust, which I think @nagisa said there is, I don't think I can complain about there being another way to do it.

Sorry, I didn’t mean to imply there’s already a way to access arbitrary memory using only safe code. What I’m trying to say that you have to be deliberately trying to write malicious safe code to reasonably expect to exploit lack of __morestack/stack probing and it won’t work most of the time anyway because of memory layout randomisation.

Sure, that is not good enough for safety Rust-without-unsafe tries to guarantee, but since we expect to implement stack probes for all platforms sometime in the future anyway…

alexcrichton · 2015-07-28T20:12:40Z

And to clarify, we don't currently use stack probes anywhere today. From what I understand it sounds like LLVM has support for stack probes on Windows but not on other platforms just yet. That being said I believe there's more detailed discussion over at #16012

bors · 2015-08-05T18:41:30Z

⌛ Testing commit 143ca53 with merge b18b3bf...

alexcrichton · 2015-08-05T18:44:46Z

@bors: r- force

Travis failed in what looks like a pretty legit fashion.

bors · 2015-08-05T18:44:48Z

⛄ The build was interrupted to prioritize another pull request.

Zoxc · 2015-08-05T23:33:33Z

src/libstd/sys/unix/stack_overflow.rs

-            term(signum);
+        // If the faulting address is within the guard page, then we print a
+        // message saying so.
+        if guard != 0 && guard - PAGE_SIZE <= addr && addr <= guard {


This should be addr < guard.

alexcrichton · 2015-08-10T15:17:55Z

@bors: r+ acbdf0a1bded880011cfff5f487066a8d16f2e0b

bors · 2015-08-10T16:18:29Z

⌛ Testing commit acbdf0a with merge aa3d701...

bors · 2015-08-10T17:06:14Z

💔 Test failed - auto-win-gnu-32-opt

This commit removes all morestack support from the compiler which entails: * Segmented stacks are no longer emitted in codegen. * We no longer build or distribute libmorestack.a * The `stack_exhausted` lang item is no longer required The only current use of the segmented stack support in LLVM is to detect stack overflow. This is no longer really required, however, because we already have guard pages for all threads and registered signal handlers watching for a segfault on those pages (to print out a stack overflow message). Additionally, major platforms (aka Windows) already don't use morestack. This means that Rust is by default less likely to catch stack overflows because if a function takes up more than one page of stack space it won't hit the guard page. This is what the purpose of morestack was (to catch this case), but it's better served with stack probes which have more cross platform support and no runtime support necessary. Until LLVM supports this for all platform it looks like morestack isn't really buying us much. cc rust-lang#16012 (still need stack probes) Closes rust-lang#26458 (a drive-by fix to help diagnostics on stack overflow)

alexcrichton · 2015-08-10T23:37:38Z

@bors: r=brson 7a3fdfb

@brson

This commit removes all morestack support from the compiler which entails: * Segmented stacks are no longer emitted in codegen. * We no longer build or distribute libmorestack.a * The `stack_exhausted` lang item is no longer required The only current use of the segmented stack support in LLVM is to detect stack overflow. This is no longer really required, however, because we already have guard pages for all threads and registered signal handlers watching for a segfault on those pages (to print out a stack overflow message). Additionally, major platforms (aka Windows) already don't use morestack. This means that Rust is by default less likely to catch stack overflows because if a function takes up more than one page of stack space it won't hit the guard page. This is what the purpose of morestack was (to catch this case), but it's better served with stack probes which have more cross platform support and no runtime support necessary. Until LLVM supports this for all platform it looks like morestack isn't really buying us much. cc #16012 (still need stack probes) Closes #26458 (a drive-by fix to help diagnostics on stack overflow) r? @brson

bors · 2015-08-10T23:40:55Z

⌛ Testing commit 7a3fdfb with merge 5aca49c...

bors · 2015-08-11T01:17:30Z

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt

On the functionality side, morestack is gone (rust-lang/rust#27338), and rustrt_native is gone (rust-lang/rust#27176). On the implementation side, connect has been renamed to join (rust-lang/rust#26957).

The Rust version is now at: ``` rustc 1.9.0-nightly (0dcc413e4 2016-03-22) ``` This includes a fix for the following build error that the new version produced: ``` src/rust_base.rs:17:1: 19:2 error: definition of an unknown language item: `stack_exhausted`. [E0522] src/rust_base.rs:17 pub extern fn stack_exhausted() { src/rust_base.rs:18 panic!("Stack exhausted"); src/rust_base.rs:19 } src/rust_base.rs:17:1: 19:2 help: run `rustc --explain E0522` to see a detailed explanation ``` It looks like the `stack_exhausted item was removed last year: rust-lang/rust#27338 This didn't cause an error so far, because the compiler didn't complain about defining lang items that don't exist. This seems to have changed in this pull request: rust-lang/rust#32264

The `stack_exhausted` lang item was removed in rust-lang/rust#27338. Therefore, our (empty) implementation of `stack_exhausted` can be removed.

rust-highfive assigned brson Jul 27, 2015

nagisa reviewed Jul 28, 2015
View reviewed changes

Zoxc reviewed Jul 28, 2015
View reviewed changes

alexcrichton force-pushed the remove-morestack branch from 4b975bb to 50e51f5 Compare July 28, 2015 16:35

geofft reviewed Jul 28, 2015
View reviewed changes

alexcrichton force-pushed the remove-morestack branch 5 times, most recently from cae8577 to ce2b8d1 Compare July 28, 2015 20:07

alexcrichton force-pushed the remove-morestack branch from ce2b8d1 to aaa1bba Compare July 28, 2015 20:15

alexcrichton force-pushed the remove-morestack branch from 143ca53 to acb8961 Compare August 5, 2015 19:01

Zoxc reviewed Aug 5, 2015
View reviewed changes

alexcrichton force-pushed the remove-morestack branch from acb8961 to acbdf0a Compare August 10, 2015 15:17

alexcrichton force-pushed the remove-morestack branch from acbdf0a to fef6937 Compare August 10, 2015 22:25

alexcrichton force-pushed the remove-morestack branch from fef6937 to 7a3fdfb Compare August 10, 2015 23:37

bors merged commit 7a3fdfb into rust-lang:master Aug 11, 2015

This was referenced Aug 11, 2015

rustc: Allow changing the default allocator #27400

Merged

Improve handling of dllexport #27416

Merged

alexcrichton deleted the remove-morestack branch August 11, 2015 06:11

ranma42 mentioned this pull request Oct 28, 2015

1.4 release notes #29416

Merged

bstrie mentioned this pull request Oct 29, 2015

Replace stack overflow checking with stack probes #16012

Closed

hawkw added a commit to sos-os/kernel that referenced this pull request Apr 2, 2016

[kernel] update for compatibility with latest nightly

9416dc7

The `stack_exhausted` lang item was removed in rust-lang/rust#27338. Therefore, our (empty) implementation of `stack_exhausted` can be removed.

lambda-fairy mentioned this pull request Jun 14, 2016

Allow for disabling the stack check in a target file #34269

Closed

qinsoon mentioned this pull request Apr 1, 2021

Stack overflow for JikesRVM mmtk/mmtk-jikesrvm#59

Open

jyn514 mentioned this pull request Apr 21, 2021

SIGBUS has to be sent twice before the rust runtime exits #84118

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove morestack support #27338

Remove morestack support #27338

alexcrichton commented Jul 27, 2015

alexcrichton commented Jul 27, 2015

steveklabnik commented Jul 27, 2015

brson commented Jul 27, 2015

brson commented Jul 27, 2015

alexcrichton commented Jul 27, 2015

eefriedman commented Jul 28, 2015

eddyb commented Jul 28, 2015

Aatch commented Jul 28, 2015

oli-obk commented Jul 28, 2015

nagisa Jul 28, 2015

nagisa commented Jul 28, 2015

Zoxc Jul 28, 2015

geofft Jul 28, 2015

alexcrichton Jul 28, 2015

pythonesque commented Jul 28, 2015

alexcrichton commented Jul 28, 2015

geofft commented Jul 28, 2015

geofft Jul 28, 2015

nagisa commented Jul 28, 2015

alexcrichton commented Jul 28, 2015

bors commented Aug 5, 2015

alexcrichton commented Aug 5, 2015

bors commented Aug 5, 2015

Zoxc Aug 5, 2015

alexcrichton commented Aug 10, 2015

bors commented Aug 10, 2015

bors commented Aug 10, 2015

alexcrichton commented Aug 10, 2015

bors commented Aug 10, 2015

bors commented Aug 11, 2015

Remove morestack support #27338

Remove morestack support #27338

Conversation

alexcrichton commented Jul 27, 2015

alexcrichton commented Jul 27, 2015

steveklabnik commented Jul 27, 2015

brson commented Jul 27, 2015

brson commented Jul 27, 2015

alexcrichton commented Jul 27, 2015

eefriedman commented Jul 28, 2015

eddyb commented Jul 28, 2015

Aatch commented Jul 28, 2015

oli-obk commented Jul 28, 2015

nagisa Jul 28, 2015

Choose a reason for hiding this comment

nagisa commented Jul 28, 2015

Zoxc Jul 28, 2015

Choose a reason for hiding this comment

geofft Jul 28, 2015

Choose a reason for hiding this comment

alexcrichton Jul 28, 2015

Choose a reason for hiding this comment

pythonesque commented Jul 28, 2015

alexcrichton commented Jul 28, 2015

geofft commented Jul 28, 2015

geofft Jul 28, 2015

Choose a reason for hiding this comment

nagisa commented Jul 28, 2015

alexcrichton commented Jul 28, 2015

bors commented Aug 5, 2015

alexcrichton commented Aug 5, 2015

bors commented Aug 5, 2015

Zoxc Aug 5, 2015

Choose a reason for hiding this comment

alexcrichton commented Aug 10, 2015

bors commented Aug 10, 2015

bors commented Aug 10, 2015

alexcrichton commented Aug 10, 2015

bors commented Aug 10, 2015

bors commented Aug 11, 2015