Closed Bug 1892699 Opened 1 month ago Closed 1 month ago

Firefox 125.0.1 (64-bit) "Arguments" array-like object have zero-length in generator function

Categories

(Core :: JavaScript Engine, defect, P1)

Firefox 125
defect

Tracking

()

RESOLVED FIXED
127 Branch
Tracking Status
relnote-firefox --- 125+
firefox-esr115 --- unaffected
firefox125 --- fixed
firefox126 --- fixed
firefox127 --- fixed

People

(Reporter: pear.knockarounder, Assigned: mgaudet)

References

(Regression)

Details

(Keywords: regression, testcase)

Attachments

(1 file)

Steps to reproduce:

When creating a generator that takes several arguments in Firefox version 125.0.1 (64 bit), I encounter a problem: the arguments arrya-like object is always empty, even if arguments are passed to the generator, although in the previous version, for example 124.0.2, it contains the passed arguments.

I run following code in developer console:

function* a(x, y, z) {
console.debug("Arguments length:", arguments.length);
yield x;
yield y;
yield z;
}
const x = a(3, 4, 5)
x.next()

Actual results:

Console output:
Arguments length: 0

Expected results:

Console output:
Arguments length: 3

This is how the code works in version 124.0.2

The Bugbug bot thinks this bug should belong to the 'Core::JavaScript Engine' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → JavaScript Engine
Product: Firefox → Core
function* a(x, y, z) {
  if (arguments.length !== 3) {
    throw "Wrong output";
  }
  yield x;
  yield y;
  yield z;
}
const x = a(3, 4, 5);
x.next();
The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/086ffc46a18e
user:        Matthew Gaudet
date:        Mon Mar 04 16:25:47 2024 +0000
summary:     Bug 1825722 - Where possible avoid allocating arguments, and use JSOp::ArgumentsLength instead r=arai

I used this slightly-modified testcase. Matt, is bug 1825722 a likely regressor?

Flags: needinfo?(mgaudet)
Keywords: regression, testcase
Regressed by: 1825722

Yes, this certainly appears to be a regression here.

For Generator functions it appears that we don't set the argument count correctly:

https://searchfox.org/mozilla-central/rev/863aef60f9fa1536884252ad90cad1d8ee9d8a41/js/src/vm/Stack-inl.h#334-335

fp->initCallFrame(prev, prevpc, prevsp, *callee, script, argv, 0, // <--- 0 is the arg Count
                  NO_CONSTRUCT);

(This also suggests that should we have ever used the ArgumentsLength intrinsic inside of a generator in self-hosted code we would have had a similar bug)

Flags: needinfo?(mgaudet)
Assignee: nobody → mgaudet
Severity: -- → S3
Priority: -- → P1
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Pushed by mgaudet@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/88940f5de974
Disable arguments.length optimization in generators and async functions r=jandem
Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 127 Branch

The patch landed in nightly and beta is affected.
:mgaudet, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox126 to wontfix.

For more information, please visit BugBot documentation.

Flags: needinfo?(mgaudet)

Comment on attachment 9397951 [details]
Bug 1892699 - Disable arguments.length optimization in generators and async functions r?jandem

Beta/Release Uplift Approval Request

  • User impact if declined: Correctness error when checking arguments.length (and not using arguments otherwise) inside of a generator or async function.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Disables broken optimization in limited circumstance.
  • String changes made/needed:
  • Is Android affected?: Yes
Flags: needinfo?(mgaudet)
Attachment #9397951 - Flags: approval-mozilla-beta?
Attachment #9397951 - Flags: approval-mozilla-release?

Comment on attachment 9397951 [details]
Bug 1892699 - Disable arguments.length optimization in generators and async functions r?jandem

Approved for 126.0b6

Attachment #9397951 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment on attachment 9397951 [details]
Bug 1892699 - Disable arguments.length optimization in generators and async functions r?jandem

Approved for 125.0.3

Attachment #9397951 - Flags: approval-mozilla-release? → approval-mozilla-release+
Flags: in-testsuite+

Added to the 125.0.3 relnotes.

Fixed a correctness error when checking arguments.length (and not using arguments otherwise) inside of a generator or async function

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: