227765 2021-07-07 12:53:05 -0700 JSArrayBufferView::byteOffsetConcurrently has a race when using PAC 2021-07-07 19:21:17 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 saam saam ews-watchlist keith_miller mark.lam msaboff tzagallo webkit-bug-importer oldest_to_newest 1775405 0 saam 2021-07-07 12:53:05 -0700 Compiler thread crash. stress/JSArrayBufferView-byteOffset-is-racy-from-compiler-thread.js.default: test_script_20: line 2: 77566 Trace/BPT trap: 5 ( "$@" ../../.vm/JavaScriptCore.framework/Helpers/jsc --useFTLJIT\=false --useFunctionDotArguments\=true --validateExceptionChecks\=true --useDollarVM\=true --maxPerThreadStackUsage\=1572864 --useFTLJIT\=true --jitPolicyScale\=0 JSArrayBufferView-byteOffset-is-racy-from-compiler-thread.js ) Exception Type: EXC_BREAKPOINT (SIGTRAP) Thread 3 Crashed:: JIT Worklist Helper Thread 0 JavaScriptCore 0x102aa03ec void* WTF::untagArrayPtr<void>(void*, unsigned long) + 20 (PtrTag.h:438) [inlined] 1 JavaScriptCore 0x102aa03ec WTF::CagedPtr<(Gigacage::Kind)0, void, true, WTF::RawPtrTraits<void> >::getMayBeNull(unsigned long) const + 92 (CagedPtr.h:75) [inlined] 2 JavaScriptCore 0x102aa03ec JSC::CagedBarrierPtr<(Gigacage::Kind)0, void, true>::getMayBeNull(unsigned long) const + 92 (CagedBarrierPtr.h:62) [inlined] 3 JavaScriptCore 0x102aa03ec JSC::JSArrayBufferView::vector() const + 120 (JSArrayBufferView.h:190) [inlined] 4 JavaScriptCore 0x102aa03ec std::__1::optional<unsigned int> JSC::JSArrayBufferView::byteOffsetImpl<(JSC::JSArrayBufferView::Requester)1, std::__1::optional<unsigned int> >() + 132 (JSArrayBufferViewInlines.h:100) [inlined] 5 JavaScriptCore 0x102aa03ec JSC::JSArrayBufferView::byteOffsetConcurrently() + 132 (JSArrayBufferViewInlines.h:120) [inlined] 6 JavaScriptCore 0x102aa03ec JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::executeEffects(unsigned int, JSC::DFG::Node*) + 53252 (DFGAbstractInterpreterInlines.h:3802) 7 JavaScriptCore 0x102a9c714 JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::executeEffects(unsigned int, JSC::DFG::Node*) + 37676 (DFGAbstractInterpreterInlines.h:3800) 8 JavaScriptCore 0x10305701c JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::execute(unsigned int) + 60 (DFGAbstractInterpreterInlines.h:4681) [inlined] 9 JavaScriptCore 0x10305701c JSC::DFG::CFAPhase::performBlockCFA(JSC::DFG::BasicBlock*) + 544 (DFGCFAPhase.cpp:232) 10 JavaScriptCore 0x103056904 JSC::DFG::CFAPhase::performForwardCFA() + 64 (DFGCFAPhase.cpp:263) [inlined] 11 JavaScriptCore 0x103056904 JSC::DFG::CFAPhase::run() + 480 (DFGCFAPhase.cpp:119) 12 JavaScriptCore 0x1030565c4 bool JSC::DFG::runAndLog<JSC::DFG::CFAPhase>(JSC::DFG::CFAPhase&) + 52 (DFGPhase.h:84) 13 JavaScriptCore 0x1030404b4 bool JSC::DFG::runPhase<JSC::DFG::CFAPhase>(JSC::DFG::Graph&) + 40 (DFGPhase.h:95) 14 JavaScriptCore 0x10318a7a8 JSC::DFG::Plan::compileInThreadImpl() + 1892 (DFGPlan.cpp:276) 15 JavaScriptCore 0x10342d454 JSC::JITPlan::compileInThread(JSC::JITWorklistThread*) + 452 (JITPlan.cpp:165) 16 JavaScriptCore 0x103474750 JSC::JITWorklistThread::work() + 252 (JITWorklistThread.cpp:123) 17 JavaScriptCore 0x10291dda8 WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0::operator()() const + 528 (AutomaticThread.cpp:229) [inlined] 18 JavaScriptCore 0x10291dda8 WTF::Detail::CallableWrapper<WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0, void>::call() + 568 (Function.h:53) 19 JavaScriptCore 0x1029688cc WTF::Function<void ()>::operator()() const + 60 (Function.h:82) [inlined] 20 JavaScriptCore 0x1029688cc WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) + 184 (Threading.cpp:186) 21 JavaScriptCore 0x10296acd4 WTF::wtfThreadEntryPoint(void*) + 16 (ThreadingPOSIX.cpp:241) 22 libsystem_pthread.dylib 0x18d80543c _pthread_start + 148 23 libsystem_pthread.dylib 0x18d8000d4 thread_start + 8 1775423 1 saam 2021-07-07 13:32:47 -0700 Seems like the bug is in: inline std::optional<unsigned> byteOffsetConcurrently(); My guess is we're detaching concurrently. 1775424 2 saam 2021-07-07 13:33:33 -0700 (In reply to Saam Barati from comment #1) > Seems like the bug is in: > inline std::optional<unsigned> byteOffsetConcurrently(); > > > My guess is we're detaching concurrently. yeah, looks like that's what the test is doing: //@ slow! //@ runDefault("--jitPolicyScale=0") // This test should not crash. script = ` let a = new Int32Array(1); for (let i = 0; i < 1000; ++i) ~a.byteOffset; transferArrayBuffer(a.buffer); eval(a.byteOffset); let description = describe(a.byteOffset); if (description !== 'Int32: 0') print(description); `; const iterations = 1000; for (let i = 0; i < iterations; i++) runString(script); 1775427 3 saam 2021-07-07 13:37:16 -0700 I think the fix is to have a different version of vector() for when run from the concurrent thread. 1775548 4 433100 saam 2021-07-07 17:44:02 -0700 Created attachment 433100 patch 1775549 5 433100 mark.lam 2021-07-07 17:46:43 -0700 Comment on attachment 433100 patch r=me. Is there anyway to assert that vectorWithoutPACValidation() is not called by the mutator? Maybe not. Just thought I'd ask. 1775554 6 saam 2021-07-07 17:55:29 -0700 (In reply to Mark Lam from comment #5) > Comment on attachment 433100 [details] > patch > > r=me. Is there anyway to assert that vectorWithoutPACValidation() is not > called by the mutator? Maybe not. Just thought I'd ask. This patch is making it so that vectorWithoutPACValidation is called on the mutator. So asserting would immediately crash. This patch aligns what the mutator thread already does when inlined in DFG/FTL's byte offset implementation. 1775582 7 ews-feeder 2021-07-07 19:20:58 -0700 Committed r279707 (239499@main): <https://commits.webkit.org/239499@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 433100. 1775583 8 webkit-bug-importer 2021-07-07 19:21:17 -0700 <rdar://problem/80301239> 433100 2021-07-07 17:44:02 -0700 2021-07-07 19:20:59 -0700 patch b-backup.diff text/plain 2296 saam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjc5Njg4KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBA CisyMDIxLTA3LTA3ICBTYWFtIEJhcmF0aSAgPHNiYXJhdGlAYXBwbGUuY29tPgorCisgICAgICAg IEpTQXJyYXlCdWZmZXJWaWV3OjpieXRlT2Zmc2V0Q29uY3VycmVudGx5IGhhcyBhIHJhY2Ugd2hl biB1c2luZyBQQUMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTIyNzc2NQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIFdlIHdlcmUgY2FsbGluZyBKU0FycmF5QnVmZmVyVmlldzo6dmVjdG9yKCksIHdoaWNoIGRv ZXMgUEFDIHZhbGlkYXRpb24KKyAgICAgICAgdXNpbmcgdGhlIGxlbmd0aCgpIG9mIHRoZSBhcnJh eS4gSG93ZXZlciwgdGhpcyBjYW4gcmFjZSB3aXRoIEpTQXJyYXlCdWZmZXJWaWV3OjpkZXRhY2gs CisgICAgICAgIHdoaWNoIHNldHMgdGhlIGxlbmd0aCB0byB6ZXJvLCBsZWFkaW5nIHRvIHNhZG5l c3Mgb24gdGhlIGNvbXBpbGVyIHRocmVhZC4KKworICAgICAgICAqIHJ1bnRpbWUvSlNBcnJheUJ1 ZmZlclZpZXcuaDoKKyAgICAgICAgKEpTQzo6SlNBcnJheUJ1ZmZlclZpZXc6OnZlY3RvcldpdGhv dXRQQUNWYWxpZGF0aW9uIGNvbnN0KToKKyAgICAgICAgKiBydW50aW1lL0pTQXJyYXlCdWZmZXJW aWV3SW5saW5lcy5oOgorICAgICAgICAoSlNDOjpKU0FycmF5QnVmZmVyVmlldzo6Ynl0ZU9mZnNl dEltcGwpOgorCiAyMDIxLTA3LTA3ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29t PgogCiAgICAgICAgIGZpdGNvcmVkIGNyYXNoZXMgYXQgSmF2YVNjcmlwdENvcmU6IEpTQzo6SGVh cDo6cmVsZWFzZUFjY2Vzc1Nsb3cKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1l L0pTQXJyYXlCdWZmZXJWaWV3LmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3Jl L3J1bnRpbWUvSlNBcnJheUJ1ZmZlclZpZXcuaAkocmV2aXNpb24gMjc5NjYyKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNBcnJheUJ1ZmZlclZpZXcuaAkod29ya2luZyBjb3B5 KQpAQCAtMTg4LDYgKzE4OCw3IEBAIHB1YmxpYzoKIAogICAgIGJvb2wgaGFzVmVjdG9yKCkgY29u c3QgeyByZXR1cm4gISFtX3ZlY3RvcjsgfQogICAgIHZvaWQqIHZlY3RvcigpIGNvbnN0IHsgcmV0 dXJuIG1fdmVjdG9yLmdldE1heUJlTnVsbChsZW5ndGgoKSk7IH0KKyAgICB2b2lkKiB2ZWN0b3JX aXRob3V0UEFDVmFsaWRhdGlvbigpIGNvbnN0IHsgcmV0dXJuIG1fdmVjdG9yLmdldFVuc2FmZSgp OyB9CiAgICAgCiAgICAgaW5saW5lIHVuc2lnbmVkIGJ5dGVPZmZzZXQoKTsKICAgICBpbmxpbmUg c3RkOjpvcHRpb25hbDx1bnNpZ25lZD4gYnl0ZU9mZnNldENvbmN1cnJlbnRseSgpOwpJbmRleDog U291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNBcnJheUJ1ZmZlclZpZXdJbmxpbmVzLmgK PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNBcnJheUJ1ZmZl clZpZXdJbmxpbmVzLmgJKHJldmlzaW9uIDI3OTY2MikKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29y ZS9ydW50aW1lL0pTQXJyYXlCdWZmZXJWaWV3SW5saW5lcy5oCSh3b3JraW5nIGNvcHkpCkBAIC05 Nyw3ICs5Nyw3IEBAIGlubGluZSBSZXN1bHRUeXBlIEpTQXJyYXlCdWZmZXJWaWV3OjpieXQKICAg ICB9CiAKICAgICBwdHJkaWZmX3QgZGVsdGEgPQotICAgICAgICBiaXR3aXNlX2Nhc3Q8dWludDhf dCo+KHZlY3RvcigpKSAtIHN0YXRpY19jYXN0PHVpbnQ4X3QqPihidWZmZXItPmRhdGEoKSk7Cisg ICAgICAgIGJpdHdpc2VfY2FzdDx1aW50OF90Kj4odmVjdG9yV2l0aG91dFBBQ1ZhbGlkYXRpb24o KSkgLSBzdGF0aWNfY2FzdDx1aW50OF90Kj4oYnVmZmVyLT5kYXRhKCkpOwogCiAgICAgdW5zaWdu ZWQgcmVzdWx0ID0gc3RhdGljX2Nhc3Q8dW5zaWduZWQ+KGRlbHRhKTsKICAgICBpZiAocmVxdWVz dGVyID09IE11dGF0b3IpCg==