2044606 - (CVE-2026-16392) Assertion failure: hasRegisterIndex(reg), at jit/RegisterSets.h:459

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P1)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: debug stack

for (var i = 0; i < 9; i++) {
  oomTest(function () {
    (function () {
      try {
        Object.keys(function () {});
      } catch (e) {}
      return [];
    })();
  });
}

(gdb) bt
#0  0x000055555812921c in MOZ_CrashSequence (aAddress=0x0, aLine=459) at /home/msf2/shell-cache/js-dbg-64-linux-x86_64-6301b01791f9-630254/objd/dist/include/mozilla/Assertions.h:261
#1  js::jit::TypedRegisterSet<js::jit::Register>::offsetOfPushedRegister (this=<optimized out>, reg=...) at /home/msf2/trees/firefox/js/src/jit/RegisterSets.h:459
#2  js::jit::MachineState::SafepointState::addressOfRegister (this=<optimized out>, reg=...) at ./../../../../../../trees/firefox/js/src/jit/JitFrames.cpp:2536
#3  0x0000555558124f66 in js::jit::MachineState::read (this=0x7ffff7805700 <_IO_stdfile_2_lock>, reg=...) at ./../../../../../../trees/firefox/js/src/jit/JitFrames.cpp:2607
#4  js::jit::SnapshotIterator::fromRegister (this=0x7fffffffb530, reg=...) at /home/msf2/trees/firefox/js/src/jit/JSJitFrameIter.h:376
#5  js::jit::SnapshotIterator::allocationValue (this=0x7fffffffb530, alloc=..., rm=<optimized out>) at ./../../../../../../trees/firefox/js/src/jit/JitFrames.cpp:1820
/snip

e43e6780c6c0-628511
e43e6780c6c0aad349a6e4fe77350ee6253d9c08 is the first interesting commit
commit e43e6780c6c0aad349a6e4fe77350ee6253d9c08
Author: Iain Ireland
Date:   Tue May 26 19:36:11 2026 +0000

    Bug 2040903: Update alias set of ObjectToIterator r=jandem

    Differential Revision: https://phabricator.services.mozilla.com/D301605

Run with --fuzzing-safe --no-threads --ion-eager, compile with AR=ar sh ~/trees/firefox/js/src/configure --enable-debug --enable-debug-symbols --with-ccache --enable-nspr-build --enable-ctypes --enable-gczeal --enable-rust-simd --disable-tests, tested on gh rev 6301b01791f9c98637ae8d5b7a81dd004d48d468.

Jan, is bug 2040903 a likely regressor? (Note that it is marked s-s)

Comment 1

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 2040903

Comment 2

[Jan de Mooij [:jandem]]

Yes this is a regression from bug 2040903, the issue is the stealResumePoint here that moves the MObjectKeys resume point to MObjectToIterator.

When we have an exception bailout for an exception thrown by the MObjectToIterator instruction itself, the recover instruction for MObjectKeysFromIterator (called forRecovery in this code) tries to read the MObjectToIterator result but it's not available because that's the instruction we're still executing.

For an exception handler bailout, we don't read the expression stack slots so strictly speaking we don't need to execute these recover instructions (the result isn't used). That's not how the code is currently structured though.

Nicolas, can you take this one? I'm not sure what's the best fix for this one, also because we probably want to uplift.

Comment 3

[Nicolas B. Pierron [:nbp] {away}]

The problem is that ObjectKeysReplacer::run decompose the MObjectKeys function in 2, which is to build the iterator and then to build the keys from the iterator, even if the second is only used while recovering instructions.

This is not possible and this is the issue raised here.

We do ignore the returned value of a call when we are running into an exception, and the recover instruction would not have any function working with the result of this call as it would only be referenced as a register.

This code changes this assumption by adding extra operations on the bailout path, except that we cannot execute anything when there is no value to execute it onto. Which is exactly what this error message complains about.

Ideally we should split the original instruction, but in such case this is a single function call, and returning an iterator in-place of the array would cause even more issues. This can be achieved by having a MNop instruction inserted after which captured the modified resume-point while the original captures the MObjectToIterator. However, this would cause issues on invalidations as the MOSIPoint would use the iterator in place of the array, when there is no allocation failure, yet the code gets invalidated while we are processing the calls from LObjectToIterator.

I do not think we can carry over the fact that we should ignore the Recover instructions, like we ignore the returned value. The reason is that the recover instructions can technically be executed ahead of the return, like done in this exception handler, and like it is possible to do when using fun.arguments to invalidate functions which are on the stack.

While incorrect, I think the simplest choice is to go with the extra MNop and cloning the resume point. Other options are likely to be much more risky from my point of view.

Comment 4

[BugBot [:suhaib / :marco]]

The bug is marked as tracked for firefox152 (beta) and tracked for firefox153 (nightly). However, the bug still has low severity.

:sdetar, could you please increase the severity for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 6

[Nicolas B. Pierron [:nbp] {away}]

Attachment: (secure)

Comment 7

[Ryan VanderMeulen [:RyanVM]]

This bug needs a severity rating still also.

Comment 8

[Nicolas B. Pierron [:nbp] {away}]

Reading the code, MachineState initialized from a SafePoint will read a register dump of all registers which are pushed on the stack.
I have not investigated the details of what might be stored in the returned register.
I will assign it a sec-moderate in the mean time.

Comment 9

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 2040903

Comment 10

[Christian Holler (:decoder)]

Found internally as well.

Comment 11

[Jan de Mooij [:jandem]]

Attachment: (secure)

Comment 12

[Jan de Mooij [:jandem]]

Attachment: (secure)

Comment 13

[BugBot [:suhaib / :marco]]

The following patch is waiting for review from a reviewer who resigned from the review:

ID	Title	Author	Reviewer Status
D305340	Bug 2044606 - Object.keys optimization should split resume points. r=jandem	nbp	jandem: Resigned from review

:nbp, could you please find another reviewer?

For more information, please visit BugBot documentation.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/9a9c26f30a96
https://hg.mozilla.org/integration/autoland/rev/42e48216dd40
Fix Object.keys resume point issue. r=mgaudet

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg-edge.mozilla.org/mozilla-central/rev/42e48216dd40

Please add a Beta uplift request on this.

Comment 16

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined/Reason for urgency: Crashes, potential security issues.
• Code covered by automated testing?: yes
• Fix verified in Nightly?: yes
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: medium
• Explanation of risk level: Patch is non-trivial but should be fairly low risk. Affects a specific optimization for Object.keys in the JIT.
• String changes made/needed?: N/A
• Is Android affected?: yes

Comment 17

[Jan de Mooij [:jandem]]

Attachment: (secure)

Original Revision: https://phabricator.services.mozilla.com/D308046

Comment 18

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/6c778f775086
https://hg.mozilla.org/releases/mozilla-beta/rev/7c39d5a702f3