520636 - sideExits holds dangling pointer on failed compilation

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

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.

Comment 1

[Luke Wagner [:luke]]

I might add "kudos!" to the writer of the "BIG FAT WARNING" comment.

Comment 2

[Andreas Gal :gal]

Why O^2? Its an array list. We could just remember the length before we start compiling and then setLength() back to that length.

Comment 3

[Luke Wagner [:luke]]

(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.

Comment 4

[Luke Wagner [:luke]]

Attachment: fix

This patch is on top of the refactoring patch in bug 517174 (which introduces TraceRecorder::finishAbort / TraceRecorder::finishSuccessfully).

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 411276 [details] [diff] [review]
fix

Does this affect 1.9.2 at all? It doesn't seem like it.

Comment 6

[Luke Wagner [:luke]]

(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.

Comment 7

[Luke Wagner [:luke]]

http://hg.mozilla.org/tracemonkey/rev/f26a372a0a4f

Comment 8

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/f26a372a0a4f