Closed Bug 879647 Opened 7 years ago Closed 6 years ago

Assertion failure: label->used(), at jit/arm/CodeGenerator-arm.cpp

Categories

(Core :: JavaScript Engine: JIT, defect, critical)

ARM
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla27
Tracking Status
firefox26 --- fixed
firefox27 + fixed
b2g-v1.2 --- fixed

People

(Reporter: gkw, Assigned: mjrosenb)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [fuzzblocker] [jsbugmon:][qa-])

Attachments

(3 files, 1 obsolete file)

Attached file stack (obsolete) —
(function () {
    (! (o == "" && typeof b == ""))
    s = [, 0 / 0, 1 / 0, 1 / 0, , , 80001, -0, -0, , 0xffffffff, 0x100000000, 0x100000001]
    return {
        c: s,
        r: o
    }
})()

asserts js debug shell on m-c changeset 7e3a4ebcf067 with --ion-eager at Assertion failure: label->used() && !label->bound(), at ion/arm/CodeGenerator-arm.cpp

This seems different from bug 829534, the regression changeset shown below is a lot later than that one.

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/63b006573c65
user:        Brian Hackett
date:        Wed May 22 11:36:29 2013 -0600
summary:     Bug 870052 - Various tweaks to reduce recompilation on asm.js style apps, r=jandem.
Flags: needinfo?(bhackett1024)
Yep, this looks like what I'm seeing too. I'll come up with a signature.
Whiteboard: [fuzzblocker] [jsbugmon:update] → [fuzzblocker] [jsbugmon:]
JSBugMon: Cannot process bug: Error: Unsupported architecture "ARM" required by bug
It's hard to tell what's going on here, but bug 870052 doesn't have anything to do with it and if anything exposed an existing bug in the assembler.
Flags: needinfo?(bhackett1024)
Setting needinfo from Marty based on comment 4.
Flags: needinfo?(mrosenberg)
... and setting needinfo from myself to retest since bug 879701 has landed, likely modifying this assert.
Flags: needinfo?(gary)
This assert turned into:

Assertion failure: label->used(), at jit/arm/CodeGenerator-arm.cpp

Tested on 32-bit debug Ubuntu Linux m-c rev d8a62355ea26 on ARM.
Flags: needinfo?(gary)
Summary: Assertion failure: label->used() && !label->bound(), at ion/arm/CodeGenerator-arm.cpp → Assertion failure: label->used(), at jit/arm/CodeGenerator-arm.cpp
Attached file Updated stack
Attachment #758397 - Attachment is obsolete: true
So looking into this, the issue appears to be:
(gdb) p lir->mir()->resultTypeSet()->empty()
$1 = true

which leads to us never generating a branch to the fail path, so the label throws an assertion due to not having anything branching to it.  I'm looking into why the result typeset is empty.
Flags: needinfo?(mrosenberg)
Assignee: general → mrosenberg
Component: JavaScript Engine → JavaScript Engine: JIT
previous assessment is wrong.  It turns out this landed in a tricky situation that the assembler buffer can't handle.  (there is code for handling it, but it is subtly wrong, so I decided to just abort ION in this case.)  anyhow, nobody told the bind code that compilation has been aborted, so it throws an assertion.
This patch just has us bail out sooner, so we don't get to the assertion case.
Attachment #815959 - Flags: review?(Jacob.Bramley)
(In reply to Marty Rosenberg [:mjrosenb] from comment #10)
> Created attachment 815959 [details] [diff] [review]
> fixAssertion-r0.patch
> 
> (there is code for handling it, but it is subtly wrong, so I decided to just
> abort ION in this case.)

Can it be fixed? Is there a TODO or bug in place?
Comment on attachment 815959 [details] [diff] [review]
fixAssertion-r0.patch

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

If the assembler buffer gets fixed, this bailout presumably isn't needed, but it will probably get forgotten. Could you add a TODO explaining that it should be removed at some point?
Attachment #815959 - Flags: review?(Jacob.Bramley) → review+
Is this ready for landing soon?
Flags: needinfo?(mrosenberg)
I'd like this to be landed before the upcoming merge window, please. This assertion is also one of the asserts blocking Android 2.2 debug tests, see bug 866795 comment 15.
https://hg.mozilla.org/mozilla-central/rev/cacd16757698
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Whiteboard: [fuzzblocker] [jsbugmon:] → [fuzzblocker] [jsbugmon:][qa-]
Is there any chance this can be nominated for backport? (this bug appears pretty frequently on ARM fuzzing prior to the fix)
Flags: needinfo?(mrosenberg)
Comment on attachment 815959 [details] [diff] [review]
fixAssertion-r0.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): IM landing
User impact if declined: assertions in debug builds, annoyance for fuzzers
Testing completed (on m-c, etc.): it has been on m-c for a while
Risk to taking this patch (and alternatives if risky): corner cases and OOM are handled slightly differently now.
String or IDL/UUID changes made by this patch:
Attachment #815959 - Flags: approval-mozilla-aurora?
Comment on attachment 815959 [details] [diff] [review]
fixAssertion-r0.patch

[
Attachment #815959 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
(In reply to bhavana bajaj [:bajaj] from comment #19)
> Comment on attachment 815959 [details] [diff] [review]
> fixAssertion-r0.patch
> 
> [

whoops hit save too fast. Switched the nom from aurora to beta given Firefox27 is already fixed here.
Attachment #815959 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.