Remove morestack support #27338

Merged
merged 1 commit into from Aug 11, 2015

Conversation

Projects
None yet
@alexcrichton
Member

alexcrichton commented Jul 27, 2015

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

This comment has been minimized.

Show comment
Hide comment
Member

alexcrichton commented Jul 27, 2015

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Jul 27, 2015

Member

🎊 amaze!

Member

steveklabnik commented Jul 27, 2015

🎊 amaze!

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Jul 27, 2015

Contributor

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.

Contributor

brson commented Jul 27, 2015

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

This comment has been minimized.

Show comment
Hide comment
@brson

brson Jul 27, 2015

Contributor

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.

Contributor

brson commented Jul 27, 2015

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

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jul 27, 2015

Member

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.

Member

alexcrichton commented Jul 27, 2015

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

This comment has been minimized.

Show comment
Hide comment
@eefriedman

eefriedman Jul 28, 2015

Contributor

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.

Contributor

eefriedman commented Jul 28, 2015

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

This comment has been minimized.

Show comment
Hide comment
@eddyb

eddyb Jul 28, 2015

Member

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 👍.

Member

eddyb commented Jul 28, 2015

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

This comment has been minimized.

Show comment
Hide comment
@Aatch

Aatch Jul 28, 2015

Contributor

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.

Contributor

Aatch commented Jul 28, 2015

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

This comment has been minimized.

Show comment
Hide comment
@oli-obk

oli-obk Jul 28, 2015

Contributor

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.

Contributor

oli-obk commented Jul 28, 2015

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.

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))

This comment has been minimized.

@nagisa

nagisa Jul 28, 2015

Contributor

left-over debug print?

@nagisa

nagisa Jul 28, 2015

Contributor

left-over debug print?

src/libstd/sys/windows/stack_overflow.rs
@@ -35,7 +35,6 @@ impl Drop for Handler {
// This is initialized in init() and only read from after
static mut PAGE_SIZE: usize = 0;
-#[no_stack_check]
extern "system" fn vectored_handler(ExceptionInfo: *mut EXCEPTION_POINTERS) -> LONG {

This comment has been minimized.

@nagisa

nagisa Jul 28, 2015

Contributor

record_sp_limit() call needs to be removed in this function?

@nagisa

nagisa Jul 28, 2015

Contributor

record_sp_limit() call needs to be removed in this function?

src/libstd/sys/unix/thread.rs
@@ -44,7 +43,7 @@ impl Thread {
assert_eq!(pthread_attr_init(&mut attr), 0);
// Reserve room for the red zone, the runtime's stack of last resort.

This comment has been minimized.

@nagisa

nagisa Jul 28, 2015

Contributor

This comment is not relevant anymore.

@nagisa

nagisa Jul 28, 2015

Contributor

This comment is not relevant anymore.

-NATIVE_LIBS := rust_builtin hoedown morestack miniz \
- rustrt_native rust_test_helpers
+NATIVE_LIBS := rust_builtin hoedown miniz \
+ rust_test_helpers morestack

This comment has been minimized.

@nagisa

nagisa Jul 28, 2015

Contributor

Are you sure you wanted to keep morestack here?

@nagisa

nagisa Jul 28, 2015

Contributor

Are you sure you wanted to keep morestack here?

This comment has been minimized.

@alexcrichton

alexcrichton Jul 28, 2015

Member

Yeah it's required for stage0 currently (it's just an empty object file), but the stage1+ compilers don't require morestack so this can be removed after a snapshot.

@alexcrichton

alexcrichton Jul 28, 2015

Member

Yeah it's required for stage0 currently (it's just an empty object file), but the stage1+ compilers don't require morestack so this can be removed after a snapshot.

@nagisa

This comment has been minimized.

Show comment
Hide comment
@nagisa

nagisa Jul 28, 2015

Contributor

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.

Contributor

nagisa commented Jul 28, 2015

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.

- _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

This comment has been minimized.

@Zoxc

Zoxc Jul 28, 2015

Contributor

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)

@Zoxc

Zoxc Jul 28, 2015

Contributor

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)

This comment has been minimized.

@geofft

geofft Jul 28, 2015

Contributor

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.)

@geofft

