Bug 186827 – Crash in sanitizeStackForVMImpl sometimes when switching threads with same VM

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Michael Saboff]

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.

[Michael Saboff]

<rdar://problem/40624786>

[Michael Saboff]

Created attachment 343121 [details]
Patch

[Saam Barati]

Comment on attachment 343121 [details]
Patch

Can we add a test for this?

[Michael Saboff]

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

[WebKit Commit Bot]

Comment on attachment 343121 [details]
Patch

Clearing flags on attachment: 343121

Committed r232998: <https://trac.webkit.org/changeset/232998>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.