Skip to content

Commit

Permalink
Support coverage instrumentation on optimized code.
Browse files Browse the repository at this point in the history 
This isn't a super important usecase, but I was curious why instrumentation was broken and it turned out it was a classic case of unintended property renaming.

coverage instrumentation works by populating a special object called '__jscov' on the window object with coverage metadata.  This means that we need to ensure that both __jscov itself and its various sub properties are not renamed since the code that ultimately reads this data is not part of the same compilation unit.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=200777314
  • Loading branch information
@lukesandberg @blickly
lukesandberg authored and blickly committed Jun 15, 2018
1 parent 0b36751 commit f725964
Show file tree
Hide file tree
Showing 13 changed files with 79 additions and 76 deletions.
17 changes: 9 additions & 8 deletions src/com/google/javascript/jscomp/BranchCoverageInstrumentationCallback.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,33 +182,34 @@ private Node newHeaderNode(NodeTraversal traversal, Node srcref) {
checkNotNull(data);

// var JSCompiler_lcov_branch_data_xx = [];
// __jscov.branchesTaken.push(JSCompiler_lcov_branch_data_xx);
// __jscov['branchesTaken'].push(JSCompiler_lcov_branch_data_xx);
String objName = CoverageInstrumentationPass.JS_INSTRUMENTATION_OBJECT_NAME;
List<Node> nodes = new ArrayList<>();
nodes.add(newArrayDeclarationNode(traversal));
nodes.add(
IR.exprResult(
IR.call(
NodeUtil.newQName(compiler, objName + ".branchesTaken.push"),
IR.getprop(IR.getelem(IR.name(objName), IR.string("branchesTaken")), "push"),
IR.name(createArrayName(traversal)))));
// __jscov.branchPresent.push(hex-data);
// __jscov['branchPresent'].push(hex-data);
nodes.add(
IR.exprResult(
IR.call(
NodeUtil.newQName(compiler, objName + ".branchPresent.push"),
IR.getprop(IR.getelem(IR.name(objName), IR.string("branchPresent")), "push"),
IR.string(data.getBranchPresentAsHexString()))));
nodes.add(newBranchesInLineNode("JSCompiler_lcov_branchesInLine", data));
// __jscov.branchesInLine.push(JSCompiler_lcov_branchesInLine);
// __jscov['branchesInLine'].push(JSCompiler_lcov_branchesInLine);
nodes.add(
IR.exprResult(
IR.call(
NodeUtil.newQName(compiler, objName + ".branchesInLine.push"),
IR.getprop(IR.getelem(IR.name(objName), IR.string("branchesInLine")), "push"),
IR.name("JSCompiler_lcov_branchesInLine"))));
// __jscov.fileNames.push(filename);
// __jscov['fileNames'].push(filename);
nodes.add(
IR.exprResult(
IR.call(
NodeUtil.newQName(compiler, objName + ".fileNames.push"), IR.string(fileName))));
IR.getprop(IR.getelem(IR.name(objName), IR.string("fileNames")), "push"),
IR.string(fileName))));
return IR.block(nodes).useSourceInfoIfMissingFromForTree(srcref);
}

Expand Down
17 changes: 8 additions & 9 deletions src/com/google/javascript/jscomp/CoverageInstrumentationCallback.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,14 @@
class CoverageInstrumentationCallback extends
NodeTraversal.AbstractPostOrderCallback {

private final AbstractCompiler compiler;
private final Map<String, FileInstrumentationData> instrumentationData;
private final CoverageReach reach;

static final String ARRAY_NAME_PREFIX = "JSCompiler_lcov_data_";

public CoverageInstrumentationCallback(
AbstractCompiler compiler,
Map<String, FileInstrumentationData> instrumentationData,
CoverageReach reach) {
this.compiler = compiler;
this.instrumentationData = instrumentationData;
this.reach = reach;
}
Expand Down Expand Up @@ -121,22 +118,24 @@ private Node newHeaderNode(NodeTraversal traversal, Node srcref) {
String objName = CoverageInstrumentationPass.JS_INSTRUMENTATION_OBJECT_NAME;

// var JSCompiler_lcov_data_xx = [];
// __jscov.executedLines.push(JSCompiler_lcov_data_xx);
// __jscov.instrumentedLines.push(hex-data);
// __jscov.fileNames.push(filename);
// __jscov['executedLines'].push(JSCompiler_lcov_data_xx);
// __jscov['instrumentedLines'].push(hex-data);
// __jscov['fileNames'].push(filename);
return IR.block(
newArrayDeclarationNode(traversal),
IR.exprResult(
IR.call(
NodeUtil.newQName(compiler, objName + ".executedLines.push"),
IR.getprop(IR.getelem(IR.name(objName), IR.string("executedLines")), "push"),
IR.name(arrayName))),
IR.exprResult(
IR.call(
NodeUtil.newQName(compiler, objName + ".instrumentedLines.push"),
IR.getprop(
IR.getelem(IR.name(objName), IR.string("instrumentedLines")), "push"),
IR.string(data.getInstrumentedLinesAsHexString()))),
IR.exprResult(
IR.call(
NodeUtil.newQName(compiler, objName + ".fileNames.push"), IR.string(fileName))))
IR.getprop(IR.getelem(IR.name(objName), IR.string("fileNames")), "push"),
IR.string(fileName))))
.useSourceInfoIfMissingFromForTree(srcref);
}

