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

Panic at break in an infinte loop, in finally block #3054

Closed
MahnurA opened this issue Jun 20, 2023 · 3 comments · Fixed by #3073
Closed

Panic at break in an infinte loop, in finally block #3054

MahnurA opened this issue Jun 20, 2023 · 3 comments · Fixed by #3073
Labels
bug Something isn't working execution Issues or PRs related to code execution

Comments

@MahnurA
Copy link

MahnurA commented Jun 20, 2023

Describe the bug
In a try-catch-finally -- in the finally block, panic in a one line break statement after an infinite loop, if there are no braces around the break statement.

To Reproduce
These JavaScripts crash with thread 'main' panicked at 'stack was empty', boa_engine/src/vm/opcode/control_flow/finally.rs:137:59

try {
}
catch {
} finally {
 while(true) 
  break;
}

or

try {
}
catch {
} finally {
 for(;;)
  break;
}

Expected behavior
Should not panic. Javascript allows one-line statements after a condition to not be encompassed by braces.

Build environment:
• OS: Ubuntu
• Version: 20.04
• Target triple: x86_64-unknown-linux-gnu
• Rustc version: rustc 1.70.0

Additional context
Note that this Javascript does not panic, i.e. if braces are present. It is also clearly breaking out of the infinite loop as "in finally" does get printed.

try {
}
catch {
} finally {
 while(true) {
  break;
 }
 console.log("in finally")
}
@MahnurA MahnurA added the bug Something isn't working label Jun 20, 2023
@jedel1043 jedel1043 added the execution Issues or PRs related to code execution label Jun 20, 2023
dirkdev98 added a commit to dirkdev98/boa that referenced this issue Jun 26, 2023
The match was too greedy, being executed for 'break' abrupt completions as well.

Closes boa-dev#3054
@dirkdev98
Copy link
Contributor

I'm sorry for not commenting on this before working on this, will make sure to do that going forward.

I think this is fixed in #3073

@HalidOdat
Copy link
Member

HalidOdat commented Jun 27, 2023

#3059 Fixes this issue by completely refactoring and redesigning the execution (removed FinallyStart/End and many many things). I should have assigned the issue to my self to avoid the work overlap 😅 .

But it's good to have a fix for the panic until #3059 is fully complete 😄

github-merge-queue bot pushed a commit that referenced this issue Jun 27, 2023
The match was too greedy, being executed for 'break' abrupt completions as well.

Closes #3054
@jedel1043
Copy link
Member

But it's good to have a fix for the panic until #3059 is fully complete 😄

Yeah, and also because we'll probably release tomorrow, so #3059 will probably arrive to our users until 0.18.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working execution Issues or PRs related to code execution
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants