Closed
Bug 746377
Opened 12 years ago
Closed 12 years ago
IonMonkey: Assertion failure: mir->type() == MIRType_Value, at ion/x86/Lowering-x86.cpp:54 or Crash [@ js::ion::MDefinition::isInitProp]
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: decoder, Assigned: nbp)
References
Details
(Keywords: assertion, testcase, Whiteboard: [jsbugmon:update,reconfirm])
Attachments
(1 file)
3.76 KB,
patch
|
sstangl
:
review+
|
Details | Diff | Splinter Review |
The following testcase asserts on ionmonkey revision 67bf9a4a1f77 (run with --ion -n): var actual = ''; test(); function test() { a = {x: 1}; b = {__proto__: a}; print(actual += test(1,2,3,4)); }
Reporter | ||
Comment 1•12 years ago
|
||
Crash when stepping through assertions looks similar to that in bug 746366: Program received signal SIGABRT, Aborted. Assertion failure: live->empty(), at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/LinearScan.cpp:719 Program received signal SIGABRT, Aborted. Program received signal SIGSEGV, Segmentation fault. 0x0847e359 in js::ion::MDefinition::isInitProp (this=0x0) at ../ion/MIR.h:441 441 MIR_OPCODE_LIST(OPCODE_CASTS) (gdb) bt 8 #0 0x0847e359 in js::ion::MDefinition::isInitProp (this=0x0) at ../ion/MIR.h:441 #1 0x0847e903 in js::ion::MDefinition::toInitProp (this=0x0) at ../ion/MIR.h:4394 #2 0x0847fd06 in js::ion::LInitProp::mir (this=0x872cf38) at ../ion/LIR-Common.h:280 #3 0x08485ec5 in js::ion::CodeGenerator::visitInitProp (this=0xffffb4a0, lir=0x872cf38) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/CodeGenerator.cpp:1008 #4 0x083eacf7 in js::ion::LInitProp::accept (this=0x872cf38, visitor=0xffffb4a0) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/LIR-Common.h:260 #5 0x08485463 in js::ion::CodeGenerator::generateBody (this=0xffffb4a0) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/CodeGenerator.cpp:824 #6 0x0848be21 in js::ion::CodeGenerator::generate (this=0xffffb4a0) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/CodeGenerator.cpp:2048 #7 0x083972ab in TestCompiler (builder=..., graph=...) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/Ion.cpp:732
Reporter | ||
Comment 2•12 years ago
|
||
JSBugMon: The testcase found in this bug no longer reproduces (tried revision de015aff650d).
Reporter | ||
Updated•12 years ago
|
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
Reporter | ||
Updated•12 years ago
|
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:update,reconfirm]
Assignee | ||
Comment 3•12 years ago
|
||
I can reproduce it with: ./js --ion --ion-eager -n ./jit-test/tests/basic/bug732693.js Assertion failure: mir->type() == MIRType_Value, at ./ion/x64/Lowering-x64.cpp:54 The assertion is caused by the lack of typePolicy function in the MIR.
Assignee: general → nicolas.b.pierron
Blocks: IonEager
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86 → All
Assignee | ||
Comment 4•12 years ago
|
||
* Fix argument order for InitProp VM function. * NewObject is not initialized the same way as NewInit. * Set mir used unconditionally by the code generator. * Add Type policy to convert the input when needed.
Attachment #618109 -
Flags: review?(sstangl)
Comment 5•12 years ago
|
||
Comment on attachment 618109 [details] [diff] [review] Fix JSOP_NEWINIT. Review of attachment 618109 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/VMFunctions.cpp @@ +223,5 @@ > > JSObject* > NewInitObject(JSContext *cx, HandleObject baseObj, types::TypeObject *type) > { > + RootedVarObject obj(cx); MethodJIT just uses "JSObject *". Should be safe to use that. @@ +224,5 @@ > JSObject* > NewInitObject(JSContext *cx, HandleObject baseObj, types::TypeObject *type) > { > + RootedVarObject obj(cx); > + if (*baseObj.address()) { This should be "if (baseObj)". For the JSOP_NEWINIT case, baseObj is NULL, and attempting to take .address() will segfault.
Attachment #618109 -
Flags: review?(sstangl) → review+
Assignee | ||
Comment 6•12 years ago
|
||
(In reply to Sean Stangl from comment #5) > Comment on attachment 618109 [details] [diff] [review] > Fix JSOP_NEWINIT. > > Review of attachment 618109 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: js/src/ion/VMFunctions.cpp > @@ +223,5 @@ > > > > JSObject* > > NewInitObject(JSContext *cx, HandleObject baseObj, types::TypeObject *type) > > { > > + RootedVarObject obj(cx); > > MethodJIT just uses "JSObject *". Should be safe to use that. If methodJIT is not using a RootedVarObject, this is likely to be a bug which will cause issue with moving GC. terrence: can you confirm this ? > @@ +224,5 @@ > > JSObject* > > NewInitObject(JSContext *cx, HandleObject baseObj, types::TypeObject *type) > > { > > + RootedVarObject obj(cx); > > + if (*baseObj.address()) { > > This should be "if (baseObj)". For the JSOP_NEWINIT case, baseObj is NULL, > and attempting to take .address() will segfault. HandleObject is never null and target the JSObject* which is pushed on the stack. HandleObject::address return the ptr_ field (JSObject**) which is never null (as ensured by callVM). Taking the address of it allow use to read the value. Another way would be to check reinterpret_cast<JSObject*>(baseObj) instead of *baseObj.address()
Comment 7•12 years ago
|
||
(In reply to Nicolas B. Pierron [:pierron] from comment #6) > HandleObject is never null and target the JSObject* which is pushed on the > stack. HandleObject::address return the ptr_ field (JSObject**) which is > never null (as ensured by callVM). Taking the address of it allow use to > read the value. Another way would be to check > reinterpret_cast<JSObject*>(baseObj) instead of *baseObj.address() Are you sure it's actually a HandleObject? The argument passing is via "pushArg(ImmGCPtr(lir->mir()->baseObj()))", where baseObj() returns a JSObject *.
Comment 8•12 years ago
|
||
(In reply to Sean Stangl from comment #7) > Are you sure it's actually a HandleObject? The argument passing is via > "pushArg(ImmGCPtr(lir->mir()->baseObj()))", where baseObj() returns a > JSObject *. Nicolas explained to me in-person that the conversion from JSObject* to HandleObject occurs magically in callVM function templates, so the above is fine.
Assignee | ||
Comment 9•12 years ago
|
||
https://hg.mozilla.org/projects/ionmonkey/rev/086958bf99e7
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 11•11 years ago
|
||
Automatically extracted testcase for this bug was committed: https://hg.mozilla.org/mozilla-central/rev/2e891e0db397
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•