Closed Bug 780838 Opened 12 years ago Closed 12 years ago

IonMonkey: Assertion failure: stackPosition_ < info_.nslots(), at ion/MIRGraph.cpp:332 or Crash near [@ js::ion::SplitCriticalEdges]

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86_64
Linux
defect
Not set
major

Tracking

()

VERIFIED FIXED
Tracking Status
firefox-esr10 --- unaffected

People

(Reporter: decoder, Assigned: djvj)

References

Details

(Keywords: assertion, testcase, Whiteboard: [ion:p1:fx18] [jsbugmon:update,ignore])

Attachments

(1 file)

The following testcase asserts on ionmonkey revision 0bc212d0183b (run with --ion -n -m --ion-eager -a):


Object.prototype.inheritsFrom = function (shuper) {
  function Inheriter() { }
  Inheriter.prototype = shuper.prototype;
  this.prototype = new Inheriter();
  this.superConstructor = shuper;
}
function Strength(strengthValue, name) {}
function Constraint(strength) {}
Constraint.prototype.addConstraint = function () {}
function UnaryConstraint(v, strength) {
  this.addConstraint('', 3, 500);
}
UnaryConstraint.inheritsFrom(Constraint);
function StayConstraint(v, str) {
  StayConstraint.superConstructor.call(this, v, str);
}
StayConstraint.inheritsFrom(UnaryConstraint);
function EditConstraint(v, str) {
  EditConstraint.superConstructor.call(this, v, str);
}
EditConstraint.inheritsFrom(UnaryConstraint);
var prev = null, first = null, last = null;
new StayConstraint(last, Strength.STRONG_DEFAULT);
var edit = new EditConstraint(first, Strength.PREFERRED);
Crashes in opt builds:

vex amd64->IR: unhandled instruction bytes: 0xFF 0xFF 0xFF 0x7F 0x1 0x0
==26473== Invalid read of size 4
==26473==    at 0x603FA42: ???
==26473==    by 0x6C7309: js::ion::SplitCriticalEdges(js::ion::MIRGenerator*, js::ion::MIRGraph&) (IonAnalysis.cpp:23)
==26473==    by 0x6C1CDA: js::ion::BuildMIR(js::ion::IonBuilder&, js::ion::MIRGraph&) (Ion.cpp:697)
==26473==    by 0x6C5A33: bool js::ion::IonCompile<&(js::ion::TestCompiler(js::ion::IonBuilder&, js::ion::MIRGraph&))>(JSContext*, JSScript*, JSFunction*, unsigned char*, bool) (Ion.cpp:833)
==26473==    by 0x6C612B: js::ion::CanEnter(JSContext*, JSScript*, js::StackFrame*, bool) (Ion.cpp:986)
==26473==    by 0x4AA3B4: js::RunScript(JSContext*, JSScript*, js::StackFrame*) (jsinterp.cpp:294)
==26473==    by 0x4AA868: js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) (jsinterp.cpp:375)
==26473==    by 0x4AAE4A: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) (jsinterp.h:119)
==26473==    by 0x72D8FF: js::ion::InvokeFunction(JSContext*, JSFunction*, unsigned int, JS::Value*, JS::Value*) (VMFunctions.cpp:65)
==26473==    by 0x4033E17: ???
==26473==    by 0x6C3E76: EnterIon(JSContext*, js::StackFrame*, void*) (Ion.cpp:1149)
==26473==    by 0x4A940B: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2495)
==26473==  Address 0x1 is not stack'd, malloc'd or (recently) free'd


Might be an invalid jump?

(This bug was filed from an airport, achievement unlocked ^_^)
Whiteboard: [jsbugmon:update] → [jsbugmon:update][ion:p1:fx18]
Assignee: general → kvijayan
Whiteboard: [jsbugmon:update][ion:p1:fx18] → [ion:p1:fx18] [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision b2382c3c24ce).
Attached patch Fix.Splinter Review
The bug here is subtle, but the fix is simple.

An assert was triggering on the |fallbackBlock->push()| indicating that the total stack slots were being exceeded.  This is because the fallbackBlock is constructed using the resumePoint immediately before the GetProp, and it subsequently pushed the GetProp, followed by the arguments.  However, this mismatched with the interpreter stack since method call bytecode uses JSOP_SWAP to place the |this| argument in the correct stack position (as opposed to explicitly pushing it).  The state immediately before the GetProp still had the |this| argument on the stack, and thus the push done by makePolyInlineDispatch was effectively duplicating the |this| instead of swapping it.

The resolution is simple: the argument pushes don't need to be done at all in the fallback block.  This is because the fallback block terminates immediately after the GETPROP instruction, and does a GOTO to the fallback end block (which does the call).  The fallback end block is constructed with the correct stack for immediately before the call, which means that any bailout before the call will recapture the right state.

The argument pushes happening in the fallbackBlock were useless: no resume point would ever have used them.
Attachment #650638 - Flags: review?(dvander)
Attachment #650638 - Flags: review?(dvander) → review+
https://hg.mozilla.org/projects/ionmonkey/rev/4804d288adae
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Group: core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: