Skip to content

Commit dcc971d

Browse files
cgeweckecgewecke
committed
Add branch coverage for logical OR conditions (#499)
* Add initial OR coverage test cases * Add logicalOR coverage for "require" conditions * Add logicalOR coverage for "while" conditions * Add logicalOR coverage for "return" conditions * Add logicalOR coverage for "if" conditions * Add logicalOR branch highlighting for html report
1 parent e69f335 commit dcc971d

29 files changed

+935
-22
lines changed

‎lib/coverage.js

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,19 @@ class Coverage {
6969
const data = collectedData[hash];
7070
const contractPath = collectedData[hash].contractPath;
7171
const id = collectedData[hash].id;
72+
73+
// NB: Any branch using the injected fn which returns boolean will have artificially
74+
// doubled hits (because of something internal to Solidity about how the stack is managed)
7275
const hits = collectedData[hash].hits;
7376

7477
switch(collectedData[hash].type){
75-
case 'line': this.data[contractPath].l[id] = hits; break;
76-
case 'function': this.data[contractPath].f[id] = hits; break;
77-
case 'statement': this.data[contractPath].s[id] = hits; break;
78-
case 'branch': this.data[contractPath].b[id][data.locationIdx] = hits; break;
79-
case 'assertPre': this.assertData[contractPath][id].preEvents = hits; break;
80-
case 'assertPost': this.assertData[contractPath][id].postEvents = hits; break;
78+
case 'line': this.data[contractPath].l[id] = hits; break;
79+
case 'function': this.data[contractPath].f[id] = hits; break;
80+
case 'statement': this.data[contractPath].s[id] = hits; break;
81+
case 'branch': this.data[contractPath].b[id][data.locationIdx] = hits; break;
82+
case 'logicalOR': this.data[contractPath].b[id][data.locationIdx] = hits / 2; break;
83+
case 'assertPre': this.assertData[contractPath][id].preEvents = hits; break;
84+
case 'assertPost': this.assertData[contractPath][id].postEvents = hits; break;
8185
}
8286
}
8387

‎lib/injector.js

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,29 @@ class Injector {
1313
}
1414

1515
_getInjectable(fileName, hash, type){
16-
return `${this._getMethodIdentifier(fileName)}(${hash}); /* ${type} */ \n`;
16+
switch(type){
17+
case 'logicalOR':
18+
return ` && ${this._getTrueMethodIdentifier(fileName)}(${hash}))`;
19+
default:
20+
return `${this._getDefaultMethodIdentifier(fileName)}(${hash}); /* ${type} */ \n`;
21+
}
1722
}
1823

1924
_getHash(fileName) {
2025
this.hashCounter++;
2126
return web3Utils.keccak256(`${fileName}:${this.hashCounter}`);
2227
}
2328

24-
_getMethodIdentifier(fileName){
29+
// Method returns void
30+
_getDefaultMethodIdentifier(fileName){
2531
return `coverage_${web3Utils.keccak256(fileName).slice(0,10)}`
2632
}
2733

34+
// Method returns boolean true
35+
_getTrueMethodIdentifier(fileName){
36+
return `coverage_true${web3Utils.keccak256(fileName).slice(0,10)}`
37+
}
38+
2839
_getInjectionComponents(contract, injectionPoint, fileName, type){
2940
const { start, end } = this._split(contract, injectionPoint);
3041
const hash = this._getHash(fileName)
@@ -39,17 +50,29 @@ class Injector {
3950
}
4051

4152
/**
42-
* Generates a solidity statement injection. Declared once per fn.
43-
* Definition is the same for every fn in file.
53+
* Generates a solidity statement injection defining a method
54+
* *which returns void* to pass instrumentation hash to.
4455
* @param {String} fileName
4556
* @return {String} ex: bytes32[1] memory _sc_82e0891
4657
*/
47-
_getHashMethodDefinition(fileName){
58+
_getDefaultMethodDefinition(fileName){
4859
const hash = web3Utils.keccak256(fileName).slice(0,10);
49-
const method = this._getMethodIdentifier(fileName);
60+
const method = this._getDefaultMethodIdentifier(fileName);
5061
return `\nfunction ${method}(bytes32 c__${hash}) public pure {}\n`;
5162
}
5263

64+
/**
65+
* Generates a solidity statement injection defining a method
66+
* *which returns boolean true* to pass instrumentation hash to.
67+
* @param {String} fileName
68+
* @return {String} ex: bytes32[1] memory _sc_82e0891
69+
*/
70+
_getTrueMethodDefinition(fileName){
71+
const hash = web3Utils.keccak256(fileName).slice(0,10);
72+
const method = this._getTrueMethodIdentifier(fileName);
73+
return `\nfunction ${method}(bytes32 c__${hash}) public pure returns (bool){ return true; }\n`;
74+
}
75+
5376
injectLine(contract, fileName, injectionPoint, injection, instrumentation){
5477
const type = 'line';
5578
const { start, end } = this._split(contract, injectionPoint);
@@ -196,7 +219,37 @@ class Injector {
196219
injectHashMethod(contract, fileName, injectionPoint, injection, instrumentation){
197220
const start = contract.instrumented.slice(0, injectionPoint);
198221
const end = contract.instrumented.slice(injectionPoint);
199-
contract.instrumented = `${start}${this._getHashMethodDefinition(fileName)}${end}`;
222+
contract.instrumented = `${start}` +
223+
`${this._getDefaultMethodDefinition(fileName)}` +
224+
`${this._getTrueMethodDefinition(fileName)}` +
225+
`${end}`;
226+
}
227+
228+
injectOpenParen(contract, fileName, injectionPoint, injection, instrumentation){
229+
const start = contract.instrumented.slice(0, injectionPoint);
230+
const end = contract.instrumented.slice(injectionPoint);
231+
contract.instrumented = `${start}(${end}`;
232+
}
233+
234+
injectLogicalOR(contract, fileName, injectionPoint, injection, instrumentation){
235+
const type = 'logicalOR';
236+
237+
const {
238+
start,
239+
end,
240+
hash,
241+
injectable
242+
} = this._getInjectionComponents(contract, injectionPoint, fileName, type);
243+
244+
instrumentation[hash] = {
245+
id: injection.branchId,
246+
locationIdx: injection.locationIdx,
247+
type: type,
248+
contractPath: fileName,
249+
hits: 0
250+
}
251+
252+
contract.instrumented = `${start}${injectable}${end}`;
200253
}
201254
};
202255

‎lib/instrumenter.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,12 @@ class Instrumenter {
8484

8585
// Line instrumentation has to happen first
8686
contract.injectionPoints[injectionPoint].sort((a, b) => {
87-
const injections = ['injectBranch', 'injectEmptyBranch', 'injectLine'];
87+
const injections = [
88+
'injectLogicalOR',
89+
'injectBranch',
90+
'injectEmptyBranch',
91+
'injectLine'
92+
];
8893
return injections.indexOf(b.type) - injections.indexOf(a.type);
8994
});
9095

‎lib/parse.js

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,47 @@ parse.Block = function(contract, expression) {
2020
}
2121
};
2222

23-
parse.BinaryOperation = function(contract, expression) {
24-
register.statement(contract, expression);
23+
parse.BinaryOperation = function(contract, expression, skipStatementRegistry) {
24+
// Regular binary operation
25+
if (!skipStatementRegistry){
26+
register.statement(contract, expression);
27+
28+
// LogicalOR conditional search...
29+
} else {
30+
parse[expression.left.type] &&
31+
parse[expression.left.type](contract, expression.left, true);
32+
33+
parse[expression.right.type] &&
34+
parse[expression.right.type](contract, expression.right, true);
35+
36+
if (expression.operator === '||'){
37+
register.logicalOR(contract, expression);
38+
}
39+
}
40+
}
41+
42+
parse.TupleExpression = function(contract, expression, skipStatementRegistry) {
43+
expression.components.forEach(component => {
44+
parse[component.type] &&
45+
parse[component.type](contract, component, skipStatementRegistry);
46+
});
2547
}
2648

27-
parse.FunctionCall = function(contract, expression) {
49+
parse.FunctionCall = function(contract, expression, skipStatementRegistry) {
2850
// In any given chain of call expressions, only the last one will fail this check.
2951
// This makes sure we don't instrument a chain of expressions multiple times.
3052
if (expression.expression.type !== 'FunctionCall') {
31-
register.statement(contract, expression);
53+
54+
// Don't register sub-expressions (like intermediate method calls)
55+
if (!skipStatementRegistry){
56+
register.statement(contract, expression);
57+
}
58+
3259
if (expression.expression.name === 'assert' || expression.expression.name === 'require') {
3360
register.assertOrRequire(contract, expression);
61+
expression.arguments.forEach(arg => {
62+
parse[arg.type] && parse[arg.type](contract, arg, true);
63+
});
3464
}
3565
parse[expression.expression.type] &&
3666
parse[expression.expression.type](contract, expression.expression);
@@ -40,10 +70,13 @@ parse.FunctionCall = function(contract, expression) {
4070
}
4171
};
4272

43-
parse.Conditional = function(contract, expression) {
44-
register.statement(contract, expression);
45-
// TODO: Investigate node structure
46-
// There are potential substatements here we aren't measuring
73+
parse.Conditional = function(contract, expression, skipStatementRegistry) {
74+
if (!skipStatementRegistry){
75+
register.statement(contract, expression);
76+
}
77+
78+
parse[expression.condition.type] &&
79+
parse[expression.condition.type](contract, expression.condition, skipStatementRegistry);
4780
};
4881

4982
parse.ContractDefinition = function(contract, expression) {
@@ -112,6 +145,10 @@ parse.FunctionDefinition = function(contract, expression) {
112145
parse.IfStatement = function(contract, expression) {
113146
register.statement(contract, expression);
114147
register.ifStatement(contract, expression);
148+
149+
parse[expression.condition.type] &&
150+
parse[expression.condition.type](contract, expression.condition, true);
151+
115152
parse[expression.trueBody.type] &&
116153
parse[expression.trueBody.type](contract, expression.trueBody);
117154

@@ -155,6 +192,10 @@ parse.SourceUnit = function(contract, expression) {
155192

156193
parse.ReturnStatement = function(contract, expression) {
157194
register.statement(contract, expression);
195+
196+
expression.expression &&
197+
parse[expression.expression.type] &&
198+
parse[expression.expression.type](contract, expression.expression, true);
158199
};
159200

160201
// TODO:Investigate node structure
@@ -174,6 +215,10 @@ parse.VariableDeclarationStatement = function (contract, expression) {
174215

175216
parse.WhileStatement = function (contract, expression) {
176217
register.statement(contract, expression);
218+
219+
parse[expression.condition.type] &&
220+
parse[expression.condition.type](contract, expression.condition, true);
221+
177222
parse[expression.body.type] &&
178223
parse[expression.body.type](contract, expression.body);
179224
};

‎lib/registrar.js

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,107 @@ class Registrar {
176176
};
177177
};
178178

179+
/**
180+
* Registers injections for branch measurements. Used by logicalOR registration method.
181+
* @param {Object} contract instrumentation target
182+
* @param {Object} expression AST node
183+
*/
184+
addNewLogicalORBranch(contract, expression) {
185+
let start;
186+
187+
// Instabul HTML highlighting location data...
188+
const leftZero = expression.left.range[0];
189+
const leftOne = expression.left.range[1];
190+
const rightZero = expression.right.range[0];
191+
const rightOne = expression.right.range[1];
192+
193+
start = contract.instrumented.slice(0, leftZero);
194+
const leftStartLine = ( start.match(/\n/g) || [] ).length + 1;
195+
const leftStartCol = leftZero - start.lastIndexOf('\n') - 1;
196+
197+
start = contract.instrumented.slice(0, leftOne);
198+
const leftEndLine = ( start.match(/\n/g) || [] ).length + 1;
199+
const leftEndCol = leftOne - start.lastIndexOf('\n') - 1;
200+
201+
start = contract.instrumented.slice(0, rightZero);
202+
const rightStartLine = ( start.match(/\n/g) || [] ).length + 1;
203+
const rightStartCol = rightZero - start.lastIndexOf('\n') - 1;
204+
205+
start = contract.instrumented.slice(0, rightOne);
206+
const rightEndLine = ( start.match(/\n/g) || [] ).length + 1;
207+
const rightEndCol = rightOne - start.lastIndexOf('\n') - 1;
208+
209+
contract.branchId += 1;
210+
211+
// NB locations for if branches in istanbul are zero
212+
// length and associated with the start of the if.
213+
contract.branchMap[contract.branchId] = {
214+
line: leftStartLine,
215+
type: 'cond-expr',
216+
locations: [{
217+
start: {
218+
line: leftStartLine, column: leftStartCol,
219+
},
220+
end: {
221+
line: leftEndLine, column: leftEndCol,
222+
},
223+
}, {
224+
start: {
225+
line: rightStartLine, column: rightStartCol,
226+
},
227+
end: {
228+
line: rightEndLine, column: rightEndCol,
229+
},
230+
}],
231+
};
232+
};
233+
234+
/**
235+
* Registers injections for logicalOR clause measurements (branches)
236+
* @param {Object} contract instrumentation target
237+
* @param {Object} expression AST node
238+
* @param {Number} injectionIdx pre/post branch index (left=0, right=1)
239+
*/
240+
logicalOR(contract, expression) {
241+
this.addNewLogicalORBranch(contract, expression);
242+
243+
// Left
244+
this._createInjectionPoint(
245+
contract,
246+
expression.left.range[0],
247+
{
248+
type: 'injectOpenParen',
249+
}
250+
);
251+
this._createInjectionPoint(
252+
contract,
253+
expression.left.range[1] + 1,
254+
{
255+
type: 'injectLogicalOR',
256+
branchId: contract.branchId,
257+
locationIdx: 0
258+
}
259+
);
260+
261+
// Right
262+
this._createInjectionPoint(
263+
contract,
264+
expression.right.range[0],
265+
{
266+
type: 'injectOpenParen',
267+
}
268+
);
269+
this._createInjectionPoint(
270+
contract,
271+
expression.right.range[1] + 1,
272+
{
273+
type: 'injectLogicalOR',
274+
branchId: contract.branchId,
275+
locationIdx: 1
276+
}
277+
);
278+
}
279+
179280
/**
180281
* Registers injections for assert/require statement measurements (branches)
181282
* @param {Object} contract instrumentation target

‎test/integration/projects/logical-or/.solcover.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Testing hooks
2+
const fn = (msg, config) => config.logger.log(msg);
3+
4+
module.exports = {
5+
skipFiles: ['Migrations.sol'],
6+
silent: process.env.SILENT ? true : false,
7+
istanbulReporter: ['json-summary', 'text'],
8+
}

‎test/integration/projects/logical-or/buidler.config.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
const { loadPluginFile } = require("@nomiclabs/buidler/plugins-testing");
2+
loadPluginFile(__dirname + "/../plugins/buidler.plugin");
3+
usePlugin("@nomiclabs/buidler-truffle5");
4+
5+
module.exports={
6+
defaultNetwork: "buidlerevm"
7+
};

0 commit comments

Comments
 (0)