Skip to content

Commit 4f007c7

Browse files
cgeweckecgewecke
authored
Enable coverage when viaIR compiler flag is true (#854)
1 parent d3a5b37 commit 4f007c7

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+450
-57
lines changed

‎.circleci/config.yml

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,9 @@ step_install_nvm: &step_install_nvm
1212
set +e
1313
export NVM_DIR="/opt/circleci/.nvm"
1414
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"
15-
nvm install v14.19.0
16-
nvm alias default v14.19.0
15+
nvm install v18
1716
echo 'export NVM_DIR="/opt/circleci/.nvm"' >> $BASH_ENV
1817
echo "[ -s \"$NVM_DIR/nvm.sh\" ] && . \"$NVM_DIR/nvm.sh\"" >> $BASH_ENV
19-
2018
jobs:
2119
unit-test:
2220
docker:
@@ -32,9 +30,13 @@ jobs:
3230
command: |
3331
yarn
3432
- run:
35-
name: Run tests
33+
name: Tests ( optimizer.enabled=false )
3634
command: |
3735
npm run test:ci
36+
- run:
37+
name: Tests ( viaIR=true )
38+
command: |
39+
npm run test:ci:viaIR
3840
- run:
3941
name: Upload coverage
4042
command: |

‎lib/api.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ class API {
5454
this.istanbulFolder = config.istanbulFolder || false;
5555
this.istanbulReporter = config.istanbulReporter || ['html', 'lcov', 'text', 'json'];
5656

57+
this.viaIR = config.viaIR;
5758
this.solcOptimizerDetails = config.solcOptimizerDetails;
5859

5960
this.setLoggingLevel(config.silent);
@@ -176,7 +177,7 @@ class API {
176177
// Hardhat
177178
async attachToHardhatVM(provider){
178179
const self = this;
179-
this.collector = new DataCollector(this.instrumenter.instrumentationData);
180+
this.collector = new DataCollector(this.instrumenter.instrumentationData, this.viaIR);
180181

181182
if ('init' in provider) {
182183
// Newer versions of Hardhat initialize the provider lazily, so we need to

‎lib/collector.js

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,31 @@
33
* coverage map constructed by the Instrumenter.
44
*/
55
class DataCollector {
6-
constructor(instrumentationData={}){
6+
constructor(instrumentationData={}, viaIR){
77
this.instrumentationData = instrumentationData;
88

99
this.validOpcodes = {
1010
"PUSH1": true,
11+
"DUP1": viaIR,
12+
"DUP2": viaIR,
13+
"DUP3": viaIR,
14+
"DUP4": viaIR,
15+
"DUP5": viaIR,
16+
"DUP6": viaIR,
17+
"DUP7": viaIR,
18+
"DUP8": viaIR,
19+
"DUP9": viaIR,
20+
"DUP10": viaIR,
21+
"DUP11": viaIR,
22+
"DUP12": viaIR,
23+
"DUP13": viaIR,
24+
"DUP14": viaIR,
25+
"DUP15": viaIR,
26+
"DUP16": viaIR,
1127
}
28+
29+
this.lastHash = null;
30+
this.viaIR = viaIR;
1231
}
1332

1433
/**
@@ -46,7 +65,26 @@ class DataCollector {
4665
hash = this._normalizeHash(hash);
4766

4867
if(this.instrumentationData[hash]){
49-
this.instrumentationData[hash].hits++;
68+
// abi.encode (used to circumvent viaIR) sometimes puts the hash on the stack twice
69+
if (this.lastHash !== hash) {
70+
this.lastHash = hash;
71+
this.instrumentationData[hash].hits++
72+
}
73+
return;
74+
}
75+
76+
// Detect and recover from viaIR mangled hashes by left-padding single `0`
77+
if(this.viaIR && hash.length === 18) {
78+
hash = hash.slice(2);
79+
hash = '0' + hash;
80+
hash = hash.slice(0,16);
81+
hash = '0x' + hash;
82+
if(this.instrumentationData[hash]){
83+
if (this.lastHash !== hash) {
84+
this.lastHash = hash;
85+
this.instrumentationData[hash].hits++
86+
}
87+
}
5088
}
5189
}
5290

@@ -56,10 +94,14 @@ class DataCollector {
5694
* but prevents left-padding shorter irrelevant hashes
5795
*
5896
* @param {String} hash data hash from evm stack.
59-
* @return {String} 0x prefixed hash of length 66.
97+
* @return {String} 0x prefixed hash of length 18.
6098
*/
6199
_normalizeHash(hash){
62-
if (hash.length < 18 && hash.length > 11){
100+
// viaIR sometimes right-pads the hashes out to 32 bytes
101+
// but it doesn't preserve leading zeroes when it does this
102+
if (this.viaIR && hash.length >= 18) {
103+
hash = hash.slice(0,18);
104+
} else if (hash.length < 18 && hash.length > 11){
63105
hash = hash.slice(2);
64106
while(hash.length < 16) hash = '0' + hash;
65107
hash = '0x' + hash

‎lib/injector.js

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
const web3Utils = require("web3-utils");
22

33
class Injector {
4-
constructor(){
4+
constructor(viaIR){
5+
this.viaIR = viaIR;
56
this.hashCounter = 0;
67
this.modifierCounter = 0;
78
this.modifiers = {};
@@ -23,7 +24,9 @@ class Injector {
2324
case 'modifier':
2425
return ` ${this._getModifierIdentifier(id)} `;
2526
default:
26-
return `${this._getDefaultMethodIdentifier(id)}(${hash}); /* ${type} */ \n`;
27+
return (this.viaIR)
28+
? `${this._getAbiEncodeStatementHash(hash)} /* ${type} */ \n`
29+
: `${this._getDefaultMethodIdentifier(id)}(${hash}); /* ${type} */ \n`;
2730
}
2831
}
2932

@@ -51,6 +54,16 @@ class Injector {
5154
return `c_mod${web3Utils.keccak256(id).slice(2,10)}`
5255
}
5356

57+
// Way to get hash on the stack with viaIR (which seems to ignore abi.encode builtin)
58+
// Tested with v0.8.17, v0.8.24
59+
_getAbiEncodeStatementHash(hash){
60+
return `abi.encode(${hash}); `
61+
}
62+
63+
_getAbiEncodeStatementVar(hash){
64+
return `abi.encode(c__${hash}); `
65+
}
66+
5467
_getInjectionComponents(contract, injectionPoint, id, type){
5568
const { start, end } = this._split(contract, injectionPoint);
5669
const hash = this._getHash(id)
@@ -73,7 +86,10 @@ class Injector {
7386
_getDefaultMethodDefinition(id){
7487
const hash = web3Utils.keccak256(id).slice(2,10);
7588
const method = this._getDefaultMethodIdentifier(id);
76-
return `\nfunction ${method}(bytes8 c__${hash}) internal pure {}\n`;
89+
90+
return (this.viaIR)
91+
? ``
92+
: `\nfunction ${method}(bytes8 c__${hash}) internal pure {}\n`;
7793
}
7894

7995
/**
@@ -85,7 +101,11 @@ class Injector {
85101
_getFileScopedHashMethodDefinition(id, contract){
86102
const hash = web3Utils.keccak256(id).slice(2,10);
87103
const method = this._getDefaultMethodIdentifier(id);
88-
return `\nfunction ${method}(bytes8 c__${hash}) pure {}\n`;
104+
const abi = this._getAbiEncodeStatementVar(hash);
105+
106+
return (this.viaIR)
107+
? `\nfunction ${method}(bytes8 c__${hash}) pure { ${abi} }\n`
108+
: `\nfunction ${method}(bytes8 c__${hash}) pure {}\n`;
89109
}
90110

91111
/**
@@ -97,7 +117,11 @@ class Injector {
97117
_getTrueMethodDefinition(id){
98118
const hash = web3Utils.keccak256(id).slice(2,10);
99119
const method = this._getTrueMethodIdentifier(id);
100-
return `function ${method}(bytes8 c__${hash}) internal pure returns (bool){ return true; }\n`;
120+
const abi = this._getAbiEncodeStatementVar(hash);
121+
122+
return (this.viaIR)
123+
? `function ${method}(bytes8 c__${hash}) internal pure returns (bool){ ${abi} return true; }\n`
124+
: `function ${method}(bytes8 c__${hash}) internal pure returns (bool){ return true; }\n`;
101125
}
102126

103127
/**
@@ -110,7 +134,11 @@ class Injector {
110134
_getFileScopeTrueMethodDefinition(id){
111135
const hash = web3Utils.keccak256(id).slice(2,10);
112136
const method = this._getTrueMethodIdentifier(id);
113-
return `function ${method}(bytes8 c__${hash}) pure returns (bool){ return true; }\n`;
137+
const abi = this._getAbiEncodeStatementVar(hash);
138+
139+
return (this.viaIR)
140+
? `function ${method}(bytes8 c__${hash}) pure returns (bool){ ${abi} return true; }\n`
141+
: `function ${method}(bytes8 c__${hash}) pure returns (bool){ return true; }\n`;
114142
}
115143

116144
/**
@@ -122,7 +150,11 @@ class Injector {
122150
_getFalseMethodDefinition(id){
123151
const hash = web3Utils.keccak256(id).slice(2,10);
124152
const method = this._getFalseMethodIdentifier(id);
125-
return `function ${method}(bytes8 c__${hash}) internal pure returns (bool){ return false; }\n`;
153+
const abi = this._getAbiEncodeStatementVar(hash);
154+
155+
return (this.viaIR)
156+
? `function ${method}(bytes8 c__${hash}) internal pure returns (bool){ ${abi} return false; }\n`
157+
: `function ${method}(bytes8 c__${hash}) internal pure returns (bool){ return false; }\n`;
126158
}
127159

128160
/**
@@ -135,7 +167,11 @@ class Injector {
135167
_getFileScopedFalseMethodDefinition(id){
136168
const hash = web3Utils.keccak256(id).slice(2,10);
137169
const method = this._getFalseMethodIdentifier(id);
138-
return `function ${method}(bytes8 c__${hash}) pure returns (bool){ return false; }\n`;
170+
const abi = this._getAbiEncodeStatementVar(hash);
171+
172+
return (this.viaIR)
173+
? `function ${method}(bytes8 c__${hash}) pure returns (bool){ ${abi} return false; }\n`
174+
: `function ${method}(bytes8 c__${hash}) pure returns (bool){ return false; }\n`;
139175
}
140176

141177
_getModifierDefinitions(contractId, instrumentation){

‎lib/instrumenter.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ class Instrumenter {
1414

1515
constructor(config={}){
1616
this.instrumentationData = {};
17-
this.injector = new Injector();
17+
this.injector = new Injector(config.viaIR);
1818
this.modifierWhitelist = config.modifierWhitelist || [];
1919
this.enabled = {
2020
statements: (config.measureStatementCoverage === false) ? false : true,

‎package.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@
1212
"scripts": {
1313
"test:unit": "./scripts/unit.sh",
1414
"test:integration": "./scripts/integration.sh",
15-
"test:ci": "./scripts/ci.sh"
15+
"test:ci": "./scripts/ci.sh",
16+
"test:uint:viaIR": "VIA_IR=true ./scripts/unit.sh",
17+
"test:integration:viaIR": "VIA_IR=true ./scripts/integration.sh",
18+
"test:ci:viaIR": "VIA_IR=true ./scripts/ci.sh"
1619
},
1720
"homepage": "https://github.com/sc-forks/solidity-coverage",
1821
"repository": {

‎plugins/hardhat.plugin.js

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,14 @@ subtask(TASK_COMPILE_SOLIDITY_GET_COMPILATION_JOB_FOR_FILE).setAction(async (_,
5454
}
5555
// Unset useLiteralContent due to solc metadata size restriction
5656
settings.metadata.useLiteralContent = false;
57-
// Override optimizer settings for all compilers
58-
settings.optimizer.enabled = false;
5957

60-
// This is fixes a stack too deep bug in ABIEncoderV2
61-
// Experimental because not sure this works as expected across versions....
58+
// Beginning with v0.8.7, we let the optimizer run if viaIR is true and
59+
// instrument using `abi.encode(bytes8 covHash)`. Otherwise turn the optimizer off.
60+
if (!settings.viaIR) settings.optimizer.enabled = false;
61+
62+
// This sometimes fixed a stack-too-deep bug in ABIEncoderV2 for coverage plugin versions up to 0.8.6
63+
// Although issue should be fixed in 0.8.7, am leaving this option in because it may still be necessary
64+
// to configure optimizer details in some cases.
6265
if (configureYulOptimizer) {
6366
if (optimizerDetails === undefined) {
6467
settings.optimizer.details = {

‎plugins/resources/nomiclabs.utils.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ function normalizeConfig(config, args={}){
3636
? sources = path.join(config.paths.sources, args.sources)
3737
: sources = config.paths.sources;
3838

39+
if (config.solidity && config.solidity.compilers.length) {
40+
config.viaIR = isUsingViaIR(config.solidity);
41+
}
42+
3943
config.workingDir = config.paths.root;
4044
config.contractsDir = sources;
4145
config.testDir = config.paths.tests;
@@ -55,6 +59,23 @@ function normalizeConfig(config, args={}){
5559
return config;
5660
}
5761

62+
function isUsingViaIR(solidity) {
63+
64+
for (compiler of solidity.compilers) {
65+
if (compiler.settings && compiler.settings.viaIR) {
66+
return true;
67+
}
68+
}
69+
if (solidity.overrides) {
70+
for (key of Object.keys(solidity.overrides)){
71+
if (solidity.overrides[key].settings && solidity.overrides[key].settings.viaIR) {
72+
return true;
73+
}
74+
}
75+
}
76+
return false;
77+
}
78+
5879
async function setupHardhatNetwork(env, api, ui){
5980
const hardhatPackage = require('hardhat/package.json');
6081
const { createProvider } = require("hardhat/internal/core/providers/construction");

‎plugins/resources/plugin.utils.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,9 @@ function loadSolcoverJS(config={}){
227227
coverageConfig = {};
228228
}
229229

230-
// Truffle writes to coverage config
230+
// viaIR is eval'd in `nomiclab.utils.normalizeConfig`
231+
coverageConfig.viaIR = config.viaIR;
232+
231233
coverageConfig.log = log;
232234
coverageConfig.cwd = config.workingDir;
233235
coverageConfig.originalContractsDir = config.contractsDir;

‎scripts/ci.sh

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
#!/usr/bin/env bash
22

3-
SILENT=true node --max-old-space-size=4096 \
3+
# Toggles optimizer on/off
4+
VIAR_IR=$VIA_IR
5+
6+
# Minimize integration test output
7+
SILENT=true
8+
9+
node --max-old-space-size=4096 \
410
./node_modules/.bin/nyc \
511
--reporter=lcov \
612
--exclude '**/sc_temp/**' \

‎scripts/integration.sh

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
#!/usr/bin/env bash
22

3+
# Toggles optimizer on/off
4+
VIA_IR=$VIA_IR
5+
36
node --max-old-space-size=4096 \
47
./node_modules/.bin/nyc \
58
--exclude '**/sc_temp/**' \

‎scripts/unit.sh

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
#!/usr/bin/env bash
22

3+
# Toggles optimizer on/off
4+
VIAR_IR=$VIA_IR
5+
36
node --max-old-space-size=4096 \
47
./node_modules/.bin/nyc \
58
--exclude '**/sc_temp/**' \

‎scripts/zeppelin.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ set -o errexit
88
# Get rid of any caches
99
sudo rm -rf node_modules
1010
echo "NVM CURRENT >>>>>" && nvm current
11+
nvm use 18
1112

1213
# Use PR env variables (for forks) or fallback on local if PR not available
1314
SED_REGEX="s/[email protected]:/https:\/\/github.com\//"

‎test/integration/standard.js

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,31 @@ describe('Hardhat Plugin: standard use cases', function() {
348348
verify.lineCoverage(expected);
349349
})
350350

351+
it('detects viaIR when specified in config overrides only', async function(){
352+
mock.installFullProject('overrides-viaIR');
353+
mock.hardhatSetupEnv(this);
354+
355+
await this.env.run("coverage");
356+
357+
const expected = [
358+
{
359+
file: mock.pathToContract(hardhatConfig, 'ContractOverA2.sol'),
360+
pct: 33.33
361+
},
362+
{
363+
file: mock.pathToContract(hardhatConfig, 'ContractOverB2.sol'),
364+
pct: 100,
365+
},
366+
{
367+
file: mock.pathToContract(hardhatConfig, 'ContractOverC2.sol'),
368+
pct: 100,
369+
},
370+
371+
];
372+
373+
verify.lineCoverage(expected);
374+
})
375+
351376
it('locates .coverage_contracts correctly when dir is subfolder', async function(){
352377
mock.installFullProject('contract-subfolders');
353378
mock.hardhatSetupEnv(this);

0 commit comments

Comments
 (0)