Iterator is not closed when breaking from finally

RESOLVED FIXED in Firefox 53

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: arai, Assigned: arai)

Tracking

Trunk
mozilla54
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr45 unaffected, firefox51 unaffected, firefox52 unaffected, firefox53 fixed, firefox54 fixed)

Details

Attachments

(1 attachment)

similar to bug 1332881.

https://dxr.mozilla.org/mozilla-central/rev/045d8fe30f546ab08466c9586ce298e6459c2069/js/src/frontend/BytecodeEmitter.cpp#2405-2427
flushPops should be done *before* handling target ForOfLoopControl :/

code:

var called = false;
var obj = {
  [Symbol.iterator]() {
    return {
      next() {
        return { value: 10, done: false };
      },
      return() {
        called = true;
        return {};
      }
    };
  }
};

for (var x of obj) {
  try {
  } finally {
    break;
  }
}
assertEq(called, true);


this time it doesn't cause crash, but just doesn't close iterator.
I should've checked that iterator is properly closed in bug 1332881's testcase.
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Attachment #8831438 - Flags: review?(shu)
Attachment #8831438 - Flags: review?(shu) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/34aa0752cd03e7c062d67ccbff83712482ce1447
Bug 1334799 - Handle stack value in correct order when leaving for-of loop from finally block. r=shu
https://hg.mozilla.org/mozilla-central/rev/34aa0752cd03
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Blocks: 1147371
Comment on attachment 8831438 [details] [diff] [review]
Handle stack value in correct order when leaving for-of loop from finally block.

Approval Request Comment
> [Feature/Bug causing the regression]
the code is added in bug 1147371,
and overlooked part in bug 1332881.

> [User impact if declined]
Wrong JavaScript behavior

> [Is this code covered by automated tests?]
Yes

> [Has the fix been verified in Nightly?]
Yes

> [Needs manual test from QE? If yes, steps to reproduce]
No

> [List of other uplifts needed for the feature/fix]
None

> [Is the change risky?]
Less risky

> [Why is the change risky/not risky?]
this changes the target stack offset of operations, like bug 1332881.
now testcase confirms it's really correct.

> [String changes made/needed]
None
Attachment #8831438 - Flags: approval-mozilla-aurora?
Comment on attachment 8831438 [details] [diff] [review]
Handle stack value in correct order when leaving for-of loop from finally block.

Fixes a JS behavior, Aurora53+
Attachment #8831438 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.