Bug 1472291 - Ensure that if, switch, do-while, with, break, and continue statements have column offsets. r=jorendorff
MozReview-Commit-ID: J1RL0xbW0qR
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -4758,16 +4758,20 @@ BytecodeEmitter::emitNumberOp(double dva
*/
MOZ_NEVER_INLINE bool
BytecodeEmitter::emitSwitch(ParseNode* pn)
{
ParseNode* cases = pn->pn_right;
MOZ_ASSERT(cases->isKind(ParseNodeKind::LexicalScope) ||
cases->isKind(ParseNodeKind::StatementList));
+ // Ensure that the column of the switch statement is set properly.
+ if (!updateSourceCoordNotes(pn->pn_pos.begin))
+ return false;
+
// Emit code for the discriminant.
if (!emitTree(pn->pn_left))
return false;
// Enter the scope before pushing the switch BreakableControl since all
// breaks are under this scope.
Maybe<TDZCheckCache> tdzCache;
Maybe<EmitterScope> emitterScope;
@@ -7017,16 +7021,21 @@ BytecodeEmitter::emitTry(ParseNode* pn)
}
bool
BytecodeEmitter::emitIf(ParseNode* pn)
{
IfEmitter ifThenElse(this);
if_again:
+ // Make sure this code is attributed to the "if" so that it gets a useful
+ // column number, instead of the default 0 value.
+ if (!updateSourceCoordNotes(pn->pn_pos.begin))
+ return false;
+
/* Emit code for the condition before pushing stmtInfo. */
if (!emitTree(pn->pn_kid1))
return false;
ParseNode* elseNode = pn->pn_kid3;
if (elseNode) {
if (!ifThenElse.emitThenElse())
return false;
@@ -7147,16 +7156,20 @@ BytecodeEmitter::emitLexicalScope(ParseN
}
return emitterScope.leave(this);
}
bool
BytecodeEmitter::emitWith(ParseNode* pn)
{
+ // Ensure that the column of the 'with' is set properly.
+ if (!updateSourceCoordNotes(pn->pn_pos.begin))
+ return false;
+
if (!emitTree(pn->pn_left))
return false;
EmitterScope emitterScope(this);
if (!emitterScope.enterWith(this))
return false;
if (!emitTree(pn->pn_right))
@@ -8285,16 +8298,20 @@ BytecodeEmitter::emitAsyncWrapper(unsign
return false;
}
return true;
}
bool
BytecodeEmitter::emitDo(ParseNode* pn)
{
+ // Ensure that the column of the 'do' is set properly.
+ if (!updateSourceCoordNotes(pn->pn_pos.begin))
+ return false;
+
/* Emit an annotated nop so IonBuilder can recognize the 'do' loop. */
unsigned noteIndex;
if (!newSrcNote(SRC_WHILE, ¬eIndex))
return false;
if (!emit1(JSOP_NOP))
return false;
unsigned noteIndex2;
@@ -10938,21 +10955,29 @@ BytecodeEmitter::emitTree(ParseNode* pn,
break;
case ParseNodeKind::For:
if (!emitFor(pn))
return false;
break;
case ParseNodeKind::Break:
+ // Ensure that the column of the 'break' is set properly.
+ if (!updateSourceCoordNotes(pn->pn_pos.begin))
+ return false;
+
if (!emitBreak(pn->as<BreakStatement>().label()))
return false;
break;
case ParseNodeKind::Continue:
+ // Ensure that the column of the 'continue' is set properly.
+ if (!updateSourceCoordNotes(pn->pn_pos.begin))
+ return false;
+
if (!emitContinue(pn->as<ContinueStatement>().label()))
return false;
break;
case ParseNodeKind::With:
if (!emitWith(pn))
return false;
break;
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/lib/assert-offset-columns.js
@@ -0,0 +1,74 @@
+// Set breakpoints "everywhere" in a function, then call the function and check that
+// the breakpoints were added are at the expected columns, and the breakpoints
+// were executed in th expected order.
+//
+// `code` is a JS Script. The final line should define a function `f` to validate.
+// `expectedBpts` is a string of spaces and carets ('^'). Throws if we don't hit
+// breakpoints on exactly the columns indicated by the carets.
+// `expectedOrdering` is a string of integer indices for the offsets that are
+// executed, in the order that then are executed. Test code can also push
+// additional items into this string using items.push("!").
+function assertOffsetColumns(code, expectedBpts, expectedOrdering = null) {
+ if (expectedOrdering === null) {
+ // The default ordering simply runs the breakpoints in order.
+ expectedOrdering = Array.from(expectedBpts.match(/\^/g), (_, i) => i).join(" ");
+ }
+
+ // Define the function `f` in a new global.
+ const global = newGlobal();
+
+ const lines = code.split(/\r?\n|\r]/g);
+ const initCode = lines.slice(0, -1).join("\n");
+ const execCode = lines[lines.length - 1];
+
+ // Treat everything but the last line as initialization code.
+ global.eval(initCode);
+
+ // Run the test code itself.
+ global.eval(execCode);
+
+ // Allow some tests to append to a log that will show up in expected ordering.
+ const hits = global.hits = [];
+ const bpts = new Set();
+
+ // Set breakpoints everywhere and call the function.
+ const dbg = new Debugger;
+ const script = dbg.addDebuggee(global).makeDebuggeeValue(global.f).script;
+ for (const offset of script.getAllColumnOffsets()) {
+ assertEq(offset.lineNumber, 1);
+ assertEq(offset.columnNumber < execCode.length, true);
+ bpts.add(offset.columnNumber);
+
+ script.setBreakpoint(offset.offset, {
+ hit(frame) {
+ hits.push(offset.columnNumber);
+ },
+ });
+ }
+ global.f(3);
+
+ const actualBpts = Array.from(execCode, (_, i) => {
+ return bpts.has(i) ? "^" : " ";
+ }).join("");
+
+ if (actualBpts.trimEnd() !== expectedBpts.trimEnd()) {
+ throw new Error(`Assertion failed:
+ code: ${execCode}
+ expected bpts: ${expectedBpts}
+ actual bpts: ${actualBpts}\n`);
+ }
+
+ const indexLookup = new Map(
+ Array.from(bpts).sort().map((col, i) => [col, i]));
+ const actualOrdering = hits
+ .map(item => typeof item === "number" ? indexLookup.get(item) : item)
+ .join(" ");
+
+ if (actualOrdering.trimEnd() !== expectedOrdering.trimEnd()) {
+ throw new Error(`Assertion failed:
+ code: ${execCode}
+ bpts: ${expectedBpts}
+ expected order: ${expectedOrdering}
+ actual order: ${actualOrdering}\n`);
+ }
+}
deleted file mode 100644
--- a/js/src/jit-test/tests/debug/Script-getAllColumnOffsets-01.js
+++ /dev/null
@@ -1,19 +0,0 @@
-// getColumnOffsets correctly places the various parts of a ForStatement.
-
-var global = newGlobal();
-Debugger(global).onDebuggerStatement = function (frame) {
- var script = frame.eval("f").return.script;
- script.getAllColumnOffsets().forEach(function (offset) {
- script.setBreakpoint(offset.offset, {
- hit: function (frame) {
- assertEq(offset.lineNumber, 1);
- global.log += offset.columnNumber + " ";
- }
- });
- });
-};
-
-global.log = '';
-global.eval("function f(n) { for (var i = 0; i < n; ++i) log += '. '; log += '! '; } debugger;");
-global.f(3);
-assertEq(global.log, "25 32 44 . 39 32 44 . 39 32 44 . 39 32 57 ! 70 ");
deleted file mode 100644
--- a/js/src/jit-test/tests/debug/Script-getAllColumnOffsets-02.js
+++ /dev/null
@@ -1,21 +0,0 @@
-// getColumnOffsets correctly places multiple variable declarations.
-
-var global = newGlobal();
-Debugger(global).onDebuggerStatement = function (frame) {
- var script = frame.eval("f").return.script;
- script.getAllColumnOffsets().forEach(function (offset) {
- script.setBreakpoint(offset.offset, {
- hit: function (frame) {
- assertEq(offset.lineNumber, 1);
- global.log += offset.columnNumber + " ";
- }
- });
- });
-};
-
-global.log = '';
-global.eval("function f(n){var w0,x1=3,y2=4,z3=9} debugger;");
-global.f(3);
-
-// Should have hit each variable declared.
-assertEq(global.log, "21 26 31 35 ");
deleted file mode 100644
--- a/js/src/jit-test/tests/debug/Script-getAllColumnOffsets-03.js
+++ /dev/null
@@ -1,20 +0,0 @@
-// getColumnOffsets correctly places comma separated expressions.
-
-var global = newGlobal();
-Debugger(global).onDebuggerStatement = function (frame) {
- var script = frame.eval("f").return.script;
- script.getAllColumnOffsets().forEach(function (offset) {
- script.setBreakpoint(offset.offset, {
- hit: function (frame) {
- assertEq(offset.lineNumber, 1);
- global.log += offset.columnNumber + " ";
- }
- });
- });
-};
-
-global.log = '';
-global.eval("function f(n){print(n),print(n),print(n)} debugger;");
-global.f(3);
-// Should hit each call that was separated by commas.
-assertEq(global.log, "14 23 32 40 ");
deleted file mode 100644
--- a/js/src/jit-test/tests/debug/Script-getAllColumnOffsets-04.js
+++ /dev/null
@@ -1,20 +0,0 @@
-// getColumnOffsets correctly places object properties.
-
-var global = newGlobal();
-Debugger(global).onDebuggerStatement = function (frame) {
- var script = frame.eval("f").return.script;
- script.getAllColumnOffsets().forEach(function (offset) {
- script.setBreakpoint(offset.offset, {
- hit: function (frame) {
- assertEq(offset.lineNumber, 1);
- global.log += offset.columnNumber + " ";
- }
- });
- });
-};
-
-global.log = '';
-global.eval("function f(n){var o={a:1,b:2,c:3}} debugger;");
-global.f(3);
-// Should hit each property in the object.
-assertEq(global.log, "18 21 25 29 33 ");
deleted file mode 100644
--- a/js/src/jit-test/tests/debug/Script-getAllColumnOffsets-05.js
+++ /dev/null
@@ -1,20 +0,0 @@
-// getColumnOffsets correctly places array properties.
-
-var global = newGlobal();
-Debugger(global).onDebuggerStatement = function (frame) {
- var script = frame.eval("f").return.script;
- script.getAllColumnOffsets().forEach(function (offset) {
- script.setBreakpoint(offset.offset, {
- hit: function (frame) {
- assertEq(offset.lineNumber, 1);
- global.log += offset.columnNumber + " ";
- }
- });
- });
-};
-
-global.log = '';
-global.eval("function f(n){var a=[1,2,n]} debugger;");
-global.f(3);
-// Should hit each item in the array.
-assertEq(global.log, "18 21 23 25 27 ");
deleted file mode 100644
--- a/js/src/jit-test/tests/debug/Script-getAllColumnOffsets-06.js
+++ /dev/null
@@ -1,28 +0,0 @@
-// getColumnOffsets correctly places function calls.
-
-var global = newGlobal();
-Debugger(global).onDebuggerStatement = function (frame) {
- var script = frame.eval("f").return.script;
- script.getAllColumnOffsets().forEach(function (offset) {
- script.setBreakpoint(offset.offset, {
- hit: function (frame) {
- assertEq(offset.lineNumber, 1);
- global.log += offset.columnNumber + " ";
- }
- });
- });
-};
-
-global.log = "";
-global.eval("function ppppp() { return 1; }");
-// 1 2 3 4
-// 01234567890123456789012345678901234567890123456789
-global.eval("function f(){ 1 && ppppp(ppppp()) && new Error() } debugger;");
-global.f();
-
-// 14 - Enter the function body
-// 25 - Inner print()
-// 19 - Outer print()
-// 37 - new Error()
-// 49 - Exit the function body
-assertEq(global.log, "14 25 19 37 49 ");
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Script-getAllColumnOffsets.js
@@ -0,0 +1,85 @@
+load(libdir + "assert-offset-columns.js");
+
+// getColumnOffsets correctly places the various parts of a ForStatement.
+assertOffsetColumns(
+ "function f(n) { for (var i = 0; i < n; ++i) hits.push('.'); hits.push('!'); }",
+ " ^ ^ ^ ^ ^ ^",
+ "0 1 3 . 2 1 3 . 2 1 3 . 2 1 4 ! 5",
+);
+
+// getColumnOffsets correctly places multiple variable declarations.
+assertOffsetColumns(
+ "function f(n){var w0,x1=3,y2=4,z3=9}",
+ " ^ ^ ^ ^",
+);
+
+// getColumnOffsets correctly places comma separated expressions.
+assertOffsetColumns(
+ "function f(n){print(n),print(n),print(n)}",
+ " ^ ^ ^ ^",
+);
+
+// getColumnOffsets correctly places object properties.
+assertOffsetColumns(
+ // Should hit each property in the object.
+ "function f(n){var o={a:1,b:2,c:3}}",
+ " ^ ^ ^ ^ ^",
+);
+
+// getColumnOffsets correctly places array properties.
+assertOffsetColumns(
+ // Should hit each item in the array.
+ "function f(n){var a=[1,2,n]}",
+ " ^ ^ ^ ^ ^",
+);
+
+// getColumnOffsets correctly places function calls.
+assertOffsetColumns(
+ "function ppppp() { return 1; }\n" +
+ "function f(){ 1 && ppppp(ppppp()) && new Error() }",
+ " ^ ^ ^ ^ ^",
+ "0 2 1 3 4",
+);
+
+// getColumnOffsets correctly places the various parts of a SwitchStatement.
+assertOffsetColumns(
+ "function f(n) { switch(n) { default: print(n); } }",
+ " ^ ^ ^",
+);
+
+// getColumnOffsets correctly places the various parts of a BreakStatement.
+assertOffsetColumns(
+ "function f(n) { do { print(n); break; } while(false); }",
+ " ^ ^ ^ ^",
+);
+
+// getColumnOffsets correctly places the various parts of a ContinueStatement.
+assertOffsetColumns(
+ "function f(n) { do { print(n); continue; } while(false); }",
+ " ^ ^ ^ ^",
+);
+
+// getColumnOffsets correctly places the various parts of a WithStatement.
+assertOffsetColumns(
+ "function f(n) { with({}) { print(n); } }",
+ " ^ ^ ^",
+);
+
+// getColumnOffsets correctly places the various parts of a IfStatement.
+assertOffsetColumns(
+ "function f(n) { if (n == 3) print(n); }",
+ " ^ ^ ^",
+);
+
+// getColumnOffsets correctly places the various parts of a IfStatement
+// with an if/else
+assertOffsetColumns(
+ "function f(n) { if (n == 2); else if (n === 3) print(n); }",
+ " ^ ^ ^ ^",
+);
+
+// getColumnOffsets correctly places the various parts of a DoWhileStatement.
+assertOffsetColumns(
+ "function f(n) { do { print(n); } while(false); }",
+ " ^ ^ ^",
+);