Bug 1362416 - fix FlowGraphSummary to handle branch target before the branch; r=jimb
authorTom Tromey <tom@tromey.com>
Fri, 05 May 2017 14:14:40 -0600
changeset 357066 b2bec2797b6d967c762e4ff2423c55702e5a862d
parent 357065 7d0f07552a6b9032d11946fbdc4c7df2f08b7cc1
child 357067 03c0ea04be4e990a8d7ecbdeb8107297ab9402e2
push id31781
push userkwierso@gmail.com
push dateMon, 08 May 2017 20:44:15 +0000
treeherdermozilla-central@e0955584782e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimb
bugs1362416
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1362416 - fix FlowGraphSummary to handle branch target before the branch; r=jimb FlowGraphSummary walks the bytecode linearly, assuming that a branch instruction will always be visited before the branch's target. However, this is not the case for JSOP_LOOPHEAD, leading to an incorrect line number (-1). This patch changes it to instead reuse the location of the previous opcode, which is correct in the case of a loop head. MozReview-Commit-ID: 5OmLmSk2uSn
js/src/jit-test/tests/debug/Frame-onStep-17.js
js/src/vm/Debugger.cpp
--- a/js/src/jit-test/tests/debug/Frame-onStep-17.js
+++ b/js/src/jit-test/tests/debug/Frame-onStep-17.js
@@ -15,17 +15,19 @@ dbg.onDebuggerStatement = function (fram
       if (thisline !== previous) {
         log += thisline;
         previous = thisline;
       }
     }
   };
 };
 
-function testOne(loopKind) {
-  let body = "var array = [2, 4, 6];\ndebugger;\nfor (let iter " +
+function testOne(decl, loopKind) {
+  let body = "var array = [2, 4, 6];\ndebugger;\nfor (" + decl + " iter " +
       loopKind + " array) {\n  print(iter);\n}\n";
   g.eval(body);
   assertEq(log, "12121212");
 }
 
-testOne("in");
-testOne("of");
+for (let decl of ["", "var", "let"]) {
+  testOne(decl, "in");
+  testOne(decl, "of");
+}
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -6034,17 +6034,22 @@ class FlowGraphSummary {
         for (BytecodeRangeWithPosition r(cx, script); !r.empty(); r.popFront()) {
             size_t lineno = prevLineno;
             size_t column = prevColumn;
             JSOp op = r.frontOpcode();
 
             if (FlowsIntoNext(prevOp))
                 addEdge(prevLineno, prevColumn, r.frontOffset());
 
-            if (BytecodeIsJumpTarget(op)) {
+            // If we visit the branch target before we visit the
+            // branch op itself, just reuse the previous location.
+            // This is reasonable for the time being because this
+            // situation can currently only arise from loop heads,
+            // where this assumption holds.
+            if (BytecodeIsJumpTarget(op) && !entries_[r.frontOffset()].hasNoEdges()) {
                 lineno = entries_[r.frontOffset()].lineno();
                 column = entries_[r.frontOffset()].column();
             }
 
             if (r.frontIsEntryPoint()) {
                 lineno = r.frontLineNumber();
                 column = r.frontColumnNumber();
             }