geofft Jul 28, 2015

Contributor

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.)

This comment has been minimized.

@alexcrichton

alexcrichton Jul 28, 2015

Member

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

@alexcrichton

alexcrichton Jul 28, 2015

Member

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

@pythonesque

This comment has been minimized.

Show comment
Hide comment
@pythonesque

pythonesque Jul 28, 2015

Contributor

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.

Contributor

pythonesque commented Jul 28, 2015

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

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jul 28, 2015

Member

@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!

Member

alexcrichton commented Jul 28, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@geofft

geofft Jul 28, 2015

Contributor

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.

Contributor

geofft commented Jul 28, 2015

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.

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

This comment has been minimized.

@geofft

geofft Jul 28, 2015

Contributor

The comments above this line are now irrelevant.

@geofft

geofft Jul 28, 2015

Contributor

The comments above this line are now irrelevant.

src/librustc_trans/trans/base.rs
- // The stack exhaustion lang item shouldn't have a split stack because
- // otherwise it would continue to be exhausted (bad), and both it and the
- // eh_personality functions need to be externally linkable.
+ // The eh_personality functions need to be externally linkable.

This comment has been minimized.

@geofft

geofft Jul 28, 2015

Contributor

"function" singular, right? There's only one of each lang-item.

@geofft

geofft Jul 28, 2015

Contributor

"function" singular, right? There's only one of each lang-item.

@nagisa

This comment has been minimized.

Show comment
Hide comment
@nagisa

nagisa Jul 28, 2015

Contributor

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…

Contributor

nagisa commented Jul 28, 2015

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

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jul 28, 2015

Member

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

Member

alexcrichton commented Jul 28, 2015

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

@eefriedman

This comment has been minimized.

Show comment
Hide comment
@eefriedman

eefriedman Jul 28, 2015

Contributor

@alexcrichton Actually, LLVM always generates stack probes on Windows because it's part of the ABI.

Contributor

eefriedman commented Jul 28, 2015

@alexcrichton Actually, LLVM always generates stack probes on Windows because it's part of the ABI.

@nagisa

This comment has been minimized.

Show comment
Hide comment
@nagisa

nagisa Jul 28, 2015

Contributor

There needs to be some warning #[no_stack_check] has no effect anymore, I think. IMO similar attribute makes little sense in presence of stack probes setup, which you want to be active all the time, so the attribute is unlikely to be reused.

Contributor

nagisa commented Jul 28, 2015

There needs to be some warning #[no_stack_check] has no effect anymore, I think. IMO similar attribute makes little sense in presence of stack probes setup, which you want to be active all the time, so the attribute is unlikely to be reused.

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jul 28, 2015

Member

@eefriedman So it does!

@nagisa I've updated to make it a normal unused attribute warning.

Member

alexcrichton commented Jul 28, 2015

@eefriedman So it does!

@nagisa I've updated to make it a normal unused attribute warning.

@jroesch

This comment has been minimized.

Show comment
Hide comment
@jroesch

jroesch Jul 31, 2015

Member

👍

Member

jroesch commented Jul 31, 2015

👍

@sstewartgallus

This comment has been minimized.

Show comment
Hide comment
@sstewartgallus

sstewartgallus Aug 1, 2015

Contributor

Does Rust have any features in it currently that take up a dynamic amount of stack space within a function similar to alloca or dynamic arrays in C99? Taking up more stack space than a page within a naive function could be triggerable if so.

Dynamic arrays example in C99:

unsigned sum(unsigned *vals, size_t n)
{
     unsigned vals_copy[n];
     for (size_t ii = 0U; ii < n; ++ii)
          vals_copy[ii] = vals;
     unsigned xx = 0U;
     for (size_t ii = 0U; ii < n; ++ii)
           xx += vals_copy[ii];
     return xx;
}
Contributor

sstewartgallus commented Aug 1, 2015

Does Rust have any features in it currently that take up a dynamic amount of stack space within a function similar to alloca or dynamic arrays in C99? Taking up more stack space than a page within a naive function could be triggerable if so.

Dynamic arrays example in C99:

