1193046 - [jsdbg2] Debugger.Environment reports "optimized out" too often

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jim Blandy :jimb]

Test case (due to Shu):

    var g = newGlobal();
    var dbg = new Debugger(g);

    g.eval(`
    function outer(unaliasedArg) {
      var unaliasedVar = unaliasedArg + 42;
      var aliasedVar = unaliasedArg;

      inner(); // offset 30
      return;

      function inner() {
        aliasedVar++;
      }
    }
    `);

    for (var script of dbg.findScripts()) {
      if (script.displayName === "inner") {
        script.setBreakpoint(0, { hit: function(frame) {
            var outerEnv = frame.environment.parent;
            print(uneval(outerEnv.getVariable('unaliasedArg'))); // optimized out
            print(uneval(outerEnv.getVariable('unaliasedVar'))); // optimized out
            print(uneval(outerEnv.getVariable('aliasedVar')));   // 42

            outerEnv = frame.older.environment;
            print(uneval(outerEnv.getVariable('unaliasedArg'))); // optimized out, SHOULD be 42
            print(uneval(outerEnv.getVariable('unaliasedVar'))); // optimized out, SHOULD be 84
            print(uneval(outerEnv.getVariable('aliasedVar')));   // 42
        }});
      }
    }

    g.outer(42);


The problem is that we're failing to clear the PREV_UP_TO_DATE bit on a frame when we mark its caller as a debuggee, which leads the code that updates liveScopes to believe that it need not produce an entry for the caller. As a consequence, we can't pair up DebugScopeObjects with the stack frames that supply values for their unaliased variables, and requests for those values yield only the "optimized out" marker.

Comment 1

[James Long (:jlongster)]

This may help bug 1186404. I'm not keen on disabling optimizations when the debugger is attached (as that bug suggests) so hopefully this helps.

Comment 2

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Thanks for filing, Jim!

Comment 3

[Shu-yu Guo [:shu]]

Attachment: Clear prevUpToDate on younger frames when toggling frame debuggeeness off->on.

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8646073 [details] [diff] [review]
Clear prevUpToDate on younger frames when toggling frame debuggeeness off->on.

Review of attachment 8646073 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/vm/ScopeObject.cpp
@@ +2271,5 @@
> +    // This are two exceptions where fp->prevUpToDate() is cleared without
> +    // popping the frame. When a frame is rematerialized or has its
> +    // debuggeeness toggled off->on, all frames younger than the frame must
> +    // have their prevUpToDate set to false. This is because unrematerialized
> +    // Ion frames have and non-debuggee frames and are skipped by

"... have and ..." ?

Is it me or did you a few words in there?

Comment 5

[Shu-yu Guo [:shu]]

Attachment: Clear prevUpToDate on younger frames when toggling frame debuggeeness off->on.

Fix comment typo.

Comment 6

[Shu-yu Guo [:shu]]

Attachment: Clear prevUpToDate on younger frames when toggling frame debuggeeness off->on.

Had a test case that didn't test anything.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/72d3337af3ec

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/72d3337af3ec