Bug 186827 - Crash in sanitizeStackForVMImpl sometimes when switching threads with same VM
Summary: Crash in sanitizeStackForVMImpl sometimes when switching threads with same VM
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Saboff
Keywords: InRadar
Depends on:
Reported: 2018-06-19 16:52 PDT by Michael Saboff
Modified: 2018-06-19 17:45 PDT (History)
6 users (show)

See Also:

Patch (1.72 KB, patch)
2018-06-19 16:57 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2018-06-19 16:52:35 PDT
Crash something like:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x0000000116f5af53 sanitizeStackForVMImpl + 15
1   com.apple.JavaScriptCore      	0x000000011754889c JSC::Heap::collectInMutatorThread() + 44
2   com.apple.JavaScriptCore      	0x0000000117548794 JSC::Heap::stopIfNecessarySlow() + 68
3   com.apple.JavaScriptCore      	0x0000000117548a0f JSC::Heap::acquireAccessSlow() + 303
4   com.apple.JavaScriptCore      	0x000000011784ae6f JSC::JSLock::didAcquireLock() + 111
5   com.apple.JavaScriptCore      	0x0000000117868f9e JSC::JSRunLoopTimer::timerDidFireCallback(__CFRunLoopTimer*, void*) + 30
6   com.apple.CoreFoundation      	0x000000010e782124 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20
7   com.apple.CoreFoundation      	0x000000010e781cc9 __CFRunLoopDoTimer + 877
8   com.apple.CoreFoundation      	0x000000010e78189e __CFRunLoopDoTimers + 333
9   com.apple.CoreFoundation      	0x000000010e77a023 __CFRunLoopRun + 2193

This can occur if the VM was used on another thread and we need to take the Heap::acquireAccessSlow() path.  This happens because we switch VM::lastStackTop after calling Heap::acquireAccess().  The slow path of acquireAccess() ends up calling Heap::collectInMutatorThread() which sanitizes the stack, and if we haven't switched lastStackTop, we'll sanitize the current stack using the prior limits of another stack.  Then we'll walk all over memory and/or crash on an unmapped page.
Comment 1 Michael Saboff 2018-06-19 16:52:46 PDT
Comment 2 Michael Saboff 2018-06-19 16:57:24 PDT
Created attachment 343121 [details]
Comment 3 Saam Barati 2018-06-19 16:58:27 PDT
Comment on attachment 343121 [details]

Can we add a test for this?
Comment 4 Michael Saboff 2018-06-19 17:06:47 PDT
(In reply to Saam Barati from comment #3)
> Comment on attachment 343121 [details]
> Patch
> Can we add a test for this?

I tried pretty hard in testapi.  The problem is how do you force Heap::acquireAccess() to take the slow path.  As a consolation, I did move the ASSERT() as well as the setting.
Comment 5 WebKit Commit Bot 2018-06-19 17:45:14 PDT
Comment on attachment 343121 [details]

Clearing flags on attachment: 343121

Committed r232998: <https://trac.webkit.org/changeset/232998>
Comment 6 WebKit Commit Bot 2018-06-19 17:45:15 PDT
All reviewed patches have been landed.  Closing bug.