1712393 - Crash evaluating static private method vs Debugger.frame.eval

Status: RESOLVED DUPLICATE of bug 1709956

(Core :: JavaScript Engine, defect, P3)

Description

[Matthew Gaudet (he/him) [:mgaudet]]

Applying the following patch (tested against 6f49d8893274) and running ./mach jit-test private-frame-eval-in-frame, we see an assertion failure Assertion failure: idval.isSymbol(), at /home/matthew/unified2/js/src/vm/Interpreter-inl.h:565

matthew@xtower:~/unified2$ hg diff
diff --git a/js/src/jit-test/tests/debug/private-methods-eval-in-frame.js b/js/src/jit-test/tests/debug/private-methods-eval-in-frame.js
--- a/js/src/jit-test/tests/debug/private-methods-eval-in-frame.js
+++ b/js/src/jit-test/tests/debug/private-methods-eval-in-frame.js
@@ -32,6 +32,12 @@ class B {
     return f();
   }
 
+  static sef(str) {
+    return evalInFrame(0, str);
+  }
+
+  static #smethod() { return 14; }
+
 }
 
 var b = new B();
@@ -54,4 +60,6 @@ assertEq(b.ef(`var x = () => { this.#x =
 // These require brand checking, and some new development to support.
 // See EmitterScope::lookupPrivate for the reasoning here.
 assertThrowsInstanceOf(() => b.ef(`this.#priv()`), Error);
-assertThrowsInstanceOf(() => b.ef(`var x = () => { return this.#priv(); } x();`), Error);
\ No newline at end of file
+assertThrowsInstanceOf(() => b.ef(`var x = () => { return this.#priv(); } x();`), Error);
+
+assertEq(B.sef(`this.#smethod()`), 14);
\ No newline at end of file

This is a debugger only problem.

Comment 1

[Matthew Gaudet (he/him) [:mgaudet]]

Here's extra fun: This test case passes, if you add another static method:

   static se(str) {
     return eval(str);
   }

Something about adding an eval makes this no longer broken. I did verify that we emit identical bytecode here too.

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

Effectively, the basic form of this problem is that with the patch, B.#smethod is not closed over, and isn't actually represented in the scope chain environment object for the eval-in-frame case.

I think a more ideal outcome would be that instead of simply crashing, we would report "B.#smethod was optimized out"

Comment 3

[Matthew Gaudet (he/him) [:mgaudet]]

This will be fixed by the patches on Bug 1709956, so duping there.