1808210 - Assertion failure: producer_ != nullptr, at jit/MIR.h:245

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20230101-492f77863dcc (debug build, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off --fast-warmup --blinterp-warmup-threshold=1 --blinterp-eager):

class LFP300442303 {}
for (let i28 = 0; i28 < 100; i28++) {
  f = function() { return new objects(); };
  for (let x26 = 0; x26 < i28; x26++) {
    obj = function() {};
    src = obj;
    objects.push({...src.bogus} ^ obj[undefined]);
  }
  objects = LFP300442303;
  objects.push = function() { return new objects(); };
  for (let i28 = 0; i28 < 100; i28++) {}
  f();
}

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x000055555789076d in js::jit::MAryInstruction<1ul>::getOperand(unsigned long) const ()
#1  0x0000555557c6f4f2 in js::jit::MDefinition::congruentIfOperandsEqual(js::jit::MDefinition const*) const ()
#2  0x00005555578a299b in js::jit::MConstantProto::congruentTo(js::jit::MDefinition const*) const ()
#3  0x000055555786a4cc in js::jit::ValueNumberer::VisibleValues::ValueHasher::match(js::jit::MDefinition*, js::jit::MDefinition const*) ()
#4  0x00005555578ad3f9 in mozilla::detail::EntrySlot<js::jit::MDefinition* const> mozilla::detail::HashTable<js::jit::MDefinition* const, mozilla::HashSet<js::jit::MDefinition*, js::jit::ValueNumberer::VisibleValues::ValueHasher, js::jit::JitAllocPolicy>::SetHashPolicy, js::jit::JitAllocPolicy>::lookup<(mozilla::detail::HashTable<js::jit::MDefinition* const, mozilla::HashSet<js::jit::MDefinition*, js::jit::ValueNumberer::VisibleValues::ValueHasher, js::jit::JitAllocPolicy>::SetHashPolicy, js::jit::JitAllocPolicy>::LookupReason)0>(js::jit::MDefinition const* const&, unsigned int) const ()
#5  0x000055555786a8c8 in js::jit::ValueNumberer::VisibleValues::forget(js::jit::MDefinition const*) ()
#6  0x000055555786b815 in js::jit::ValueNumberer::releaseOperands(js::jit::MDefinition*) ()
#7  0x000055555786b04a in js::jit::ValueNumberer::discardDef(js::jit::MDefinition*, js::jit::ValueNumberer::AllowEffectful) ()
#8  0x000055555786add7 in js::jit::ValueNumberer::discardDefsRecursively(js::jit::MDefinition*, js::jit::ValueNumberer::AllowEffectful) ()
#9  0x000055555786de25 in js::jit::ValueNumberer::visitUnreachableBlock(js::jit::MBasicBlock*) ()
#10 0x000055555786e518 in js::jit::ValueNumberer::visitDominatorTree(js::jit::MBasicBlock*) ()
#11 0x000055555786e993 in js::jit::ValueNumberer::visitGraph() ()
#12 0x000055555786f81b in js::jit::ValueNumberer::run(js::jit::ValueNumberer::UpdateAliasAnalysisFlag) ()
#13 0x0000555557b8d3e1 in js::jit::OptimizeMIR(js::jit::MIRGenerator*) ()
#14 0x0000555557b96a5f in js::jit::CompileBackEnd(js::jit::MIRGenerator*, js::jit::WarpSnapshot*) ()
#15 0x0000555557b97d9a in js::jit::Compile(JSContext*, JS::Handle<JSScript*>, js::jit::BaselineFrame*, unsigned char*) ()
#16 0x0000555557b987de in IonCompileScriptForBaseline(JSContext*, js::jit::BaselineFrame*, unsigned char*) ()
#17 0x0000555557b98e8a in js::jit::IonCompileScriptForBaselineOSR(JSContext*, js::jit::BaselineFrame*, unsigned int, unsigned char*, js::jit::IonOsrTempData**) ()
#18 0x0000019f74a07558 in ?? ()
#19 0x00007fffffffc060 in ?? ()
#20 0x00007fffffffbfc8 in ?? ()
#21 0x0000177b6593e030 in ?? ()
#22 0x0000000000000000 in ?? ()
rax	0x5555559241c4	93824996229572
rbx	0x7ffff5a5dff8	140737314676728
rcx	0x55555837af78	93825040625528
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7fffffffb380	140737488335744
rsp	0x7fffffffb380	140737488335744
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f99800	140737353717760
r10	0x0	0
r11	0x0	0
r12	0x0	0
r13	0x0	0
r14	0x7ffff60526d0	140737320920784
r15	0x1	1
rip	0x55555789076d <js::jit::MAryInstruction<1ul>::getOperand(unsigned long) const+77>
=> 0x55555789076d <_ZNK2js3jit15MAryInstructionILm1EE10getOperandEm+77>:	movl   $0xf5,0x0
   0x555557890778 <_ZNK2js3jit15MAryInstructionILm1EE10getOperandEm+88>:	callq  0x555556c7cf24 <abort>

Marking s-s because this is a JIT related assertion.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Bugmon [:jkratzer for issues]]

Verified bug as reproducible on mozilla-central 20230101212612-492f77863dcc.
The bug appears to have been introduced in the following build range:

Start: 6212248e60a47a7a019b2e46c5089172dadbca52 (20221213001201)
End: bd78e2e5b1fe852424cf7c035580e44de70ac135 (20221213001429)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=6212248e60a47a7a019b2e46c5089172dadbca52&tochange=bd78e2e5b1fe852424cf7c035580e44de70ac135

Comment 4

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

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

Comment 5

[Andrew McCreight [:mccr8]]

A getter unexpectedly returning a null doesn't sound particularly dangerous to me, but hopefully somebody who is familiar with this code can confirm that.

Comment 6

[Alex Thayer [:alexical] (she/her)]

Yeah I don't think this is dangerous - the issue goes away if you just reorder the checks in MConstantProto::congruentTo. However, I'm still looking to understand precisely why that is and what's going on here.

Comment 7

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1808210 - Check ptr equality in MConstantProto::congruentTo r?iain

It looks like basically what's going on here is VisibleValues::forget
can fail to be found in set_ because it says it doesn't match itself,
because its receiverObject_ is null. I don't know if this is the
preferred fix, or if we should check pointer equality in
ValueHasher::match. One interesting thing to note is that in
ValueNumberer::leader, we explicitly check a node for conruency against
itself in order to detect if it has opted out of redundance elimination, so
it would be somewhat awkward for ValueHasher::match to not allow nodes to
return false for congruency against themselves, although I think everything
would still be correct.

Comment 8

[Andrew McCreight [:mccr8]]

Is this a security issue?

Comment 9

[Alex Thayer [:alexical] (she/her)]

I feel pretty confident at this point that this is not a security issue. Basically, we're leaving a bogus MDefinition in a hash set because it fails to match with itself. We clean up that bogus value in everything but the hash set, and then we later try to remove a different value which collides with it, so we check for congruency and hit an assertion failure because the old MDefinition is bogus. However if we didn't hit that assertion failure, we'd just continue on to the next one and try to remove it. We end up cluttering a hash set, but the hash set will never yield those bogus values back out because they will always fail to match with anything at all, including themselves.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e0bd0bb80d0f
Check ptr equality in MConstantProto::congruentTo r=iain

Comment 11

[Marian-Vasile Laza]

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

Comment 12

[Bugmon [:jkratzer for issues]]

Verified bug as fixed on rev mozilla-central 20230107093442-7272e73dca36.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.