Closed Bug 909586 Opened 7 years ago Closed 6 years ago

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

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla28
Tracking Status
firefox27 --- wontfix
firefox28 --- fixed
firefox-esr24 --- wontfix
b2g18 --- wontfix
b2g-v1.1hd --- wontfix
b2g-v1.2 --- wontfix
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- unaffected

People

(Reporter: gkw, Assigned: djvj)

References

Details

(4 keywords, Whiteboard: [jsbugmon:][adv-main28+])

Attachments

(3 files, 3 obsolete files)

Attached file 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.
Flags: needinfo?(kvijayan)
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
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?
Blocks: 839335
Flags: needinfo?(bhackett1024)
(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.
Flags: needinfo?(bhackett1024)
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.
Assignee: general → kvijayan
Flags: needinfo?(kvijayan)
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?
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>
Kannan, any luck with the comment 6 compile options?
Flags: needinfo?(kvijayan)
Haven't tried it yet.  Needed to update my OSX build env and I was busy with other work.  Looking at it now.
Flags: needinfo?(kvijayan)
So I was able to reproduce this on my mac with the given configuration parameters.  Thanks gary!  Trying to work on it now.
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.
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.
sec-moderate because it is profiler-only.
Keywords: sec-moderate
Attachment #810524 - Attachment is obsolete: true
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.
Kannan, this *may* block finding other SPS bugs. Perhaps it fell off the radar?
Flags: needinfo?(kvijayan)
Attached patch bug-909586.patch (obsolete) — Splinter Review
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.
Flags: needinfo?(kvijayan)
Comment on attachment 8341393 [details] [diff] [review]
bug-909586.patch

Yes, this fixes the assert.
Attachment #8341393 - Flags: feedback+
Attached patch bug-909586.patch (obsolete) — Splinter Review
Updated version of previous patch, with a big fat comment explaining the change.
Attachment #8341393 - Attachment is obsolete: true
Attachment #8341408 - Flags: review?(bhackett1024)
Attachment #8341408 - Flags: review?(bhackett1024) → review+
Attached patch bug-909586.patchSplinter Review
Just fixed up comment grammar from r+-ed patch.
Attachment #8341408 - Attachment is obsolete: true
Attachment #8341423 - Flags: checkin?
Keywords: checkin-needed
Comment on attachment 8341423 [details] [diff] [review]
bug-909586.patch

Forwarding r+ from obsoleted patch.
Attachment #8341423 - Flags: review+
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.
Attachment #8341423 - Flags: checkin? → checkin+
https://hg.mozilla.org/mozilla-central/rev/de5cfdf73a3c

Not sure what's affected.
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
How far back does this issue go? Seems like all currently supported branches.
Whiteboard: [jsbugmon:] → [jsbugmon:][adv-main28+]
Group: core-security
You need to log in before you can comment on or make changes to this bug.