1918303 - Assertion failure: toSpace.mallocedBufferBytes >= nbytes, at gc/Nursery-inl.h:77

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect, P1)

Description

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

Attachment: debug stack

function f() {
    var x = Debugger();
    var y =
        "12345678901234567890123456789012345678901234567890" +
        "12345678901234567890123456789012345678901234567890" +
        "12345678901234567890123456789012345678901234567890" +
        "123456789012345678901234567890" + f.toString() +
        "12345678901234567890";
    y[1] = y;
    var z = {
      [y]: y,
    };
    x.memory.takeCensus();
}
oomTest(f);

77        MOZ_ASSERT(toSpace.mallocedBufferBytes >= nbytes);
(gdb) bt
#0  js::Nursery::removeExtensibleStringBuffer (this=this@entry=0x7ffff6b2e928, s=0x17847800428) at /home/yksubu/trees/mozilla-central/js/src/gc/Nursery-inl.h:77
#1  0x0000555557749671 in UpdateNurseryBuffersOnTransfer (nursery=..., from=0x0, from@entry=0x17847800428, to=to@entry=0x17847800448, chars=chars@entry=0x7ffff592d408, size=0) at /home/yksubu/trees/mozilla-central/js/src/vm/StringType.cpp:940
#2  0x0000555557763c21 in JSRope::flattenInternal<(JSRope::UsingBarrier)0, unsigned char> (root=0x17847800448) at /home/yksubu/trees/mozilla-central/js/src/vm/StringType.cpp:1107
#3  0x0000555557729cc8 in JSRope::flattenInternal<(JSRope::UsingBarrier)0> (this=0x7ffff7a1ca60 <_IO_stdfile_2_lock>) at /home/yksubu/trees/mozilla-central/js/src/vm/StringType.cpp:1012
#4  0x0000555557729b7d in JSRope::flatten (this=<optimized out>, maybecx=0x7ffff6b36200) at /home/yksubu/trees/mozilla-central/js/src/vm/StringType.cpp:990
#5  0x0000555557505ccf in JSString::ensureLinear (this=0x17847800448, cx=0x7ffff6b36200) at /home/yksubu/trees/mozilla-central/js/src/vm/StringType.h:2174
#6  js::AtomizeString (cx=0x7ffff6b36200, str=str@entry=0x17847800448) at /home/yksubu/trees/mozilla-central/js/src/vm/JSAtomUtils.cpp:715
#7  0x00005555572cc6b3 in js::PrimitiveValueToId<(js::AllowGC)1> (cx=0x7ffff7a1ca60 <_IO_stdfile_2_lock>, v=..., idp=...) at /home/yksubu/trees/mozilla-central/js/src/vm/JSAtomUtils-inl.h:54
#8  0x00005555581990f7 in ValueToNameOrSymbolId (cx=0x7ffff7a1ca60 <_IO_stdfile_2_lock>, idVal=..., id=..., nameOrSymbol=<optimized out>) at /home/yksubu/trees/mozilla-central/js/src/jit/CacheIR.cpp:399
#9  js::jit::SetPropIRGenerator::tryAttachStub (this=0x7fffffffc078) at /home/yksubu/trees/mozilla-central/js/src/jit/CacheIR.cpp:4415
#10 0x0000555557e9877b in js::jit::DoSetElemFallback (cx=0x7ffff6b36200, frame=0x7fffffffc338, stub=0x7ffff607d790, stack=0x7fffffffc300, objv=..., index=..., rhs=...) at /home/yksubu/trees/mozilla-central/js/src/jit/BaselineIC.cpp:866
#11 0x000028a924c14d2f in ?? ()
#12 0x00007fffffffc2f8 in ?? ()
#13 0x000028a924c47f4f in ?? ()
#14 0x000000000000007d in ?? ()
#15 0x00007fffffffc380 in ?? ()
/snip

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/39e5feb1b0b8
user:        Jan de Mooij
date:        Wed Sep 04 10:49:51 2024 +0000
summary:     Bug 1915328 - Update mallocedBufferBytes for string buffers used by nursery strings. r=jonco

