1305333 - Uninitialized lexial is cleared to undefined in BaselineFrame::trace, when GC happens before entering the lexixal scope.

Status: RESOLVED DUPLICATE of bug 1304649

(Core :: JavaScript Engine: JIT, defect)

Description

[Tooru Fujisawa [:arai]]

Attachment: (testing patch) Add unused self-hosted functions and self-hosting intrinsics.

While rebasing bug 1185106 patches, I hit the following strange behavior:

with the attached patch applied, the following test fails:

base revision:
  m-c 29beaebdfacc

environment:
  OSX 10.11.6

configure flags:
  --disable-optimize --enable-debug --enable-stdcxx-compat --enable-ctypes --disable-shared-js --enable-nspr-build

environment variable:
  JS_GC_ZEAL=14

runtime flags:
  --baseline-eager

code (modified version of js/src/jit-test/tests/basic/bug1091757.js):
  function f() {
      try {
          (function() {
              let a = 3;
              let XY = XY;
              return function() { return a; };
          })();
          assertEq(0, 1);
      } catch(e) {
          assertEq(e instanceof ReferenceError, true);
          assertEq(e.message.includes("XY"), true);
      }
  }

  for (let i = 0; i < 10; i++) {
      f();
  }

expected result:
  it runs successfully

actual result:
  test.js:10:11 Error: Assertion failed: got false, expected true

  but if I change the test code to show the exception, the issue disappears and just passes.


or run jit-test js/src/jit-test/tests/basic/bug1091757.js with --baseline-eager.


the attached code is an almost minimal testcase, reduced from bug 1185106's patch, that adds unused self-hosted functions and self-hosting intrinsics.

Comment 1

[Tooru Fujisawa [:arai]]

updated code:
  function f() {
      try {
          (function() {
              let a = 3;
              let XY = XY;
              return function() { return a; };
          })();
          assertEq(0, 1);
      } catch(e) {
          print("message:", e.message);
          assertEq(e instanceof ReferenceError, true);
          assertEq(e.message.includes("XY"), true);
      }
  }

  for (let i = 0; i < 10; i++) {
      f();
  }

it prints:
  message: can't access lexical declaration `XY' before initialization
  message: Assertion failed: got 0, expected 1
  /tmp/test.js:11:11 Error: Assertion failed: got false, expected true
  Stack:
    f@/tmp/test.js:11:11
    @/tmp/test.js:17:7

so looks like |let XY = XY| doesn't throw on 2nd call.

Comment 2

[Tooru Fujisawa [:arai]]

this is the actual issue I hit with bug 1185106 patch:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=4c72867941f6&selectedJob=28022332

Comment 3

[Tooru Fujisawa [:arai]]

I see the stack value of XY is changed from MagicValue(JS_UNINITIALIZED_LEXICAL) to undefined here:

https://dxr.mozilla.org/mozilla-central/rev/29beaebdfaccbdaeb4c1ee5a43a9795ab015ef49/js/src/jit/BaselineFrame.cpp#80
>         // Clear dead block-scoped locals.
>         while (nfixed > nlivefixed)
>             unaliasedLocal(--nfixed).setUndefined();

and the stack value is not changed back to MagicValue(JS_UNINITIALIZED_LEXICAL).

therefore JSOP_CHECKLEXICAL doesn't throw.

Comment 4

[Tooru Fujisawa [:arai]]

https://dxr.mozilla.org/mozilla-central/rev/29beaebdfaccbdaeb4c1ee5a43a9795ab015ef49/js/src/jit/BaselineFrame.cpp#71
>     if (nfixed == nlivefixed) {
>         // All locals are live.
>         MarkLocals(this, trc, 0, numValueSlots());
>     } else {

if I change the condition to |if (true)|, the issue disappears.

Comment 5

[Tooru Fujisawa [:arai]]

This happens after bug 1263355.

when I apply the patch to 181336fdda66, the issue doesn't happen.
when I apply the patch to dbf7b0e7dc66, the issue happens.

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: (testing patch for dbf7b0e7dc66) Add unused self-hosted functions and self-hosting intrinsics.

here's rebased patch for m-c dbf7b0e7dc66

Comment 7

[Tooru Fujisawa [:arai]]

here's the bytecode for the function in testcase.
  00000:  uninitialized
  00001:  initlexical 0
  00005:  pop
  00006:  pushlexicalenv lexical {a: env slot 2, XY: frame slot 0}
  00011:  int8 3
  00013:  initaliasedlexical "a" (hops = 0, slot = 2)
  00018:  pop
  00019:  checklexical 0
  00023:  getlocal 0
  00027:  initlexical 0
  00031:  pop
  00032:  lambda (function () { return a; })
  00037:  setrval
  00038:  poplexicalenv
  00039:  retrval
  00040:  poplexicalenv
  00041:  retrval

GC happens in JSOP_PUSHLEXICALENV in pc=00006, and |lookupScope(pc)| in JSScript::calculateLiveFixed returns null, because it's not yet inside the lexical scope (lexical scope is available between pc=00011 and pc=00040).
|nlivefixed| in BaselineFrame::trace becomes 0, and uninitialized magic value created by JSOP_UNINITIALIZED and JSOP_INITLEXICAL (pc=00000 and pc=00001) is set to undefined.

Comment 8

[Tooru Fujisawa [:arai]]

shu, can you take a look?

Comment 9

[Tooru Fujisawa [:arai]]

talked in IRC, and this should be a dupe of bug 1304649.

confirmed the fix with the bug 1304649 patch applied, both for the testcase in this bug and bug 1185106.