1877406 - Assertion failure: slots == calculateDynamicSlots(), at vm/JSObject-inl.h:45

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

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

Attachment: debug stack

for (let i = 0; i < 2; i++) {
  let x = [];
  x[Symbol.toPrimitive] = function () {
    /i/.exec();
  };
  oomTest(function () {
    new ArrayBuffer(x);
  });
}
dumpHeap();

(gdb) bt
#0  js::NativeObject::numDynamicSlots (this=0x3812aac497b0) at /home/gen32gx500/trees/mozilla-central/js/src/vm/JSObject-inl.h:45
#1  0x00005555574ac058 in js::NativeObject::slotInRange (this=0x7ffff7e37700 <_IO_stdfile_2_lock>, slot=0, 
    sentinel=(js::NativeObject::SENTINEL_ALLOWED | unknown: 0xf7e36562)) at /home/gen32gx500/trees/mozilla-central/js/src/vm/NativeObject.cpp:192
#2  0x000055555743267d in js::NativeObject::getSlotRef (this=0x3812aac497b0, slot=0) at /home/gen32gx500/trees/mozilla-central/js/src/vm/NativeObject.h:1174
#3  JSObject::traceChildren (this=0x3812aac497b0, trc=0x7fffffffc780) at /home/gen32gx500/trees/mozilla-central/js/src/vm/JSObject.cpp:3433
#4  0x0000555557c71c1b in JS::TraceChildren(JSTracer*, JS::GCCellPtr)::$_0::operator()<JSObject*>(JSObject*) const (t=0x3812aac497b0, this=<optimized out>)
    at /home/gen32gx500/trees/mozilla-central/js/src/gc/Tracer.cpp:62
#5  JS::MapGCThingTyped<JS::TraceChildren(JSTracer*, JS::GCCellPtr)::$_0>(void*, JS::TraceKind, JS::TraceChildren(JSTracer*, JS::GCCellPtr)::$_0&&) (thing=0x3812aac497b0, 
    traceKind=<optimized out>, f=...) at /home/gen32gx500/shell-cache/js-dbg-64-linux-x86_64-49f49182fc50/objdir-js/dist/include/js/TraceKind.h:253
#6  JS::ApplyGCThingTyped<JS::TraceChildren(JSTracer*, JS::GCCellPtr)::$_0>(void*, JS::TraceKind, JS::TraceChildren(JSTracer*, JS::GCCellPtr)::$_0&&) (
    thing=0x3812aac497b0, traceKind=<optimized out>, f=...) at /home/gen32gx500/shell-cache/js-dbg-64-linux-x86_64-49f49182fc50/objdir-js/dist/include/js/TraceKind.h:268
#7  JS::TraceChildren (trc=trc@entry=0x7fffffffc780, thing=...) at /home/gen32gx500/trees/mozilla-central/js/src/gc/Tracer.cpp:59
#8  0x0000555558423255 in DumpHeapVisitCell (rt=<optimized out>, data=0x7fffffffc780, cellptr=..., thingSize=<optimized out>, nogc=...)
    at /home/gen32gx500/trees/mozilla-central/js/src/util/DumpFunctions.cpp:577
#9  0x0000555557c38033 in IterateRealmsArenasCellsUnbarriered (cx=cx@entry=0x7ffff662e100, zone=zone@entry=0x7ffff6444000, data=data@entry=0x7fffffffc780, 
    realmCallback=realmCallback@entry=0x555558422f50 <DumpHeapVisitRealm(JSContext*, void*, JS::Realm*, JS::AutoRequireNoGC const&)>, 
    arenaCallback=arenaCallback@entry=0x555558423080 <DumpHeapVisitArena(JSRuntime*, void*, js::gc::Arena*, JS::TraceKind, unsigned long, JS::AutoRequireNoGC const&)>, 
    cellCallback=cellCallback@entry=0x555558423120 <DumpHeapVisitCell(JSRuntime*, void*, JS::GCCellPtr, unsigned long, JS::AutoRequireNoGC const&)>, nogc=...)
    at /home/gen32gx500/trees/mozilla-central/js/src/gc/PublicIterators.cpp:37
