RESOLVED FIXED 157864
Code that null checks the VM pointer before any use should ref the VM. 
https://bugs.webkit.org/show_bug.cgi?id=157864
Summary Code that null checks the VM pointer before any use should ref the VM.
Mark Lam
Reported 2016-05-18 15:18:15 PDT
Specifically, in JSLock::willReleaseLock() and HeapTimer::timerDidFire(). Otherwise, there's no guarantee that the VM won't be deleted after the null check. Patch coming.
Attachments
proposed patch. (3.65 KB, patch)
2016-05-18 15:45 PDT, Mark Lam 		keith_miller: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Mark Lam
Comment 1 2016-05-18 15:21:28 PDT
<rdar://problem/26129156>
Mark Lam
Comment 2 2016-05-18 15:45:45 PDT
Created attachment 279304 [details] proposed patch. Still need to run tests.
WebKit Commit Bot
Comment 3 2016-05-18 15:48:00 PDT
Attachment 279304 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/JSLock.cpp:180: 'vm' is incorrectly named. It should be named 'protector' or 'protectedVm'. [readability/naming/protected] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 4 2016-05-18 15:49:04 PDT
Comment on attachment 279304 [details] proposed patch. Nice!
Keith Miller
Comment 5 2016-05-18 15:54:31 PDT
Comment on attachment 279304 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=279304&action=review > Source/JavaScriptCore/ChangeLog:3 > + Code that need to null check the VM pointer before use should ref the VM. I think this would be less confusing as "Code that null checks the VM pointer before any use should ref the VM."
Keith Miller
Comment 6 2016-05-18 15:55:00 PDT
Comment on attachment 279304 [details] proposed patch. r=me too.
Mark Lam
Comment 7 2016-05-18 16:27:10 PDT
(In reply to comment #5) > Comment on attachment 279304 [details] > proposed patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=279304&action=review > > > Source/JavaScriptCore/ChangeLog:3 > > + Code that need to null check the VM pointer before use should ref the VM. > > I think this would be less confusing as "Code that null checks the VM > pointer before any use should ref the VM." I'll make the change.
Mark Lam
Comment 8 2016-05-19 14:03:35 PDT
The patch has passed the layout tests and JSC tests on x86_64. I also did an ad hoc smoke test by running a few apps with it on ARM64.
Mark Lam
Comment 9 2016-05-19 14:04:16 PDT
Landed in r201180: <http://trac.webkit.org/r201180>.
Note You need to log in before you can comment on or make changes to this bug.