909586 - Assertion failure: frame->script->code <= pc && pc < frame->script->code + frame->script->length, at vm/SPSProfiler.h

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

Attachment: stack

function f(x) {
    this["__iterator__"] = x;
    for (var m in this)
    return this;
}
for each(let z in this) {
    try {
        f(z);
    } catch (e) {}
}

asserts js debug (64-bit threadsafe deterministic) shell on m-c changeset 17143a9a0d83 with --ion-eager at Assertion failure: frame->script->code <= pc && pc < frame->script->code + frame->script->length, at vm/SPSProfiler.h

Setting s-s because Kannan has mentioned that SPS bugs may be bad.

Comment 1

[Christian Holler (:decoder)]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 2

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

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/73861b907300
user:        Brian Hackett
date:        Fri Feb 08 06:50:54 2013 -0700
summary:     Bug 839335 - Baseline compile assorted stubbed opcodes, r=jandem.

(Not sure how correct this is)

Brian, is bug 839335 a possible regressor?

Comment 3

[Brian Hackett [Laid off!]]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #2)
> Brian, is bug 839335 a possible regressor?

No, this bug was just allowing us to baseline compile more scripts.  This is probably some bug in how SPS works with baseline and/or ion.

Comment 4

[Kannan Vijayan [:djvj]]

I'm not taking this bug immediately because I have a full plate, but I'm clearing the needinfo and I'll pick it up (if it hasn't already) when I'm able to clear my plate.

Comment 5

[Kannan Vijayan [:djvj]]

I cannot reproduce this bug.  On same revision, with deterministic, threadsafe build, with profiling enabled.

Gary, can you tell me if this still reproduces for you?  Or can I get a copy of the build directory to play with locally?

Comment 6

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

This definitely reproduces for me on rev 52218c11ec89 with the following compile options:

CC="clang -Qunused-arguments" AR=ar CXX="clang++ -Qunused-arguments" sh ./configure --target=x86_64-apple-darwin11.4.0 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <more NSPR compile options>

Comment 7

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 8

[David Bolter [:davidb] (NeedInfo me for attention)]

Kannan, any luck with the comment 6 compile options?

Comment 9

[Kannan Vijayan [:djvj]]

Haven't tried it yet.  Needed to update my OSX build env and I was busy with other work.  Looking at it now.

Comment 10

[Kannan Vijayan [:djvj]]

So I was able to reproduce this on my mac with the given configuration parameters.  Thanks gary!  Trying to work on it now.

Comment 11

[Kannan Vijayan [:djvj]]

This reproduces with a pure debug build as well.  Here is a more appropriate stack trace:

(gdb) bt
#0  js::SPSInstrumentation<js::jit::MacroAssembler, js::jit::Register>::leave (
    this=0x10512d198, pc=0x10305c5ff "R", masm=@0x10512c850, scratch=
      {code_ = JSC::X86Registers::eax}) at SPSProfiler.h:356
#1  0x0000000100568f53 in js::jit::IonInstrumentation::leave (
    this=0x10512d198, masm=@0x10512c850, reg={code_ = JSC::X86Registers::eax})
    at IonInstrumentation.h:33
#2  0x0000000100568107 in js::jit::MacroAssembler::leaveSPSFrame (
    this=0x10512c850) at IonMacroAssembler.h:820
#3  0x00000001005657e3 in js::jit::MacroAssembler::callWithABI<void*> (
    this=0x10512c850, fun=@0x7fff5fbf71f8,
    result=js::jit::MacroAssemblerX64::GENERAL) at IonMacroAssembler.h:758
#4  0x0000000100641b97 in js::jit::CodeGeneratorShared::verifyOsiPointRegs (
    this=0x10512c800, safepoint=0x105117b48)
    at /Users/kvijayan/Checkouts/mozilla-central/js/src/jit/shared/CodeGenerator-shared.cpp:572
#5  0x000000010064cccf in js::jit::CodeGenerator::visitOsiPoint (
    this=0x10512c800, lir=0x105117c90)
    at /Users/kvijayan/Checkouts/mozilla-central/js/src/jit/CodeGenerator.cpp:827
#6  0x00000001007848e3 in js::jit::LOsiPoint::accept (this=0x105117c90,
    visitor=0x10512c800) at LIR-Common.h:72
#7  0x00000001006546d5 in js::jit::CodeGenerator::generateBody (
    this=0x10512c800)
    at /Users/kvijayan/Checkouts/mozilla-central/js/src/jit/CodeGenerator.cpp:2755
#8  0x00000001006623a3 in js::jit::CodeGenerator::generate (this=0x10512c800)
    at /Users/kvijayan/Checkouts/mozilla-central/js/src/jit/CodeGenerator.cpp:5582
#9  0x00000001006a4a4f in js::jit::GenerateCode (mir=0x1050536c8,
    lir=0x105109bb0, maybeMasm=0x0)
    at /Users/kvijayan/Checkouts/mozilla-central/js/src/jit/Ion.cpp:1488
#10 0x00000001006a4b97 in js::jit::CompileBackEnd (mir=0x1050536c8,
    maybeMasm=0x0)
    at /Users/kvijayan/Checkouts/mozilla-central/js/src/jit/Ion.cpp:1507
