1063188 - Segfault in CompactBufferReader::readByte()

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Botond Ballo [:botond]]

I ran into the following segfault when starting up debug build of B2G.

The Gecko revision I was using is 39a8d9b2b639, from a few days ago. I don't see it with today's build.

Please let me know if there's any other information I can provide.


Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 301.643]
0xb5b233a2 in readByte (this=<optimized out>) at ../../../../refactoring/js/src/jit/CompactBuffer.h:57
57              return *buffer_++;
(gdb) i s
#0  0xb5b233a2 in readByte (this=<optimized out>) at ../../../../refactoring/js/src/jit/CompactBuffer.h:57
#1  readVariableLength (this=<optimized out>) at ../../../../refactoring/js/src/jit/CompactBuffer.h:40
#2  readUnsigned (this=<optimized out>) at ../../../../refactoring/js/src/jit/CompactBuffer.h:77
#3  read (this=<synthetic pointer>) at ../../../../refactoring/js/src/jit/arm/Assembler-arm.cpp:628
#4  js::jit::Assembler::TraceJumpRelocations (trc=0xb0d8c9a8, code=0xaefa9420, reader=<optimized out>) at ../../../../refactoring/js/src/jit/arm/Assembler-arm.cpp:791
#5  0xb5a26d8e in js::jit::JitCode::trace (this=0xaefa9420, trc=0xb0d8c9a8) at ../../../../refactoring/js/src/jit/Ion.cpp:767
#6  0xb59bc6e4 in MarkChildren (code=0xaefa9420, trc=0xb0d8c9a8) at ../../../../refactoring/js/src/gc/Marking.cpp:1477
#7  js::GCMarker::processMarkStackOther (this=0xb0d8c9a8, tag=<optimized out>, addr=2935657504) at ../../../../refactoring/js/src/gc/Marking.cpp:1651
#8  0xb59bce5a in js::GCMarker::drainMarkStack (this=0xb0d8c9a8, budget=...) at ../../../../refactoring/js/src/gc/Marking.cpp:1814
#9  0xb5b6224e in js::gc::GCRuntime::drainMarkStack (this=0xb0d8c1d8, sliceBudget=..., phase=<optimized out>) at ../../../../refactoring/js/src/jsgc.cpp:4817
#10 0xb5b9d92a in js::gc::GCRuntime::incrementalCollectSlice (this=0xb0d8c1d8, budget=<optimized out>, reason=JS::gcreason::DOM_WORKER) at ../../../../refactoring/js/src/jsgc.cpp:5351
#11 0xb5b9e07e in js::gc::GCRuntime::gcCycle (this=0xb0d8c1d8, incremental=<optimized out>, budget=0, gckind=js::GC_SHRINK, reason=JS::gcreason::DOM_WORKER) at ../../../../refactoring/js/src/jsgc.cpp:5573
#12 0xb5b9e298 in js::gc::GCRuntime::collect (this=0xb0d8c1d8, incremental=false, budget=0, gckind=js::GC_SHRINK, reason=JS::gcreason::DOM_WORKER) at ../../../../refactoring/js/src/jsgc.cpp:5700
#13 0xb5ba03e8 in gc (reason=<optimized out>, gckind=js::GC_SHRINK, this=<optimized out>) at ../../../../refactoring/js/src/jsgc.cpp:5745
#14 JS::ShrinkingGC (rt=<optimized out>, reason=<optimized out>) at ../../../../refactoring/js/src/jsfriendapi.cpp:206
#15 0xb4f6ea72 in mozilla::dom::workers::WorkerPrivate::GarbageCollectInternal (this=0xb27b1c00, aCx=0xb1a05d20, aShrinking=<optimized out>, aCollectChildren=<optimized out>)
    at ../../../../refactoring/dom/workers/WorkerPrivate.cpp:5593
#16 0xb4f6eac0 in (anonymous namespace)::GarbageCollectRunnable::WorkerRun (this=<optimized out>, aCx=<optimized out>, aWorkerPrivate=<optimized out>)
    at ../../../../refactoring/dom/workers/WorkerPrivate.cpp:1645
