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 while executing eval inside block inside function #2317

Closed
jedel1043 opened this issue Sep 30, 2022 · 3 comments
Closed

Panic while executing eval inside block inside function #2317

jedel1043 opened this issue Sep 30, 2022 · 3 comments
Assignees
Labels
bug Something isn't working E-Medium Medium difficulty problem help wanted Extra attention is needed vm Issues and PRs related to the Boa Virtual Machine.
Milestone

Comments

@jedel1043
Copy link
Member

Describe the bug
The interpreter panics while executing the following snippet:

(function(){
  {
    eval("var x;")
  }
})();

Additional context
While doing some preliminar investigation, I realized functions don't push a new environment:

-------------------------Compiled Output: ''--------------------------
Location  Count   Opcode                     Operands

000000    0000    RestParameterPop           
000001    0001    PushDeclarativeEnvironment 0, 0
000010    0002    GetName                    0000: 'eval'
000015    0003    PushUndefined              
000016    0004    Swap                       
000017    0005    PushLiteral                0
000022    0006    CallEval                   1
000027    0007    Pop                        
000028    0008    PopEnvironment             
000029    0009    PushUndefined              
000030    0010    Return                     

Literals:
    0000: <string> "var x;"

Bindings:
    0000: eval

Functions:
    <empty>           

Notice how the inner block pushes a declarative environment, but the function doesn't. However, if you define argument initializers, the function pushes a function environment, and this fixes the bug:

(function(y=5){
  {
    eval("var x;")
  }
})();
-------------------------Compiled Output: ''--------------------------
Location  Count   Opcode                     Operands

000000    0000    JumpIfNotUndefined         7
000005    0001    PushInt8                   5
000007    0002    DefInitArg                 0000: 'y'
000012    0003    RestParameterPop           
000013    0004    PushFunctionEnvironment    0, 1
000022    0005    PushDeclarativeEnvironment 0, 0
000031    0006    GetName                    0001: 'eval'
000036    0007    PushUndefined              
000037    0008    Swap                       
000038    0009    PushLiteral                0
000043    0010    CallEval                   1
000048    0011    Pop                        
000049    0012    PopEnvironment             
000050    0013    PushUndefined              
000051    0014    Return                     

Literals:
    0000: <string> "var x;"

Bindings:
    0000: y
    0001: eval

Functions:
    <empty>

cc @raskad and @HalidOdat which are the most familiar with the vm.

@jedel1043 jedel1043 added bug Something isn't working E-Medium Medium difficulty problem vm Issues and PRs related to the Boa Virtual Machine. labels Sep 30, 2022
@Razican Razican added the help wanted Extra attention is needed label Oct 21, 2022
@jedel1043 jedel1043 moved this to To do in Boa pre-v1 Nov 8, 2022
@veera-sivarajan
Copy link
Contributor

Hi, can I work on this?

@jedel1043
Copy link
Member Author

@veera-sivarajan Nice! I think this issue is a good candidate to use our newest debugging tool: Instruction flowgraphs. This should be pretty useful to determine exactly how the VM is pushing and popping the environments.

@veera-sivarajan
Copy link
Contributor

I have narrowed down the issue to this part where a new function environment is pushed only when the parameters have an expression.

let env_label = if parameters.has_expressions() {
compiler.code_block.num_bindings = compiler.context.get_binding_number();
compiler.context.push_compile_time_environment(true);
compiler.code_block.function_environment_push_location =
compiler.next_opcode_location();
Some(compiler.emit_opcode_with_two_operands(Opcode::PushFunctionEnvironment))
} else {
None
};

However, based on NOTE 1, 10.2.11.27 and 10.2.11.28 in the spec, the compiler should push either

  1. A declarative environment and a function environment when parameters have expressions.
  2. Only a function environment when parameters don't have expressions.

I attempted to implement that by conditionally (if parameters have expression) pushing an environment before parameters gets binded and popping it after the function statements are compiled but it did not work as expected. Would appreciate hearing your thoughts on how my approach can be improved.

cc @raskad and @HalidOdat

jedel1043 added a commit that referenced this issue Apr 10, 2023
@jedel1043 jedel1043 moved this from To do to In Progress in Boa pre-v1 Apr 10, 2023
@bors bors bot closed this as completed in 1e75fd0 Apr 10, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in Boa pre-v1 Apr 10, 2023
@jedel1043 jedel1043 added this to the v0.17.0 milestone May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working E-Medium Medium difficulty problem help wanted Extra attention is needed vm Issues and PRs related to the Boa Virtual Machine.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants