1833517 - Assertion failure: !templateObj->hasDynamicSlots(), at jit/WarpBuilder.cpp:325

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

enableShellAllocationMetadataBuilder();
(function f() {
  (function () {
    f;
  });
})();

Thread 1 "js-dbg-64-linux" received signal SIGSEGV, Segmentation fault.
js::jit::WarpBuilder::buildNamedLambdaEnv (this=this@entry=0x7fffffffbe40, callee=callee@entry=0x7ffff699e798, env=env@entry=0x7ffff699e810, templateObj=<optimized out>)
    at /home/gen16gx500/trees/mozilla-central/js/src/jit/WarpBuilder.cpp:325
325	  MOZ_ASSERT(!templateObj->hasDynamicSlots());
(gdb) bt
#0  js::jit::WarpBuilder::buildNamedLambdaEnv (this=this@entry=0x7fffffffbe40, callee=callee@entry=0x7ffff699e798, env=env@entry=0x7ffff699e810, templateObj=<optimized out>)
    at /home/gen16gx500/trees/mozilla-central/js/src/jit/WarpBuilder.cpp:325
#1  0x0000555557c7bdd5 in js::jit::WarpBuilder::buildEnvironmentChain()::$_2::operator()(js::jit::FunctionEnvironment const&) const (env=..., this=<optimized out>)
    at /home/gen16gx500/trees/mozilla-central/js/src/jit/WarpBuilder.cpp:396
#2  mozilla::detail::VariantImplementation<unsigned char, 2ul, js::jit::FunctionEnvironment>::matchN<mozilla::Variant<js::jit::NoEnvironment, js::jit::WarpGCPtr<JSObject*>, js::jit::FunctionEnvironment> const&, js::jit::WarpBuilder::buildEnvironmentChain()::$_2>(mozilla::Variant<js::jit::NoEnvironment, js::jit::WarpGCPtr<JSObject*>, js::jit::FunctionEnvironment> const&, js::jit::WarpBuilder::buildEnvironmentChain()::$_2&&) (aV=..., aMatcher=...) at /home/gen16gx500/shell-cache/js-dbg-64-linux-x86_64-8ebc261d0f07/objdir-js/dist/include/mozilla/Variant.h:202
#3  mozilla::detail::VariantImplementation<unsigned char, 1ul, js::jit::WarpGCPtr<JSObject*>, js::jit::FunctionEnvironment>::matchN<mozilla::Variant<js::jit::NoEnvironment, js::jit::WarpGCPtr<JSObject*>, js::jit::FunctionEnvironment> const&, js::jit::WarpBuilder::buildEnvironmentChain()::$_1, js::jit::WarpBuilder::buildEnvironmentChain()::$_2>(mozilla::Variant<js::jit::NoEnvironment, js::jit::WarpGCPtr<JSObject*>, js::jit::FunctionEnvironment> const&, js::jit::WarpBuilder::buildEnvironmentChain()::$_1&&, js::jit::WarpBuilder::buildEnvironmentChain()::$_2&&) (
    aV=..., aMi=..., aMs=...) at /home/gen16gx500/shell-cache/js-dbg-64-linux-x86_64-8ebc261d0f07/objdir-js/dist/include/mozilla/Variant.h:318
#4  mozilla::detail::VariantImplementation<unsigned char, 0ul, js::jit::NoEnvironment, js::jit::WarpGCPtr<JSObject*>, js::jit::FunctionEnvironment>::matchN<mozilla::Variant<js::jit::NoEnvironment, js::jit::WarpGCPtr<JSObject*>, js::jit::FunctionEnvironment> const&, js::jit::WarpBuilder::buildEnvironmentChain()::$_0, js::jit::WarpBuilder::buildEnvironmentChain()::$_1, js::jit::WarpBuilder::buildEnvironmentChain()::$_2>(mozilla::Variant<js::jit::NoEnvironment, js::jit::WarpGCPtr<JSObject*>, js::jit::FunctionEnvironment> const&, js::jit::WarpBuilder::buildEnvironmentChain()::$_0&&, js::jit::WarpBuilder::buildEnvironmentChain()::$_1&&, js::jit::WarpBuilder::buildEnvironmentChain()::$_2&&) (aV=..., aMi=..., aMs=..., aMs=...)
    at /home/gen16gx500/shell-cache/js-dbg-64-linux-x86_64-8ebc261d0f07/objdir-js/dist/include/mozilla/Variant.h:318
