IonMonkey: Crash [@ scopeChain] or [@ js::AbstractFramePtr::evalPrevScopeChain] or Assertion failure: ins->type() == MIRType_Value, at ion/MIR.h or Assertion failure: false (Unexpected state), at vm/Stack.cpp

RESOLVED FIXED in Firefox 23

Status

()

--
critical
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: gkw, Assigned: sstangl)

Tracking

(Blocks: 2 bugs, 4 keywords)

Trunk
mozilla23
assertion, regression, sec-high, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox20 unaffected, firefox21 unaffected, firefox22 unaffected, firefox23+ fixed, firefox-esr17 unaffected, b2g18 unaffected, b2g18-v1.0.0 unaffected, b2g18-v1.0.1 unaffected)

Details

(Whiteboard: [jsbugmon:testComment=9,update,bisectfix][adv-main23-], crash signature)

Attachments

(2 attachments)

Created attachment 734306 [details]
stack

The upcoming testcase asserts js debug shell on m-c changeset 768af8d8fad4 with --ion-eager and --no-jm at Assertion failure: ins->type() == MIRType_Value, at ion/MIR.h

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   127209:ed51ae24fee2
user:        Kannan Vijayan
date:        Wed Jan 30 12:09:10 2013 -0500
summary:     Bug 805877 - Bailout from Ion to Baseline. r=jandem

Locking s-s because similar bug 858617 is also marked s-s. I have tested the patch in that bug does not fix this issue.
Hardware: x86_64 → x86
Summary: BaselineCompiler: Assertion failure: ins->type() == MIRType_Value, at ion/MIR.h → BaselineCompiler: Assertion failure: ins->type() == MIRType_Value, at ion/MIR.h or Assertion failure: false (Unexpected state), at vm/Stack.cpp
Whiteboard: [jsbugmon:update,testComment=1]
fwiw, the assertion(s) go(es) away with --no-baseline.
Turns out the regressor in comment 0 was for comment 2, for comment 0, autoBisect pointed to another possible regressor but I'm not sure which is correct:

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   127248:d0639bd4b896
user:        Brian Hackett
date:        Thu Feb 07 12:10:50 2013 -0700
summary:     Bug 838820 - Compile NAME and ALIASEDVAR related opcodes, r=djvj.
Flags: needinfo?(kvijayan)
Whiteboard: [jsbugmon:update,testComment=1] → [jsbugmon:testComment=1]
JSBugMon: Cannot process bug: Error: Failed to isolate original revision for test
JSBugMon should automatically pick up the attachments, but testComment must not point to a comment without HG revision (except origRev is used to specify one).
Whiteboard: [jsbugmon:testComment=1] → [jsbugmon:update]
I think that neither of these bugs are directly related to the issue.  Somehow the |thisDefn| of the CallInfo struct is becoming NULL during the IonBuilder's handling of JSOP_CALL.  This should not happen, and it's highly unlinkely that its occurrence is happening because of baseline issues.

More likely, the two bugs are falsely identified as regressors because:
1. The bailout patch changes how Ion bails out, and thus changes when Ion is re-entered.  Unapplying the patch is just hiding the issue, not removing it.
2. The NAME and ALIASEDVAR patch allows some scripts to be baseline compiled which would otherwise not have been.  Unapplying it probably just disables the scripts exposing the bug in ion from not running.

My gut says to mark this as an Ion bug and debug it as such.
Flags: needinfo?(kvijayan)
Marking as an IonMonkey bug based on comment 7.
Summary: BaselineCompiler: Assertion failure: ins->type() == MIRType_Value, at ion/MIR.h or Assertion failure: false (Unexpected state), at vm/Stack.cpp → IonMonkey: Assertion failure: ins->type() == MIRType_Value, at ion/MIR.h or Assertion failure: false (Unexpected state), at vm/Stack.cpp
Created attachment 736503 [details]
debug and opt stacks

x = Set
function f() {
    eval("isPrototypeOf.call(Set, x)")
}
y = [0, 0]
Array.prototype.some.call(y, f)
Set = 0[0]
Array.prototype.sort.call(y, f)

crashes js opt shell at scopeChain with js::AbstractFramePtr::evalPrevScopeChain on the stack and asserts js debug shell at Assertion failure: false (Unexpected state), at vm/Stack.cpp

