Closed
Bug 725067
Opened 12 years ago
Closed 12 years ago
IonMonkey: Crash [@ js::gc::ChunkBitmap::markIfUnmarked]
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: decoder, Assigned: dvander)
References
Details
(Keywords: crash, testcase)
Crash Data
Attachments
(3 files)
17.31 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
7.72 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
3.88 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
The following testcase crashes on ionmonkey revision d66c148e0756 (run with --ion -n), tested on 64 bit: var o0 = []; var o4 = {}; var o5 = Math; function f6(o) { o[("Keywords")] = o;}; for(var i=0; i<20; i++) { f6(o0); f6(o4); f6(o5); } gc();
Assignee | ||
Comment 1•12 years ago
|
||
The bug here is that lhs == rhs in the register allocator, and then we generate code like: push lhs mov [lhs.slots], lhs mov rhs, [lhs] pop lhs So we just need to get an extra register. But before that I'd like to refactor the SetProperty stuff so it looks more like GetProperty/GetElement.
Assignee | ||
Comment 2•12 years ago
|
||
This patch splits MGenericSetProperty into MCallSetProperty and MSetPropertyCache. LCallSetPropertyV/T are then combined into one unspecialized LCallSetProperty.
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #595521 -
Flags: review?(jdemooij)
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #595537 -
Flags: review?(jdemooij)
Comment 6•12 years ago
|
||
Comment on attachment 595519 [details] [diff] [review] part 1: split MGenericSetProperty up Review of attachment 595519 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/MIR.h @@ +2959,5 @@ > bool strict_; > + > + protected: > + MSetPropertyInstruction(MDefinition *obj, MDefinition *value, JSAtom *atom, > + bool strict) Nit: this needs some extra spaces
Attachment #595519 -
Flags: review?(jdemooij) → review+
Comment 7•12 years ago
|
||
Comment on attachment 595521 [details] [diff] [review] part 2, rename LCacheSetProperty to LSetPropertyCache Review of attachment 595521 [details] [diff] [review]: ----------------------------------------------------------------- What about renaming GetPropertyCache so these instructions use either a Call* or a Cache* prefix?
Attachment #595521 -
Flags: review?(jdemooij) → review+
Updated•12 years ago
|
Attachment #595537 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 8•12 years ago
|
||
http://hg.mozilla.org/projects/ionmonkey/rev/9cd94217ee4f http://hg.mozilla.org/projects/ionmonkey/rev/d546f1b141b9 http://hg.mozilla.org/projects/ionmonkey/rev/28c66941856b I think I prefer Call/Cache as suffixes, but it's not a strong preference.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 9•11 years ago
|
||
A testcase for this bug was automatically identified at js/src/jit-test/tests/ion/bug725067.js.
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•