Closed Bug 1217061 Opened 4 years ago Closed 4 years ago

ARM Sim: Undetected OOM in gc/oomInArrayProtoTest.js

Categories

(Core :: JavaScript Engine: JIT, defect)

ARM
Unspecified
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: jolesen, Assigned: jolesen)

References

Details

Attachments

(1 file)

Running the jit-test suite under the arm simulator exposes an undetected OOM error:

Assertion failure: cx->isExceptionPending(), at /Users/jolesen/gecko-dev/js/src/builtin/TestingFunctions.cpp:1161
Exit code: -11
FAIL - gc/oomInArrayProtoTest.js

This failure is intermittent, but seems to be more likely when applying the patches in bug 1207827, preventing these patches from landing.

:jonco says:
- Is there a trick to finding the stack that lost an OOM when getting the "cx->isExceptionPending(), at js/src/builtin/TestingFunctions.cpp:1161" assertion?
- Yes, set OOM_VERBOSE in the environment to see the last allocation that we failed.
  Then set a breakpoint on js_failedAllocBreakpoint and continue the appropriate number of times.
  You will need to configure with --enable-oom-breakpoint.
Reproducing on OS X:

Compile SM with --enable-simulator=arm --enable-om-breakpoint.
The assertion happens after 807 oom failures:

OOM_VERBOSE=1 lldb -- dist/bin/js ../js/src/jit-test/tests/gc/oomInArrayProtoTest.js
(lldb) break set -i 806 -n js_failedAllocBreakpoint
Breakpoint 1: 45 locations.
(lldb) r
Process 1536 launched: '/Users/jolesen/gecko-dev/obj-a32simdev/dist/bin/js' (i386)
thread 1
  allocation 1
  allocation 2
  allocation 3
...
  allocation 806
  allocation 807
Process 1536 stopped
* thread #1: tid = 0xacc341, 0x0099ae90 js`js_failedAllocBreakpoint() at Utility.h:116, queue = 'com.apple.main-thread', stop reason = breakpoint 1.31
    frame #0: 0x0099ae90 js`js_failedAllocBreakpoint() at Utility.h:116
   113 	extern JS_PUBLIC_DATA(bool) OOM_failAlways;
   114 	
   115 	#ifdef JS_OOM_BREAKPOINT
-> 116 	static MOZ_NEVER_INLINE void js_failedAllocBreakpoint() { asm(""); }
   117 	#define JS_OOM_CALL_BP_FUNC() js_failedAllocBreakpoint()
   118 	#else
   119 	#define JS_OOM_CALL_BP_FUNC() do {} while(0)
(lldb) bt


* thread #1: tid = 0xacc341, 0x0099ae90 js`js_failedAllocBreakpoint() at Utility.h:116, queue = 'com.apple.main-thread', stop reason = breakpoint 1.31
  * frame #0: 0x0099ae90 js`js_failedAllocBreakpoint() at Utility.h:116
    frame #1: 0x0099ad7f js`js::oom::ShouldFailWithOOM() + 79 at Utility.h:148
    frame #2: 0x009adc0d js`js::LifoAlloc::alloc(this=0x052695d0, n=48) + 29 at LifoAlloc.h:274
    frame #3: 0x003c5b17 js`js::jit::JitcodeGlobalEntry* js::LifoAlloc::new_<js::jit::JitcodeGlobalEntry>(this=0x052695d0) + 39 at LifoAlloc.h:413
    frame #4: 0x003b7868 js`js::jit::JitcodeGlobalTable::allocateEntry(this=0x052695d0) + 72 at JitcodeMap.cpp:711
    frame #5: 0x003b729f js`js::jit::JitcodeGlobalTable::addEntry(this=0x052695d0, entry=0xbfff97c8, rt=0x03d36000) + 399 at JitcodeMap.cpp:535
    frame #6: 0x00361a0a js`js::jit::JitcodeGlobalTable::addEntry(this=0x052695d0, entry=0xbfff98b0, rt=0x03d36000) + 74 at JitcodeMap.h:1049
    frame #7: 0x00321b3a js`js::jit::IonCache::linkAndAttachStub(this=0x03d6e0c0, cx=0x03d8f020, masm=0xbfff9a38, attacher=0xbfff9a00, ion=0x03d6e000, attachKind="dense array", trackedOutcome=ICGetElemStub_Dense) + 1370 at IonCaches.cpp:356
    frame #8: 0x0032e9ba js`js::jit::GetPropertyIC::tryAttachDenseElement(this=0x03d6e0c0, cx=0x03d8f020, outerScript=JS::HandleScript @ 0xbfffa108, ion=0x03d6e000, obj=JS::HandleObject @ 0xbfffa110, idval=JS::HandleValue @ 0xbfffa114, emitted=0xbfffa41f) + 1018 at IonCaches.cpp:3664
    frame #9: 0x0032d00b js`js::jit::GetPropertyIC::tryAttachStub(this=0x03d6e0c0, cx=0x03d8f020, outerScript=JS::HandleScript @ 0xbfffa328, ion=0x03d6e000, obj=JS::HandleObject @ 0xbfffa330, idval=JS::HandleValue @ 0xbfffa334, emitted=0xbfffa41f) + 2363 at IonCaches.cpp:2085
    frame #10: 0x0032f485 js`js::jit::GetPropertyIC::update(cx=0x03d8f020, outerScript=JS::HandleScript @ 0xbfffa464, cacheIndex=0, obj=JS::HandleObject @ 0xbfffa46c, idval=JS::HandleValue @ 0xbfffa470, vp=JS::MutableHandleValue @ 0xbfffa474) + 341 at IonCaches.cpp:2122
    frame #11: 0x005f0c44 js`js::jit::Simulator::softwareInterrupt(this=0x03d8e000, instr=0x03d26c64) + 2148 at Simulator-arm.cpp:2361
    frame #12: 0x005f6fd0 js`js::jit::Simulator::decodeType7(this=0x03d8e000, instr=0x03d26c64) + 80 at Simulator-arm.cpp:3483
    frame #13: 0x005ed116 js`js::jit::Simulator::instructionDecode(this=0x03d8e000, instr=0x03d26c64) + 390 at Simulator-arm.cpp:4405
    frame #14: 0x006152f5 js`void js::jit::Simulator::execute<false>(this=0x03d8e000) + 165 at Simulator-arm.cpp:4460
