Closed
Bug 520636
Opened 15 years ago
Closed 15 years ago
sideExits holds dangling pointer on failed compilation
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: luke, Assigned: luke)
References
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file)
4.55 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
VMSideExits are allocated in the traceAlloc, which is reset on failed compilations. Before recording aborts, pointers to VMSideExits can be added to TreeInfo::sideExits (which is used in snapshot() to merge side exits). Since the list outlives a failed compilation, these pointers will become invalid and may be used in future snapshots. Simply removing all VMSideExits created during recording would be O(n^2) (and n can be big), so I propose keeping a TraceRecorder::recordedSideExits list that is only added to TreeInfo::sideExits on successful recording. snapshot() would then search both lists.
Assignee | ||
Comment 1•15 years ago
|
||
I might add "kudos!" to the writer of the "BIG FAT WARNING" comment.
Comment 2•15 years ago
|
||
Why O^2? Its an array list. We could just remember the length before we start compiling and then setLength() back to that length.
Assignee | ||
Comment 3•15 years ago
|
||
(In reply to comment #2) > Why O^2? Its an array list. We could just remember the length before we start > compiling and then setLength() back to that length. Oh right, I was thinking ahead to bug 519567 where there can be multiple simultaneous recorders. But yes, as it is currently, sideExits is stack-like, so it could be zippy.
Assignee | ||
Comment 4•15 years ago
|
||
This patch is on top of the refactoring patch in bug 517174 (which introduces TraceRecorder::finishAbort / TraceRecorder::finishSuccessfully).
Comment on attachment 411276 [details] [diff] [review] fix Does this affect 1.9.2 at all? It doesn't seem like it.
Attachment #411276 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 6•15 years ago
|
||
(In reply to comment #5) > (From update of attachment 411276 [details] [diff] [review]) > Does this affect 1.9.2 at all? It doesn't seem like it. Since we are using traceAlloc in 1.9.2, I think it does.
Assignee | ||
Comment 7•15 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/f26a372a0a4f
Whiteboard: fixed-in-tracemonkey
Comment 8•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/f26a372a0a4f
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•