186827 2018-06-19 16:52:35 -0700 Crash in sanitizeStackForVMImpl sometimes when switching threads with same VM 2018-06-19 17:45:15 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 msaboff msaboff commit-queue ews-watchlist keith_miller mark.lam saam webkit-bug-importer oldest_to_newest 1434636 0 msaboff 2018-06-19 16:52:35 -0700 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. 1434637 1 msaboff 2018-06-19 16:52:46 -0700 <rdar://problem/40624786> 1434638 2 343121 msaboff 2018-06-19 16:57:24 -0700 Created attachment 343121 Patch 1434639 3 343121 saam 2018-06-19 16:58:27 -0700 Comment on attachment 343121 Patch Can we add a test for this? 1434645 4 msaboff 2018-06-19 17:06:47 -0700 (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. 1434661 5 343121 commit-queue 2018-06-19 17:45:14 -0700 Comment on attachment 343121 Patch Clearing flags on attachment: 343121 Committed r232998: <https://trac.webkit.org/changeset/232998> 1434662 6 commit-queue 2018-06-19 17:45:15 -0700 All reviewed patches have been landed. Closing bug. 343121 2018-06-19 16:57:24 -0700 2018-06-19 17:45:14 -0700 Patch 186827.patch text/plain 1764 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjMyOTkzKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBA CisyMDE4LTA2LTE5ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIENyYXNoIGluIHNhbml0aXplU3RhY2tGb3JWTUltcGwgc29tZXRpbWVzIHdoZW4gc3dpdGNo aW5nIHRocmVhZHMgd2l0aCBzYW1lIFZNCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0xODY4MjcKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBOZWVkIHRvIHNldCBWTTo6bGFzdFN0YWNrVG9wIGJlZm9yZSBhbnkg cG9zc2libGUgY2FsbHMgdG8gc2FuaXRpemVTdGFjaygpLgorCisgICAgICAgICogcnVudGltZS9K U0xvY2suY3BwOgorICAgICAgICAoSlNDOjpKU0xvY2s6OmRpZEFjcXVpcmVMb2NrKToKKwogMjAx OC0wNi0xOSAgVGFkZXUgWmFnYWxsbyAgPHR6YWdhbGxvQGFwcGxlLmNvbT4KIAogICAgICAgICBT aGFkb3dDaGlja2VuIGNyYXNoZXMgd2l0aCBzdGFjayBvdmVyZmxvdyBpbiB0aGUgTExJbnQKSW5k ZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTTG9jay5jcHAKPT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQot LS0gU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNMb2NrLmNwcAkocmV2aXNpb24gMjMy OTkzKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNMb2NrLmNwcAkod29ya2lu ZyBjb3B5KQpAQCAtMTM0LDYgKzEzNCw5IEBAIHZvaWQgSlNMb2NrOjpkaWRBY3F1aXJlTG9jaygp CiAgICAgbV9lbnRyeUF0b21pY1N0cmluZ1RhYmxlID0gdGhyZWFkLnNldEN1cnJlbnRBdG9taWNT dHJpbmdUYWJsZShtX3ZtLT5hdG9taWNTdHJpbmdUYWJsZSgpKTsKICAgICBBU1NFUlQobV9lbnRy eUF0b21pY1N0cmluZ1RhYmxlKTsKIAorICAgIG1fdm0tPnNldExhc3RTdGFja1RvcCh0aHJlYWQu c2F2ZWRMYXN0U3RhY2tUb3AoKSk7CisgICAgQVNTRVJUKHRocmVhZC5zdGFjaygpLmNvbnRhaW5z KG1fdm0tPmxhc3RTdGFja1RvcCgpKSk7CisKICAgICBpZiAobV92bS0+aGVhcC5oYXNBY2Nlc3Mo KSkKICAgICAgICAgbV9zaG91bGRSZWxlYXNlSGVhcEFjY2VzcyA9IGZhbHNlOwogICAgIGVsc2Ug ewpAQCAtMTQ1LDkgKzE0OCw2IEBAIHZvaWQgSlNMb2NrOjpkaWRBY3F1aXJlTG9jaygpCiAgICAg dm9pZCogcCA9ICZwOyAvLyBBIHByb3h5IGZvciB0aGUgY3VycmVudCBzdGFjayBwb2ludGVyLgog ICAgIG1fdm0tPnNldFN0YWNrUG9pbnRlckF0Vk1FbnRyeShwKTsKIAotICAgIG1fdm0tPnNldExh c3RTdGFja1RvcCh0aHJlYWQuc2F2ZWRMYXN0U3RhY2tUb3AoKSk7Ci0gICAgQVNTRVJUKHRocmVh ZC5zdGFjaygpLmNvbnRhaW5zKG1fdm0tPmxhc3RTdGFja1RvcCgpKSk7Ci0gICAgCiAgICAgbV92 bS0+aGVhcC5tYWNoaW5lVGhyZWFkcygpLmFkZEN1cnJlbnRUaHJlYWQoKTsKICNpZiBFTkFCTEUo V0VCQVNTRU1CTFkpCiAgICAgV2FzbTo6c3RhcnRUcmFja2luZ0N1cnJlbnRUaHJlYWQoKTsK