#5  mozilla::Variant<js::jit::NoEnvironment, js::jit::WarpGCPtr<JSObject*>, js::jit::FunctionEnvironment>::matchN<mozilla::Variant<js::jit::NoEnvironment, js::jit::WarpGCPtr<JSObject*>, js::jit::FunctionEnvironment> const&, js::jit::WarpBuilder::buildEnvironmentChain()::$_0, js::jit::WarpBuilder::buildEnvironmentChain()::$_1, js::jit::WarpBuilder::buildEnvironmentChain()::$_2>(mozilla::Variant<js::jit::NoEnvironment, js::jit::WarpGCPtr<JSObject*>, js::jit::FunctionEnvironment> const&, js::jit::WarpBuilder::buildEnvironmentChain()::$_0&&, js::jit::WarpBuilder::buildEnvironmentChain()::$_1&&, js::jit::WarpBuilder::buildEnvironmentChain()::$_2&&) (aVariant=..., aM0=..., aM1=..., aMs=...)
    at /home/gen16gx500/shell-cache/js-dbg-64-linux-x86_64-8ebc261d0f07/objdir-js/dist/include/mozilla/Variant.h:902
#6  mozilla::Variant<js::jit::NoEnvironment, js::jit::WarpGCPtr<JSObject*>, js::jit::FunctionEnvironment>::match<js::jit::WarpBuilder::buildEnvironmentChain()::$_0, js::jit::WarpBuilder::buildEnvironmentChain()::$_1, js::jit::WarpBuilder::buildEnvironmentChain()::$_2>(js::jit::WarpBuilder::buildEnvironmentChain()::$_0&&, js::jit::WarpBuilder::buildEnvironmentChain()::$_1&&, js::jit::WarpBuilder::buildEnvironmentChain()::$_2&&) const & (this=<optimized out>, aM0=..., aM1=..., aMs=...)
    at /home/gen16gx500/shell-cache/js-dbg-64-linux-x86_64-8ebc261d0f07/objdir-js/dist/include/mozilla/Variant.h:845
#7  js::jit::WarpBuilder::buildEnvironmentChain (this=this@entry=0x7fffffffbe40) at /home/gen16gx500/trees/mozilla-central/js/src/jit/WarpBuilder.cpp:384
#8  0x0000555557c795cc in js::jit::WarpBuilder::buildPrologue (this=0x7fffffffbe40) at /home/gen16gx500/trees/mozilla-central/js/src/jit/WarpBuilder.cpp:460
#9  0x0000555557c7907e in js::jit::WarpBuilder::build (this=0x7ffff7bf8a00) at /home/gen16gx500/trees/mozilla-central/js/src/jit/WarpBuilder.cpp:290
#10 0x0000555557c68861 in js::jit::CompileBackEnd (mir=mir@entry=0x7ffff699e178, snapshot=snapshot@entry=0x7ffff699e2d0) at /home/gen16gx500/trees/mozilla-central/js/src/jit/Ion.cpp:1559
#11 0x0000555557c69b23 in js::jit::IonCompile (cx=0x7ffff772e100, script=..., osrPc=<optimized out>) at /home/gen16gx500/trees/mozilla-central/js/src/jit/Ion.cpp:1694
#12 js::jit::Compile (cx=cx@entry=0x7ffff772e100, script=script@entry=..., osrFrame=osrFrame@entry=0x0, osrPc=osrPc@entry=0x0)
    at /home/gen16gx500/trees/mozilla-central/js/src/jit/Ion.cpp:1861
#13 0x0000555557c68f73 in js::jit::CanEnterIon (cx=cx@entry=0x7ffff772e100, state=...) at /home/gen16gx500/trees/mozilla-central/js/src/jit/Ion.cpp:1952
#14 0x0000555557d2c76c in js::jit::MaybeEnterJit (cx=cx@entry=0x7ffff772e100, state=...) at /home/gen16gx500/trees/mozilla-central/js/src/jit/Jit.cpp:170
#15 0x0000555556ea8dfa in js::RunScript (cx=cx@entry=0x7ffff772e100, state=...) at /home/gen16gx500/trees/mozilla-central/js/src/vm/Interpreter.cpp:448
#16 0x0000555556ea978d in js::InternalCallOrConstruct (cx=0x7ffff772e100, args=..., construct=construct@entry=js::NO_CONSTRUCT, reason=js::CallReason::Call)
    at /home/gen16gx500/trees/mozilla-central/js/src/vm/Interpreter.cpp:612
