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

Stack probe #29

Open
antoyo opened this issue Jan 20, 2018 · 9 comments
Open

Stack probe #29

antoyo opened this issue Jan 20, 2018 · 9 comments

Comments

@antoyo
Copy link

antoyo commented Jan 20, 2018

Hi.
To have better safety, could we use guard pages and stack probes in may?
I'm not sure it's possible, just asking.
Thanks.

@Xudong-Huang
Copy link
Owner

Xudong-Huang commented Jan 22, 2018

thanks for the link!
currently May doesn't use guard page for coroutines stacks, this need alloc extra one more page for each coroutine. Now we only has a very unsafe way to probe the stack overflow implemented by checking the footprint of the coroutine stack. So this is why exceed stack could cause undefined behavior.

I check the __rust_probestack src code and find that this function needs to be called in the user's code, not in the May library, because it only used for one big single function frame to detect stack overflow, and for libary design we don't have any information about how the stack frames would grow. If users forget call this for a large stack frame (bigger than one page) function it still triggers undefined behavior even we has a stack gurad page here.

@vitiral
Copy link

vitiral commented Feb 5, 2018

I check the __rust_probestack src code and find that this function needs to be called in the user's code, not in the May library, because it only used for one big single function frame to detect stack overflow, and for libary design we don't have any information about how the stack frames would grow.

I may not be understanding, but if it has to be called "in the user's code" then couldn't the go! macro call it?

@Xudong-Huang
Copy link
Owner

@vitiral: go! macro is just a thin wrapper, which doesn't need stack probe at all. only those leaf functions that have very big stack frame would be benefit from __rust_probestack when they are running with a guard page on the stack. But may doesn't have guard pages for coroutines now, so call __rust_probestack is no help.

@vitiral
Copy link

vitiral commented Mar 14, 2018

I feel the big issue with may is that it does not guard against undefined behavior. I think most use cases don't care that they have to limit their stack size (most co-routines will spawn a single function call!) and wouldn't mind if the coroutine paniced in that case. Undefined behavior, however, is a huge problem.

Is it possible to prevent undefined behavior and instead panic... or even abort?

@Xudong-Huang
Copy link
Owner

Xudong-Huang commented Mar 15, 2018

yes, it's possible to add a stack guard page for each coroutine. And this would need an extra one page memory allocated. for most cases this is not a big issue which I preferred to add the behavior in future. if stack overflow detected, OS will kill the program with a segment fault(maybe with better output by rust, this is not a UB, I think) just like normal thread stack overflow implemented by rust.

I don't think we can have a panic if stack overflow happened, when panic happened, it will need more stack which would make things worse. If we are using guard page, there is no place to put the panic code, rust run-time takes over the action for stack overflow.

@vitiral
Copy link

vitiral commented Mar 15, 2018

cool, thanks for the response!

I think it makes sense for go! to use the stack probe, but allow users to access a coroutine without probed memory (through an unsafe function) if they really want to squeeze performance.

Thanks!

@killme2008
Copy link

I think this is a serious issue that prevent people to use may in production.People are not afraid of panic but undefined. 😄

Do you have any plan to fix this issue?

@Xudong-Huang
Copy link
Owner

Xudong-Huang commented Apr 23, 2019 via email

@Xudong-Huang
Copy link
Owner

now the generator lib use guarded stack, may will automatically has the desired behavior.

ref Xudong-Huang/generator-rs#12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants