Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase the size of the sigaltstack. #1315

Merged
merged 6 commits into from
Apr 3, 2020
Merged

Conversation

sunfishcode
Copy link
Member

Rust's stack overflow handler installs a sigaltstack stack with size
SIGSTKSZ, which is too small for some of the things we do in signal
handlers, and as of this writing lacks a guard page. Install bigger
sigaltstack stacks so that we have enough space, and have a guard page.

This is similar to #1298 but rewritten in Rust

Rust's stack overflow handler installs a sigaltstack stack with size
SIGSTKSZ, which is too small for some of the things we do in signal
handlers, and as of this writing lacks a guard page. Install bigger
sigaltstack stacks so that we have enough space, and have a guard page.
@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Mar 13, 2020
@github-actions
Copy link

Subscribe to Label Action

This issue or pull request has been labeled: "wasmtime:api"

Users Subscribed to "wasmtime:api"

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

crates/runtime/src/traphandlers.rs Outdated Show resolved Hide resolved
crates/runtime/src/traphandlers.rs Outdated Show resolved Hide resolved
crates/runtime/src/traphandlers.rs Outdated Show resolved Hide resolved
crates/runtime/src/traphandlers.rs Outdated Show resolved Hide resolved
crates/runtime/src/traphandlers.rs Outdated Show resolved Hide resolved

/// The size of the sigaltstack (not including the guard, which will be added).
/// Make this large enough to run our signal handlers.
static STACK_SIZE: usize = libc::SIGSTKSZ * 4;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI in Dfinity we use std::cmp::max(SIGSTKSZ, 24 * 1024) here, because SIGSTKSZ on MacOS is already huge (128K) and reserving 512K per thread for the signal stack seemed like too much

Copy link
Member Author

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Github won't let me approve since it was originally my PR, but this LGTM.

return Err(Trap::oom());
}

// Prepare the stack with readable/writable memory and then reigster it
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/reigster/register/

@alexcrichton alexcrichton merged commit 9ca3bf5 into master Apr 3, 2020
@alexcrichton alexcrichton deleted the bigger-sigaltstack-rust branch April 3, 2020 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants