Bug 227765 – JSArrayBufferView::byteOffsetConcurrently has a race when using PAC

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Saam Barati]

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

[Saam Barati]

Seems like the bug is in:
    inline std::optional<unsigned> byteOffsetConcurrently();


My guess is we're detaching concurrently.

[Saam Barati]

(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);

[Saam Barati]

I think the fix is to have a different version of vector() for when run from the concurrent thread.

[Saam Barati]

Created attachment 433100 [details]
patch

[Mark Lam]

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.

[Saam Barati]

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

[EWS]

Committed r279707 (239499@main): <https://commits.webkit.org/239499@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433100 [details].

[Radar WebKit Bug Importer]

<rdar://problem/80301239>