#17 0xb4f77752 in mozilla::dom::workers::WorkerRunnable::Run (this=0xb0d7ebc0) at ../../../../refactoring/dom/workers/WorkerRunnable.cpp:326
#18 0xb4f72a86 in mozilla::dom::workers::WorkerPrivate::ProcessAllControlRunnablesLocked (this=0xb27b1c00) at ../../../../refactoring/dom/workers/WorkerPrivate.cpp:4485
#19 0xb4f76128 in mozilla::dom::workers::WorkerPrivate::DoRunLoop (this=0xb27b1c00, aCx=0xb1a05d20) at ../../../../refactoring/dom/workers/WorkerPrivate.cpp:3974
#20 0xb4f5bdb0 in (anonymous namespace)::WorkerThreadPrimaryRunnable::Run (this=0xb21bb060) at ../../../../refactoring/dom/workers/RuntimeService.cpp:2760
#21 0xb45330a8 in nsThread::ProcessNextEvent (this=0xb1ac3de0, aMayWait=<optimized out>, aResult=0xb19ffe27) at ../../../../refactoring/xpcom/threads/nsThread.cpp:823
#22 0xb45475f0 in NS_ProcessNextEvent (aThread=0xb1ac3de0, aMayWait=<optimized out>) at /home/botond/dev/mozilla/refactoring/xpcom/glue/nsThreadUtils.cpp:265
#23 0xb46f9082 in mozilla::ipc::MessagePumpForNonMainThreads::Run (this=0xb27c24f0, aDelegate=0xb213b080) at ../../../../refactoring/ipc/glue/MessagePump.cpp:326
#24 0xb46e6e16 in MessageLoop::RunInternal (this=0xb213b080) at ../../../../refactoring/ipc/chromium/src/base/message_loop.cc:229
#25 0xb46e6e2e in RunHandler (this=0xb213b080) at ../../../../refactoring/ipc/chromium/src/base/message_loop.cc:222
#26 MessageLoop::Run (this=0xb213b080) at ../../../../refactoring/ipc/chromium/src/base/message_loop.cc:196
#27 0xb45341d8 in nsThread::ThreadFunc (aArg=0xb1ac3de0) at ../../../../refactoring/xpcom/threads/nsThread.cpp:350
#28 0xb6a7f820 in _pt_root (arg=0xb2751180) at ../../../../../../refactoring/nsprpub/pr/src/pthreads/ptthread.c:212
#29 0xb6f2bba4 in __thread_entry (func=0xb6a7f779 <_pt_root>, arg=0xb2751180, tls=0xb19fff00) at bionic/libc/bionic/pthread_create.cpp:92
#30 0xb6f2bd20 in pthread_create (thread_out=0xbeaf9c0c, attr=<optimized out>, start_routine=0x78, arg=0xb2751180) at bionic/libc/bionic/pthread_create.cpp:201
#31 0x00000000 in ?? ()
(gdb) p buffer_
value has been optimized out
(gdb) x/20i $pc-20
   0xb5b2338e <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+22>:       add     sp, #28
   0xb5b23390 <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+24>:       ldmia.w sp!, {r4, r5, r6, r7, r8, r9, r10, r11, pc}
   0xb5b23394 <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+28>:
    ldr r7, [pc, #436]  ; (0xb5b2354c <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+468>)
   0xb5b23396 <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+30>:       movw    r9, #65296      ; 0xff10
   0xb5b2339a <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+34>:       movt    r9, #303        ; 0x12f
   0xb5b2339e <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+38>:       add     r7, pc
   0xb5b233a0 <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+40>:       mov     r2, r4
=> 0xb5b233a2 <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+42>:       ldrb.w  r1, [r2], #1
   0xb5b233a6 <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+46>:       lsrs    r3, r1, #1
   0xb5b233a8 <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+48>:       lsls    r1, r1, #31
   0xb5b233aa <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+50>:
    bpl.w       0xb5b23506 <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+398>
   0xb5b233ae <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+54>:       cmp     r5, r2
   0xb5b233b0 <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+56>:
    bls.w       0xb5b2350e <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+406>
   0xb5b233b4 <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+60>:       ldrb    r2, [r4, #1]
   0xb5b233b6 <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+62>:       adds    r1, r4, #2
   0xb5b233b8 <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+64>:       lsrs    r0, r2, #1
   0xb5b233ba <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+66>:       orr.w   r3, r3, r0, lsl #7
   0xb5b233be <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+70>:       lsls    r0, r2, #31
   0xb5b233c0 <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+72>:
    bpl.w       0xb5b2350a <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+402>
   0xb5b233c4 <js::jit::Assembler::TraceJumpRelocations(JSTracer*, js::jit::JitCode*, js::jit::CompactBufferReader&)+76>:       cmp     r1, r5
(gdb) p (void*)$r2
$1 = (void *) 0xaefe1310
(gdb) p (void*)$r1
$2 = (void *) 0xaefa9420
(gdb) p (void*)$r4
$3 = (void *) 0xaefe1310
(gdb)

Comment 1

[Botond Ballo [:botond]]

My debugger breaks at this segfault literally every time I debug B2G. Is there a way to disable this?

Comment 2

[Gregor Wagner [:gwagner]]

Yep this is pretty painful. I have to hit continue a few times to stat the phone.
Naveed, can we fix this?

Comment 3

[Jan de Mooij [:jandem]]

Does setting the JS_NO_SIGNALS=1 environment variable help?

Do we know how other b2g devs do this?

Comment 4

[Nicolas B. Pierron [:nbp]]

(In reply to Jan de Mooij [:jandem] from comment #3)
> Does setting the JS_NO_SIGNALS=1 environment variable help?

You can modify the b2g script in /system/bin/ to export this variable before starting Gecko.

Comment 5

[Gregor Wagner [:gwagner]]

(In reply to Nicolas B. Pierron [:nbp] from comment #4)
> (In reply to Jan de Mooij [:jandem] from comment #3)
> > Does setting the JS_NO_SIGNALS=1 environment variable help?
> 
> You can modify the b2g script in /system/bin/ to export this variable before
> starting Gecko.

We shouldn't have to fix the debug scripts for basic debugging.

Comment 6

[Naveed Ihsanullah [:naveed]]

Is this happening because (buffer_ < end_) is no longer true? If they are encountering this in delay debugging we should address this for them. That would be really annoying to hit all the time. What can we do to help them out?

Comment 7

[Nicolas B. Pierron [:nbp]]

(In reply to Naveed Ihsanullah [:naveed] from comment #6)
> Is this happening because (buffer_ < end_) is no longer true? If they are
> encountering this in delay debugging we should address this for them. That
> would be really annoying to hit all the time. What can we do to help them
> out?

No this is happening because the memory is marked as non-readable in addition to be marked as non-executable.  I fixed it once [1], but the patch got reverted because of issues on MacOSX.  Maybe we should redo the modification I made, but only for Gonk, which would remove this false-positive signature.

Still this does not fix the fact that gdb catch segv before the segfault handler.

[1] Bug 970643 comment 18

Comment 8

[Luke Wagner [:luke]]

I had an idea on this: the whole point of mprotecting the code is to halt the executing thread so we can do tricky stuff (either move the pc for asm.js or flip the backedges for ion).  What if, instead, we just halted the thread directly:
 - SuspendThread + GetThreadContext on Win32
 - pthread_kill (passing a signal that doesn't freak out gdb) on Unix

I vaguely remember considering this before; I forget whether there were other challenges or whether it just seemed simpler to reuse the existing signal handling mechanism that was set up for out-of-bounds handling.

Comment 9

[Jon Coppeard (:jonco)]

(In reply to Luke Wagner [:luke] from comment #8)
>  - SuspendThread + GetThreadContext on Win32

I seem to remember that this interacts badly with structured exception handling on Windows.  Specifically, if the thread is handling an exception then changes to the PC via the context are sometimes ignored.  If we removed all uses of exceptions in normal operation then I don't think this would be a problem though.

Comment 10

[Luke Wagner [:luke]]

Ah, I should have given more detail: at least for Ion, we'd just use SuspendThread/pthread_kill to halt the thread while we flipped the backedges.

For asm.js, I was thinking we could check whether the pc was in jit code and, only if it was, redirect the pc.  However, that assumes that every path into and out of C++ code checks the interrupt flag which isn't true for the math builtins, so maybe we'd keep the mprotect for asm.js.

Comment 11

[Luke Wagner [:luke]]

Is this fixed now that bug 1091912 has landed?

Comment 12

[Wayne Mery (:wsmwk)]

(In reply to Luke Wagner [:luke] from comment #11)
> Is this fixed now that bug 1091912 has landed?

Comment 13

[Botond Ballo [:botond]]

I haven't seen this in a while. Will reopen if I see it again.