From 7a41a5249fcb8457cb8beb823ce9a1362cdbfa98 Mon Sep 17 00:00:00 2001 From: perissology Date: Thu, 14 Jun 2018 11:17:04 -0700 Subject: Collect coverage for provided sources When solidity generates source maps during contract compilation, the contracts are referred to by an id, which corresponds to an array index. We may not want to cover all sources that were included in a compilation, but because we use array indexes (vs. the id that is provided by solidity compiler) to map the contract to the sourceMap, the provided sourceCodes array must include the code at the correct index. This can result in empty slots in the sourceCodes array. This commit allows the coverage to only be collected for the contracts with provided sourceCode. --- packages/sol-cov/src/collect_coverage_entries.ts | 2 +- packages/sol-cov/src/coverage_subprovider.ts | 6 ++++++ packages/sol-cov/src/source_maps.ts | 5 ++++- 3 files changed, 11 insertions(+), 2 deletions(-) (limited to 'packages/sol-cov/src') diff --git a/packages/sol-cov/src/collect_coverage_entries.ts b/packages/sol-cov/src/collect_coverage_entries.ts index b145f044e..3fc85008c 100644 --- a/packages/sol-cov/src/collect_coverage_entries.ts +++ b/packages/sol-cov/src/collect_coverage_entries.ts @@ -10,7 +10,7 @@ const coverageEntriesBySourceHash: { [sourceHash: string]: CoverageEntriesDescri export const collectCoverageEntries = (contractSource: string) => { const sourceHash = ethUtil.sha3(contractSource).toString('hex'); - if (_.isUndefined(coverageEntriesBySourceHash[sourceHash])) { + if (_.isUndefined(coverageEntriesBySourceHash[sourceHash]) && !_.isUndefined(contractSource)) { const ast = parser.parse(contractSource, { range: true }); const locationByOffset = getLocationByOffset(contractSource); const visitor = new ASTVisitor(locationByOffset); diff --git a/packages/sol-cov/src/coverage_subprovider.ts b/packages/sol-cov/src/coverage_subprovider.ts index 0fa7f873e..2f0bcbb93 100644 --- a/packages/sol-cov/src/coverage_subprovider.ts +++ b/packages/sol-cov/src/coverage_subprovider.ts @@ -66,6 +66,12 @@ export const coverageHandler: SingleFileSubtraceHandler = ( ): Coverage => { const absoluteFileName = contractData.sources[fileIndex]; const coverageEntriesDescription = collectCoverageEntries(contractData.sourceCodes[fileIndex]); + + // if the source wasn't provided for the fileIndex, we can't cover the file + if (_.isUndefined(coverageEntriesDescription)) { + return {}; + } + let sourceRanges = _.map(subtrace, structLog => pcToSourceRange[structLog.pc]); sourceRanges = _.compact(sourceRanges); // Some PC's don't map to a source range and we just ignore them. // By default lodash does a shallow object comparasion. We JSON.stringify them and compare as strings. diff --git a/packages/sol-cov/src/source_maps.ts b/packages/sol-cov/src/source_maps.ts index f9503e16c..c95c7d2b0 100644 --- a/packages/sol-cov/src/source_maps.ts +++ b/packages/sol-cov/src/source_maps.ts @@ -12,6 +12,9 @@ export interface SourceLocation { } export function getLocationByOffset(str: string): LocationByOffset { + if (_.isUndefined(str)) { + return {}; + } const locationByOffset: LocationByOffset = { 0: { line: 1, column: 0 } }; let currentOffset = 0; for (const char of str.split('')) { @@ -56,7 +59,7 @@ export function parseSourceMap( length, fileIndex, }; - if (parsedEntry.fileIndex !== -1) { + if (parsedEntry.fileIndex !== -1 && !_.isUndefined(locationByOffsetByFileIndex[parsedEntry.fileIndex])) { const sourceRange = { location: { start: locationByOffsetByFileIndex[parsedEntry.fileIndex][parsedEntry.offset], -- cgit v1.2.3 From 613a78bcf62c10d27d7def8f0ff54efadc2faefb Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Mon, 18 Jun 2018 17:28:38 -0700 Subject: Include source code snippets in revert stack traces --- packages/sol-cov/src/get_source_range_snippet.ts | 179 +++++++++++++++++++++++ packages/sol-cov/src/revert_trace_subprovider.ts | 63 ++++++-- packages/sol-cov/src/types.ts | 10 ++ packages/sol-cov/src/utils.ts | 6 + 4 files changed, 249 insertions(+), 9 deletions(-) create mode 100644 packages/sol-cov/src/get_source_range_snippet.ts (limited to 'packages/sol-cov/src') diff --git a/packages/sol-cov/src/get_source_range_snippet.ts b/packages/sol-cov/src/get_source_range_snippet.ts new file mode 100644 index 000000000..48c74dd27 --- /dev/null +++ b/packages/sol-cov/src/get_source_range_snippet.ts @@ -0,0 +1,179 @@ +import * as ethUtil from 'ethereumjs-util'; +import * as _ from 'lodash'; +import * as Parser from 'solidity-parser-antlr'; + +import { SingleFileSourceRange, SourceRange, SourceSnippet } from './types'; +import { utils } from './utils'; + +interface ASTInfo { + type: string; + node: Parser.ASTNode; + name: string | null; + range: SingleFileSourceRange; +} + +// Parsing source code for each transaction/code is slow and therefore we cache it +const parsedSourceByHash: { [sourceHash: string]: Parser.ASTNode } = {}; + +export function getSourceRangeSnippet(sourceRange: SourceRange, sourceCode: string): SourceSnippet | null { + const sourceHash = ethUtil.sha3(sourceCode).toString('hex'); + if (_.isUndefined(parsedSourceByHash[sourceHash])) { + parsedSourceByHash[sourceHash] = Parser.parse(sourceCode, { loc: true }); + } + const astNode = parsedSourceByHash[sourceHash]; + const visitor = new ASTInfoVisitor(); + Parser.visit(astNode, visitor); + const astInfo = visitor.getASTInfoForRange(sourceRange); + if (astInfo === null) { + return null; + } + const sourceCodeInRange = utils.getRange(sourceCode, sourceRange.location); + return { + ...astInfo, + source: sourceCodeInRange, + fileName: sourceRange.fileName, + }; +} + +// A visitor which collects ASTInfo for contract definitions, function +// definitions, and other types of statements. +class ASTInfoVisitor { + private _astInfos: ASTInfo[] = []; + public getASTInfoForRange(sourceRange: SourceRange): ASTInfo | null { + // HACK(albrow): Sometimes the source range doesn't exactly match that + // of astInfo. To work around that we try with a +/-1 offset on + // end.column. If nothing matches even with the offset, we return null. + const offset = { + start: { + line: 0, + column: 0, + }, + end: { + line: 0, + column: 0, + }, + }; + let astInfo = this._getASTInfoForRange(sourceRange, offset); + if (astInfo !== null) { + return astInfo; + } + offset.end.column += 1; + astInfo = this._getASTInfoForRange(sourceRange, offset); + if (astInfo !== null) { + return astInfo; + } + offset.end.column -= 2; + astInfo = this._getASTInfoForRange(sourceRange, offset); + if (astInfo !== null) { + return astInfo; + } + return null; + } + public ContractDefinition(ast: Parser.ContractDefinition): void { + this._visitContractDefinition(ast); + } + public IfStatement(ast: Parser.IfStatement): void { + this._visitStatement(ast); + } + public FunctionDefinition(ast: Parser.FunctionDefinition): void { + this._visitFunctionLikeDefinition(ast); + } + public ModifierDefinition(ast: Parser.ModifierDefinition): void { + this._visitFunctionLikeDefinition(ast); + } + public ForStatement(ast: Parser.ForStatement): void { + this._visitStatement(ast); + } + public ReturnStatement(ast: Parser.ReturnStatement): void { + this._visitStatement(ast); + } + public BreakStatement(ast: Parser.BreakStatement): void { + this._visitStatement(ast); + } + public ContinueStatement(ast: Parser.ContinueStatement): void { + this._visitStatement(ast); + } + public EmitStatement(ast: any /* TODO: Parser.EmitStatement */): void { + this._visitStatement(ast); + } + public VariableDeclarationStatement(ast: Parser.VariableDeclarationStatement): void { + this._visitStatement(ast); + } + public Statement(ast: Parser.Statement): void { + this._visitStatement(ast); + } + public WhileStatement(ast: Parser.WhileStatement): void { + this._visitStatement(ast); + } + public SimpleStatement(ast: Parser.SimpleStatement): void { + this._visitStatement(ast); + } + public ThrowStatement(ast: Parser.ThrowStatement): void { + this._visitStatement(ast); + } + public DoWhileStatement(ast: Parser.DoWhileStatement): void { + this._visitStatement(ast); + } + public ExpressionStatement(ast: Parser.ExpressionStatement): void { + this._visitStatement(ast.expression); + } + public InlineAssemblyStatement(ast: Parser.InlineAssemblyStatement): void { + this._visitStatement(ast); + } + public ModifierInvocation(ast: Parser.ModifierInvocation): void { + const BUILTIN_MODIFIERS = ['public', 'view', 'payable', 'external', 'internal', 'pure', 'constant']; + if (!_.includes(BUILTIN_MODIFIERS, ast.name)) { + this._visitStatement(ast); + } + } + private _visitStatement(ast: Parser.ASTNode): void { + this._astInfos.push({ + type: ast.type, + node: ast, + name: null, + range: ast.loc, + }); + } + private _visitFunctionLikeDefinition(ast: Parser.ModifierDefinition | Parser.FunctionDefinition): void { + this._astInfos.push({ + type: ast.type, + node: ast, + name: ast.name, + range: ast.loc, + }); + } + private _visitContractDefinition(ast: Parser.ContractDefinition): void { + this._astInfos.push({ + type: ast.type, + node: ast, + name: ast.name, + range: ast.loc, + }); + } + private _getASTInfoForRange(sourceRange: SourceRange, offset: SingleFileSourceRange): ASTInfo | null { + const offsetSourceRange = { + ...sourceRange, + location: { + start: { + line: sourceRange.location.start.line + offset.start.line, + column: sourceRange.location.start.column + offset.start.column, + }, + end: { + line: sourceRange.location.end.line + offset.end.line, + column: sourceRange.location.end.column + offset.end.column, + }, + }, + }; + for (const astInfo of this._astInfos) { + if ( + astInfo.range.start.column === offsetSourceRange.location.start.column && + astInfo.range.start.line === offsetSourceRange.location.start.line && + astInfo.range.end.column === offsetSourceRange.location.end.column && + astInfo.range.end.line === offsetSourceRange.location.end.line + ) { + return astInfo; + } + } + return null; + } +} diff --git a/packages/sol-cov/src/revert_trace_subprovider.ts b/packages/sol-cov/src/revert_trace_subprovider.ts index b1d4da10c..fb2215eaa 100644 --- a/packages/sol-cov/src/revert_trace_subprovider.ts +++ b/packages/sol-cov/src/revert_trace_subprovider.ts @@ -4,10 +4,11 @@ import { getLogger, levels, Logger } from 'loglevel'; import { AbstractArtifactAdapter } from './artifact_adapters/abstract_artifact_adapter'; import { constants } from './constants'; +import { getSourceRangeSnippet } from './get_source_range_snippet'; import { getRevertTrace } from './revert_trace'; import { parseSourceMap } from './source_maps'; import { TraceCollectionSubprovider } from './trace_collection_subprovider'; -import { ContractData, EvmCallStack, SourceRange } from './types'; +import { ContractData, EvmCallStack, SourceRange, SourceSnippet } from './types'; import { utils } from './utils'; /** @@ -53,7 +54,7 @@ export class RevertTraceSubprovider extends TraceCollectionSubprovider { } } private async _printStackTraceAsync(evmCallStack: EvmCallStack): Promise { - const sourceRanges: SourceRange[] = []; + const sourceSnippets: SourceSnippet[] = []; if (_.isUndefined(this._contractsData)) { this._contractsData = await this._artifactAdapter.collectContractsDataAsync(); } @@ -97,18 +98,62 @@ export class RevertTraceSubprovider extends TraceCollectionSubprovider { continue; } } - sourceRanges.push(sourceRange); + const fileIndex = contractData.sources.indexOf(sourceRange.fileName); + const sourceSnippet = getSourceRangeSnippet(sourceRange, contractData.sourceCodes[fileIndex]); + if (sourceSnippet !== null) { + sourceSnippets.push(sourceSnippet); + } } - if (sourceRanges.length > 0) { + const filteredSnippets = filterSnippets(sourceSnippets); + if (filteredSnippets.length > 0) { this._logger.error('\n\nStack trace for REVERT:\n'); - _.forEach(_.reverse(sourceRanges), sourceRange => { - this._logger.error( - `${sourceRange.fileName}:${sourceRange.location.start.line}:${sourceRange.location.start.column}`, - ); + _.forEach(_.reverse(filteredSnippets), snippet => { + const traceString = getStackTraceString(snippet); + this._logger.error(traceString); }); this._logger.error('\n'); } else { - this._logger.error('Could not determine stack trace'); + this._logger.error('REVERT detected but could not determine stack trace'); + } + } +} + +// removes duplicates and if statements +function filterSnippets(sourceSnippets: SourceSnippet[]): SourceSnippet[] { + if (sourceSnippets.length === 0) { + return []; + } + const results: SourceSnippet[] = [sourceSnippets[0]]; + let prev = sourceSnippets[0]; + for (const sourceSnippet of sourceSnippets) { + if (sourceSnippet.type === 'IfStatement') { + continue; + } else if (sourceSnippet.source === prev.source) { + prev = sourceSnippet; + continue; } + results.push(sourceSnippet); + prev = sourceSnippet; + } + return results; +} + +function getStackTraceString(sourceSnippet: SourceSnippet): string { + let result = `${sourceSnippet.fileName}:${sourceSnippet.range.start.line}:${sourceSnippet.range.start.column}`; + const snippetString = getSourceSnippetString(sourceSnippet); + if (snippetString !== '') { + result += `:\n ${snippetString}`; + } + return result; +} + +function getSourceSnippetString(sourceSnippet: SourceSnippet): string { + switch (sourceSnippet.type) { + case 'ContractDefinition': + return `contract ${sourceSnippet.name}`; + case 'FunctionDefinition': + return `function ${sourceSnippet.name}`; + default: + return `${sourceSnippet.source}`; } } diff --git a/packages/sol-cov/src/types.ts b/packages/sol-cov/src/types.ts index cef7141cb..54ade0400 100644 --- a/packages/sol-cov/src/types.ts +++ b/packages/sol-cov/src/types.ts @@ -1,4 +1,5 @@ import { StructLog } from 'ethereum-types'; +import * as Parser from 'solidity-parser-antlr'; export interface LineColumn { line: number; @@ -114,3 +115,12 @@ export interface EvmCallStackEntry { } export type EvmCallStack = EvmCallStackEntry[]; + +export interface SourceSnippet { + source: string; + fileName: string; + type: string; + node: Parser.ASTNode; + name: string | null; + range: SingleFileSourceRange; +} diff --git a/packages/sol-cov/src/utils.ts b/packages/sol-cov/src/utils.ts index 4f16a1cda..d31696636 100644 --- a/packages/sol-cov/src/utils.ts +++ b/packages/sol-cov/src/utils.ts @@ -66,4 +66,10 @@ export const utils = { } return structLogs; }, + getRange(sourceCode: string, range: SingleFileSourceRange): string { + const lines = sourceCode.split('\n').slice(range.start.line - 1, range.end.line); + lines[lines.length - 1] = lines[lines.length - 1].slice(0, range.end.column); + lines[0] = lines[0].slice(range.start.column); + return lines.join('\n'); + }, }; -- cgit v1.2.3 From e4d55242d88695a5d95cad7689ca304ecafb2252 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Tue, 19 Jun 2018 13:01:06 -0700 Subject: Update to match latest type definitions and other small changes --- packages/sol-cov/src/ast_visitor.ts | 5 +++-- packages/sol-cov/src/get_source_range_snippet.ts | 15 ++++++++------- 2 files changed, 11 insertions(+), 9 deletions(-) (limited to 'packages/sol-cov/src') diff --git a/packages/sol-cov/src/ast_visitor.ts b/packages/sol-cov/src/ast_visitor.ts index 16984b5ec..564f0f7d2 100644 --- a/packages/sol-cov/src/ast_visitor.ts +++ b/packages/sol-cov/src/ast_visitor.ts @@ -116,8 +116,9 @@ export class ASTVisitor { this._statementMap[this._entryId++] = this._getExpressionRange(ast); } private _getExpressionRange(ast: Parser.ASTNode): SingleFileSourceRange { - const start = this._locationByOffset[ast.range[0]]; - const end = this._locationByOffset[ast.range[1] + 1]; + const astRange = ast.range as [number, number]; + const start = this._locationByOffset[astRange[0]]; + const end = this._locationByOffset[astRange[1] + 1]; const range = { start, end, diff --git a/packages/sol-cov/src/get_source_range_snippet.ts b/packages/sol-cov/src/get_source_range_snippet.ts index 48c74dd27..30d6ec802 100644 --- a/packages/sol-cov/src/get_source_range_snippet.ts +++ b/packages/sol-cov/src/get_source_range_snippet.ts @@ -9,7 +9,7 @@ interface ASTInfo { type: string; node: Parser.ASTNode; name: string | null; - range: SingleFileSourceRange; + range?: SingleFileSourceRange; } // Parsing source code for each transaction/code is slow and therefore we cache it @@ -30,13 +30,13 @@ export function getSourceRangeSnippet(sourceRange: SourceRange, sourceCode: stri const sourceCodeInRange = utils.getRange(sourceCode, sourceRange.location); return { ...astInfo, + range: astInfo.range as SingleFileSourceRange, source: sourceCodeInRange, fileName: sourceRange.fileName, }; } -// A visitor which collects ASTInfo for contract definitions, function -// definitions, and other types of statements. +// A visitor which collects ASTInfo for most nodes in the AST. class ASTInfoVisitor { private _astInfos: ASTInfo[] = []; public getASTInfoForRange(sourceRange: SourceRange): ASTInfo | null { @@ -165,11 +165,12 @@ class ASTInfoVisitor { }, }; for (const astInfo of this._astInfos) { + const astInfoRange = astInfo.range as SingleFileSourceRange; if ( - astInfo.range.start.column === offsetSourceRange.location.start.column && - astInfo.range.start.line === offsetSourceRange.location.start.line && - astInfo.range.end.column === offsetSourceRange.location.end.column && - astInfo.range.end.line === offsetSourceRange.location.end.line + astInfoRange.start.column === offsetSourceRange.location.start.column && + astInfoRange.start.line === offsetSourceRange.location.start.line && + astInfoRange.end.column === offsetSourceRange.location.end.column && + astInfoRange.end.line === offsetSourceRange.location.end.line ) { return astInfo; } -- cgit v1.2.3 From c5ea985a7003fef8430555e8477a130262407137 Mon Sep 17 00:00:00 2001 From: perissology Date: Thu, 21 Jun 2018 06:24:50 -0700 Subject: only call getLocationByOffset if source if defined --- packages/sol-cov/src/source_maps.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'packages/sol-cov/src') diff --git a/packages/sol-cov/src/source_maps.ts b/packages/sol-cov/src/source_maps.ts index c95c7d2b0..39b83ea0d 100644 --- a/packages/sol-cov/src/source_maps.ts +++ b/packages/sol-cov/src/source_maps.ts @@ -12,9 +12,6 @@ export interface SourceLocation { } export function getLocationByOffset(str: string): LocationByOffset { - if (_.isUndefined(str)) { - return {}; - } const locationByOffset: LocationByOffset = { 0: { line: 1, column: 0 } }; let currentOffset = 0; for (const char of str.split('')) { @@ -39,7 +36,7 @@ export function parseSourceMap( ): { [programCounter: number]: SourceRange } { const bytecode = Uint8Array.from(Buffer.from(bytecodeHex, 'hex')); const pcToInstructionIndex: { [programCounter: number]: number } = getPcToInstructionIndexMapping(bytecode); - const locationByOffsetByFileIndex = _.map(sourceCodes, getLocationByOffset); + const locationByOffsetByFileIndex = _.map(sourceCodes, (s) => _.isUndefined(s) ? {} : getLocationByOffset(s)); const entries = srcMap.split(';'); let lastParsedEntry: SourceLocation = {} as any; const instructionIndexToSourceRange: { [instructionIndex: number]: SourceRange } = {}; -- cgit v1.2.3 From ade8e95d2e1c46bb1a3f8212a6eac5f396bc3e94 Mon Sep 17 00:00:00 2001 From: perissology Date: Thu, 21 Jun 2018 09:12:30 -0700 Subject: fix linter issues --- packages/sol-cov/src/source_maps.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/sol-cov/src') diff --git a/packages/sol-cov/src/source_maps.ts b/packages/sol-cov/src/source_maps.ts index 39b83ea0d..90b21dda1 100644 --- a/packages/sol-cov/src/source_maps.ts +++ b/packages/sol-cov/src/source_maps.ts @@ -36,7 +36,7 @@ export function parseSourceMap( ): { [programCounter: number]: SourceRange } { const bytecode = Uint8Array.from(Buffer.from(bytecodeHex, 'hex')); const pcToInstructionIndex: { [programCounter: number]: number } = getPcToInstructionIndexMapping(bytecode); - const locationByOffsetByFileIndex = _.map(sourceCodes, (s) => _.isUndefined(s) ? {} : getLocationByOffset(s)); + const locationByOffsetByFileIndex = _.map(sourceCodes, s => (_.isUndefined(s) ? {} : getLocationByOffset(s))); const entries = srcMap.split(';'); let lastParsedEntry: SourceLocation = {} as any; const instructionIndexToSourceRange: { [instructionIndex: number]: SourceRange } = {}; -- cgit v1.2.3