???t?\x9d?) + 1266 at Simulator-arm.cpp:4548tor::callInternal(this=0x03d8e000, entry="?O-?\x04?M??-?
???t?\x9d?, argument_count=8) + 583 at Simulator-arm.cpp:463103d8e000, entry="?O-?\x04?M??-?
 
It looks like lldb gets confused about the stack frames around here. Single-stepping out of GetPropertyIC::update() takes us to Simulator::softwareInterrupt():

          case Args_General6: {
            Prototype_General6 target = reinterpret_cast<Prototype_General6>(external);
            int64_t result = target(arg0, arg1, arg2, arg3, arg4, arg5);
            scratchVolatileRegisters(/* scratchFloat = true */);
            setCallResult(result);
            break;
          }

Note that Prototype_General6 is a function returning int64_t, but the callee GetPropertyIC::update() returns a bool. A function returning a bool will return garbage in %edx which holds the high 32 bits of the int64_t result:

(lldb) reg read eax edx
     eax = 0x00000000
     edx = 0xbfff9a00 
(lldb) p/x result
(int64_t) $2 = 0xbfff9a0000000000

This is then handed off to:

Simulator::setCallResult(int64_t res)
{
    set_register(r0, static_cast<int32_t>(res));
    set_register(r1, static_cast<int32_t>(res >> 32));
}

This means we get garbage in the simulated 'r1' which may or may not be a problem. It's probably fine.
It's a kCallRtRedirected(=0x10) SWI:

    0x3e26c60: 0x0032f330   ; nativeFunction_ = GetPropertyIC::update
    0x3e26c64: 0xef000010   svc    #0x10
    0x3e26c68: 0x00001555   ; type_ = Args_General6
    0x3e26c6c: 0x03e26c50   ; next

Here's the code that called the native function:

(lldb) disas -b -arch armv7 -c 20 -s 'registers_[15]-12'
    0x2ec2c9c: 0xe306cc64   movw   r12, #0x6c64
    0x2ec2ca0: 0xe340c3e2   movt   r12, #0x3e2  ; r12 = 0x3e26c64: 0xef000010   svc    #0x10
    0x2ec2ca4: 0xe12fff3c   blx    r12
    0x2ec2ca8: 0xe28dd00c   add    sp, sp, #12
    0x2ec2cac: 0xe59dd000   ldr    sp, [sp]
    0x2ec2cb0: 0xe31000ff   tst    r0, #255     ; Any low bits in return value?
    0x2ec2cb4: 0x0a000003   beq    0x2ec2cc8    ; false_return
    0x2ec2cb8: 0xe49d2004   ldr    r2, [sp], #4
    0x2ec2cbc: 0xe49d3004   ldr    r3, [sp], #4
    0x2ec2cc0: 0xe28dd008   add    sp, sp, #8
    0x2ec2cc4: 0xe49df01c   ldr    pc, [sp], #28
false_return:
    0x2ec2cc8: 0xe30dc270   movw   r12, #0xd270
    0x2ec2ccc: 0xe340c2eb   movt   r12, #0x2eb
    0x2ec2cd0: 0xe12fff1c   bx     r12
 