unsigned sum(unsigned *vals, size_t n)
{
     unsigned vals_copy[n];
     for (size_t ii = 0U; ii < n; ++ii)
          vals_copy[ii] = vals;
     unsigned xx = 0U;
     for (size_t ii = 0U; ii < n; ++ii)
           xx += vals_copy[ii];
     return xx;
}
@eddyb

This comment has been minimized.

Show comment
Hide comment
@eddyb

eddyb Aug 1, 2015

Member

Rust does not have VLAs right now, but if it did, stack probing could be implemented for them, just like it is present in some alloca implementations.

Member

eddyb commented Aug 1, 2015

Rust does not have VLAs right now, but if it did, stack probing could be implemented for them, just like it is present in some alloca implementations.

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Aug 5, 2015

Contributor

☔️ The latest upstream changes (presumably #27393) made this pull request unmergeable. Please resolve the merge conflicts.

Contributor

bors commented Aug 5, 2015

☔️ The latest upstream changes (presumably #27393) made this pull request unmergeable. Please resolve the merge conflicts.

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
Member

alexcrichton commented Aug 5, 2015

ping r? @brson

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Aug 5, 2015

Contributor

@bors r+

Nice simplification. 😢

Contributor

brson commented Aug 5, 2015

@bors r+

Nice simplification. 😢

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Aug 5, 2015

Contributor

📌 Commit 143ca53 has been approved by brson

Contributor

bors commented Aug 5, 2015

📌 Commit 143ca53 has been approved by brson

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Aug 5, 2015

Contributor

⌛️ Testing commit 143ca53 with merge b18b3bf...

Contributor

bors commented Aug 5, 2015

⌛️ Testing commit 143ca53 with merge b18b3bf...

bors added a commit that referenced this pull request Aug 5, 2015

Auto merge of #27338 - alexcrichton:remove-morestack, r=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
@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Aug 5, 2015

Member

@bors: r- force

Travis failed in what looks like a pretty legit fashion.

Member

alexcrichton commented Aug 5, 2015

@bors: r- force

Travis failed in what looks like a pretty legit fashion.

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Aug 5, 2015

Contributor

⛄️ The build was interrupted to prioritize another pull request.

Contributor

bors commented Aug 5, 2015

⛄️ The build was interrupted to prioritize another pull request.

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 comment has been minimized.

@Zoxc

Zoxc Aug 5, 2015

Contributor

This should be addr < guard.

@Zoxc

Zoxc Aug 5, 2015

Contributor

This should be addr < guard.

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
Member

alexcrichton commented Aug 10, 2015

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Aug 10, 2015

Contributor

⌛️ Testing commit acbdf0a with merge aa3d701...

Contributor

bors commented Aug 10, 2015

⌛️ Testing commit acbdf0a with merge aa3d701...

bors added a commit that referenced this pull request Aug 10, 2015

Auto merge of #27338 - alexcrichton:remove-morestack, r=alexcrichton
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

This comment has been minimized.

Show comment
Hide comment
@bors

bors Aug 10, 2015

Contributor

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

Contributor

bors commented Aug 10, 2015

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

Remove morestack support
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)
@alexcrichton

This comment has been minimized.

Show comment
Hide comment
Member

alexcrichton commented Aug 10, 2015

@bors: r=brson 7a3fdfb

bors added a commit that referenced this pull request Aug 10, 2015

Auto merge of #27338 - alexcrichton:remove-morestack, r=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

This comment has been minimized.

Show comment
Hide comment
@bors

bors Aug 10, 2015

Contributor

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

Contributor

bors commented Aug 10, 2015

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

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

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
homu Test successful
Details

@alexcrichton alexcrichton deleted the alexcrichton:remove-morestack branch Aug 11, 2015

geofft added a commit to geofft/std-with-cargo that referenced this pull request Aug 31, 2015

Update cargo-ify.patch for current Rust
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).

@ranma42 ranma42 referenced this pull request Oct 28, 2015

Merged

1.4 release notes #29416

hannobraun added a commit to hannobraun/embedded that referenced this pull request Mar 24, 2016

Update Rust
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

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

[kernel] update for compatibility with latest nightly
The `stack_exhausted` lang item was removed in rust-lang/rust#27338.
Therefore, our (empty) implementation of `stack_exhausted` can be
removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment