1531018 - Crash [@ mozilla::HashBytes] with BigInt and GC

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 110ea2a7c3d4 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off):

evalInWorker(`
  function f() {
    return gczeal(14,1);
  }
  f();
  const one = BigInt(1);
  new Map([[one, 42]]).has(one);
`);

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x00005555558a4898 in mozilla::HashBytes (aBytes=0x1, aLength=38675482157104) at mfbt/HashFunctions.cpp:22
#1  0x00005555559b3c04 in JS::BigInt::hash (this=0x232cd664a030) at js/src/vm/BigIntType.cpp:177
#2  0x0000555555946a02 in HashValue (v=..., hcs=...) at js/src/builtin/MapObject.cpp:84
#3  0x0000555555946d28 in js::HashableValue::hash (hcs=..., this=<optimized out>) at js/src/builtin/MapObject.cpp:95
#4  js::HashableValue::Hasher::hash (hcs=..., v=...) at js/src/builtin/MapObject.h:35
#5  js::detail::OrderedHashTable<js::OrderedHashMap<js::HashableValue, js::HeapPtr<JS::Value>, js::HashableValue::Hasher, js::ZoneAllocPolicy>::Entry, js::OrderedHashMap<js::HashableValue, js::HeapPtr<JS::Value>, js::HashableValue::Hasher, js::ZoneAllocPolicy>::MapOps, js::ZoneAllocPolicy>::prepareHash (l=..., this=<optimized out>) at js/src/ds/OrderedHashTable.h:617
#6  js::detail::OrderedHashTable<js::OrderedHashMap<js::HashableValue, js::HeapPtr<JS::Value>, js::HashableValue::Hasher, js::ZoneAllocPolicy>::Entry, js::OrderedHashMap<js::HashableValue, js::HeapPtr<JS::Value>, js::HashableValue::Hasher, js::ZoneAllocPolicy>::MapOps, js::ZoneAllocPolicy>::Range::rekeyFront (k=..., this=0x7ffff68fc270) at js/src/ds/OrderedHashTable.h:472
#7  TraceKey<js::detail::OrderedHashTable<js::OrderedHashMap<js::HashableValue, js::HeapPtr<JS::Value>, js::HashableValue::Hasher, js::ZoneAllocPolicy>::Entry, js::OrderedHashMap<js::HashableValue, js::HeapPtr<JS::Value>, js::HashableValue::Hasher, js::ZoneAllocPolicy>::MapOps, js::ZoneAllocPolicy>::Range> (trc=0x7ffff68fc418, key=..., r=...) at js/src/builtin/MapObject.cpp:431
#8  js::MapObject::trace (trc=0x7ffff68fc418, obj=<optimized out>) at js/src/builtin/MapObject.cpp:438
#9  0x0000555555b1875b in js::Class::doTrace (this=<optimized out>, obj=0x232cd6611080, trc=0x7ffff68fc418) at dist/include/js/Class.h:872
#10 JSObject::traceChildren (this=this@entry=0x232cd6611080, trc=trc@entry=0x7ffff68fc418) at js/src/vm/JSObject.cpp:4091
#11 0x0000555555fcc93e in UpdateCellPointers<JSObject> (cell=0x232cd6611080, trc=0x7ffff68fc410) at js/src/gc/GC.cpp:2511
#12 UpdateArenaPointersTyped<JSObject> (trc=trc@entry=0x7ffff68fc410, arena=<optimized out>) at js/src/gc/GC.cpp:2517
#13 0x0000555555fdd508 in UpdateArenaPointers (arena=0x232cd6611000, trc=0x7ffff68fc410) at js/src/gc/GC.cpp:2533
#14 js::gc::UpdatePointersTask::updateArenas (this=this@entry=0x7ffff68fc650) at js/src/gc/GC.cpp:2649
#15 0x0000555555fddb68 in js::gc::UpdatePointersTask::run (this=0x7ffff68fc650) at js/src/gc/GC.cpp:2658
#16 0x0000555555ad04e8 in js::GCParallelTask::runTask (this=0x7ffff68fc650) at js/src/gc/GCParallelTask.h:128
#17 js::GCParallelTask::runFromMainThread (this=this@entry=0x7ffff68fc650, rt=<optimized out>) at js/src/vm/HelperThreads.cpp:1614
#18 0x0000555555fb23fd in js::gc::GCRuntime::updateCellPointers (this=this@entry=0x7ffff4d8d6b8, zone=zone@entry=0x7ffff4d7c000, kinds=..., bgTaskCount=bgTaskCount@entry=4) at js/src/gc/GC.cpp:2744
#19 0x0000555555fe65fd in js::gc::GCRuntime::updateAllCellPointers (this=this@entry=0x7ffff4d8d6b8, trc=trc@entry=0x7ffff68fc9f0, zone=zone@entry=0x7ffff4d7c000) at js/src/gc/GC.cpp:2813
#20 0x0000555555fe6812 in js::gc::GCRuntime::updateZonePointersToRelocatedCells (this=this@entry=0x7ffff4d8d6b8, zone=zone@entry=0x7ffff4d7c000) at js/src/gc/GC.cpp:2845
#21 0x0000555555fee18b in js::gc::GCRuntime::compactPhase (this=this@entry=0x7ffff4d8d6b8, reason=reason@entry=JS::GCReason::DEBUG_GC, sliceBudget=..., session=...) at js/src/gc/GC.cpp:6627
#22 0x0000555555ff2cd2 in js::gc::GCRuntime::incrementalSlice (this=this@entry=0x7ffff4d8d6b8, budget=..., reason=reason@entry=JS::GCReason::DEBUG_GC, session=...) at js/src/gc/GC.cpp:7080
#23 0x0000555555ff35fa in js::gc::GCRuntime::gcCycle (this=this@entry=0x7ffff4d8d6b8, nonincrementalByAPI=nonincrementalByAPI@entry=true, budget=..., reason=reason@entry=JS::GCReason::DEBUG_GC) at js/src/gc/GC.cpp:7374
#24 0x0000555555ff3c6d in js::gc::GCRuntime::collect (this=this@entry=0x7ffff4d8d6b8, nonincrementalByAPI=nonincrementalByAPI@entry=true, budget=..., reason=reason@entry=JS::GCReason::DEBUG_GC) at js/src/gc/GC.cpp:7545
#25 0x0000555555ff4109 in js::gc::GCRuntime::gc (this=this@entry=0x7ffff4d8d6b8, gckind=gckind@entry=GC_SHRINK, reason=reason@entry=JS::GCReason::DEBUG_GC) at js/src/gc/GC.cpp:7633
#26 0x0000555555ff5c79 in js::gc::GCRuntime::runDebugGC (this=this@entry=0x7ffff4d8d6b8) at js/src/gc/GC.cpp:8169
#27 0x0000555555ff5d9e in js::gc::GCRuntime::gcIfNeededAtAllocation (this=this@entry=0x7ffff4d8d6b8, cx=cx@entry=0x7ffff4d71000) at js/src/gc/Allocator.cpp:338
#28 0x000055555602c518 in js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1> (this=this@entry=0x7ffff4d8d6b8, cx=cx@entry=0x7ffff4d71000, kind=kind@entry=js::gc::AllocKind::OBJECT2_BACKGROUND) at js/src/gc/Allocator.cpp:302
#29 0x000055555602c75f in js::Allocate<JSObject, (js::AllowGC)1> (cx=cx@entry=0x7ffff4d71000, kind=<optimized out>, nDynamicSlots=nDynamicSlots@entry=0, heap=heap@entry=js::gc::TenuredHeap, clasp=clasp@entry=0x555557b9b020 <js::PlainObject::class_>) at js/src/gc/Allocator.cpp:56
#30 0x00005555559d3fbb in js::NativeObject::create (cx=0x7ffff4d71000, kind=<optimized out>, heap=js::gc::TenuredHeap, shape=..., group=...) at js/src/vm/NativeObject-inl.h:491
#31 0x0000555555b28f2d in NewObject (cx=<optimized out>, group=..., kind=<optimized out>, newKind=js::TenuredObject, initialShapeFlags=<optimized out>) at js/src/vm/JSObject.cpp:802
#32 0x0000555555b2986d in js::NewObjectWithClassProtoCommon (cx=<optimized out>, cx@entry=0x7ffff4d71000, clasp=0x555557b9b020 <js::PlainObject::class_>, protoArg=..., protoArg@entry=..., allocKind=allocKind@entry=js::gc::AllocKind::OBJECT2_BACKGROUND, newKind=newKind@entry=js::TenuredObject) at js/src/vm/JSObject.cpp:934
#33 0x00005555558d7866 in js::NewObjectWithClassProto (newKind=js::TenuredObject, allocKind=js::gc::AllocKind::OBJECT2_BACKGROUND, proto=..., clasp=<optimized out>, cx=0x7ffff4d71000) at js/src/vm/JSObject-inl.h:489
#34 js::NewBuiltinClassInstance (newKind=js::TenuredObject, allocKind=js::gc::AllocKind::OBJECT2_BACKGROUND, clasp=<optimized out>, cx=0x7ffff4d71000) at js/src/vm/JSObject-inl.h:522
#35 js::NewBuiltinClassInstance<js::PlainObject> (newKind=js::TenuredObject, allocKind=js::gc::AllocKind::OBJECT2_BACKGROUND, cx=0x7ffff4d71000) at js/src/vm/JSObject-inl.h:541
#36 js::CopyInitializerObject (cx=0x7ffff4d71000, baseobj=..., newKind=js::TenuredObject) at js/src/vm/NativeObject-inl.h:653
#37 0x00005555558d8b10 in js::NewObjectOperation (cx=0x7ffff4d71000, script=..., pc=0x7ffff5f7b10c "[", newKind=js::TenuredObject) at js/src/vm/Interpreter.cpp:5126
#38 0x00005555558df32e in Interpret (cx=0x7ffff4d71000, state=...) at js/src/vm/Interpreter.cpp:3729
#39 0x00005555558e7b06 in js::RunScript (cx=0x7ffff4d71000, state=...) at js/src/vm/Interpreter.cpp:420
#40 0x00005555558e838f in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7ffff4d71000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:560
#41 0x00005555558e87ed in InternalCall (cx=cx@entry=0x7ffff4d71000, args=...) at js/src/vm/Interpreter.cpp:587
#42 0x00005555558e8980 in js::Call (cx=cx@entry=0x7ffff4d71000, fval=..., fval@entry=..., thisv=..., thisv@entry=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:603
#43 0x0000555555cc5d18 in js::CallSelfHostedFunction (cx=0x7ffff4d71000, name=..., thisv=..., thisv@entry=..., args=..., rval=...) at js/src/vm/SelfHosting.cpp:1906
#44 0x000055555596e80d in js::MapObject::construct (cx=<optimized out>, cx@entry=0x7ffff4d71000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/MapObject.cpp:664
#45 0x00005555558f63d9 in CallJSNative (cx=0x7ffff4d71000, native=native@entry=0x55555596e570 <js::MapObject::construct(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:440
[...]
#54 0x00005555558608a1 in WorkerMain (input=<optimized out>) at js/src/shell/js.cpp:4038
#55 0x0000555555863102 in js::detail::ThreadTrampoline<void (&)(WorkerInput*), WorkerInput*&>::callMain<0ul> (this=0x7ffff4d770e0) at js/src/threading/Thread.h:239
#56 js::detail::ThreadTrampoline<void (&)(WorkerInput*), WorkerInput*&>::Start (aPack=0x7ffff4d770e0) at js/src/threading/Thread.h:232
#57 0x00007ffff7bc16ba in start_thread (arg=0x7ffff68ff700) at pthread_create.c:333
#58 0x00007ffff6c2c41d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
rax	0x0	0
rbx	0x232cd664a030	38675482452016
rcx	0x232cd6602030	38675482157104
rdx	0x0	0
rsi	0x232cd6602030	38675482157104
rdi	0x1	1
rbp	0x7ffff68fc1c0	140737330004416
rsp	0x7ffff68fc1c0	140737330004416
r8	0x555556c5cd6c	93825016384876
r9	0x4000001	67108865
r10	0x1	1
r11	0x7ffff4deff10	140737301643024
r12	0x555556ab87e2	93825014663138
r13	0x7ffff68fc268	140737330004584
r14	0x7ffff5f5ef00	140737319923456
r15	0xfffaffffffffffff	-1407374883553281
rip	0x5555558a4898 <mozilla::HashBytes(void const*, unsigned long)+24>
=> 0x5555558a4898 <mozilla::HashBytes(void const*, unsigned long)+24>:	mov    (%rdi,%rdx,1),%r8
   0x5555558a489c <mozilla::HashBytes(void const*, unsigned long)+28>:	rol    $0x5,%eax

