Bug 1378808 - Use ::Arguments or ::PropertyName location for method call column offsets. r=jorendorff draft
authorLogan F Smyth <loganfsmyth@gmail.com>
Thu, 12 Jul 2018 11:51:17 -0700
changeset 826416 c72426a76436f0cfa1490a0e7e832a21ec9349d8
parent 823737 3b4f4db5d3d78170faf00bdf9fe2a562ad155fd5
push id118325
push userbmo:loganfsmyth@gmail.com
push dateFri, 03 Aug 2018 20:04:13 +0000
reviewersjorendorff
bugs1378808
milestone63.0a1
Bug 1378808 - Use ::Arguments or ::PropertyName location for method call column offsets. r=jorendorff MozReview-Commit-ID: G8mG1qsIO21
js/src/frontend/BytecodeEmitter.cpp
js/src/jit-test/lib/assert-offset-columns.js
js/src/jit-test/tests/debug/Script-getAllColumnOffsets.js
js/src/jsapi-tests/testSavedStacks.cpp
js/xpconnect/tests/unit/test_getCallerLocation.js
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -9690,27 +9690,72 @@ BytecodeEmitter::emitCallOrNew(ParseNode
             if (!emitDupAt(argc + 1))
                 return false;
         } else {
             if (!emitDupAt(2))
                 return false;
         }
     }
 
+    ParseNode* coordNode = pn;
+    if (pn->isOp(JSOP_CALL) || pn->isOp(JSOP_SPREADCALL)) {
+        switch (pn_callee->getKind()) {
+          case ParseNodeKind::Dot: {
+
+            // Check if this member is a simple chain of simple chain of
+            // property accesses, e.g. x.y.z, this.x.y, super.x.y
+            bool simpleDotChain = false;
+            for (ParseNode* cur = pn_callee; cur->isKind(ParseNodeKind::Dot); cur = cur->pn_left) {
+                ParseNode* left = cur->pn_left;
+                if (left->isKind(ParseNodeKind::Name) || left->isKind(ParseNodeKind::This) ||
+                    left->isKind(ParseNodeKind::SuperBase))
+                {
+                    simpleDotChain = true;
+                }
+            }
+
+            if (!simpleDotChain) {
+                // obj().aprop() // expression
+                //       ^       // column coord
+                //
+                // Note: Because of the constant folding logic in FoldElement,
+                // this case also applies for constant string properties.
+                //
+                // obj()['aprop']() // expression
+                //       ^          // column coord
+                coordNode = pn_callee->pn_right;
+            }
+            break;
+          }
+          case ParseNodeKind::Elem:
+            // obj[expr]() // expression
+            //          ^  // column coord
+            coordNode = pn_args;
+            break;
+          default:
+            break;
+        }
+    }
+
     if (!spread) {
         if (pn->getOp() == JSOP_CALL && valueUsage == ValueUsage::IgnoreValue) {
-            if (!emitCall(JSOP_CALL_IGNORES_RV, argc, pn))
+            if (!emitCall(JSOP_CALL_IGNORES_RV, argc, coordNode))
                 return false;
             checkTypeSet(JSOP_CALL_IGNORES_RV);
         } else {
-            if (!emitCall(pn->getOp(), argc, pn))
+            if (!emitCall(pn->getOp(), argc, coordNode))
                 return false;
             checkTypeSet(pn->getOp());
         }
     } else {
+        if (coordNode) {
+            if (!updateSourceCoordNotes(coordNode->pn_pos.begin))
+                return false;
+        }
+
         if (!emit1(pn->getOp()))
             return false;
         checkTypeSet(pn->getOp());
     }
     if (pn->isOp(JSOP_EVAL) ||
         pn->isOp(JSOP_STRICTEVAL) ||
         pn->isOp(JSOP_SPREADEVAL) ||
         pn->isOp(JSOP_STRICTSPREADEVAL))
--- a/js/src/jit-test/lib/assert-offset-columns.js
+++ b/js/src/jit-test/lib/assert-offset-columns.js
@@ -28,17 +28,22 @@ function assertOffsetColumns(code, expec
     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;
+    let debuggeeFn = dbg.addDebuggee(global).makeDebuggeeValue(global.f);
+    if (debuggeeFn.isBoundFunction) {
+        debuggeeFn = debuggeeFn.boundTargetFunction;
+    }
+
+    const { script } = debuggeeFn;
     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);
--- a/js/src/jit-test/tests/debug/Script-getAllColumnOffsets.js
+++ b/js/src/jit-test/tests/debug/Script-getAllColumnOffsets.js
@@ -78,8 +78,63 @@ assertOffsetColumns(
     "                ^                 ^            ^         ^",
 );
 
 // getColumnOffsets correctly places the various parts of a DoWhileStatement.
 assertOffsetColumns(
     "function f(n) { do { print(n); } while(false); }",
     "                ^    ^                         ^",
 );
