Skip to content

Remove alternate stack with sigaltstack before unmaping it. #31684

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

Merged
merged 1 commit into from Feb 19, 2016
Merged

Remove alternate stack with sigaltstack before unmaping it. #31684

merged 1 commit into from Feb 19, 2016

Conversation

ghost
Copy link

@ghost ghost commented Feb 15, 2016

Remove alternate stack with sigaltstack before unmaping it.

Also reuse existing signal stack if already set, this is especially
useful when working with sanitizers that configure alternate stack
themselves.

This change depends on SS_DISABLE recently introduced in libc crate and updates
this git submodule accordingly.

@ghost
Copy link
Author

ghost commented Feb 16, 2016

r? @alexcrichton

Handler { _data: stack.ss_sp as *mut libc::c_void }
let mut stack = mem::zeroed();
sigaltstack(ptr::null(), &mut stack);
if stack.ss_flags & SS_DISABLE != 0 {
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment explaining what this check is doing? I think it's just checking to see if one's already set, right?

@alexcrichton
Copy link
Member

Thanks @tmiasko!

@ghost
Copy link
Author

ghost commented Feb 16, 2016

I have added comment before checking for SS_DISABLE,
and updated the test to use ignore-windows.

@alexcrichton
Copy link
Member

@bors: r+ 4022ff164884a4628ac9498c9d9557cca643c8bb

@bors
Copy link
Collaborator

bors commented Feb 17, 2016

⌛ Testing commit 4022ff1 with merge ec445e3...

@bors
Copy link
Collaborator

bors commented Feb 17, 2016

💔 Test failed - auto-mac-64-opt

Also reuse existing signal stack if already set, this is especially
useful when working with sanitizers that configure alternate stack
themselves.
@ghost
Copy link
Author

ghost commented Feb 18, 2016

The buildbot test failure on Mac OS is real. While I don't have Mac to test
this on, this appears be a bug in Mac OS sigaltstk implementation:
https://opensource.apple.com/source/Libc/Libc-1044.1.2/compat-43/sigaltstk.c
The check for stack size should not be there when disabling the stack. The
ss_size should be ignored in this case.

I incorporated a workaround that passes SIGSTKSZ when disabling the stack.
Please take another look.

@alexcrichton
Copy link
Member

@bors: r+ 77922b8

Nice catch!

bors added a commit that referenced this pull request Feb 18, 2016
Remove alternate stack with sigaltstack before unmaping it.

Also reuse existing signal stack if already set, this is especially
useful when working with sanitizers that configure alternate stack
themselves.

This change depends on SS_DISABLE recently introduced in libc crate and updates
this git submodule accordingly.
@bors
Copy link
Collaborator

bors commented Feb 18, 2016

⌛ Testing commit 77922b8 with merge 8842e28...

@bors bors merged commit 77922b8 into rust-lang:master Feb 19, 2016
@ghost ghost deleted the alternate-stack branch September 26, 2016 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants