commit 7557fe41b5d2e7c7441f49fe88cef9ff5b5b58ae
parent c08c3ea0df5664f6e1dae83b7b3d31c8f0176bf4
Author: Jan de Mooij <jdemooij@mozilla.com>
Date: Fri, 7 Nov 2025 06:54:20 +0000
Bug 1982378 part 2 - Fix Debugger.Script.getOffset{Metadata,Location} return value for prologue bytecode ops. r=arai
If the offset was in the prologue, `getOffsetMetadata` and `getOffsetLocation`
were returning information for the first 'main' op instead. This could result in
confusing stepping behavior in DevTools if the first main op is a valid stepping
location, because the debugger would pause in the prologue.
This patch adds a `SkipPrologueOps` argument to `BytecodeRangeWithPosition` and requires
callers to specify whether they want to skip prologue ops.
Differential Revision: https://phabricator.services.mozilla.com/D271579
Diffstat:
4 files changed, 83 insertions(+), 18 deletions(-)
diff --git a/js/src/debugger/Script.cpp b/js/src/debugger/Script.cpp
@@ -921,7 +921,8 @@ class DebuggerScript::GetPossibleBreakpointsMatcher {
return false;
}
- for (BytecodeRangeWithPosition r(cx_, script); !r.empty(); r.popFront()) {
+ for (BytecodeRangeWithPosition r(cx_, script, SkipPrologueOps::Yes);
+ !r.empty(); r.popFront()) {
if (!r.frontIsBreakablePoint()) {
continue;
}
@@ -1023,10 +1024,14 @@ class DebuggerScript::GetOffsetMetadataMatcher {
return false;
}
- BytecodeRangeWithPosition r(cx_, script);
+ // Use SkipPrologueOps::No to ensure we return isBreakpoint = false and
+ // isStepStart = false for prologue ops, instead of the metadata for the
+ // first 'main' op.
+ BytecodeRangeWithPosition r(cx_, script, SkipPrologueOps::No);
while (!r.empty() && r.frontOffset() < offset_) {
r.popFront();
}
+ MOZ_ASSERT(r.frontOffset() == offset_);
RootedValue value(cx_, NumberValue(r.frontLineNumber()));
if (!DefineDataProperty(cx_, result_, cx_->names().lineNumber, value)) {
@@ -1215,7 +1220,8 @@ class FlowGraphSummary {
uint32_t prevLineno = script->lineno();
uint32_t prevColumn = 1;
JSOp prevOp = JSOp::Nop;
- for (BytecodeRangeWithPosition r(cx, script); !r.empty(); r.popFront()) {
+ for (BytecodeRangeWithPosition r(cx, script, SkipPrologueOps::Yes);
+ !r.empty(); r.popFront()) {
uint32_t lineno = prevLineno;
uint32_t column = prevColumn;
JSOp op = r.frontOpcode();
@@ -1336,12 +1342,14 @@ class DebuggerScript::GetOffsetLocationMatcher {
return false;
}
- BytecodeRangeWithPosition r(cx_, script);
+ // Use SkipPrologueOps::No to ensure we return isEntryPoint = false for
+ // prologue ops, instead of the value for the first 'main' op.
+ BytecodeRangeWithPosition r(cx_, script, SkipPrologueOps::No);
while (!r.empty() && r.frontOffset() < offset_) {
r.popFront();
}
+ MOZ_ASSERT(r.frontOffset() == offset_);
- size_t offset = r.frontOffset();
bool isEntryPoint = r.frontIsEntryPoint();
// Line numbers are only correctly defined on entry points. Thus looks
@@ -1378,9 +1386,9 @@ class DebuggerScript::GetOffsetLocationMatcher {
}
// The same entry point test that is used by getAllColumnOffsets.
- isEntryPoint = (isEntryPoint && !flowData[offset].hasNoEdges() &&
- (flowData[offset].lineno() != r.frontLineNumber() ||
- flowData[offset].columnOrSentinel() !=
+ isEntryPoint = (isEntryPoint && !flowData[offset_].hasNoEdges() &&
+ (flowData[offset_].lineno() != r.frontLineNumber() ||
+ flowData[offset_].columnOrSentinel() !=
r.frontColumnNumber().oneOriginValue()));
value.setBoolean(isEntryPoint);
if (!DefineDataProperty(cx_, result_, cx_->names().isEntryPoint, value)) {
@@ -1772,7 +1780,8 @@ bool DebuggerScript::CallData::getAllOffsets() {
if (!result) {
return false;
}
- for (BytecodeRangeWithPosition r(cx, script); !r.empty(); r.popFront()) {
+ for (BytecodeRangeWithPosition r(cx, script, SkipPrologueOps::Yes);
+ !r.empty(); r.popFront()) {
if (!r.frontIsEntryPoint()) {
continue;
}
@@ -1881,7 +1890,8 @@ class DebuggerScript::GetAllColumnOffsetsMatcher {
return false;
}
- for (BytecodeRangeWithPosition r(cx_, script); !r.empty(); r.popFront()) {
+ for (BytecodeRangeWithPosition r(cx_, script, SkipPrologueOps::Yes);
+ !r.empty(); r.popFront()) {
uint32_t lineno = r.frontLineNumber();
JS::LimitedColumnNumberOneOrigin column = r.frontColumnNumber();
size_t offset = r.frontOffset();
@@ -1964,7 +1974,8 @@ class DebuggerScript::GetLineOffsetsMatcher {
}
// Second pass: build the result array.
- for (BytecodeRangeWithPosition r(cx_, script); !r.empty(); r.popFront()) {
+ for (BytecodeRangeWithPosition r(cx_, script, SkipPrologueOps::Yes);
+ !r.empty(); r.popFront()) {
if (!r.frontIsEntryPoint()) {
continue;
}
@@ -2424,7 +2435,8 @@ bool DebuggerScript::CallData::getOffsetsCoverage() {
RootedValue countValue(cx);
// Iterate linearly over the bytecode.
- for (BytecodeRangeWithPosition r(cx, script); !r.empty(); r.popFront()) {
+ for (BytecodeRangeWithPosition r(cx, script, SkipPrologueOps::Yes);
+ !r.empty(); r.popFront()) {
size_t offset = r.frontOffset();
// The beginning of each non-branching sequences of instruction set the
diff --git a/js/src/jit-test/tests/debug/Script-prologue-offsets.js b/js/src/jit-test/tests/debug/Script-prologue-offsets.js
@@ -0,0 +1,48 @@
+let g = newGlobal({newCompartment: true});
+let dbg = new Debugger(g);
+let count = 0;
+dbg.onEnterFrame = function(frame) {
+ if (frame.type === "eval") {
+ return;
+ }
+
+ // This test assumes the hello1 and hello2 functions have prologue bytecode.
+ assertEq(frame.script.mainOffset > 0, true);
+
+ // Prologue bytecode ops are never marked as break/step points.
+ let prologueData = frame.script.getOffsetMetadata(0);
+ assertEq(prologueData.isBreakpoint, false);
+ assertEq(prologueData.isStepStart, false);
+
+ // Prologue ops are also never marked as entry points.
+ let location = frame.script.getOffsetLocation(0);
+ assertEq(location.isEntryPoint, false);
+ if (count == 0) {
+ // hello1
+ assertEq(location.lineNumber, 3);
+ assertEq(location.columnNumber, 5);
+ } else {
+ // hello2
+ assertEq(location.lineNumber, 5);
+ assertEq(location.columnNumber, 18);
+ }
+
+ // getPossibleBreakpointOffsets shouldn't return prologue offsets.
+ for (let offset of frame.script.getPossibleBreakpointOffsets()) {
+ assertEq(offset >= frame.script.mainOffset, true);
+ }
+
+ count++;
+};
+g.eval(` // 1
+ function hello1(name) { // 2
+ return [].some((r) => r === name); // 3
+ } // 4
+ function hello2(name, x=1) { // 5
+ return [].some((r) => r === name); // 6
+ } // 7
+ hello1("world"); // 8
+ hello2("world"); // 9
+`);
+
+assertEq(count, 2);
diff --git a/js/src/vm/BytecodeUtil-inl.h b/js/src/vm/BytecodeUtil-inl.h
@@ -108,6 +108,8 @@ class BytecodeRange {
jsbytecode* end;
};
+enum class SkipPrologueOps { No, Yes };
+
class BytecodeRangeWithPosition : private BytecodeRange {
public:
using BytecodeRange::empty;
@@ -115,7 +117,8 @@ class BytecodeRangeWithPosition : private BytecodeRange {
using BytecodeRange::frontOpcode;
using BytecodeRange::frontPC;
- BytecodeRangeWithPosition(JSContext* cx, JSScript* script)
+ BytecodeRangeWithPosition(JSContext* cx, JSScript* script,
+ SkipPrologueOps skipPrologueOps)
: BytecodeRange(cx, script),
initialLine(script->lineno()),
lineno(script->lineno()),
@@ -130,10 +133,12 @@ class BytecodeRangeWithPosition : private BytecodeRange {
snpc += sn->delta();
}
updatePosition();
- while (frontPC() != mainPC) {
- popFront();
+ if (skipPrologueOps == SkipPrologueOps::Yes) {
+ while (frontPC() != mainPC) {
+ popFront();
+ }
+ MOZ_ASSERT(entryPointState != EntryPointState::NotEntryPoint);
}
- MOZ_ASSERT(entryPointState != EntryPointState::NotEntryPoint);
}
void popFront() {
diff --git a/js/src/vm/BytecodeUtil.cpp b/js/src/vm/BytecodeUtil.cpp
@@ -2769,8 +2769,8 @@ static bool GetPCCountJSON(JSContext* cx, const ScriptAndCounts& sac,
json.beginListProperty("opcodes");
uint64_t hits = 0;
- for (BytecodeRangeWithPosition range(cx, script); !range.empty();
- range.popFront()) {
+ for (BytecodeRangeWithPosition range(cx, script, SkipPrologueOps::Yes);
+ !range.empty(); range.popFront()) {
jsbytecode* pc = range.frontPC();
size_t offset = script->pcToOffset(pc);
JSOp op = JSOp(*pc);
