Profiler debug assertion: idx >= 0 && uint32_t(idx) < script()->length, at SPSProfiler.cpp:267

RESOLVED FIXED in mozilla26

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jld, Assigned: jld)

Tracking

Trunk
mozilla26
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
I can reliably reproduce this by starting a debug build with profiling enabled.  The failure is:

F/MOZ_Assert(17795): Assertion failure: idx >= 0 && uint32_t(idx) < script()->length, at /home/jld/src/B2G/gecko/js/src/vm/SPSProfiler.cpp:267

Apparently caused by (in a profiler sample):

(gdb) p aStack->mStack[i].idx
$12 = 1024
(gdb) p aStack->mStack[i].script_->length
$15 = 169

If I understand correctly, this has to be getting written in by jitcode, because the only path to write it from C++ has an equivalent assertion.  We presumably need more assertions to track down the bad index at its source.  (This might be ARM-specific, though I haven't tried it with an x86 build yet, nor do I know if our regular testing would cover this on other platforms.)

In addition to making the Gecko profiler unusable for investigating debug build overhead, which is something that may need to happen on b2g, this also makes it somewhat difficult to do development on the profiling infrastructure.
This error can be worked around by disabling the 'js' feature. Of course this isn't something we can do long term. We need someone from js to look at this.
Summary: Profiler causes assertion failure on B2G debug builds → Profiler debug assertion: idx >= 0 && uint32_t(idx) < script()->length, at SPSProfiler.cpp:267

Updated

4 years ago
Assignee: nobody → general
Component: Gecko Profiler → JavaScript Engine

Updated

4 years ago
Duplicate of this bug: 897716
I also saw this, on x86_64-linux.
(In reply to Jed Davis [:jld] from comment #0)
> I can reliably reproduce this by starting a debug build with profiling
> enabled.

Does this happen with the js shell or do we need to build the whole browser? Doesn't seem to reproduce on x86_64-linux with the js shell only (built with --enable-profiling).
Flags: needinfo?(jld)
(Assignee)

Comment 5

4 years ago
I tried running a simple program in the JS shell, on b2g, on a debug+profiling build, and it didn't fail.  But, doesn't the jitcode profiling instrumentation need a dynamic flag to be set, as well as --enable-profiling?

Also, if it helps, I've pastebinned the stack of a failure: http://pastebin.mozilla.org/2891906 .  These are the lines with the bad bytecode index (accompanied by their indices within the stack array):

10:  {{string = 0x474d7d00 "csp_toString (resource://gre/modules/CSPUtils.jsm:765)", sp = 0x0, script_ = 0x44a4ae00, idx = 1024}},
11:  {{string = 0x4715cba0 "CSPSourceList.prototype.toString (resource://gre/modules/CSPUtils.jsm:1134)", sp = 0x0, script_ = 0x44a4f400, idx = 1024}},
15:  {{string = 0x4715cc40 "CSPSource.prototype.toString (resource://gre/modules/CSPUtils.jsm:1593)", sp = 0x0, script_ = 0x44a4fc00, idx = 1024}},
16:  {{string = 0x4715cc40 "CSPSource.prototype.toString (resource://gre/modules/CSPUtils.jsm:1593)", sp = 0x0, script_ = 0x44a4fc00, idx = 1024}},
Flags: needinfo?(jld)
(Assignee)

Comment 6

4 years ago
Created attachment 793734 [details] [diff] [review]
bug890143-workaround.diff

This patch prevents the assertion failure for me, by not setting the pseudo-stack index from a jitcode register if it's equal to 1024.  I therefore conclude that the bad values are coming in through MacroAssembler::spsUpdatePCIdx(SPSProfiler*, Register, Register), which seems to be used only in BaselineIC.cpp to copy values from the pcOffset_ fields of the various kinds of baseline frames.
(Assignee)

Comment 7

4 years ago
I think I know what's going on here: the register holding the bytecode index is the same as the ScratchRegister that spsProfileEntryAddressSafe clobbers when it does branch32 with an AbsoluteAddress.

So, the mysterious 1024 is the number of elements in the pseudostack.

It makes sense that this would also affect x86_64 (but not 32-bit x86), because that architecture also can't do arithmetic with a memory operand at an arbitrary absolute address without using a temporary.
Assignee: general → jld
(Assignee)

Comment 8

4 years ago
Created attachment 793796 [details] [diff] [review]
BaselineSecondScratchReg is not available in Baseline mode.

This should take care of things for ARM.  It looks like profiling is only relevant in that nothing in BaselineIC happens to get r6 and use it across a conflicting operation otherwise (or maybe it does but we don't notice, which is a scary thought).

I don't have an answer for x86_64 yet.
Attachment #793734 - Attachment is obsolete: true
Attachment #793796 - Flags: review?(mrosenberg)
(Assignee)

Comment 9

4 years ago
(In reply to Jed Davis [:jld] from comment #8)
> I don't have an answer for x86_64 yet.

I've reproduced a crash on x86_64 Linux, but it's a different assertion: in SPSInstrumentation::leave, computing the offset for the immediate version of spsUpdatePCIdx, getting a bytecode pointer that's obviously out of range for the script it's supposed to be inside.  I'll file a separate bug.

I'm currently thinking that my previous guess was wrong, and this bug is in fact ARM-backend-specific.
(Assignee)

Updated

4 years ago
Attachment #793796 - Flags: review?(mrosenberg) → review?(kvijayan)
Comment on attachment 793796 [details] [diff] [review]
BaselineSecondScratchReg is not available in Baseline mode.

Review of attachment 793796 [details] [diff] [review]:
-----------------------------------------------------------------

Nice find.
Attachment #793796 - Flags: review?(kvijayan) → review+
(Assignee)

Comment 11

4 years ago
Also, for whatever it's worth at this point, I did some try builds last week:
https://tbpl.mozilla.org/?tree=Try&rev=2eb3fba9017a
https://tbpl.mozilla.org/?tree=Try&rev=9783bb53b9d2

(Built on all platforms, ran tests on ARM.)
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/cce567042b93
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/cce567042b93
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.