Closed Bug 520636 Opened 15 years ago Closed 15 years ago

sideExits holds dangling pointer on failed compilation

Categories

(Core :: JavaScript Engine, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: luke, Assigned: luke)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

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.
I might add "kudos!" to the writer of the "BIG FAT WARNING" comment.
Why O^2? Its an array list. We could just remember the length before we start compiling and then setLength() back to that length.
(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.
Attached patch fixSplinter Review
This patch is on top of the refactoring patch in bug 517174 (which introduces TraceRecorder::finishAbort / TraceRecorder::finishSuccessfully).
Assignee: general → lw
Status: NEW → ASSIGNED
Attachment #411276 - Flags: review?(dvander)
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+
(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.
http://hg.mozilla.org/tracemonkey/rev/f26a372a0a4f
Whiteboard: fixed-in-tracemonkey
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.

Attachment

General

Created:
Updated:
Size: