From d02d4625a8448d9bfc338bbd4aad32b3179c2d25 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Wed, 30 Aug 2023 01:30:47 +0100 Subject: [PATCH] `with` statements WIP 1 --- TODO.md | 9 +++++ lib/init/index.js | 52 +++++++++++++++++++++++- lib/instrument/blocks.js | 1 + lib/instrument/visitors/eval.js | 2 + lib/instrument/visitors/function.js | 1 + lib/instrument/visitors/identifier.js | 54 +++++++++++++++++++++---- lib/instrument/visitors/statement.js | 8 +--- lib/instrument/visitors/with.js | 58 +++++++++++++++++++++++++++ lib/serialize/functions.js | 6 ++- lib/serialize/parseFunction.js | 12 +++++- lib/shared/tracker.js | 23 ++++++++++- 11 files changed, 205 insertions(+), 21 deletions(-) create mode 100644 TODO.md create mode 100644 lib/instrument/visitors/with.js diff --git a/TODO.md b/TODO.md new file mode 100644 index 00000000..29afd424 --- /dev/null +++ b/TODO.md @@ -0,0 +1,9 @@ +# TODO + +* Create `with () {}` block in output + * Make sure that scope function is sloppy mode +* Prevent vars below `with () {}` having their names mangled + * Including vars internal to functions +* Deal with `with` in `eval()` +* Tests +* Remove note in README that `with` is not supported diff --git a/lib/init/index.js b/lib/init/index.js index 596e7fb1..0f77ad2f 100644 --- a/lib/init/index.js +++ b/lib/init/index.js @@ -11,8 +11,8 @@ const getScopeId = require('./getScopeId.js'), addEvalFunctionsToTracker = require('./eval.js'), internal = require('../shared/internal.js'), - {tracker} = require('../shared/tracker.js'), - {COMMON_JS_MODULE} = require('../shared/constants.js'); + {tracker, getIsGettingScope} = require('../shared/tracker.js'), + {COMMON_JS_MODULE, INTERNAL_VAR_NAMES_PREFIX} = require('../shared/constants.js'); // Exports @@ -43,11 +43,59 @@ module.exports = (filename, module, require, nextBlockId, prefixNum) => { localTracker.nextBlockId = nextBlockId; localTracker.prefixNum = prefixNum; addEvalFunctionsToTracker(localTracker, filename); + addWrapWithFunctionToTracker(localTracker, prefixNum); // Return tracker and `getScopeId` functions return [localTracker, getScopeId]; }; +function addWrapWithFunctionToTracker(localTracker, prefixNum) { + // `.wrapWith` wraps an object used as object of a `with ()` statement. + // + // Filter out any accesses to Livepack's internal vars, to prevent the `with` object + // obscuring access to them. + // + // There can be no valid accesses to Livepack's internal vars, otherwise they'd have + // been renamed to avoid clashes with any existing vars. + // Only exception is in `eval()`, where var names cannot be predicted in advance + // e.g. `with ({livepack_tracker: 1}) { eval('livepack_tracker = 2'); }`. + // This should set the property on the `with` object. + // However, this is a pre-existing problem with `eval()`, and can manifest without `with`, + // so not going to try to solve it here either. + // TODO: Try to solve this. + // + // Also always returns false from `has` trap if getting scope vars for a function. + // This renders the `with` object transparent, so tracker can get values of variables + // outside of the `with () {}` block. e.g. `let f, x = 123; with ({x: 1}) { f = () => x; }` + // Tracker in `f` needs to be able to get the value of `x` in outer scope. + // + // Proxy an empty object, and forward operations to the `with` object, + // rather than proxying the `with` object directly, to avoid breaking Proxy `has` trap's invariant + // that cannot report a property as non-existent if it's non-configurable. + // e.g. `with (Object.freeze({livepack_tracker: 123})) { ... }` + const internalVarsPrefix = `${INTERNAL_VAR_NAMES_PREFIX}${prefixNum || ''}_`; + localTracker.wrapWith = withObj => new Proxy(Object.create(null), { + has(target, key) { + // Act as if object has no properties if currently getting scope vars for a function + if (getIsGettingScope()) return false; + // Act as if properties named like Livepack's internal vars don't exist + if (key.startsWith(internalVarsPrefix)) return false; + // Forward to `with` object + return Reflect.has(withObj, key); + }, + get(target, key) { + return Reflect.get(withObj, key, withObj); + }, + set(target, key, value) { + return Reflect.set(withObj, key, value, withObj); + }, + deleteProperty(target, key) { + return Reflect.deleteProperty(withObj, key); + } + // NB: These are the only traps which can be triggered + }); +} + // Imports // These imports are after export to avoid circular requires in Jest tests const captureFunctions = require('./functions.js'), diff --git a/lib/instrument/blocks.js b/lib/instrument/blocks.js index 1bb662a4..f398b507 100644 --- a/lib/instrument/blocks.js +++ b/lib/instrument/blocks.js @@ -199,6 +199,7 @@ function getOrCreateExternalVar(externalVars, block, varName, bindingOrExternalV varNode: bindingOrExternalVar.varNode, isReadFrom: false, isAssignedTo: false, + isNameLocked: false, argNames: bindingOrExternalVar.argNames, trails: [] }; diff --git a/lib/instrument/visitors/eval.js b/lib/instrument/visitors/eval.js index 76613198..39a2a4ab 100644 --- a/lib/instrument/visitors/eval.js +++ b/lib/instrument/visitors/eval.js @@ -115,6 +115,7 @@ function instrumentEvalCall(callNode, block, fn, isStrict, canUseSuper, state) { const varDefsNodes = []; for (const [varName, binding] of Object.entries(block.bindings)) { + // TODO: Capture `with` object if (varNamesUsed.has(varName)) continue; if (isStrict && varName !== 'this' && isReservedWord(varName)) continue; @@ -145,6 +146,7 @@ function instrumentEvalCall(callNode, block, fn, isStrict, canUseSuper, state) { if (blockIsExternalToFunction && varName !== 'new.target') { activateBinding(binding, varName); const externalVar = getOrCreateExternalVar(externalVars, block, varName, binding); + // TODO: Set `isNameLocked` externalVar.isReadFrom = true; if (!isConst) externalVar.isAssignedTo = true; } diff --git a/lib/instrument/visitors/function.js b/lib/instrument/visitors/function.js index 67fcb644..315dd5cb 100644 --- a/lib/instrument/visitors/function.js +++ b/lib/instrument/visitors/function.js @@ -728,6 +728,7 @@ function createFunctionInfoFunction(fn, scopes, astJson, fnInfoVarNode, state) { return { isReadFrom: varProps.isReadFrom || undefined, isAssignedTo: varProps.isAssignedTo || undefined, + isNameLocked: varProps.isNameLocked || undefined, trails: varProps.trails }; }) diff --git a/lib/instrument/visitors/identifier.js b/lib/instrument/visitors/identifier.js index e9d4dba8..88c1dcd0 100644 --- a/lib/instrument/visitors/identifier.js +++ b/lib/instrument/visitors/identifier.js @@ -16,7 +16,7 @@ module.exports = { // Imports const visitEval = require('./eval.js'), - {getOrCreateExternalVar, activateBlock, activateBinding} = require('../blocks.js'), + {getOrCreateExternalVar, activateBlock, activateBinding, createBlockTempVar} = require('../blocks.js'), {checkInternalVarNameClash} = require('../internalVars.js'), {createArrayOrPush} = require('../../shared/functions.js'), { @@ -81,7 +81,7 @@ function ThisExpression(node, state) { // Ignore if internal to function, unless in class constructor or prototype class property // of class with super class. if (block.id < fn.id) { - recordExternalVar(binding, block, 'this', fn, [...state.trail], true, false, state); + recordExternalVar(binding, block, 'this', fn, [...state.trail], true, false, false, state); } else if (fn.hasSuperClass) { createArrayOrPush(fn.internalVars, 'this', [...state.trail]); } @@ -105,7 +105,7 @@ function NewTargetExpression(node, state) { const block = state.currentThisBlock; if (block.id < fn.id) { recordExternalVar( - block.bindings['new.target'], block, 'new.target', fn, [...state.trail], true, false, state + block.bindings['new.target'], block, 'new.target', fn, [...state.trail], true, false, false, state ); } } @@ -148,7 +148,7 @@ function visitIdentifier(node, varName, isReadFrom, isAssignedTo, state) { function resolveIdentifierInSecondPass(node, block, varName, fn, isReadFrom, isAssignedTo, state) { state.secondPass( resolveIdentifier, - node, block, varName, fn, [...state.trail], isReadFrom, isAssignedTo, state.isStrict, state + node, block, varName, fn, [...state.trail], isReadFrom, isAssignedTo, false, state.isStrict, state ); } @@ -161,14 +161,25 @@ function resolveIdentifierInSecondPass(node, block, varName, fn, isReadFrom, isA * @param {Array} trail - Trail * @param {boolean} isReadFrom - `true` if variable is read from * @param {boolean} isAssignedTo - `true` if variable is assigned to + * @param {boolean} isBehindWith - `true` if variable may be shadowed by a `with () {}` statement * @param {boolean} isStrict - `true` if variable used in strict mode * @param {Object} state - State object * @returns {undefined} */ -function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssignedTo, isStrict, state) { +function resolveIdentifier( + node, block, varName, fn, trail, isReadFrom, isAssignedTo, isBehindWith, isStrict, state +) { // Find binding - let binding; + let binding, + isWithBinding = false; do { + // Check for `with () {}` block which can intercept any variable access + binding = block.bindings.with; + if (binding) { + isWithBinding = true; + break; + } + binding = block.bindings[varName]; } while (!binding && (block = block.parent)); // eslint-disable-line no-cond-assign @@ -183,10 +194,33 @@ function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssign // Record if internal var if (block.id >= fn.id) { + if (isWithBinding) { + // Continue searching for binding further down the scope chain + resolveIdentifier( + node, block.parent, varName, fn, trail, isReadFrom, isAssignedTo, true, isStrict, state + ); + return; + } + + // TODO: Freeze var name if behind `with () {}` if (!binding.isFunction && !binding.argNames) createArrayOrPush(fn.internalVars, varName, trail); return; } + // If is a `with () {}` block, activate block + // and continue to search for binding further down the scope chain + if (isWithBinding) { + activateBlock(block, state); + if (!binding.varNode) binding.varNode = createBlockTempVar(block, state); + const externalVar = getOrCreateExternalVar(fn.externalVars, block, 'with', binding); + externalVar.isReadFrom = true; + + resolveIdentifier( + node, block.parent, varName, fn, trail, isReadFrom, isAssignedTo, true, isStrict, state + ); + return; + } + // Record external var if (isAssignedTo && binding.isConst) { // Record const violation @@ -204,7 +238,7 @@ function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssign isAssignedTo = false; } - recordExternalVar(binding, block, varName, fn, trail, isReadFrom, isAssignedTo, state); + recordExternalVar(binding, block, varName, fn, trail, isReadFrom, isAssignedTo, isBehindWith, state); } /** @@ -216,14 +250,18 @@ function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssign * @param {Array} trail - Trail * @param {boolean} isReadFrom - `true` if variable is read from * @param {boolean} isAssignedTo - `true` if variable is assigned to + * @param {boolean} isBehindWith - `true` if variable may be shadowed by a `with () {}` statement * @param {Object} state - State object * @returns {undefined} */ -function recordExternalVar(binding, block, varName, fn, trail, isReadFrom, isAssignedTo, state) { +function recordExternalVar( + binding, block, varName, fn, trail, isReadFrom, isAssignedTo, isBehindWith, state +) { activateBlock(block, state); activateBinding(binding, varName); const externalVar = getOrCreateExternalVar(fn.externalVars, block, varName, binding); if (isReadFrom) externalVar.isReadFrom = true; if (isAssignedTo) externalVar.isAssignedTo = true; + if (isBehindWith) externalVar.isNameLocked = true; externalVar.trails.push(trail); } diff --git a/lib/instrument/visitors/statement.js b/lib/instrument/visitors/statement.js index 1122f4d4..130a7444 100644 --- a/lib/instrument/visitors/statement.js +++ b/lib/instrument/visitors/statement.js @@ -19,6 +19,7 @@ const VariableDeclaration = require('./variableDeclaration.js'), SwitchStatement = require('./switch.js'), TryStatement = require('./try.js'), ThrowStatement = require('./unary.js'), + WithStatement = require('./with.js'), {visitKey, visitKeyMaybe} = require('../visit.js'); // Exports @@ -71,13 +72,6 @@ function ReturnStatement(node, state) { visitKeyMaybe(node, 'argument', Expression, state); } -function WithStatement(node, state) { - // TODO: Maintain a state property `currentWithBlock` which can be used in `resolveBinding()` - // to flag functions which access a var which would be affected by `with` - visitKey(node, 'object', Expression, state); - visitKey(node, 'body', Statement, state); -} - function LabeledStatement(node, state) { visitKey(node, 'body', Statement, state); } diff --git a/lib/instrument/visitors/with.js b/lib/instrument/visitors/with.js new file mode 100644 index 00000000..300c0616 --- /dev/null +++ b/lib/instrument/visitors/with.js @@ -0,0 +1,58 @@ +/* -------------------- + * livepack module + * Code instrumentation visitor for `with` statements + * ------------------*/ + +'use strict'; + +// Export +module.exports = WithStatement; + +// Modules +const t = require('@babel/types'); + +// Imports +const Expression = require('./expression.js'), + Statement = require('./statement.js'), + {createAndEnterBlock, createBindingWithoutNameCheck} = require('../blocks.js'), + {createTrackerVarNode} = require('../internalVars.js'), + {visitKey} = require('../visit.js'); + +// Exports + +/** + * Visitor for `with () {}` statement. + * @param {Object} node - Statement AST node + * @param {Object} state - State object + * @returns {undefined} + */ +function WithStatement(node, state) { + // Visit object i.e. expression inside `with (...)` + visitKey(node, 'object', Expression, state); + + // Create block for `with` object + const parentBlock = state.currentBlock; + const block = createAndEnterBlock('with', false, state); + const binding = createBindingWithoutNameCheck(block, 'with', {isConst: true}, state); + + // Visit body + visitKey(node, 'body', Statement, state); + + // Exit block + state.currentBlock = parentBlock; + + // Queue action to wrap `with` object + state.secondPass(instrumentWithObj, node, binding, state); +} + +function instrumentWithObj(node, binding, state) { + // `with (o) {}` -> `with ( livepack_tracker.wrapWith(livepack_temp2 = o) ) {}` + node.object = t.callExpression( + t.memberExpression(createTrackerVarNode(state), t.identifier('wrapWith')), + [ + binding.varNode + ? t.assignmentExpression('=', binding.varNode, node.object) + : node.object + ] + ); +} diff --git a/lib/serialize/functions.js b/lib/serialize/functions.js index 36b92e57..a96ccb19 100644 --- a/lib/serialize/functions.js +++ b/lib/serialize/functions.js @@ -15,7 +15,9 @@ const util = require('util'), t = require('@babel/types'); // Imports -const {activateTracker, getTrackerResult, trackerError} = require('../shared/tracker.js'), +const { + activateTracker, getTrackerResult, trackerError, setIsGettingScope + } = require('../shared/tracker.js'), specialFunctions = require('../shared/internal.js').functions, { TRACKER_COMMENT_PREFIX, @@ -309,11 +311,13 @@ module.exports = { // Call `getScopes()` to get scope vars let scopeVars; if (!errorMessage) { + setIsGettingScope(true); try { scopeVars = getScopes(); } catch (err) { errorMessage = getErrorMessage(err); } + setIsGettingScope(false); } assertBug( diff --git a/lib/serialize/parseFunction.js b/lib/serialize/parseFunction.js index 3615ae1e..77e2abfc 100644 --- a/lib/serialize/parseFunction.js +++ b/lib/serialize/parseFunction.js @@ -81,7 +81,11 @@ module.exports = function parseFunction( blockName, vars: mapValues(vars, (varProps, varName) => { externalVars[varName] = []; - return {isReadFrom: !!varProps.isReadFrom, isAssignedTo: !!varProps.isAssignedTo}; + return { + isReadFrom: !!varProps.isReadFrom, + isAssignedTo: !!varProps.isAssignedTo, + isNameLocked: !!varProps.isNameLocked + }; }) }); } @@ -865,11 +869,15 @@ function resolveFunctionInfo( const {blockId} = scope; if (blockId < fnId) { // External var - for (const [varName, {isReadFrom, isAssignedTo, trails}] of Object.entries(scope.vars)) { + for ( + const [varName, {isReadFrom, isAssignedTo, isNameLocked, trails}] + of Object.entries(scope.vars) + ) { if (isNestedFunction) { const scopeDefVar = scopeDefs.get(blockId).vars[varName]; if (isReadFrom) scopeDefVar.isReadFrom = true; if (isAssignedTo) scopeDefVar.isAssignedTo = true; + if (isNameLocked) scopeDefVar.isNameLocked = true; } externalVars[varName].push(...trailsToNodes(fnNode, trails, varName)); diff --git a/lib/shared/tracker.js b/lib/shared/tracker.js index 60ef7a8f..8d677303 100644 --- a/lib/shared/tracker.js +++ b/lib/shared/tracker.js @@ -14,10 +14,13 @@ module.exports = { tracker, activateTracker, getTrackerResult, - trackerError + trackerError, + getIsGettingScope, + setIsGettingScope }; let trackerIsActive = false, + isGettingScope = false, trackerResult; /** @@ -62,3 +65,21 @@ function getTrackerResult() { trackerResult = undefined; return result || {getFnInfo: undefined, getScopes: undefined}; } + +/** + * Get whether currently getting scope vars for function. + * Used by `livepack_tracker.wrapWith()`. + * @returns {boolean} - `true` if currently getting scope vars + */ +function getIsGettingScope() { + return isGettingScope; +} + +/** + * Set whether currently getting scope vars for function. + * @param {boolean} isGetting - `true` if currently getting scope vars + * @returns {undefined} + */ +function setIsGettingScope(isGetting) { + isGettingScope = isGetting; +}