2005583 - Test failure in Iterator/zip/iterables-iteration-get-iterator-flattenable-abrupt-completion.js

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Dan Minor [:dminor]]

This tests passes locally, but fails in CI:
[task 2025-12-11T14:54:07.224+00:00] ## test262/built-ins/Iterator/zip/iterables-iteration-get-iterator-flattenable-abrupt-completion.js: rc = 3, run time = 0.024858
[task 2025-12-11T14:54:07.224+00:00] /builds/worker/checkouts/gecko/js/src/tests/test262/shell.js:97:13 uncaught exception: Test262Error: Expected a ExpectedError but got a Test262Error
[task 2025-12-11T14:54:07.224+00:00] Stack:
[task 2025-12-11T14:54:07.224+00:00] assert.throws@/builds/worker/checkouts/gecko/js/src/tests/test262/shell.js:97:13
[task 2025-12-11T14:54:07.225+00:00] @/builds/worker/checkouts/gecko/js/src/tests/test262/built-ins/Iterator/zip/iterables-iteration-get-iterator-flattenable-abrupt-completion.js:152:10
[task 2025-12-11T14:54:07.225+00:00] TEST-UNEXPECTED-FAIL | test262/built-ins/Iterator/zip/iterables-iteration-get-iterator-flattenable-abrupt-completion.js | (args: "--dll /builds/worker/fetches/injector/libbreakpadinjector.so --baseline-eager --write-protect-code=off") [0.0 s]
[task 2025-12-11T14:54:07.225+00:00] TEST-PASS | test262/built-ins/Iterator/zip/proto.js | (args: "--dll /builds/worker/fetches/injector/libbreakpadinjector.so") [0.0 s]
[task 2025-12-11T14:54:07.225+00:00] TEST-PASS | test262/built-ins/Iterator/zip/proto.js | (args: "--dll /builds/worker/fetches/injector/libbreakpadinjector.so --no-blinterp --no-baseline --no-ion --more-compartments") [0.0 s]
[task 2025-12-11T14:54:07.227+00:00] TEST-PASS | test262/built-ins/Iterator/zip/proto.js | (args: "--dll /builds/worker/fetches/injector/libbreakpadinjector.so --baseline-eager --write-protect-code=off") [0.0 s]
[task 2025-12-11T14:54:07.227+00:00] TEST-PASS | test262/built-ins/Iterator/zip/suspended-start-iterator-close-calls-next.js | (args: "--dll /builds/worker/fetches/injector/libbreakpadinjector.so") [0.0 s]
[task 2025-12-11T14:54:07.227+00:00] TEST-PASS | test262/built-ins/Iterator/zip/iterator-zip-iteration-strict-iterator-step-abrupt-completion.js | (args: "--dll /builds/worker/fetches/injector/libbreakpadinjector.so --ion-eager --ion-offthread-compile=off --more-compartments") [0.0 s]
[task 2025-12-11T14:54:07.227+00:00] TEST-PASS | test262/built-ins/Iterator/zip/suspended-start-iterator-close-calls-next.js | (args: "--dll /builds/worker/fetches/injector/libbreakpadinjector.so --baseline-eager --write-protect-code=off") [0.0 s]
[task 2025-12-11T14:54:07.228+00:00] ## test262/built-ins/Iterator/zip/iterables-iteration-get-iterator-flattenable-abrupt-completion.js: rc = 3, run time = 0.049466
[task 2025-12-11T14:54:07.228+00:00] /builds/worker/checkouts/gecko/js/src/tests/test262/shell.js:97:13 uncaught exception: Test262Error: Expected a ExpectedError but got a Test262Error
[task 2025-12-11T14:54:07.228+00:00] Stack:
[task 2025-12-11T14:54:07.228+00:00] assert.throws@/builds/worker/checkouts/gecko/js/src/tests/test262/shell.js:97:13
[task 2025-12-11T14:54:07.228+00:00] @/builds/worker/checkouts/gecko/js/src/tests/test262/built-ins/Iterator/zip/iterables-iteration-get-iterator-flattenable-abrupt-completion.js:152:10
[task 2025-12-11T14:54:07.228+00:00] TEST-UNEXPECTED-FAIL | test262/built-ins/Iterator/zip/iterables-iteration-get-iterator-flattenable-abrupt-completion.js | (args: "--dll /builds/worker/fetches/injector/libbreakpadinjector.so --ion-eager --ion-offthread-compile=off --more-compartments") [0.0 s]
[task 2025-12-11T14:54:07.228+00:00] TEST-PASS | test262/built-ins/Iterator/zip/basic-longest.js | (args: "--dll /builds/worker/fetches/injector/libbreakpadinjector.so --baseline-eager --write-protect-code=off") [0.1 s]