Expand Down
17 changes: 9 additions & 8 deletions src/com/google/javascript/jscomp/CoverageInstrumentationPass.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,7 @@ public void process(Node externsNode, Node rootNode) {
new BranchCoverageInstrumentationCallback(compiler, instrumentationData));
} else {
NodeTraversal.traverse(
compiler,
rootNode,
new CoverageInstrumentationCallback(compiler, instrumentationData, reach));
compiler, rootNode, new CoverageInstrumentationCallback(instrumentationData, reach));
}
Node firstScript = rootNode.getFirstChild();
checkState(firstScript.isScript());
Expand All @@ -102,19 +100,22 @@ public void process(Node externsNode, Node rootNode) {
}

private Node createConditionalObjectDecl(String name, Node srcref) {
// Make sure to quote properties so they are not renamed.
String jscovData;
if (instrumentOption == InstrumentOption.BRANCH_ONLY) {
jscovData = "{fileNames:[], branchPresent:[], branchesInLine: [], branchesTaken: []}";
jscovData = "{'fileNames':[], 'branchPresent':[], 'branchesInLine': [], 'branchesTaken': []}";
} else if (instrumentOption == InstrumentOption.LINE_ONLY) {
jscovData = "{fileNames:[], instrumentedLines: [], executedLines: []}";
jscovData = "{'fileNames':[], 'instrumentedLines': [], 'executedLines': []}";
} else {
jscovData =
"{fileNames:[], instrumentedLines: [], executedLines: [],"
+ " branchPresent:[], branchesInLine: [], branchesTaken: []}";
"{'fileNames:[], 'instrumentedLines: [], 'executedLines': [],"
+ " 'branchPresent':[], 'branchesInLine': [], 'branchesTaken': []}";
}

// Add the __jscov var to the window as a quoted key so it can be found even if property
// renaming is enabled.
String jscovDecl =
" var " + name + " = window.top.__jscov || " + "(window.top.__jscov = " + jscovData + ");";
" var " + name + " = window.top['__jscov'] || (window.top['__jscov'] = " + jscovData + ");";

Node script = compiler.parseSyntheticCode(jscovDecl);
Node var = script.removeFirstChild();
Expand Down
20 changes: 11 additions & 9 deletions test/com/google/javascript/jscomp/bundle/CoverageInstrumenterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,17 @@ private static Source source(Path path, String code) {

public void testCompilerSupplier_instruments() {
CoverageInstrumenter.CompileResult result = compiler.compile(SOURCE_JS, "var x = 42;");
String[] expected = new String[]{
"if(!self.window){self.window=self;self.window.top=self}",
"var __jscov=window.top.__jscov||",
"(window.top.__jscov={fileNames:[],instrumentedLines:[],executedLines:[]});",
"var JSCompiler_lcov_data_source_js=[];",
"__jscov.executedLines.push(JSCompiler_lcov_data_source_js);",
"__jscov.instrumentedLines.push(\"01\");",
"__jscov.fileNames.push(\"source.js\");",
"JSCompiler_lcov_data_source_js[0]=true;var x=42;"};
String[] expected =
new String[] {
"if(!self.window){self.window=self;self.window.top=self}",
"var __jscov=window.top[\"__jscov\"]||",
"(window.top[\"__jscov\"]={\"fileNames\":[],\"instrumentedLines\":[],\"executedLines\":[]});",
"var JSCompiler_lcov_data_source_js=[];",
"__jscov[\"executedLines\"].push(JSCompiler_lcov_data_source_js);",
"__jscov[\"instrumentedLines\"].push(\"01\");",
"__jscov[\"fileNames\"].push(\"source.js\");",
"JSCompiler_lcov_data_source_js[0]=true;var x=42;"
};

assertThat(result.source).isEqualTo(Joiner.on("").join(expected));
assertThat(result.errors).isEmpty();
Expand Down
8 changes: 4 additions & 4 deletions ...google/javascript/jscomp/testdata/CoverageInstrumentationPassTest/ArrowBlockGolden.jsdata
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ if (!self.window) {
self.window = self;
self.window.top = self;
}
var __jscov = window.top.__jscov || (window.top.__jscov = {fileNames:[], instrumentedLines:[], executedLines:[]});
var __jscov = window.top["__jscov"] || (window.top["__jscov"] = {"fileNames":[], "instrumentedLines":[], "executedLines":[]});
var JSCompiler_lcov_data_CoverageInstrumentationPassTest_ArrowBlock_jsdata = [];
__jscov.executedLines.push(JSCompiler_lcov_data_CoverageInstrumentationPassTest_ArrowBlock_jsdata);
__jscov.instrumentedLines.push("01");
__jscov.fileNames.push("CoverageInstrumentationPassTest/ArrowBlock.jsdata");
__jscov["executedLines"].push(JSCompiler_lcov_data_CoverageInstrumentationPassTest_ArrowBlock_jsdata);
__jscov["instrumentedLines"].push("01");
__jscov["fileNames"].push("CoverageInstrumentationPassTest/ArrowBlock.jsdata");
var f = function() {
JSCompiler_lcov_data_CoverageInstrumentationPassTest_ArrowBlock_jsdata[0] = true;
JSCompiler_lcov_data_CoverageInstrumentationPassTest_ArrowBlock_jsdata[0] = true;
Expand Down
8 changes: 4 additions & 4 deletions ...e/javascript/jscomp/testdata/CoverageInstrumentationPassTest/ArrowExpressionGolden.jsdata
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ if (!self.window) {
self.window = self;
self.window.top = self;
}
var __jscov = window.top.__jscov || (window.top.__jscov = {fileNames:[], instrumentedLines:[], executedLines:[]});
var __jscov = window.top["__jscov"] || (window.top["__jscov"] = {"fileNames":[], "instrumentedLines":[], "executedLines":[]});
var JSCompiler_lcov_data_CoverageInstrumentationPassTest_ArrowExpression_jsdata = [];
__jscov.executedLines.push(JSCompiler_lcov_data_CoverageInstrumentationPassTest_ArrowExpression_jsdata);
__jscov.instrumentedLines.push("01");
__jscov.fileNames.push("CoverageInstrumentationPassTest/ArrowExpression.jsdata");
__jscov["executedLines"].push(JSCompiler_lcov_data_CoverageInstrumentationPassTest_ArrowExpression_jsdata);
__jscov["instrumentedLines"].push("01");
__jscov["fileNames"].push("CoverageInstrumentationPassTest/ArrowExpression.jsdata");
var f = function(x) {
JSCompiler_lcov_data_CoverageInstrumentationPassTest_ArrowExpression_jsdata[0] = true;
JSCompiler_lcov_data_CoverageInstrumentationPassTest_ArrowExpression_jsdata[0] = true;
Expand Down
10 changes: 5 additions & 5 deletions ...javascript/jscomp/testdata/CoverageInstrumentationPassTest/DoWhileLoopBranchGolden.jsdata
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@ if (!self.window) {
self.window = self;
self.window.top = self;
}
var __jscov = window.top.__jscov || (window.top.__jscov = {fileNames:[], branchPresent:[], branchesInLine:[], branchesTaken:[]});
var __jscov = window.top["__jscov"] || (window.top["__jscov"] = {"fileNames":[], "branchPresent":[], "branchesInLine":[], "branchesTaken":[]});
var JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_DoWhileLoopBranch_jsdata = [];
__jscov.branchesTaken.push(JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_DoWhileLoopBranch_jsdata);
__jscov.branchPresent.push("04");
__jscov["branchesTaken"].push(JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_DoWhileLoopBranch_jsdata);
__jscov["branchPresent"].push("04");
{
var JSCompiler_lcov_branchesInLine = [];
JSCompiler_lcov_branchesInLine[0] = 2;
}
__jscov.branchesInLine.push(JSCompiler_lcov_branchesInLine);
__jscov.fileNames.push("CoverageInstrumentationPassTest/DoWhileLoopBranch.jsdata");
__jscov["branchesInLine"].push(JSCompiler_lcov_branchesInLine);
__jscov["fileNames"].push("CoverageInstrumentationPassTest/DoWhileLoopBranch.jsdata");
function AlwaysTrue(x) {
var i = 0;
do {
Expand Down
10 changes: 5 additions & 5 deletions ...t/jscomp/testdata/CoverageInstrumentationPassTest/DoWhileLoopMultiLineBranchGolden.jsdata
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@ if (!self.window) {
self.window = self;
self.window.top = self;
}
var __jscov = window.top.__jscov || (window.top.__jscov = {fileNames:[], branchPresent:[], branchesInLine:[], branchesTaken:[]});
var __jscov = window.top["__jscov"] || (window.top["__jscov"] = {"fileNames":[], "branchPresent":[], "branchesInLine":[], "branchesTaken":[]});
var JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_DoWhileLoopMultiLineBranch_jsdata = [];
__jscov.branchesTaken.push(JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_DoWhileLoopMultiLineBranch_jsdata);
__jscov.branchPresent.push("04");
__jscov["branchesTaken"].push(JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_DoWhileLoopMultiLineBranch_jsdata);
__jscov["branchPresent"].push("04");
{
var JSCompiler_lcov_branchesInLine = [];
JSCompiler_lcov_branchesInLine[0] = 2;
}
__jscov.branchesInLine.push(JSCompiler_lcov_branchesInLine);
__jscov.fileNames.push("CoverageInstrumentationPassTest/DoWhileLoopMultiLineBranch.jsdata");
__jscov["branchesInLine"].push(JSCompiler_lcov_branchesInLine);
__jscov["fileNames"].push("CoverageInstrumentationPassTest/DoWhileLoopMultiLineBranch.jsdata");
function AlwaysTrue(x) {
var i = 0;
do {
Expand Down
10 changes: 5 additions & 5 deletions ...gle/javascript/jscomp/testdata/CoverageInstrumentationPassTest/ForLoopBranchGolden.jsdata
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@ if (!self.window) {
self.window = self;
self.window.top = self;
}
var __jscov = window.top.__jscov || (window.top.__jscov = {fileNames:[], branchPresent:[], branchesInLine:[], branchesTaken:[]});
var __jscov = window.top["__jscov"] || (window.top["__jscov"] = {"fileNames":[], "branchPresent":[], "branchesInLine":[], "branchesTaken":[]});
var JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_ForLoopBranch_jsdata = [];
__jscov.branchesTaken.push(JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_ForLoopBranch_jsdata);
__jscov.branchPresent.push("02");
__jscov["branchesTaken"].push(JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_ForLoopBranch_jsdata);
__jscov["branchPresent"].push("02");
{
var JSCompiler_lcov_branchesInLine = [];
JSCompiler_lcov_branchesInLine[0] = 2;
}
__jscov.branchesInLine.push(JSCompiler_lcov_branchesInLine);
__jscov.fileNames.push("CoverageInstrumentationPassTest/ForLoopBranch.jsdata");
__jscov["branchesInLine"].push(JSCompiler_lcov_branchesInLine);
__jscov["fileNames"].push("CoverageInstrumentationPassTest/ForLoopBranch.jsdata");
function IsPostive(x) {
for (var i = 0; i < x; i++) {
JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_ForLoopBranch_jsdata[0] = true;
Expand Down
8 changes: 4 additions & 4 deletions ...m/google/javascript/jscomp/testdata/CoverageInstrumentationPassTest/FunctionGolden.jsdata
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ if (!self.window) {
self.window = self;
self.window.top = self;
}
var __jscov = window.top.__jscov || (window.top.__jscov = {fileNames:[], instrumentedLines:[], executedLines:[]});
var __jscov = window.top["__jscov"] || (window.top["__jscov"] = {"fileNames":[], "instrumentedLines":[], "executedLines":[]});
var JSCompiler_lcov_data_CoverageInstrumentationPassTest_Function_jsdata = [];
__jscov.executedLines.push(JSCompiler_lcov_data_CoverageInstrumentationPassTest_Function_jsdata);
__jscov.instrumentedLines.push("01");
__jscov.fileNames.push("CoverageInstrumentationPassTest/Function.jsdata");
__jscov["executedLines"].push(JSCompiler_lcov_data_CoverageInstrumentationPassTest_Function_jsdata);
__jscov["instrumentedLines"].push("01");
__jscov["fileNames"].push("CoverageInstrumentationPassTest/Function.jsdata");
function f() {
JSCompiler_lcov_data_CoverageInstrumentationPassTest_Function_jsdata[0] = true;
console.log("hi");
Expand Down
10 changes: 5 additions & 5 deletions ...m/google/javascript/jscomp/testdata/CoverageInstrumentationPassTest/IfBranchGolden.jsdata
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@ if (!self.window) {
self.window = self;
self.window.top = self;
}
var __jscov = window.top.__jscov || (window.top.__jscov = {fileNames:[], branchPresent:[], branchesInLine:[], branchesTaken:[]});
var __jscov = window.top["__jscov"] || (window.top["__jscov"] = {"fileNames":[], "branchPresent":[], "branchesInLine":[], "branchesTaken":[]});
var JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_IfBranch_jsdata = [];
__jscov.branchesTaken.push(JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_IfBranch_jsdata);
__jscov.branchPresent.push("02");
__jscov["branchesTaken"].push(JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_IfBranch_jsdata);
__jscov["branchPresent"].push("02");
{
var JSCompiler_lcov_branchesInLine = [];
JSCompiler_lcov_branchesInLine[0] = 2;
}
__jscov.branchesInLine.push(JSCompiler_lcov_branchesInLine);
__jscov.fileNames.push("CoverageInstrumentationPassTest/IfBranch.jsdata");
__jscov["branchesInLine"].push(JSCompiler_lcov_branchesInLine);
__jscov["fileNames"].push("CoverageInstrumentationPassTest/IfBranch.jsdata");
function IsPostive(x) {
if (x > 0) {
JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_IfBranch_jsdata[0] = true;
Expand Down
10 changes: 5 additions & 5 deletions ...ogle/javascript/jscomp/testdata/CoverageInstrumentationPassTest/IfElseBranchGolden.jsdata
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@ if (!self.window) {
self.window = self;
self.window.top = self;
}
var __jscov = window.top.__jscov || (window.top.__jscov = {fileNames:[], branchPresent:[], branchesInLine:[], branchesTaken:[]});
var __jscov = window.top["__jscov"] || (window.top["__jscov"] = {"fileNames":[], "branchPresent":[], "branchesInLine":[], "branchesTaken":[]});
var JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_IfElseBranch_jsdata = [];
__jscov.branchesTaken.push(JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_IfElseBranch_jsdata);
__jscov.branchPresent.push("02");
__jscov["branchesTaken"].push(JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_IfElseBranch_jsdata);
__jscov["branchPresent"].push("02");
{
var JSCompiler_lcov_branchesInLine = [];
JSCompiler_lcov_branchesInLine[0] = 2;
}
__jscov.branchesInLine.push(JSCompiler_lcov_branchesInLine);
__jscov.fileNames.push("CoverageInstrumentationPassTest/IfElseBranch.jsdata");
__jscov["branchesInLine"].push(JSCompiler_lcov_branchesInLine);
__jscov["fileNames"].push("CoverageInstrumentationPassTest/IfElseBranch.jsdata");
function IsPostive(x) {
if (x > 0) {
JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_IfElseBranch_jsdata[0] = true;
Expand Down
10 changes: 5 additions & 5 deletions ...e/javascript/jscomp/testdata/CoverageInstrumentationPassTest/WhileLoopBranchGolden.jsdata
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@ if (!self.window) {
self.window = self;
self.window.top = self;
}
var __jscov = window.top.__jscov || (window.top.__jscov = {fileNames:[], branchPresent:[], branchesInLine:[], branchesTaken:[]});
var __jscov = window.top["__jscov"] || (window.top["__jscov"] = {"fileNames":[], "branchPresent":[], "branchesInLine":[], "branchesTaken":[]});
var JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_WhileLoopBranch_jsdata = [];
__jscov.branchesTaken.push(JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_WhileLoopBranch_jsdata);
__jscov.branchPresent.push("04");
__jscov["branchesTaken"].push(JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_WhileLoopBranch_jsdata);
__jscov["branchPresent"].push("04");
{
var JSCompiler_lcov_branchesInLine = [];
JSCompiler_lcov_branchesInLine[0] = 2;
}
__jscov.branchesInLine.push(JSCompiler_lcov_branchesInLine);
__jscov.fileNames.push("CoverageInstrumentationPassTest/WhileLoopBranch.jsdata");
__jscov["branchesInLine"].push(JSCompiler_lcov_branchesInLine);
__jscov["fileNames"].push("CoverageInstrumentationPassTest/WhileLoopBranch.jsdata");
function IsPostive(x) {
for (var i = 0; i < x;) {
JSCompiler_lcov_branch_data_CoverageInstrumentationPassTest_WhileLoopBranch_jsdata[0] = true;
Expand Down

0 comments on commit f725964

Please sign in to comment.