Marking s-s because the test uses gczeal.

Comment 1

[Andy Wingo [:wingo]]

Robin kindly agreed to have a look; could be related to https://bugzilla.mozilla.org/show_bug.cgi?id=1530406

Comment 2

[Jon Coppeard (:jonco)]

This is happening during compacting GC. I reproduced it, and looking down the call stack I found the following comment when tracing Map keys:

// The hash function only uses the bits of the Value, so it is safe to
// rekey even when the object or string has been modified by the GC.

https://searchfox.org/mozilla-central/source/js/src/builtin/MapObject.cpp#429-430

This doesn't seem to be true for BigInts though which use data in the BigInt object itself to calculate the has.

Also there's a comment here explaining how these hashes are calculated:

https://searchfox.org/mozilla-central/source/js/src/builtin/MapObject.cpp#68-75

Comment 3

[Jon Coppeard (:jonco)]

Attachment: bug1531018-bigint-hashing

We need to make HashValue() account for the fact that BigInts may have been forwarded.

Comment 4

[Andy Wingo [:wingo]]

Patch LGTM, for what it's worth!

Comment 5

[Andy Wingo [:wingo]]

And thank you!!

Comment 6

[Andrew McCreight (out of office until 8/21) [:mccr8]]

I'm going to assume this is trunk only.

Comment 7

[Fuzzing Team]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/7f6e5294fb0f
user:        Andy Wingo
date:        Mon Feb 25 15:28:42 2019 +0000
summary:     Bug 1454862 - Enable compaction for BigInt values r=tcampbell,jonco

This iteration took 508.341 seconds to run.

Comment 8

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/664a5bdd8ab159d09bb75148a59ff5eaeafdabe8
https://hg.mozilla.org/mozilla-central/rev/664a5bdd8ab1

Comment 9

[Fuzzing Team]

JSBugMon: This bug has been automatically verified fixed.