#10 0x0000555557c37af4 in js::IterateHeapUnbarriered(JSContext*, void*, void (*)(JSRuntime*, void*, JS::Zone*, JS::AutoRequireNoGC const&), void (*)(JSContext*, void*, JS::Realm*, JS::AutoRequireNoGC const&), void (*)(JSRuntime*, void*, js::gc::Arena*, JS::TraceKind, unsigned long, JS::AutoRequireNoGC const&), void (*)(JSRuntime*, void*, JS::GCCellPtr, unsigned long, JS::AutoRequireNoGC const&))::$_0::operator()(JS::Zone*) const (zone=0x7ffff6444000, this=<optimized out>)
    at /home/gen32gx500/trees/mozilla-central/js/src/gc/PublicIterators.cpp:54
#11 js::IterateHeapUnbarriered (cx=cx@entry=0x7ffff662e100, data=data@entry=0x7fffffffc780, 
    zoneCallback=0x555558422f30 <DumpHeapVisitZone(JSRuntime*, void*, JS::Zone*, JS::AutoRequireNoGC const&)>, 
    realmCallback=0x555558422f50 <DumpHeapVisitRealm(JSContext*, void*, JS::Realm*, JS::AutoRequireNoGC const&)>, 
    arenaCallback=0x555558423080 <DumpHeapVisitArena(JSRuntime*, void*, js::gc::Arena*, JS::TraceKind, unsigned long, JS::AutoRequireNoGC const&)>, 
    cellCallback=0x555558423120 <DumpHeapVisitCell(JSRuntime*, void*, JS::GCCellPtr, unsigned long, JS::AutoRequireNoGC const&)>)
    at /home/gen32gx500/trees/mozilla-central/js/src/gc/PublicIterators.cpp:64
/snip

This seems to go back prior to m-c rev e963fffcb3a0, let me know if it's worth it to try and go back further to find the real regressor. Note that at this revision, it shows a crash at js::NativeObject::shape instead.

Run with --fuzzing-safe --no-threads --no-baseline --no-ion --no-ggc, 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 49f49182fc50.

Jan, I'm not sure if this is a GC issue or not, perhaps you'd like to take a look first? Setting s-s to be safe.

Comment 1

[Jan de Mooij [:jandem]]

We call js::CreateRegExpMatchResult => js::NewDenseFullyAllocatedArrayWithShape. This OOMs under allocateInitialSlots.

We correctly return nullptr, but we still have an object in the heap for which the number of dynamic slots doesn't match the slots in the shape, so this asserts under js::DumpHeap.

Jon, could this be a regression from cd8376894002?

Comment 2

[Jan de Mooij [:jandem]]

We could potentially change the object's shape to global->arrayShapeWithDefaultProto when we OOM. This pointer should be non-nullptr at this point I think considering we allocate an array under RegExpRealm::createMatchResultShape. This might not be true for other callers though...

Comment 3

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

Since this did not seem like a non-s-s issue immediately at first glance, upon waking up this morning after a late-night bug file, I got the following confirmation:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/cd8376894002
user:        Jon Coppeard
date:        Fri Apr 14 09:38:40 2023 +0000
summary:     Bug 1827918 - Part 2: Move dynamic slot allocation out of GC allocation path r=jandem

Comment 4

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

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

Comment 5

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

oomTest(Debugger);
oomTest(Debugger);
async function* f() {}
f().return();
dumpHeap(f);

Here's another testcase that does not require --no-ggc.

Tested with m-c rev 2cac2f68bfdc, run with --fuzzing-safe --no-threads --no-baseline --no-ion.

Comment 6

[Jon Coppeard (:jonco)]

Similar to bug 1828396 except observing the partially initialized object with dumpHeap().

Comment 7

[Jon Coppeard (:jonco)]

Not security sensitive. The object we're asserting on is unreachable and requires some debugging functionality (e.g. dumpHeap) for us to touch with it.

Comment 8

[Jon Coppeard (:jonco)]

Attachment: Bug 1877406 - Set native object shape to a safe value if allocating dynamic slots fails r?jandem

The assertion is failing beacuse the number of slots allocated (zero) doesn't
match the number we expect given the object's shape. Since allocation is
failing and the object will be unreachable, the patch sets the shape to that of
a plain object with zero slots. This should be a safe default.

We have to also ensure that this shape always exists so we can allocate it when
the global is initialized.

This means we can also remove the workaround fix for bug 1828396.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c03c917c8c39
Set native object shape to a safe value if allocating dynamic slots fails r=jandem

Comment 10

[Natalia Csoregi [:nataliaCs]]

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

Comment 11

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

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

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

For more information, please visit BugBot documentation.

Comment 12

[Jon Coppeard (:jonco)]

It's not necessary to uplift this as it's not something that will happen in normal use.