From efc78a9bf906ff94105c4ec751a472edcb510d97 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Wed, 22 Nov 2023 00:51:24 +0000 Subject: [PATCH] WIP 5 --- TODO.md | 17 +++++++++++++++++ lib/instrument/visitors/function.js | 20 ++++++++++++++++---- lib/instrument/visitors/identifier.js | 25 ++++++++++++++++++------- lib/instrument/visitors/super.js | 3 ++- lib/serialize/parseFunction.js | 15 +++++++++------ lib/shared/constants.js | 6 +++--- 6 files changed, 65 insertions(+), 21 deletions(-) diff --git a/TODO.md b/TODO.md index 388abdd3..aecbe2c0 100644 --- a/TODO.md +++ b/TODO.md @@ -14,3 +14,20 @@ * Tests for binding in higher scope which is shadowed still not being renamed e.g. `() => { let x = 1; { let x; eval('typeof a !== "undefined" && a = 2'); } return x; }` - make sure param `x` not renamed to `a` * Make sure this includes vars which are illegal to access inside `eval()` if `eval()` is strict mode. +* Test for const violations with frozen vars when internal to function: +```js +module.exports = function(module, exports) { + const x = 1; + eval('x'); + return () => { x = 2; }; +}; +``` +Also where another binding which isn't frozen: +```js +module.exports = function(module, exports) { + const x = 1; + eval('x'); + { const x = 2; } + return () => { x = 2; }; +}; +``` diff --git a/lib/instrument/visitors/function.js b/lib/instrument/visitors/function.js index 72b001ea..36b5d3f6 100644 --- a/lib/instrument/visitors/function.js +++ b/lib/instrument/visitors/function.js @@ -43,7 +43,8 @@ const Statement = require('./statement.js'), {combineArraysWithDedup} = require('../../shared/functions.js'), { FN_TYPE_FUNCTION, FN_TYPE_ASYNC_FUNCTION, FN_TYPE_GENERATOR_FUNCTION, - FN_TYPE_ASYNC_GENERATOR_FUNCTION, TRACKER_COMMENT_PREFIX + FN_TYPE_ASYNC_GENERATOR_FUNCTION, TRACKER_COMMENT_PREFIX, + CONST_VIOLATION_NEEDS_VAR, CONST_VIOLATION_NEEDS_NO_VAR } = require('../../shared/constants.js'); // Exports @@ -740,6 +741,19 @@ function createFunctionInfoFunction(fn, state) { } } + // Compile amendments. + // Reverse order so deepest are first. + // Remove the need for an internal var if binding has frozen name. + let amendments; + if (fn.amendments.length > 0) { + amendments = fn.amendments.map(({type, blockId, trail, binding}) => { + if (type === CONST_VIOLATION_NEEDS_VAR && binding.isFrozenName) { + type = CONST_VIOLATION_NEEDS_NO_VAR; + } + return [type, blockId, ...trail]; + }).reverse(); + } + // Create JSON function info string const {children} = fn; let argNames; @@ -767,9 +781,7 @@ function createFunctionInfoFunction(fn, state) { internalVars, reservedVarNames: reservedVarNames.size !== 0 ? [...reservedVarNames] : undefined, globalVarNames: fn.globalVarNames.size !== 0 ? [...fn.globalVarNames] : undefined, - amendments: fn.amendments.length !== 0 - ? fn.amendments.map(({type, blockId, trail}) => [type, blockId, ...trail]).reverse() - : undefined, + amendments, hasSuperClass: fn.hasSuperClass || undefined, firstSuperStatementIndex: fn.firstSuperStatementIndex, returnsSuper: fn.returnsSuper || undefined, diff --git a/lib/instrument/visitors/identifier.js b/lib/instrument/visitors/identifier.js index 120898e4..a4fb9d55 100644 --- a/lib/instrument/visitors/identifier.js +++ b/lib/instrument/visitors/identifier.js @@ -19,7 +19,7 @@ const visitEval = require('./eval.js'), {getOrCreateExternalVar, activateBlock, activateBinding} = require('../blocks.js'), {checkInternalVarNameClash} = require('../internalVars.js'), { - CONST_VIOLATION_CONST, CONST_VIOLATION_FUNCTION_THROWING, CONST_VIOLATION_FUNCTION_SILENT + CONST_VIOLATION_NEEDS_VAR, CONST_VIOLATION_NEEDS_NO_VAR, CONST_VIOLATION_SILENT } = require('../../shared/constants.js'); // Exports @@ -173,15 +173,26 @@ function resolveIdentifier(node, block, varName, fn, trail, isReadFrom, isAssign // Record external var if (isAssignedTo && binding.isConst) { - // Record const violation + // Record const violation. + // `CONST_VIOLATION_NEEDS_VAR` violation type is for if this const violation is output + // in a parent function where the const binding is internal to the function, + // it will need to be added to `internalVars`. + // This is not the case if var is being read from, as an external var is created below, + // which will be converted to an internal var. + // It's also not needed if binding is a function name or frozen var, as they're not renamed + // and so aren't treated as internal vars. + // Not possible to know at this point whether binding is frozen or not (an `eval()` might be later + // in the function), so store the binding, and type will be changed to `CONST_VIOLATION_NEEDS_NO_VAR` + // later on if the binding is frozen. fn.amendments.push({ type: binding.isSilentConst && !isStrict - ? CONST_VIOLATION_FUNCTION_SILENT - : binding.isFunction - ? CONST_VIOLATION_FUNCTION_THROWING - : CONST_VIOLATION_CONST, + ? CONST_VIOLATION_SILENT + : isReadFrom || binding.isFrozenName || binding.isFunction + ? CONST_VIOLATION_NEEDS_NO_VAR + : CONST_VIOLATION_NEEDS_VAR, blockId: block.id, - trail + trail, + binding }); if (!isReadFrom) return; diff --git a/lib/instrument/visitors/super.js b/lib/instrument/visitors/super.js index 7d2123cb..2882384c 100644 --- a/lib/instrument/visitors/super.js +++ b/lib/instrument/visitors/super.js @@ -206,7 +206,8 @@ function recordAmendmentForSuper(node, superBlock, isSuperCall, fn, trail) { fn.amendments.push({ type: isSuperCall ? SUPER_CALL : SUPER_EXPRESSION, blockId: superBlock.id, - trail + trail, + binding: undefined // Not used, but keeps same object shape for all amendments }); } diff --git a/lib/serialize/parseFunction.js b/lib/serialize/parseFunction.js index 6c53ee98..0e638fe6 100644 --- a/lib/serialize/parseFunction.js +++ b/lib/serialize/parseFunction.js @@ -18,7 +18,7 @@ const {isJsIdentifier, isNumberKey, setAddFrom} = require('./utils.js'), getProp, getProps, setProp, traverseAll } = require('../shared/functions.js'), { - SUPER_CALL, SUPER_EXPRESSION, CONST_VIOLATION_CONST, CONST_VIOLATION_FUNCTION_SILENT + SUPER_CALL, SUPER_EXPRESSION, CONST_VIOLATION_SILENT, CONST_VIOLATION_NEEDS_VAR } = require('../shared/constants.js'), assertBug = require('../shared/assertBug.js'); @@ -338,10 +338,10 @@ module.exports = function parseFunction( replaceSuperExpression( trail, trailNodes, superVarNode, thisVarNode, superIsProto, internalVars, copyLoc ); - } else if (amendmentType === CONST_VIOLATION_FUNCTION_SILENT) { + } else if (amendmentType === CONST_VIOLATION_SILENT) { replaceConstViolation(trail, trailNodes, true, internalVars, copyLoc); } else { - // amendmentType === CONST_VIOLATION_CONST || amendmentType === CONST_VIOLATION_FUNCTION_THROWING + // A non-silent const violation replaceConstViolation(trail, trailNodes, false, internalVars, copyLoc); } } @@ -916,10 +916,13 @@ function resolveFunctionInfo( for (const [type, blockId, ...trail] of thisAmendments) { if (blockId < fnId) { amendments.push({type, trail, trailNodes: getProps(fnNode, trail)}); - } else if (type === CONST_VIOLATION_CONST) { + } else if (type === CONST_VIOLATION_NEEDS_VAR) { // Const violation where var is internal to function. Add to internal vars instead. - // Ignore CONST_VIOLATION_FUNCTION_THROWING and CONST_VIOLATION_FUNCTION_SILENT violation types - // because they refer to function names, which are not treated as internal vars. + // If var is frozen, no internal var is needed. In these cases, type will be + // `CONST_VIOLATION_SILENT` or `CONST_VIOLATION_NEEDS_NO_VAR`. + // If violation is read-write (e.g. `x += 1`), then it would also appear in `externalVars` + // so will have been converted to an internal var already above. Type will be + // `CONST_VIOLATION_NEEDS_NO_VAR`. const node = getProp(fnNode, trail); internalVars[node.name].push(node); } diff --git a/lib/shared/constants.js b/lib/shared/constants.js index 5ef48dec..9b615ec8 100644 --- a/lib/shared/constants.js +++ b/lib/shared/constants.js @@ -27,9 +27,9 @@ module.exports = { FN_TYPE_CLASS: 'c', SUPER_CALL: 1, SUPER_EXPRESSION: 2, - CONST_VIOLATION_CONST: 3, - CONST_VIOLATION_FUNCTION_THROWING: 4, - CONST_VIOLATION_FUNCTION_SILENT: 5, + CONST_VIOLATION_NEEDS_VAR: 3, + CONST_VIOLATION_NEEDS_NO_VAR: 4, + CONST_VIOLATION_SILENT: 5, GLOBAL: 1, MODULE: 2, VALUE: 3,