If the low 8 bits of the GetPropertyIC::update() return value are clear, the code tail calls to: 0x2ebd270:

    0x2ebd270: 0xe24dd020   sub    sp, sp, #32
    0x2ebd274: 0xe1a0000d   mov    r0, sp
    0x2ebd278: 0xe1a0100d   mov    r1, sp
    0x2ebd27c: 0xe3cdd007   bic    sp, sp, #7       ; Align stack to 8 bytes.
    0x2ebd280: 0xe52d1004   str    r1, [sp, #-0x4]!
    0x2ebd284: 0xe24dd004   sub    sp, sp, #4       ; Now 8 bytes + 4 unaligned
    0x2ebd288: 0xe31d0007   tst    sp, #7           ; Test for stack alignment?!
    0x2ebd28c: 0x0a000000   beq    0x2ebd294        ; Never taken.
    0x2ebd290: 0xe1200078   bkpt   #0x8
    0x2ebd294: 0xe306c4e4   movw   r12, #0x64e4
    0x2ebd298: 0xe340c3e2   movt   r12, #0x3e2
    0x2ebd29c: 0xe12fff3c   blx    r12
    0x2ebd2a0: 0xe28dd004   add    sp, sp, #4
    0x2ebd2a4: 0xe59dd000   ldr    sp, [sp]
    0x2ebd2a8: 0xe59d000c   ldr    r0, [sp, #0xc]
    0x2ebd2ac: 0xe3500000   cmp    r0, #0
    0x2ebd2b0: 0x0a000008   beq    0x2ebd2d8
    0x2ebd2b4: 0xe3500001   cmp    r0, #1
    0x2ebd2b8: 0x0a00000a   beq    0x2ebd2e8
    0x2ebd2bc: 0xe3500002   cmp    r0, #2
    0x2ebd2c0: 0x0a00000c   beq    0x2ebd2f8
    0x2ebd2c4: 0xe3500003   cmp    r0, #3
    0x2ebd2c8: 0x0a000016   beq    0x2ebd328
    0x2ebd2cc: 0xe3500004   cmp    r0, #4
    0x2ebd2d0: 0x0a000022   beq    0x2ebd360
    0x2ebd2d4: 0xe1200079   bkpt   #0x9
    0x2ebd2d8: 0xe3e0307b   mvn    r3, #123
    0x2ebd2dc: 0xe3a0200d   mov    r2, #13
    0x2ebd2e0: 0xe59dd004   ldr    sp, [sp, #0x4]
    0x2ebd2e4: 0xe49df004   ldr    pc, [sp], #4
    0x2ebd2e8: 0xe59d0008   ldr    r0, [sp, #0x8]
    0x2ebd2ec: 0xe59db000   ldr    r11, [sp]
    0x2ebd2f0: 0xe59dd004   ldr    sp, [sp, #0x4]
    0x2ebd2f4: 0xe12fff10   bx     r0
The code at 0x2ebd270 was generated by MacroAssemblerARMCompat::handleFailureWithHandlerTail(HandleException).

It does not take the breakpoint at 0x2ebd290. I can't read code.

HandleException(rfe) returned with rfe->kind == RESUME_ENTRY_FRAME, and cx->throwing is false. No OOM exception has been logged.

This fixes this particular problem:

diff --git a/js/src/jit/IonCaches.cpp b/js/src/jit/IonCaches.cpp
index b5235a8..a5a29af 100644
--- a/js/src/jit/IonCaches.cpp
+++ b/js/src/jit/IonCaches.cpp
@@ -355,6 +355,7 @@ IonCache::linkAndAttachStub(JSContext* cx, MacroAssembler& masm, StubAttacher& a
         JitcodeGlobalTable* globalTable = cx->runtime()->jitRuntime()->getJitcodeGlobalTable();
         if (!globalTable->addEntry(entry, cx->runtime())) {
             entry.destroy();
+            ReportOutOfMemory(cx);
             return false;
         }
 
It looks like IonCaches.cpp never reports OOM errors.
The method IonCache::linkAndAttachStub() can run out of memory in a few
ways: When adding entries to the global jit code table, and when
generating code through the macro assembler.

Make sure to call ReportOutOfMemory() before returning false when that
happens. Otherwise we won't get the right of error reported, and OOM
simulation tests fail.

The Linker already calls ReportOutOfMemory(), so we don't need to
handle those calls.
Attachment #8677675 - Flags: review?(kvijayan)
Attachment #8677675 - Flags: review?(kvijayan) → review+
https://hg.mozilla.org/mozilla-central/rev/08493e40a4ad
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.