Comment 1

[André Bargull [:anba]]

This is probably a JIT bug, because it's not reproducible when running in the interpreter only.

Running with --tbpl to also test JIT variants should make the test fail locally:

tests/jstests.py --tbpl test262/built-ins/Iterator/zip/iterables-iteration-get-iterator-flattenable-abrupt-completion.js

Comment 2

[Dan Minor [:dminor]]

Thanks, that helped. Offhand, it seems likely the problem is occurring in IteratorCloseAllForException.

Comment 3

[André Bargull [:anba]]

Simplified test case, fails unless JITs are disabled with --no-blinterp. Maybe this part is missing in the JITs?

var iterable = {
  [Symbol.iterator]() {
    return this;
  },
  next() {
    return { done: false };
  },
  return() {
    throw "ReturnError";
  }
};

function test() {
  try {
    for (var v of iterable) {
      throw "NextError";
    }
  } catch (e) {
    assertEq(e, "NextError");
  }
}

for (var i = 0; i < 100; ++i) {
  test();
}

Comment 4

[Iain Ireland [:iain]]

JSOp::CloseIter has a CompletionKind operand to control throwing behaviour. It looks like self-hosted IteratorClose always emits CompletionKind::Normal. Maybe we need CompletionKind::Throw here?

Comment 5

[André Bargull [:anba]]

Attachment: Bug 2005583: Add synthetic try-catch around CloseIter with throw-completion. r=iain!

Add a synthetic try-catch block around CloseIter with a throw-completion, so
that exceptions thrown from CloseIter are correctly ignored when called from
scripted-return CloseIter ICs.

Comment 6

[André Bargull [:anba]]

Attachment: Bug 2005583: Don't attach a scripted-return CloseIter IC for throw completions. r=iain!

Don't attach an IC because the generated JIT code doesn't clear new exceptions
thrown when calling scripted functions.

Comment 7

[André Bargull [:anba]]

I've uploaded two different approaches to fix this bug:

1. D276159: Disable JIT inlining support for throw-completions for the scripted-return case.
2. D276158: Add a synthetic try-catch block around CloseIter with a throw-completion to ignore new exceptions.

D276158 keeps the JIT support, but further increases the bytecode size for iteration protocol related operations. D276159 doesn't enlarge the bytecode size, but removes JIT support.

Comment 8

[Iain Ireland [:iain]]

Ugh, good catch. This is kind of unfortunate.

This is hopefully not a hot path, so losing JIT support isn't the end of the world. On the other hand, a little more bytecode shouldn't hurt too badly either.

I've gone back and forth on this, eventually landing on a soft preference for removing JIT support. What finally convinced me is that we will still have JIT support for the common case where the iterator doesn't have a return method (and therefore can't throw).

If we ever needed to make this path fast, I think we could speed it up a little bit by adding an IC calling a VM trampoline that implements steps 4c and 5 of CloseIterOperation. It's significantly worse than a callJit, but it lets us use an IC for the GetProp. I hope that it will never be necessary, though.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/54cfe2b0ceb7
https://hg.mozilla.org/integration/autoland/rev/af989af6973a
Don't attach a scripted-return CloseIter IC for throw completions. r=iain

Comment 10

[amarc]

https://hg.mozilla.org/mozilla-central/rev/af989af6973a