Bug 150351 - GCAwareJITStubRoutineWithExceptionHandler has a stale CodeBlock pointer in its destructor
Summary: GCAwareJITStubRoutineWithExceptionHandler has a stale CodeBlock pointer in it...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-19 18:33 PDT by Saam Barati
Modified: 2015-10-20 11:39 PDT (History)
10 users (show)

See Also:


Attachments
patch (3.36 KB, patch)
2015-10-19 18:38 PDT, Saam Barati
mark.lam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2015-10-19 18:33:02 PDT
The problem is that we may regenerate many GCAwareJITStubRoutineWithExceptionHandler stubs per one PolymorphicAccess.
Only the last GCAwareJITStubRoutineWithExceptionHandler stub that was generated will get the CodeBlock's aboutToDie()
notification. All other GCAwareJITStubRoutineWithExceptionHandler stubs will still be holding a stale CodeBlock pointer
that they will use in their destructor. The solution is to have GCAwareJITStubRoutineWithExceptionHandler remove its
exception handler in observeZeroRefCount() instead of its destructor. observeZeroRefCount() will run when a PolymorphicAccess
replaces its m_stubRoutine.
Comment 1 Saam Barati 2015-10-19 18:38:35 PDT
Created attachment 263539 [details]
patch
Comment 2 Mark Lam 2015-10-19 19:05:06 PDT
Comment on attachment 263539 [details]
patch

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

r=me

> Source/JavaScriptCore/jit/GCAwareJITStubRoutine.cpp:129
> +GCAwareJITStubRoutineWithExceptionHandler::~GCAwareJITStubRoutineWithExceptionHandler()
> +{
>  }

maybe get rid of the destructor now since you don't need it anymore.

> Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:101
>      ~GCAwareJITStubRoutineWithExceptionHandler() override;

No need to override if we don't need it.
Comment 3 Saam Barati 2015-10-20 11:39:09 PDT
Thanks for the review. I've included your suggestions in the landed patch.

landed in:
http://trac.webkit.org/changeset/191350