Run with --fuzzing-safe --no-threads --no-baseline --no-ion, compile with AR=ar sh ../configure --enable-debug --enable-debug-symbols --with-ccache --enable-nspr-build --enable-ctypes --enable-gczeal --enable-rust-simd --disable-tests, tested on m-c rev 606085de1f6c.

Setting s-s just in case. Jan, is bug 1915328 a likely regressor?

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 2

[Andrew McCreight [:mccr8]]

takeCensus is a chrome only method that crawls over the entire heap, so this might not really be a security issue. But it is also possible that this is just the easiest way to find a heap inconsistency that would cause problems normally.

Comment 3

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

(In reply to Andrew McCreight [:mccr8] from comment #2)

takeCensus is a chrome only method that crawls over the entire heap, so this might not really be a security issue. But it is also possible that this is just the easiest way to find a heap inconsistency that would cause problems normally.

var x = 0;
function f() {
  undefined ^= (x += [0]);
  oomTest(f);
}
for (let i = 0; i < 99; i++) {
  f();
}

This is another testcase that does not involve Debugger nor takeCensus.

Comment 4

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

var x = 0;
function f() {
  undefined ^= (x += [0]);
  oomTest(f);
}
for (let i = 0; i < 99; i++) {
  f();
}

Run with --fuzzing-safe --no-threads --no-baseline --no-ion, compile with AR=ar sh ../configure --enable-debug --enable-debug-symbols --with-ccache --enable-nspr-build --enable-ctypes --enable-gczeal --enable-rust-simd --disable-tests, also tested on m-c rev 606085de1f6c.

Comment 5

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

Attachment: debug stack for second testcase

(gdb) bt
#0  js::Nursery::removeExtensibleStringBuffer (this=this@entry=0x7ffff6b2e928, s=0x3d469e030508) at /home/yksubu/trees/mozilla-central/js/src/gc/Nursery-inl.h:77
#1  0x0000555557749671 in UpdateNurseryBuffersOnTransfer (nursery=..., from=0x0, from@entry=0x3d469e030508, to=to@entry=0x3d469e0305e0, chars=chars@entry=0x7ffff5952008, size=0) at /home/yksubu/trees/mozilla-central/js/src/vm/StringType.cpp:940
#2  0x0000555557763c21 in JSRope::flattenInternal<(JSRope::UsingBarrier)0, unsigned char> (root=0x3d469e0305e0) at /home/yksubu/trees/mozilla-central/js/src/vm/StringType.cpp:1107
#3  0x0000555557729cc8 in JSRope::flattenInternal<(JSRope::UsingBarrier)0> (this=0x7ffff7a1ca60 <_IO_stdfile_2_lock>) at /home/yksubu/trees/mozilla-central/js/src/vm/StringType.cpp:1012
#4  0x0000555557729b7d in JSRope::flatten (this=<optimized out>, maybecx=0x7ffff6b36200) at /home/yksubu/trees/mozilla-central/js/src/vm/StringType.cpp:990
#5  0x000055555795f9eb in JSString::ensureLinear (this=0x3d469e0305e0, cx=0x7ffff6b36200) at /home/yksubu/trees/mozilla-central/js/src/vm/StringType.h:2174
#6  js::StringToNumber (cx=0x7ffff6b36200, str=0x3d469e0305e0, result=0x7fffffffbaf0) at /home/yksubu/trees/mozilla-central/js/src/jsnum.cpp:2007
/snip

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1918303 - Fix OOM issue in UpdateNurseryBuffersOnTransfer. r?jonco!

addExtensibleStringBuffer is fallible so we should call it first.

This fixes a memory leak because we'd not release a string buffer reference
from an extensible string in the nursery.

Comment 7

[Jan de Mooij [:jandem]]

Good find. If we transfer an extensible string's buffer to another string, we could call removeExtensibleStringBuffer multiple times if the call to addExtensibleStringBuffer after it failed.

Incorrectly removing the string-buffer from the nursery's string-buffer list means we don't release it when we do a nursery GC and the string is dead. That's a memory leak but otherwise harmless.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/661579d4e99a
Fix OOM issue in UpdateNurseryBuffersOnTransfer. r=jonco

Comment 9

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/661579d4e99a