Bug 146285 – Crash on gog.com due to PolymorphicCallNode's having stale references to CallLinkInfo

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Saboff]

CallLinkInfo's contain a RefPtr to a PolymorphicCallStubRoutine, named stub, which contains a collection of PolymorphicCallNode.  Those PolymorphicCallNodes have a reference back to the CallLinkInfo.  When a CallLinkInfo replaces or clears "stub", the ref count of the PolymorphicCallStubRoutine is decremented as expected, but since it inherits from GCAwareJITStubRoutine, it isn't actually deleted until GC.  In the mean time, the original CallLinkInfo can go away.  If PolymorphicCallNode::unlink() is called at that point, it will try to unlink a now deleted CallLinkInfo and crash as a result.

The fix is to clear the CallLinkInfo* in the contained PolymorphicCallNode of a PolymorphicCallStubRoutine when that CallLinkInfo no longer references the PolymorphicCallStubRoutine.

rdar://problem/20701417

[Michael Saboff]

Created attachment 255504 [details]
Patch

[Michael Saboff]

I'll remove the inadvertent change to runtime/Options.h.  That was from debugging.

[Michael Saboff]

Created attachment 255507 [details]
Rebased patch, updated the ChangeLog with suggestions and removed Options.h change

[Darin Adler]

Comment on attachment 255507 [details]
Rebased patch, updated the ChangeLog with suggestions and removed Options.h change

View in context: https://bugs.webkit.org/attachment.cgi?id=255507&action=review

> Source/JavaScriptCore/ChangeLog:25
> +        In the process I refactored CallLinkInfo from a struct to a class with proper accessors and
> +        made all the data elements private.

I know this is a pain, but ideally we would land a first patch with just the refactoring, and the review a second patch with the actual change in behavior.

[Michael Saboff]

(In reply to comment #4)
> Comment on attachment 255507 [details]
> Rebased patch, updated the ChangeLog with suggestions and removed Options.h
> change
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=255507&action=review
> 
> > Source/JavaScriptCore/ChangeLog:25
> > +        In the process I refactored CallLinkInfo from a struct to a class with proper accessors and
> > +        made all the data elements private.
> 
> I know this is a pain, but ideally we would land a first patch with just the
> refactoring, and the review a second patch with the actual change in
> behavior.

I'll refactor in https://bugs.webkit.org/show_bug.cgi?id=146292 and then land the simple clearing change here.

[Michael Saboff]

Created attachment 255520 [details]
Patch after CallLinkInfo refactoring was landed separately.  Reviewed in person.

[Michael Saboff]

Committed r185932: <http://trac.webkit.org/changeset/185932>