[gjs] coverage: Handle variable call arguments in functions
- From: Sam Spilsbury <samspilsbury src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs] coverage: Handle variable call arguments in functions
- Date: Thu, 16 Jul 2015 00:58:07 +0000 (UTC)
commit 0a0346a60b5b12ddb2ac6dbfbb5a7aa97da61cac
Author: Sam Spilsbury <smspillaz gmail com>
Date: Fri Jul 3 03:26:42 2015 +0800
coverage: Handle variable call arguments in functions
Previously we were assuming that the number of arguments
that a function had was the number of arguments that would
be given to us by the debugger in frame.callee.params. This
is not correct - frame.callee.params represents the arguments
actually provided and not the arguments that the function has.
As such, function call recording didn't work correctly for
varargs functions and won't work correctly for functions
which define default parameters.
This means that a point of potential disambiguation might not
be as useful now. Now, when we are trying to identify the
function that was actually called, we look at the name,
the line it is defined on and then if there are still
multiple optons, the function with the closest number
of arguments to what this function was called with.
This isn't a perfect heuristic, but it gets us close
enough. Where two similar looking functions are defined
on the same line with little opportunity to disambiguate,
a warning will be issued.
Fixes BGO #751732
installed-tests/js/testCoverage.js | 178 ++++++++++++++++++++++++++++++++----
modules/coverage.js | 130 +++++++++++++++++++++-----
2 files changed, 263 insertions(+), 45 deletions(-)
---
diff --git a/installed-tests/js/testCoverage.js b/installed-tests/js/testCoverage.js
index 0b1aab8..839aaf1 100644
--- a/installed-tests/js/testCoverage.js
+++ b/installed-tests/js/testCoverage.js
@@ -992,10 +992,72 @@ function testFunctionCounterMapReturnedForFunctionKeys() {
};
let detectedFunctions = Coverage.functionsForAST(ast);
- let functionKey = Coverage._getFunctionKeyFromReflectedFunction(ast.body[0]);
- let functionCounters = Coverage._functionsToFunctionCounters(detectedFunctions);
+ let functionCounters = Coverage._functionsToFunctionCounters('script',
+ detectedFunctions);
+
+ JSUnit.assertEquals(0, functionCounters.name['1']['0'].hitCount);
+}
+
+function _fetchLogMessagesFrom(func) {
+ let oldLog = window.log;
+ let collectedMessages = [];
+ window.log = function(message) {
+ collectedMessages.push(message);
+ };
+
+ try {
+ func.apply(this, arguments);
+ } finally {
+ window.log = oldLog;
+ }
+
+ return collectedMessages;
+}
+
+function testErrorReportedWhenTwoIndistinguishableFunctionsPresent() {
+ let ast = {
+ body: [{
+ type: 'FunctionDeclaration',
+ id: {
+ name: '(anonymous)'
+ },
+ loc: {
+ start: {
+ line: 1
+ }
+ },
+ params: [],
+ body: {
+ type: 'BlockStatement',
+ body: []
+ }
+ }, {
+ type: 'FunctionDeclaration',
+ id: {
+ name: '(anonymous)'
+ },
+ loc: {
+ start: {
+ line: 1
+ }
+ },
+ params: [],
+ body: {
+ type: 'BlockStatement',
+ body: []
+ }
+ }]
+ };
+
+ let detectedFunctions = Coverage.functionsForAST(ast);
+ let messages = _fetchLogMessagesFrom(function() {
+ Coverage._functionsToFunctionCounters('script', detectedFunctions);
+ });
- JSUnit.assertEquals(0, functionCounters[functionKey].hitCount);
+ JSUnit.assertEquals('script:1 Function identified as (anonymous):1:0 ' +
+ 'already seen in this file. Function coverage will ' +
+ 'be incomplete.',
+ messages[0]);
}
function testKnownFunctionsArrayPopulatedForFunctions() {
@@ -1012,14 +1074,65 @@ function testKnownFunctionsArrayPopulatedForFunctions() {
}
function testIncrementFunctionCountersForFunctionOnSameExecutionStartLine() {
- let functionKey = 'f:1:0';
- let functionCounters = {};
+ let functionCounters = Coverage._functionsToFunctionCounters('script', [
+ { key: 'f:1:0',
+ line: 1,
+ n_params: 0 }
+ ]);
+ Coverage._incrementFunctionCounters(functionCounters, null, 'f', 1, 0);
- functionCounters[functionKey] = { hitCount: 0 };
+ JSUnit.assertEquals(functionCounters.f['1']['0'].hitCount, 1);
+}
- Coverage._incrementFunctionCounters(functionCounters, null, 'f', 1, 0);
+function testIncrementFunctionCountersCanDisambiguateTwoFunctionsWithSameName() {
+ let functionCounters = Coverage._functionsToFunctionCounters('script', [
+ { key: '(anonymous):1:0',
+ line: 1,
+ n_params: 0 },
+ { key: '(anonymous):2:0',
+ line: 2,
+ n_params: 0 }
+ ]);
+ Coverage._incrementFunctionCounters(functionCounters, null, '(anonymous)', 1, 0);
+ Coverage._incrementFunctionCounters(functionCounters, null, '(anonymous)', 2, 0);
- JSUnit.assertEquals(functionCounters[functionKey].hitCount, 1);
+ JSUnit.assertEquals(functionCounters['(anonymous)']['1']['0'].hitCount, 1);
+ JSUnit.assertEquals(functionCounters['(anonymous)']['2']['0'].hitCount, 1);
+}
+
+function testIncrementFunctionCountersCanDisambiguateTwoFunctionsOnSameLineWithDifferentParams() {
+ let functionCounters = Coverage._functionsToFunctionCounters('script', [
+ { key: '(anonymous):1:0',
+ line: 1,
+ n_params: 0 },
+ { key: '(anonymous):1:1',
+ line: 1,
+ n_params: 1 }
+ ]);
+ Coverage._incrementFunctionCounters(functionCounters, null, '(anonymous)', 1, 0);
+ Coverage._incrementFunctionCounters(functionCounters, null, '(anonymous)', 1, 1);
+
+ JSUnit.assertEquals(functionCounters['(anonymous)']['1']['0'].hitCount, 1);
+ JSUnit.assertEquals(functionCounters['(anonymous)']['1']['1'].hitCount, 1);
+}
+
+function testIncrementFunctionCountersCanDisambiguateTwoFunctionsOnSameLineByGuessingClosestParams() {
+ let functionCounters = Coverage._functionsToFunctionCounters('script', [
+ { key: '(anonymous):1:0',
+ line: 1,
+ n_params: 0 },
+ { key: '(anonymous):1:3',
+ line: 1,
+ n_params: 3 }
+ ]);
+
+ /* Eg, we called the function with 3 params with just two arguments. We
+ * should be able to work out that we probably intended to call the
+ * latter function as opposed to the former. */
+ Coverage._incrementFunctionCounters(functionCounters, null, '(anonymous)', 1, 2);
+
+ JSUnit.assertEquals(functionCounters['(anonymous)']['1']['0'].hitCount, 0);
+ JSUnit.assertEquals(functionCounters['(anonymous)']['1']['3'].hitCount, 1);
}
function testIncrementFunctionCountersForFunctionOnEarlierStartLine() {
@@ -1043,15 +1156,15 @@ function testIncrementFunctionCountersForFunctionOnEarlierStartLine() {
};
let detectedFunctions = Coverage.functionsForAST(ast);
- let functionKey = Coverage._getFunctionKeyFromReflectedFunction(ast.body[0]);
let knownFunctionsArray = Coverage._populateKnownFunctions(detectedFunctions, 3);
- let functionCounters = Coverage._functionsToFunctionCounters(detectedFunctions);
+ let functionCounters = Coverage._functionsToFunctionCounters('script',
+ detectedFunctions);
/* We're entering at line two, but the function definition was actually
* at line one */
Coverage._incrementFunctionCounters(functionCounters, knownFunctionsArray, 'name', 2, 0);
- JSUnit.assertEquals(functionCounters[functionKey].hitCount, 1);
+ JSUnit.assertEquals(functionCounters.name['1']['0'].hitCount, 1);
}
function testIncrementFunctionCountersThrowsErrorOnUnexpectedFunction() {
@@ -1076,7 +1189,8 @@ function testIncrementFunctionCountersThrowsErrorOnUnexpectedFunction() {
let detectedFunctions = Coverage.functionsForAST(ast);
let functionKey = Coverage._getFunctionKeyFromReflectedFunction(ast.body[0]);
let knownFunctionsArray = Coverage._populateKnownFunctions(detectedFunctions, 3);
- let functionCounters = Coverage._functionsToFunctionCounters(detectedFunctions);
+ let functionCounters = Coverage._functionsToFunctionCounters('script',
+ detectedFunctions);
/* We're entering at line two, but the function definition was actually
* at line one */
@@ -1161,10 +1275,22 @@ function testBranchTrackerFindsNextBranch() {
}
function testConvertFunctionCountersToArray() {
- let functionsMap = {};
-
- functionsMap['(anonymous):2:0'] = { hitCount: 1 };
- functionsMap['name:1:0'] = { hitCount: 0 };
+ let functionsMap = {
+ '(anonymous)': {
+ '2': {
+ '0': {
+ hitCount: 1
+ },
+ },
+ },
+ 'name': {
+ '1': {
+ '0': {
+ hitCount: 0
+ },
+ },
+ }
+ };
let expectedFunctionCountersArray = [
{ name: '(anonymous):2:0', hitCount: 1 },
@@ -1182,10 +1308,22 @@ function testConvertFunctionCountersToArray() {
}
function testConvertFunctionCountersToArrayIsSorted() {
- let functionsMap = {};
-
- functionsMap['name:1:0'] = { hitCount: 0 };
- functionsMap['(anonymous):2:0'] = { hitCount: 1 };
+ let functionsMap = {
+ '(anonymous)': {
+ '2': {
+ '0': {
+ hitCount: 1
+ },
+ },
+ },
+ 'name': {
+ '1': {
+ '0': {
+ hitCount: 0
+ },
+ },
+ }
+ };
let expectedFunctionCountersArray = [
{ name: '(anonymous):2:0', hitCount: 1 },
diff --git a/modules/coverage.js b/modules/coverage.js
index 9d41178..13f4ca6 100644
--- a/modules/coverage.js
+++ b/modules/coverage.js
@@ -477,13 +477,27 @@ function _branchesToBranchCounters(branches, nLines) {
return counters;
}
-function _functionsToFunctionCounters(functions) {
+function _functionsToFunctionCounters(script, functions) {
let functionCounters = {};
functions.forEach(function(func) {
- functionCounters[func.key] = {
- hitCount: 0
- };
+ let [name, line, args] = func.key.split(':');
+
+ if (functionCounters[name] === undefined) {
+ functionCounters[name] = {};
+ }
+
+ if (functionCounters[name][line] === undefined) {
+ functionCounters[name][line] = {};
+ }
+
+ if (functionCounters[name][line][args] === undefined) {
+ functionCounters[name][line][args] = { hitCount: 0 };
+ } else {
+ log(script + ':' + line + ' Function identified as ' +
+ func.key + ' already seen in this file. Function coverage ' +
+ 'will be incomplete.');
+ }
});
return functionCounters;
@@ -499,6 +513,59 @@ function _populateKnownFunctions(functions, nLines) {
return knownFunctions;
}
+function _identifyFunctionCounterInLinePartForDescription(linePart,
+ nArgs) {
+ /* There is only one potential option for this line. We might have been
+ * called with the wrong number of arguments, but it doesn't matter. */
+ if (Object.getOwnPropertyNames(linePart).length === 1)
+ return linePart[Object.getOwnPropertyNames(linePart)[0]];
+
+ /* Have to disambiguate using nArgs and we have an exact match. */
+ if (linePart[nArgs] !== undefined)
+ return linePart[nArgs];
+
+ /* There are multiple options on this line. Pick the one where
+ * we satisfy the number of arguments exactly, or failing that,
+ * go through each and pick the closest. */
+ let allNArgsOptions = Object.keys(linePart).map(function(key) {
+ return parseInt(key);
+ });
+
+ let closest = allNArgsOptions.reduce(function(prev, current, index, array) {
+ let nArgsOption = array[index];
+ if (Math.abs(nArgsOption - nArgs) < Math.abs(current - nArgs)) {
+ return nArgsOption;
+ }
+
+ return current;
+ });
+
+ return linePart[String(closest)];
+}
+
+function _identifyFunctionCounterForDescription(functionCounters,
+ name,
+ line,
+ nArgs) {
+ let candidateCounter = functionCounters[name];
+
+ if (candidateCounter === undefined)
+ return null;
+
+ if (Object.getOwnPropertyNames(candidateCounter).length === 1) {
+ let linePart = candidateCounter[Object.getOwnPropertyNames(candidateCounter)[0]];
+ return _identifyFunctionCounterInLinePartForDescription(linePart, nArgs);
+ }
+
+ let linePart = functionCounters[name][line];
+
+ if (linePart === undefined) {
+ return null;
+ }
+
+ return _identifyFunctionCounterInLinePartForDescription(linePart, nArgs);
+}
+
/**
* _incrementFunctionCounters
*
@@ -517,25 +584,30 @@ function _incrementFunctionCounters(functionCounters,
name,
line,
nArgs) {
- let functionKey = name + ':' + line + ':' + nArgs;
- let functionCountersForKey = functionCounters[functionKey];
+ let functionCountersForKey = _identifyFunctionCounterForDescription(functionCounters,
+ name,
+ line,
+ nArgs);
/* Its possible that the JS Engine might enter a funciton
* at an executable line which is a little bit past the
* actual definition. Roll backwards until we reach the
* last known function definition line which we kept
* track of earlier to see if we can find this function first */
- if (functionCountersForKey === undefined) {
+ if (functionCountersForKey === null) {
do {
--line;
- functionKey = name + ':' + line + ':' + nArgs;
- functionCountersForKey = functionCounters[functionKey];
+ functionCountersForKey = _identifyFunctionCounterForDescription(functionCounters,
+ name,
+ line,
+ nArgs);
} while (linesWithKnownFunctions[line] !== true && line > 0);
}
- if (functionCountersForKey !== undefined) {
+ if (functionCountersForKey !== null) {
functionCountersForKey.hitCount++;
} else {
+ let functionKey = [name, line, nArgs].join(':');
throw new Error("expected Reflect to find function " + functionKey);
}
}
@@ -615,16 +687,24 @@ function _BranchTracker(branchCounters) {
function _convertFunctionCountersToArray(functionCounters) {
let arrayReturn = [];
- /* functionCounters is an object so convert it to
+ /* functionCounters is an object so explore it to create a
+ * set of function keys and then convert it to
* an array-of-object using the key as a property
* of that object */
- for (let key of Object.getOwnPropertyNames(functionCounters)) {
- let func = functionCounters[key];
- /* The name of the function contains its line, after the first
- * colon. Split the name and retrieve it here */
- arrayReturn.push({ name: key,
- line: Number(key.split(':')[1]),
- hitCount: func.hitCount });
+ for (let name of Object.getOwnPropertyNames(functionCounters)) {
+ let namePart = functionCounters[name];
+
+ for (let line of Object.getOwnPropertyNames(namePart)) {
+ let linePart = functionCounters[name][line];
+
+ for (let nArgs of Object.getOwnPropertyNames(linePart)) {
+ let functionKey = [name, line, nArgs].join(':');
+ arrayReturn.push({ name: functionKey,
+ line: Number(line),
+ nArgs: nArgs,
+ hitCount: linePart[nArgs].hitCount });
+ }
+ }
}
arrayReturn.sort(function(left, right) {
@@ -663,7 +743,7 @@ function _fetchCountersFromCache(filename, cache, nLines) {
return {
expressionCounters: _expressionLinesToCounters(cache_for_file.lines, nLines),
branchCounters: _branchesToBranchCounters(cache_for_file.branches, nLines),
- functionCounters: _functionsToFunctionCounters(functions),
+ functionCounters: _functionsToFunctionCounters(filename, functions),
linesWithKnownFunctions: _populateKnownFunctions(functions, nLines),
nLines: nLines
};
@@ -672,14 +752,14 @@ function _fetchCountersFromCache(filename, cache, nLines) {
return null;
}
-function _fetchCountersFromReflection(contents, nLines) {
+function _fetchCountersFromReflection(filename, contents, nLines) {
let reflection = Reflect.parse(contents);
let functions = functionsForAST(reflection);
return {
expressionCounters: _expressionLinesToCounters(expressionLinesForAST(reflection), nLines),
branchCounters: _branchesToBranchCounters(branchesForAST(reflection), nLines),
- functionCounters: _functionsToFunctionCounters(functions),
+ functionCounters: _functionsToFunctionCounters(filename, functions),
linesWithKnownFunctions: _populateKnownFunctions(functions, nLines),
nLines: nLines
};
@@ -704,7 +784,7 @@ function CoverageStatisticsContainer(prefixes, cache) {
let counters = _fetchCountersFromCache(filename, cachedASTs, nLines);
if (counters === null) {
cacheMisses++;
- counters = _fetchCountersFromReflection(contents, nLines);
+ counters = _fetchCountersFromReflection(filename, contents, nLines);
}
if (counters === null)
@@ -735,10 +815,10 @@ function CoverageStatisticsContainer(prefixes, cache) {
checksum: mtime === null ? getFileChecksum(filename) : null,
lines: [],
branches: [],
- functions: Object.keys(statisticsForFilename.functionCounters).map(function(key) {
+ functions:
_convertFunctionCountersToArray(statisticsForFilename.functionCounters).map(function(func) {
return {
- key: key,
- line: Number(key.split(':')[1])
+ key: func.name,
+ line: func.line
};
})
};
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
