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

Run SpiderMonkey on split stacks #131

Closed
brson opened this issue Oct 18, 2012 · 6 comments
Closed

Run SpiderMonkey on split stacks #131

brson opened this issue Oct 18, 2012 · 6 comments
Labels

Comments

@brson
Copy link
Contributor

@brson brson commented Oct 18, 2012

This will make stack overflows from js impossible. I don't know SpiderMonkey, but I recall that it has somewhat complicated checks for recursion that could be removed.

I currently expect Rust to limit recursion by aborting the process at some arbitrary depth. This may be ok for a multi-process Servo.

Thoughts:

  • Clang probably already knows how to do it, but it will involve invoking split stack features that Rust doesn't currently use (so may not work). Probably requires new assembly
  • We will have to figure out how split-stack SpiderMonkey code calls into the dynamic system libraries it links to. gcc has some sort of solution for this
  • The JIT will need to be modified to insert __morestack checks.
  • We can still use very large stack segments for performance
  • The checks will have some performance impact, but I think it is not large.
@bzbarsky
Copy link
Contributor

@bzbarsky bzbarsky commented Oct 18, 2012

There are web pages out there that rely on the out-of-stack exception JS engines throw to function correctly. As in, they run code that's theoretically an infinite recursion loop and depend on the out-of-stack exception to terminate the code. If we make that situation terminate the process, those web pages would no longer be browsable to... Worse yet, this was a bug in a library that was used across a number of websites, so it might have pretty broad effects.

@bzbarsky
Copy link
Contributor

@bzbarsky bzbarsky commented Oct 18, 2012

Oh, and in particular said web pages expect to hit the out-of-stack case quickly; otherwise you get a user-visible hang....

@bzbarsky
Copy link
Contributor

@bzbarsky bzbarsky commented Oct 18, 2012

@brson
Copy link
Contributor Author

@brson brson commented Oct 18, 2012

Oh my. That makes it much harder. Is this even a worthwhile goal to pursue?

I do think Rust can eventually recover safely from too-deep recursion, but it is hard. The problem is that our opportunity to detect recursion happens before the function prologue even runs, so if we decide we need to unwind then the function arguments get lost and destructors don't run. Fixing this in an obvious and efficient way seems to require the same improvements to LLVM as precise GC, though we may be most of the way there with @elliottslaughter's patches.

Of course that only helps Rust code. I cannot think of a way to recover safely from a __morestack allocation failure in general. gcc's __morestack code also abort()s if the allocation fails.

Possibly we could have Rust's limit set very deep and abort on failure, then have a much smaller limit that is more 'advisory', setting a flag that SpiderMonkey has to check periodically (presumably exactly where it's doing checks now). Would that simplify anything for SpiderMonkey?

@bzbarsky
Copy link
Contributor

@bzbarsky bzbarsky commented Oct 18, 2012

That seems like the obvious way to go, yes... Right now SpiderMonkey certainly has an arbitrary stack limit that is not "as much address space I can get".

@pcwalton
Copy link
Contributor

@pcwalton pcwalton commented May 8, 2014

This is no longer necessary since Rust does not use split stacks.

@pcwalton pcwalton closed this May 8, 2014
ChrisParis pushed a commit to ChrisParis/servo that referenced this issue Sep 7, 2014
glennw added a commit to glennw/servo that referenced this issue Jan 16, 2017
First pass of GPU clipping support.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.