1811199 - DevTools points to wrong position on searchfox

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Kagami Rosylight [:saschanaz] (they/them)]

Thank you for helping make Firefox better. If you are reporting a defect, please complete the following:

What were you doing?

Please tell us what site you were on, and what steps led to the error you are reporting

4. Open https://searchfox.org/mozilla-central/rev/daf613efc5c358f3a94961d73b90472c00703838/dom/streams/ReadableStream.h#99
5. Open Debugger and make sure it pauses on uncaught exceptions
6. Click on any C++ parameter name, for example aCx.
7. An exception happens

What happened?

It somehow point to line 203 of context-menu.js

What should have happened?

The debugger should point to line 22, which is inside fmt() and is the exact position the JavaScript TypeError shows:

Uncaught TypeError: can't access property "replace", data is undefined
    fmt https://searchfox.org/mozilla-central/static/js/context-menu.js:22
    tryShowOnClick https://searchfox.org/mozilla-central/static/js/context-menu.js:194
    ContextMenu https://searchfox.org/mozilla-central/static/js/context-menu.js:18
    ContextMenu https://searchfox.org/mozilla-central/static/js/context-menu.js:18
    <anonymous> https://searchfox.org/mozilla-central/static/js/context-menu.js:1

Anything else we should know?

This happens on Nightly 2022-01-16 from mozregression, so it's not a regression. Chrome and Safari properly points to line 22.

Comment 1

[Kagami Rosylight [:saschanaz] (they/them)]

Will you be able to wait until we get a minimal repro before fixing the searchfox issue? Thanks 👍

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

I set up this simpler case: https://ffx-devtools-pause-on-exception-wrong-location.glitch.me/

Here's the script that throws:

function fmt(s) {
  return s.replace("_", "");
}

const x = [];
for (const search of [1]) {
  x.push(
    fmt()
  );
}

It seems like being in a for loop might be the important part

Comment 3

[Kagami Rosylight [:saschanaz] (they/them)]

Amazing, thanks!

Comment 4

[Alexandre Poirot [:ochameau]]

This fails because the debugger is confused right here:
https://searchfox.org/mozilla-central/rev/df68a65540f2227e27a12ed0b491188e2927f6d5/devtools/server/actors/thread.js#1952

for (let frame = youngestFrame; frame != null; frame = frame.older) {
     if (frame.script.isInCatchScope(frame.offset)) {
        willBeCaught = true;

DebuggerAPI.onExceptionUnwind works fine.

Given the following code:

function f() {
  function foo() {
    throw 16; // <= first frame, "youngestFrame" correctly refers to here, and its isInCatchScope is false, as excepted.
  }

  for (const _ of [1]) { // <= this only fails with "of"
    foo(); // <= the second iteration in the thread actor code, refer to youndestFrame.older, but here isInCatchScope is true, which is unexpected
  }
}

isInCatchScope is computed over there:
https://searchfox.org/mozilla-central/rev/df68a65540f2227e27a12ed0b491188e2927f6d5/js/src/debugger/Script.cpp#2248-2251

    for (const TryNote& tn : script->trynotes()) {
      if (tn.start <= offset_ && offset_ < tn.start + tn.length &&
          tn.kind() == TryNoteKind::Catch) {
        isInCatch_ = true;

Unfortunately, this is on the edge of my hability to dig this further.

Comment 5

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1811199 - [devtools] Failing jit test.

Comment 6

[Iain Ireland [:iain]]

Attachment: Bug 1811199: Skip synthetic for-of catch blocks in isInCatchScope r=tcampbell

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38e892c41583
Skip synthetic for-of catch blocks in isInCatchScope r=tcampbell

Comment 8

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/38e892c41583