+
+// getColumnOffsets correctly places the part of normal ::Dot node with identifier root.
+assertOffsetColumns(
+    "var args = [];\n" +
+    "var obj = { base: { a(){ return { b(){} }; } } };\n" +
+    "function f(n) { obj.base.a().b(...args); }",
+    "                ^            ^ ^         ^",
+    "0 2 1 3",
+);
+
+// getColumnOffsets correctly places the part of normal ::Dot node with "this" root.
+assertOffsetColumns(
+    "var args = [];\n" +
+    "var obj = { base: { a(){ return { b(){} }; } } };\n" +
+    "var f = function() { this.base.a().b(...args);  }.bind(obj);",
+    "                     ^             ^ ^          ^",
+    "0 2 1 3",
+);
+
+// getColumnOffsets correctly places the part of normal ::Dot node with "super" base.
+assertOffsetColumns(
+    "var args = [];\n" +
+    "var obj = { base: { a(){ return { b(){} }; } } };\n" +
+    "var f = { __proto__: obj, f(n) { super.base.a().b(...args); } }.f;",
+    "                                 ^              ^ ^         ^",
+    "0 2 1 3",
+);
+
+// getColumnOffsets correctly places the part of normal ::Dot node with other base.
+assertOffsetColumns(
+    "var args = [];\n" +
+    "var obj = { base: { a(){ return { b(){} }; } } };\n" +
+    "function f(n) { (0, obj).base.a().b(...args); }",
+    "                 ^  ^         ^   ^ ^         ^",
+    "0 1 2 4 3 5",
+);
+
+// getColumnOffsets correctly places the part of folded ::Elem node.
+assertOffsetColumns(
+    "var args = [];\n" +
+    "var obj = { base: { a(){ return { b(){} }; } } };\n" +
+    // Constant folding makes the static string behave like a dot access.
+    "function f(n) { obj.base['a']()['b'](...args); }",
+    "                ^               ^    ^         ^",
+    "0 2 1 3",
+);
+
+// getColumnOffsets correctly places the part of computed ::Elem node.
+assertOffsetColumns(
+    "var args = [], a = 'a', b = 'b';\n" +
+    "var obj = { base: { a(){ return { b(){} }; } } };\n" +
+    "function f(n) { obj.base[a]()[b](...args); }",
+    "                ^          ^    ^^         ^",
+    "0 1 3 2 4",
+);
--- a/js/src/jsapi-tests/testSavedStacks.cpp
+++ b/js/src/jsapi-tests/testSavedStacks.cpp
@@ -247,17 +247,17 @@ BEGIN_TEST(testSavedStacks_selfHostedFra
     CHECK(result == JS::SavedFrameResult::Ok);
     CHECK_EQUAL(line, 3U);
 
     // Column
     uint32_t column = 123;
     result = JS::GetSavedFrameColumn(cx, selfHostedFrame, &column,
                                      JS::SavedFrameSelfHosted::Exclude);
     CHECK(result == JS::SavedFrameResult::Ok);
-    CHECK_EQUAL(column, 5U);
+    CHECK_EQUAL(column, 9U);
 
     // Function display name
     result = JS::GetSavedFrameFunctionDisplayName(cx, selfHostedFrame, &str,
                                                   JS::SavedFrameSelfHosted::Exclude);
     CHECK(result == JS::SavedFrameResult::Ok);
     lin = str->ensureLinear(cx);
     CHECK(lin);
     CHECK(js::StringEqualsAscii(lin, "one"));
--- a/js/xpconnect/tests/unit/test_getCallerLocation.js
+++ b/js/xpconnect/tests/unit/test_getCallerLocation.js
@@ -34,33 +34,33 @@ add_task(async function() {
   `, sandbox, undefined, "thing.js");
 
   Cu.exportFunction(foo, sandbox, {defineAs: "foo"});
 
   let frame = sandbox.thing();
 
   equal(frame.source, "thing.js", "Frame source");
   equal(frame.line, 5, "Frame line");
-  equal(frame.column, 14, "Frame column");
+  equal(frame.column, 18, "Frame column");
   equal(frame.functionDisplayName, "it", "Frame function name");
   equal(frame.parent, null, "Frame parent");
 
-  equal(String(frame), "it@thing.js:5:14\n", "Stringified frame");
+  equal(String(frame), "it@thing.js:5:18\n", "Stringified frame");
 
 
   // reportError
 
   let {messages} = await AddonTestUtils.promiseConsoleOutput(() => {
     Cu.reportError("Meh", frame);
   });
 
   let [msg] = messages.filter(m => m.message.includes("Meh"));
 
   equal(msg.stack, frame, "reportError stack frame");
-  equal(msg.message, '[JavaScript Error: "Meh" {file: "thing.js" line: 5}]\nit@thing.js:5:14\n');
+  equal(msg.message, '[JavaScript Error: "Meh" {file: "thing.js" line: 5}]\nit@thing.js:5:18\n');
 
   Assert.throws(() => { Cu.reportError("Meh", {}); },
                 err => err.result == Cr.NS_ERROR_INVALID_ARG,
                 "reportError should throw when passed a non-SavedFrame object");
 
 
   // createError
 
@@ -77,10 +77,10 @@ add_task(async function() {
   equal(Cu.getGlobalForObject(error), sandbox,
         "createError creates errors in the global of the SavedFrame");
   equal(error.stack, String(cloned),
         "createError creates errors with the correct stack");
 
   equal(error.message, "Meh", "Error message");
   equal(error.fileName, "thing.js", "Error filename");
   equal(error.lineNumber, 5, "Error line");
-  equal(error.columnNumber, 14, "Error column");
+  equal(error.columnNumber, 18, "Error column");
 });