From be9bbe429f1ff851e1d54fc331a4c416b4cea31a Mon Sep 17 00:00:00 2001 From: Harry Altman Date: Thu, 21 May 2020 23:58:09 -0400 Subject: [PATCH] Unify Solidity/Yul variable handling --- packages/codec/lib/ast/utils.ts | 11 + packages/debugger/lib/data/actions/index.js | 24 +- packages/debugger/lib/data/reducers.js | 216 +++++------------- packages/debugger/lib/data/sagas/index.js | 47 ++-- packages/debugger/lib/data/selectors/index.js | 103 +++------ 5 files changed, 127 insertions(+), 274 deletions(-) diff --git a/packages/codec/lib/ast/utils.ts b/packages/codec/lib/ast/utils.ts index 8b8b1fbe88c..560b8807953 100644 --- a/packages/codec/lib/ast/utils.ts +++ b/packages/codec/lib/ast/utils.ts @@ -23,6 +23,10 @@ export function typeString(definition: AstNode): string { * @category Definition Reading */ export function typeStringWithoutLocation(definition: AstNode): string { + if (definition.nodeType === "YulTypedName") { + //for handling Yul variables + return "bytes32"; + } return typeString(definition).replace( / (storage|memory|calldata)( slice)?$/, "" @@ -37,6 +41,10 @@ export function typeStringWithoutLocation(definition: AstNode): string { * @category Definition Reading */ export function typeClass(definition: AstNode): string { + if (definition.nodeType === "YulTypedName") { + //for handling Yul variables + return "bytes"; + } return typeIdentifier(definition).match(/t_([^$_0-9]+)/)[1]; } @@ -85,6 +93,9 @@ export function visibility(definition: AstNode): Common.Visibility { * @category Definition Reading */ export function specifiedSize(definition: AstNode): number { + if (definition.nodeType === "YulTypedName") { + return 32; //for handling Yul variables + } let specified = typeIdentifier(definition).match(/t_[a-z]+([0-9]+)/); if (!specified) { diff --git a/packages/debugger/lib/data/actions/index.js b/packages/debugger/lib/data/actions/index.js index 89f08e985b4..fef068f05bc 100644 --- a/packages/debugger/lib/data/actions/index.js +++ b/packages/debugger/lib/data/actions/index.js @@ -11,28 +11,12 @@ export function scope(id, pointer, parentId, sourceId, compilationId) { } export const DECLARE = "DATA_DECLARE_VARIABLE"; -export function declare(node, compilationId) { +export function declare(name, idOrPath, scopeIdOrPath, compilationId) { return { type: DECLARE, - node, - compilationId - }; -} - -export const YUL_DECLARE = "DATA_YUL_DECLARE"; -export function yulDeclare( - node, - pointer, - scopePointer, - sourceId, - compilationId -) { - return { - type: YUL_DECLARE, - node, - pointer, - scopePointer, - sourceId, + name, + idOrPath, + scopeIdOrPath, compilationId }; } diff --git a/packages/debugger/lib/data/reducers.js b/packages/debugger/lib/data/reducers.js index b7d1a0c6f2a..53b12c876d3 100644 --- a/packages/debugger/lib/data/reducers.js +++ b/packages/debugger/lib/data/reducers.js @@ -19,145 +19,66 @@ function scopes(state = DEFAULT_SCOPES, action) { switch (action.type) { case actions.SCOPE: { - let { compilationId, id, sourceId, parentId, pointer } = action; - if (id !== null) { - newState = { - byCompilationId: { - ...state.byCompilationId, - [compilationId]: { - ...state.byCompilationId[compilationId] //just setting this up to avoid errors later - } - } - }; - - //apologies for this multi-stage setup, but JS is like that... - - newState.byCompilationId[compilationId] = { - ...newState.byCompilationId[compilationId], - byId: { - ...newState.byCompilationId[compilationId].byId - } - }; - - scope = newState.byCompilationId[compilationId].byId[id]; - - newState.byCompilationId[compilationId].byId[id] = { - ...scope, - id, - sourceId, - parentId, //may be null - pointer, - compilationId - }; - - return newState; - } else { - debug("scoping by pointer!"); - newState = { - byCompilationId: { - ...state.byCompilationId, - [compilationId]: { - ...state.byCompilationId[compilationId] //just setting this up to avoid errors later - } - } - }; - - //apologies for this multi-stage setup, but JS is like that... - - newState.byCompilationId[compilationId] = { - ...newState.byCompilationId[compilationId], - bySourceAndPointer: { - ...newState.byCompilationId[compilationId].bySourceAndPointer - } - }; - - let sourceAndPointer = sourceId + ":" + pointer; - debug("sourceAndPointer: %s", sourceAndPointer); - - scope = - newState.byCompilationId[compilationId].bySourceAndPointer[ - sourceAndPointer - ]; - - newState.byCompilationId[compilationId].bySourceAndPointer[ - sourceAndPointer - ] = { - ...scope, - parentId, //is usually undefined - sourceId, - pointer, - compilationId - }; - - return newState; - } - } - - case actions.DECLARE: { - let { compilationId, node } = action; - - //note: we can assume the compilation already exists! - scope = - state.byCompilationId[compilationId].byId[action.node.scope] || {}; - variables = scope.variables || []; + const { compilationId, id, sourceId, parentId, pointer } = action; + const idOrPath = id !== undefined ? id : sourceId + ":" + pointer; - return { + newState = { byCompilationId: { ...state.byCompilationId, [compilationId]: { - ...state.byCompilationId[compilationId], - byId: { - ...state.byCompilationId[compilationId].byId, - - [node.scope]: { - ...scope, + ...state.byCompilationId[compilationId] //just setting this up to avoid errors later + } + } + }; - variables: [ - ...variables, + //apologies for this multi-stage setup, but JS is like that... - { - name: node.name, - id: node.id, - compilationId - } - ] - } - } - } + newState.byCompilationId[compilationId] = { + ...newState.byCompilationId[compilationId], + byIdOrPath: { + ...newState.byCompilationId[compilationId].byIdOrPath } }; + + scope = newState.byCompilationId[compilationId].byIdOrPath[idOrPath]; + + newState.byCompilationId[compilationId].byIdOrPath[idOrPath] = { + ...scope, + id, + sourceId, + parentId, //may be null or undefined + pointer, + compilationId + }; + + return newState; } - case actions.YUL_DECLARE: { - debug("yul declaration!"); - let { node, pointer, scopePointer, sourceId, compilationId } = action; + case actions.DECLARE: { + let { compilationId, name, idOrPath, scopeIdOrPath } = action; //note: we can assume the compilation already exists! - let sourceAndPointer = sourceId + ":" + scopePointer; scope = - state.byCompilationId[compilationId].bySourceAndPointer[ - sourceAndPointer - ] || {}; + state.byCompilationId[compilationId].byIdOrPath[scopeIdOrPath] || {}; variables = scope.variables || []; - debug("node: %o", node); return { byCompilationId: { ...state.byCompilationId, [compilationId]: { ...state.byCompilationId[compilationId], - bySourceAndPointer: { - ...state.byCompilationId[compilationId].bySourceAndPointer, + byIdOrPath: { + ...state.byCompilationId[compilationId].byIdOrPath, - [sourceAndPointer]: { + [scopeIdOrPath]: { ...scope, variables: [ ...variables, { - name: node.name, - sourceAndPointer: sourceId + ":" + pointer, + name, + idOrPath, compilationId } ] @@ -247,59 +168,32 @@ function assignments(state = DEFAULT_ASSIGNMENTS, action) { debug("action.type %O", action.type); debug("action.assignments %O", action.assignments); return Object.values(action.assignments).reduce((acc, assignment) => { - let { id, astId, sourceAndPointer, compilationId } = assignment; + let { id, idOrPath, compilationId } = assignment; //we assume for now that only ordinary variables will be assigned this //way, and not globals; globals are handled in DEFAULT_ASSIGNMENTS - if (astId !== undefined) { - return { - ...acc, - byId: { - ...acc.byId, - [id]: assignment - }, - byCompilationId: { - ...acc.byCompilationId, - [compilationId]: { - ...acc.byCompilationId[compilationId], - byAstId: { - ...(acc.byCompilationId[compilationId] || {}).byAstId, - [astId]: [ - ...new Set([ - ...(((acc.byCompilationId[compilationId] || {}).byAstId || - {})[astId] || []), - id - ]) - ] - } - } - } - }; - } else { - return { - ...acc, - byId: { - ...acc.byId, - [id]: assignment - }, - byCompilationId: { - ...acc.byCompilationId, - [compilationId]: { - ...acc.byCompilationId[compilationId], - bySourceAndPointer: { - ...(acc.byCompilationId[compilationId] || {}) - .bySourceAndPointer, - [sourceAndPointer]: [ - ...new Set([ - ...(((acc.byCompilationId[compilationId] || {}) - .bySourceAndPointer || {})[sourceAndPointer] || []), - id - ]) - ] - } + return { + ...acc, + byId: { + ...acc.byId, + [id]: assignment + }, + byCompilationId: { + ...acc.byCompilationId, + [compilationId]: { + ...acc.byCompilationId[compilationId], + byIdOrPath: { + ...(acc.byCompilationId[compilationId] || {}).byIdOrPath, + [idOrPath]: [ + ...new Set([ + ...(((acc.byCompilationId[compilationId] || {}) + .byIdOrPath || {})[idOrPath] || []), + id + ]) + ] } } - }; - } + } + }; }, state); case actions.RESET: diff --git a/packages/debugger/lib/data/sagas/index.js b/packages/debugger/lib/data/sagas/index.js index 326606a4644..10405a38773 100644 --- a/packages/debugger/lib/data/sagas/index.js +++ b/packages/debugger/lib/data/sagas/index.js @@ -24,11 +24,13 @@ export function* scope(nodeId, pointer, parentId, sourceId, compilationId) { } export function* declare(node, compilationId) { - yield put(actions.declare(node, compilationId)); + yield put(actions.declare(node.name, node.id, node.scope, compilationId)); } export function* yulScope(pointer, sourceId, compilationId, parentId) { - yield put(actions.scope(null, pointer, parentId, sourceId, compilationId)); + yield put( + actions.scope(undefined, pointer, parentId, sourceId, compilationId) + ); } export function* yulDeclare( @@ -39,7 +41,12 @@ export function* yulDeclare( compilationId ) { yield put( - actions.yulDeclare(node, pointer, scopePointer, sourceId, compilationId) + actions.declare( + node.name, + sourceId + ":" + pointer, + sourceId + ":" + scopePointer, + compilationId + ) ); } @@ -322,11 +329,15 @@ function* variablesAndMappingsSaga() { inModifier ? { compilationId, - sourceAndPointer, + idOrPath: sourceAndPointer, stackframe: currentDepth, modifierDepth } - : { compilationId, sourceAndPointer, stackframe: currentDepth }, + : { + compilationId, + idOrPath: sourceAndPointer, + stackframe: currentDepth + }, { location: "stack", from: position, //all Yul variables are size 1 @@ -348,7 +359,7 @@ function* variablesAndMappingsSaga() { assignments = {}; for (let id in allocation.members) { id = Number(id); //not sure why we're getting them as strings, but... - let idObj = { compilationId, astId: id, address }; + let idObj = { compilationId, idOrPath: id, address }; let fullId = stableKeccak256(idObj); //we don't use makeAssignment here as we had to compute the ID anyway assignment = { @@ -397,11 +408,11 @@ function* variablesAndMappingsSaga() { inModifier ? { compilationId, - astId: varId, + idOrPath: varId, stackframe: currentDepth, modifierDepth } - : { compilationId, astId: varId, stackframe: currentDepth }, + : { compilationId, idOrPath: varId, stackframe: currentDepth }, { location: "stack", from: top - Codec.Ast.Utils.stackSize(node) + 1, @@ -448,13 +459,13 @@ function* variablesAndMappingsSaga() { inModifier ? { compilationId, - sourceAndPointer: variableSourceAndPointer, + idOrPath: variableSourceAndPointer, stackframe: currentDepth, modifierDepth } : { compilationId, - sourceAndPointer: variableSourceAndPointer, + idOrPath: variableSourceAndPointer, stackframe: currentDepth }, { @@ -721,11 +732,11 @@ function* decodeMappingKeyCore(indexDefinition, keyDefinition) { let indexIdObj = inModifier ? { compilationId, - astId: indexId, + idOrPath: indexId, stackframe: currentDepth, modifierDepth } - : { compilationId, astId: indexId, stackframe: currentDepth }; + : { compilationId, idOrPath: indexId, stackframe: currentDepth }; let fullIndexId = stableKeccak256(indexIdObj); const indexReference = (currentAssignments.byId[fullIndexId] || {}).ref; @@ -900,11 +911,11 @@ function literalAssignments( inModifier ? { compilationId, - astId: node.id, + idOrPath: node.id, stackframe: currentDepth, modifierDepth } - : { compilationId, astId: node.id, stackframe: currentDepth }, + : { compilationId, idOrPath: node.id, stackframe: currentDepth }, { location: "stackliteral", literal } ); @@ -938,11 +949,11 @@ function assignParameters( forModifier ? { compilationId, - astId: parameter.id, + idOrPath: parameter.id, stackframe: functionDepth, modifierDepth } - : { compilationId, astId: parameter.id, stackframe: functionDepth }, + : { compilationId, idOrPath: parameter.id, stackframe: functionDepth }, pointer ); assignments[assignment.id] = assignment; @@ -964,13 +975,13 @@ function fetchBasePath( inModifier ? { compilationId, - astId: baseNode.id, + idOrPath: baseNode.id, stackframe: currentDepth, modifierDepth } : { compilationId, - astId: baseNode.id, + idOrPath: baseNode.id, stackframe: currentDepth } ); diff --git a/packages/debugger/lib/data/selectors/index.js b/packages/debugger/lib/data/selectors/index.js index a3a68c2c9fa..d5aecfacb84 100644 --- a/packages/debugger/lib/data/selectors/index.js +++ b/packages/debugger/lib/data/selectors/index.js @@ -224,7 +224,7 @@ const data = createSelectorTree({ ) .filter( variable => - inlined[compilationId][variable.id].definition + inlined[compilationId][variable.idOrPath].definition .visibility !== "private" //filter out private variables from the base classes ) @@ -236,7 +236,7 @@ const data = createSelectorTree({ //how to read. they'll just clutter things up. debug("variable %O", variable); let definition = - inlined[compilationId][variable.id].definition; + inlined[compilationId][variable.idOrPath].definition; return ( !definition.constant || Codec.Ast.Utils.isSimpleConstant(definition.value) @@ -256,18 +256,13 @@ const data = createSelectorTree({ * * the raw scopes data, just with intermediate * layers cut out - * - * ...and Solidity and Yul grouped together, HACK */ raw: createLeaf(["/info/scopes"], scopes => Object.assign( {}, ...Object.entries(scopes.byCompilationId).map( - ([ - compilationId, - { byId: nodes, bySourceAndPointer: yulNodes } - ]) => ({ - [compilationId]: { ...nodes, ...yulNodes } + ([compilationId, { byIdOrPath: nodes }]) => ({ + [compilationId]: { ...nodes } }) ) ) @@ -1049,8 +1044,7 @@ const data = createSelectorTree({ * data.current.identifiers (selector) * * returns identifers and corresponding definition node ID or builtin name - * (object entries look like [name]: {astId: id}, [name]: {builtin: name}, - * or [name]: {sourceAndPointer: sourceId + ":" + pointer}) + * (object entries look like [name]: {idOrPath: idOrPath}, [name]: {builtin: name}) */ _: createLeaf( [ @@ -1073,10 +1067,7 @@ const data = createSelectorTree({ .filter(variable => variable.name !== "") //exclude anonymous output params .filter(variable => variables[variable.name] == undefined) //don't add shadowed vars .map(variable => ({ - [variable.name]: - variable.id !== undefined - ? { astId: variable.id } - : { sourceAndPointer: variable.sourceAndPointer } + [variable.name]: { idOrPath: variable.idOrPath } })) ); //NOTE: because these assignments are processed in order, that means @@ -1135,26 +1126,11 @@ const data = createSelectorTree({ let variables = Object.assign( {}, ...Object.entries(identifiers).map(([identifier, variable]) => { - if (variable.astId !== undefined) { - let { definition } = scopes[variable.astId]; + if (variable.idOrPath !== undefined) { + let { definition } = scopes[variable.idOrPath]; return { [identifier]: definition }; - } else if (variable.sourceAndPointer !== undefined) { - let { definition } = scopes[variable.sourceAndPointer]; - //for Yul variables - let splicedDefinition = { - ...definition, - id: -1, - nodeType: "VariableDeclaration", - //NOTE: the actual only-existing type in Yul at the moment is u256, - //which officially corresponds to uint256, not bytes32. - //However, I think it makes more sense at the moment to treat it as - //bytes32 despite that, so that's what I'm doing. - typeDescriptions: { - typeIdentifier: "t_bytes32", - typeString: "bytes32" - } - }; - return { [identifier]: splicedDefinition }; + //there used to be separate code for Yul variables here, + //but now that's handled in definitionToType } else { return {}; //skip over builtins; we'll handle those separately } @@ -1220,25 +1196,18 @@ const data = createSelectorTree({ Object.assign( {}, ...Object.entries(identifiers).map( - ([identifier, { astId, sourceAndPointer, builtin }]) => { + ([identifier, { idOrPath, builtin }]) => { let id; - debug("astId: %d", astId); + debug("idOrPath: %o", idOrPath); debug("builtin: %s", builtin); //is this an ordinary variable or a builtin? - if (astId !== undefined || sourceAndPointer !== undefined) { + if (idOrPath !== undefined) { //if not a builtin, first check if it's a contract var let compilationAssignments = - assignments.byCompilationId[compilationId] || {}; - let solidityAssignments = - compilationAssignments.byAstId || {}; - let yulAssignments = - compilationAssignments.bySourcAndPointer || {}; - id = ( - solidityAssignments[astId] || - yulAssignments[sourceAndPointer] || - [] - ).find( + (assignments.byCompilationId[compilationId] || {}) + .byIdOrPath || {}; + id = (compilationAssignments[idOrPath] || []).find( idHash => assignments.byId[idHash].address === address ); debug("id after global: %s", id); @@ -1247,35 +1216,18 @@ const data = createSelectorTree({ if (id === undefined) { //if we're in a modifier, include modifierDepth if (inModifier) { - if (astId !== undefined) { - id = stableKeccak256({ - astId, - compilationId, - stackframe: currentDepth, - modifierDepth - }); - } else { - id = stableKeccak256({ - sourceAndPointer, - compilationId, - stackframe: currentDepth, - modifierDepth - }); - } + id = stableKeccak256({ + idOrPath, + compilationId, + stackframe: currentDepth, + modifierDepth + }); } else { - if (astId !== undefined) { - id = stableKeccak256({ - astId, - compilationId, - stackframe: currentDepth - }); - } else { - id = stableKeccak256({ - sourceAndPointer, - compilationId, - stackframe: currentDepth - }); - } + id = stableKeccak256({ + idOrPath, + compilationId, + stackframe: currentDepth + }); } } debug("id after local: %s", id); @@ -1283,6 +1235,7 @@ const data = createSelectorTree({ //otherwise, it's a builtin //NOTE: for now we assume there is only one assignment per //builtin, but this will change in the future + debug("builtin: %s", builtin); id = assignments.byBuiltin[builtin][0]; }