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

feat: add configurable interpreter step limit for moc.js #3618

Merged
merged 2 commits into from
Dec 5, 2022

Conversation

rvanasa
Copy link
Contributor

@rvanasa rvanasa commented Dec 3, 2022

Adds an optional deterministic step limit to the moc.js interpreter.

This feature will protect users from accidentally crashing the browser with a code snippet such as while true {} now that we are using a trampoline (#3541), so I suppose this is a nice problem to have.

@rvanasa rvanasa requested a review from crusso December 3, 2022 00:57
@github-actions
Copy link

github-actions bot commented Dec 3, 2022

Comparing from e602092 to 81a1932:
The produced WebAssembly code seems to be completely unchanged.

@rvanasa rvanasa added interpreter automerge-squash When ready, merge (using squash) labels Dec 3, 2022
Copy link
Contributor

@crusso crusso left a comment

Choose a reason for hiding this comment

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

If needs must.

Does the browser hang indefinitely or offer a cancel script prompt eventually?

If the former, I wonder why a while (true) { }diverges, but the similar bounded loop on the trampoline PR stack-overflowed, There, decrementing a counter for 163840 steps exits with stack overflow (but 16384 is ok).

@mergify mergify bot merged commit 112f9eb into master Dec 5, 2022
@mergify mergify bot deleted the ryan/js-interpreter-limit branch December 5, 2022 16:37
@mergify mergify bot removed the automerge-squash When ready, merge (using squash) label Dec 5, 2022
@rvanasa
Copy link
Contributor Author

rvanasa commented Dec 5, 2022

Does the browser hang indefinitely or offer a cancel script prompt eventually?

Browser hangs indefinitely and requires reopening the tab.

I wonder why a while (true) { } diverges, but the similar bounded loop on the trampoline PR stack-overflowed

Yeah, same. I'll try to reverse-engineer the stack trace in case there's a way we can mitigate whatever is going on here.

@crusso
Copy link
Contributor

crusso commented Dec 5, 2022

Yeah, I actually didn't think to look a the developer tool pane to figure out why the overflow was happening. It could be that we are missing some extra trampolines in other interpreter functions, but I experimented with adding some and didn't get anywhere (or maybe didn't refresh the browser hard enough).

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

Successfully merging this pull request may close these issues.

2 participants