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

Instrumentation can block creation of vars in outside scope with sloppy-mode direct eval() #457

Open
overlookmotel opened this issue Nov 19, 2022 · 2 comments
Labels
bug Something isn't working eval Issue related to `eval` instrumentation Relates to instrumentation

Comments

@overlookmotel
Copy link
Owner

overlookmotel commented Nov 19, 2022

Problem

Input:

// CommonJS - sloppy mode
eval('var x = 123; let livepack_tracker;');
assert(typeof x === 'number');

In original code, assertion passes. In instrumented code, it fails.

Due to usage of a var livepack_tracker in the eval()-ed code, Livepack's instrumentation wraps the eval code in an arrow function to populate internal livepack_tracker1 var. The code which is executed in eval() is:

(
  (livepack1_tracker, livepack1_getScopeId) =>
  eval('var x = 123; let livepack_tracker;')
)(livepack_tracker, livepack_getScopeId)

This means var x is added to the scope of the arrow function, not the outer scope.

Only a occurs if:

  1. eval() code is sloppy mode. In strict mode var declarations are bound within the eval script scope.
  2. eval()-ed code uses vars which could clash with Livepack's internal vars in outer scope (very uncommon case)

Solution

Can avoid the arrow function wrapper, by instead transforming eval()-ed code to:

const livepack1_tracker = livepack_tracker,
  livepack1_getScopeId = livepack_getScopeId;
eval('var x = 123; let livepack_tracker;');

NB Indirect eval also has similar problems, but solution is more complex. Will open separate issue for that.

@overlookmotel
Copy link
Owner Author

overlookmotel commented Nov 19, 2022

Actually, this problem occurs even if no internal vars are used.

Input:

// CommonJS - sloppy mode
eval('var x = 123;');
assert(typeof x === 'number');

This is instrumented as:

livepack_tracker.evalDirect(
  'var x = 123',
  eval,
  livepack_temp_3 => eval(livepack_temp_3),
  livepack_temp_3 => eval(...livepack_temp_3),
  [[1, "index", livepack_scopeId_2, ["module"], ["exports"], ["this", 1], ["new.target", 1]]],
  false
);

So eval() is still called within an arrow function.

To solve it, would need to convert to something like:

eval( livepack_tracker.instrumentEval( 'var x = 123', eval, [ /* ... */ ] ) )

Previously took this approach (function was called livepack_preval) but later changed it, but can't remember why.

@overlookmotel
Copy link
Owner Author

See #137 (comment) for a solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working eval Issue related to `eval` instrumentation Relates to instrumentation
Projects
None yet
Development

No branches or pull requests

1 participant