#17 0x0000555556eaa46d in InternalCall (cx=0x7ffff7bf8a00, args=..., reason=1481763648, reason@entry=js::CallReason::Call)
    at /home/gen16gx500/trees/mozilla-central/js/src/vm/Interpreter.cpp:647
#18 0x0000555556eaa3d7 in js::CallFromStack (cx=0x7ffff7bf8a00, args=..., reason=js::CallReason::CallContent, reason@entry=js::CallReason::Call)
    at /home/gen16gx500/trees/mozilla-central/js/src/vm/Interpreter.cpp:652
#19 0x00005555578e06fb in js::jit::DoCallFallback (cx=0x7ffff7bf8a00, frame=0x7fffffffcbc8, stub=0x7ffff77a9888, argc=0, vp=0x7fffffffcb88, res=...)
    at /home/gen16gx500/trees/mozilla-central/js/src/jit/BaselineIC.cpp:1591
#20 0x00000c718f7e4487 in ?? ()
#21 0x00007ffff772e118 in ?? ()
#22 0x00007fffffffcb40 in ?? ()
#23 0xfff9800000000000 in ?? ()
#24 0x00005555584d2450 in js::jit::vmFunctions ()
#25 0x00007fffffffcba0 in ?? ()
#26 0x00000c718f8096e4 in ?? ()
#27 0x0000000000000002 in ?? ()
#28 0x00007fffffffcbc8 in ?? ()
#29 0x00007ffff77a9888 in ?? ()
#30 0x0000000000000000 in ?? ()
(gdb)

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/fd4f1d5035c2
user:        Jon Coppeard
date:        Tue May 09 15:25:17 2023 +0000
summary:     Bug 1828455 - Part 1: Use dynamic slots header to store unique IDs for native objects r=jandem

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

Jon, is bug 1828455 a likely regressor? Setting s-s just-in-case, esp since bug 746376 was previously sec-critical, but this involves enableShellAllocationMetadataBuilder so I'm not sure.

Comment 1

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

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

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1833517 - Fix code generation for object allocation now that dynamic slots allocation can be present but empty r?jandem

Native objects may now have a dynamic slots allocation to store their unique ID
without using any dynamic slots.

The assertion that fails is not a problem and can be updated to check the
number of dynamic slots used. However code generation for object creation needs
to check the number of slots used everywhere. Currently I think we can create
objects with uninitialised dynamic slots if the template object had a unique ID
set and no dynamic slots.

Comment 3

[Daniel Veditz [:dveditz]]

What's the difference between !hasDynamicSlots() and numDynamicSlots == 0 ? I think this patch is not doing everything you said in comment 2 -- do we need to wait for that part of the fix? I have no idea what severity to assign here.

Comment 4

[Jon Coppeard (:jonco)]

(In reply to Daniel Veditz [:dveditz] from comment #3)
hasDynamicSlots() returns whether a dynamic slots allocation exists, whereas numDynamicSlots() returns the number of slots in that allocation which can be zero (previously that wasn't the case). The naming is not great here. I've filed bug 1833827 to address that.

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Fix code generation for object allocation now that dynamic slots allocation can be present but empty r=jandem
https://hg.mozilla.org/integration/autoland/rev/6778a82ddeb64dd0569a6aca5b17dd0d9c8a0a9c
https://hg.mozilla.org/mozilla-central/rev/6778a82ddeb6

Comment 6

[Andrew McCreight [:mccr8]]

What are the security implications of this issue? For bug bounty purposes, we should rate it. Are we going to potentially be reading from an uninitialized JS object? Do we initialize to a fixed value? Is the type of the thing we'd be reading as uninitialized fixed? etc. Thanks.

Comment 7

[Jon Coppeard (:jonco)]

(In reply to Andrew McCreight [:mccr8] from comment #6)
It means a potential read from uninitialized memory if one of the JIT's template objects ends up with a unique ID. I know this can happen with if the allocation metadata builder is enabled (i.e. requires devtools), but I'm not sure otherwise. Jan do you know if this can happen more generally?

Comment 8

[Jan de Mooij [:jandem]]

(In reply to Jon Coppeard (:jonco) from comment #7)

(In reply to Andrew McCreight [:mccr8] from comment #6)
It means a potential read from uninitialized memory if one of the JIT's template objects ends up with a unique ID. I know this can happen with if the allocation metadata builder is enabled (i.e. requires devtools), but I'm not sure otherwise. Jan do you know if this can happen more generally?

That sounds right to me. We don't expose template objects to JS, so the metadata callback is probably the only thing that will be able to see those objects.