Tested on 64-bit opt deterministic threadsafe shell with --ion-eager --no-jm --no-baseline, on m-c changeset 9d5f05a6d497.
Crash Signature: [@ scopeChain] [@ js::AbstractFramePtr::evalPrevScopeChain]
Summary: IonMonkey: Assertion failure: ins->type() == MIRType_Value, at ion/MIR.h or Assertion failure: false (Unexpected state), at vm/Stack.cpp → IonMonkey: Crash [@ scopeChain] or [@ js::AbstractFramePtr::evalPrevScopeChain] or Assertion failure: ins->type() == MIRType_Value, at ion/MIR.h or Assertion failure: false (Unexpected state), at vm/Stack.cpp
OS: Mac OS X → All
Hardware: x86 → All
I f2f discussed this with Sean, and he mentioned he might be able to take a look.
Flags: needinfo?(sstangl)
Assignee: general → sstangl
Keywords: sec-high
If we're sure this is only on FF23, then tracking to ensure we get a backout or forward fix on this before ship.
tracking-firefox23: ? → +
Crash Signature: [@ scopeChain] [@ js::AbstractFramePtr::evalPrevScopeChain] → [@ scopeChain] [@ js::AbstractFramePtr::evalPrevScopeChain]
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 8b1a7228674a).
Crash Signature: [@ scopeChain] [@ js::AbstractFramePtr::evalPrevScopeChain] → [@ scopeChain] [@ js::AbstractFramePtr::evalPrevScopeChain]
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:update,testComment=9]
Crash Signature: [@ scopeChain] [@ js::AbstractFramePtr::evalPrevScopeChain] → [@ scopeChain] [@ js::AbstractFramePtr::evalPrevScopeChain]
Whiteboard: [jsbugmon:update,testComment=9] → [jsbugmon:testComment=9]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #9)
> Created attachment 736503 [details]
> debug and opt stacks
> 
> x = Set
> function f() {
>     eval("isPrototypeOf.call(Set, x)")
> }
> y = [0, 0]
> Array.prototype.some.call(y, f)
> Set = 0[0]
> Array.prototype.sort.call(y, f)
> 
> crashes js opt shell at scopeChain with
> js::AbstractFramePtr::evalPrevScopeChain on the stack and asserts js debug
> shell at Assertion failure: false (Unexpected state), at vm/Stack.cpp
> 
> Tested on 64-bit opt deterministic threadsafe shell with --ion-eager --no-jm
> --no-baseline, on m-c changeset 9d5f05a6d497.

I can still reproduce this on m-c changeset 8b1a7228674a.
Crash Signature: [@ scopeChain] [@ js::AbstractFramePtr::evalPrevScopeChain] → [@ scopeChain] [@ js::AbstractFramePtr::evalPrevScopeChain]
I looked into the testcase provided in comment 9 .
In that testcase we both use FastInvoke for IM (bug 797131) for array_sort and DirectEvalFromIon (introduced in bug 743394). I have no proof, but I wouldn't be surprised if this is caused by a faulty interaction between the two.

Just to make it more readable, this is equivalent testcase (doesn't crashes or run)
x = Set
y = [0, 0]
function f() {
    eval("x.isPrototypeOf(Set)")
}
y.some(f)
x = undefined
y.sort(f)

The problem arises in the eval'd code that "x" isn't an object (but undefined) for y.sort(f). So it throws an error. But it goes wrong when unwinding scope. We need "evalPrevScopeChain", but during settle, we get the wrong stackFrame. We get the Native, instead of the Ion frame. (And we die/assert/crash, because we can't get the scopeChain from Native.)

Now I found where we make the wrong decision in "StackIter::settleOnNewState()".
vm/Stack.cpp:1384
if (containsFrame && (!containsCall || (Value *)data_.fp_ >= data_.calls_->array())) {

This should be true. containsFrame and containsCall are true. (This is correct).
But "(Value *)data_.fp_ >= data_.calls_->array()" should also be true. But it is false!

When removing the "some"-call or replacing with another "sort"-call, this is indeed true.
Also when running the testcase and setting this to true on the right moment, makes it not crash, but report the right error message!

Now I'm a bit in the dark as to what this expression does and where it is set and where we could miss it. I'm going to leave this now in the hands of people that are more experienced with this part of the code.
Flags: needinfo?(jdemooij)
Bug 868990 fixed this by taking out all the StackIter native call code.
Flags: needinfo?(jdemooij)

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [jsbugmon:testComment=9] → [jsbugmon:testComment=9,update,bisectfix]
(Assignee)

Updated

5 years ago
Flags: needinfo?(sstangl)
status-firefox23: affected → fixed
Target Milestone: --- → mozilla23
Whiteboard: [jsbugmon:testComment=9,update,bisectfix] → [jsbugmon:testComment=9,update,bisectfix][adv-main23-]
Group: core-security
You need to log in before you can comment on or make changes to this bug.