#11 0x00000001006a9f25 in IonCompile (cx=0x103800770, script=0x103cbedc0,
    baselineFrame=0x7fff5fbf7ae8, osrPc=0x10305c692 "�\001V",
    constructing=false, executionMode=js::jit::SequentialExecution)
    at /Users/kvijayan/Checkouts/mozilla-central/js/src/jit/Ion.cpp:1675
#12 0x00000001006a5a86 in Compile (cx=0x103800770, script=
      {<js::HandleBase<JSScript *>> = {<No data fields>}, ptr = 0x7fff5fbf7850}, osrFrame=0x7fff5fbf7ae8, osrPc=0x10305c692 "�\001V", constructing=false,
    executionMode=js::jit::SequentialExecution)
    at /Users/kvijayan/Checkouts/mozilla-central/js/src/jit/Ion.cpp:1833
#13 0x00000001006a5496 in js::jit::CanEnterAtBranch (cx=0x103800770,
    script=0x103cbedc0, osrFrame=0x7fff5fbf7ae8, pc=0x10305c692 "�\001V",
    isConstructing=false)
    at /Users/kvijayan/Checkouts/mozilla-central/js/src/jit/Ion.cpp:1880
#14 0x00000001005fe01b in EnsureCanEnterIon (cx=0x103800770, stub=0x103f2fc68,
    frame=0x7fff5fbf7ae8, script=
      {<js::HandleBase<JSScript *>> = {<No data fields>}, ptr = 0x7fff5fbf79e8}, pc=0x10305c692 "�\001V", jitcodePtr=0x7fff5fbf79b8)
    at /Users/kvijayan/Checkouts/mozilla-central/js/src/jit/BaselineIC.cpp:732
#15 0x00000001005d0a93 in DoUseCountFallback (cx=0x103800770,
    stub=0x103f2fc68, frame=0x7fff5fbf7ae8, infoPtr=0x7fff5fbf7a78)
    at /Users/kvijayan/Checkouts/mozilla-central/js/src/jit/BaselineIC.cpp:918
#16 0x00000001027aca9e in ?? ()
#17 0x0000000103f2fc68 in ?? ()



Don't know exactly what's happening yet, but the reason the assert is triggering is because the frame script in the SPS instrumentation refers to an inline-compiled script's frame, but the PC being passed in is from the top-level script that the Ion compile was called on.

The relevant scripts are self-hosted.  The inlined function is |List| in selfhosted.js, and the top-level function being compiled is |CanonicalizeLanguageTag| in selfhosted.js.

Comment 12

[Kannan Vijayan [:djvj]]

When slow profiling is turned on, all MFunctionBoundaries have OSI points associated with them, because they can emit a slow call.

The MFunctionBoundary(Inline_Enter) instruction is actually added to the caller's block (not the callee).  So the PC it's associated with
is the caller's PC.

During codeine, when the LFunctionBoundary(Inline_Enter) is processed, the lastPC_ gets updated to point to the PC associated with the MFunctionBoundary, which is a PC in the CALLER's code.  However, processing of the LFunctionBoundary(Inline_Enter) also pushes a new SPS frame.

We subsequently hit the OSI point for the LFunctionBoundary.  It does a callWithABI, which tries to emit an SPS "leave".  However, the lastPC_ on record at this point is actually the caller's lastPC, but the SPS frame which is currently pushed is for the inner function (callee).

This triggers the assert.

Comment 13

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 14

[Andrew McCreight [:mccr8]]

sec-moderate because it is profiler-only.

Comment 15

[Kannan Vijayan [:djvj]]

Ok this is a bit tricky to fix.  Originally I thought it should be as simple as moving the MFunctionBoundary into the callee's block.

However this doesn't work, because then the "leave" action for the _caller_ (entering the callee) gets the callee pc instead of the caller PC, which asserts once again.

Comment 16

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

Kannan, this *may* block finding other SPS bugs. Perhaps it fell off the radar?

Comment 17

[Kannan Vijayan [:djvj]]

Attachment: bug-909586.patch

Hey gary.  Sorry, it did fall off my radar, and I was having a hard time thinking of a way of fixing the codegen so that it would work.  However, my attempts were not that successful.

This attachment is an attempt at a cheap fix.  My build environment on the mac has gotten a bit stale, so I can't test this out directly.. can you try this out for me?  It's a simple fix and just removes profiling for the callWithABI that's causing this.  This is OK, because this callWithABI only happens for debug-build "heavyweight verification" runs, which probably don't need to be profiled.

Comment 18

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

Comment on attachment 8341393 [details] [diff] [review]
bug-909586.patch

Yes, this fixes the assert.

Comment 19

[Kannan Vijayan [:djvj]]

Attachment: bug-909586.patch

Updated version of previous patch, with a big fat comment explaining the change.

Comment 20

[Kannan Vijayan [:djvj]]

Attachment: bug-909586.patch

Just fixed up comment grammar from r+-ed patch.

Comment 21

[Kannan Vijayan [:djvj]]

Comment on attachment 8341423 [details] [diff] [review]
bug-909586.patch

Forwarding r+ from obsoleted patch.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8341423 [details] [diff] [review]
bug-909586.patch

https://hg.mozilla.org/integration/mozilla-inbound/rev/de5cfdf73a3c

Please make sure future patches have commit information included when requesting checkin.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

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

Not sure what's affected.

Comment 24

[Al Billings [:abillings - ex-MoCo]]

How far back does this issue go? Seems like all currently supported branches.