Bug 199997 - JSC: assertion failure in SpeculativeJIT::compileGetByValOnIntTypedArray
Summary: JSC: assertion failure in SpeculativeJIT::compileGetByValOnIntTypedArray
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Safari 12
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-07-22 02:44 PDT by Samuel Groß
Modified: 2019-08-05 13:02 PDT (History)
13 users (show)

See Also:


Attachments
Patch (3.67 KB, patch)
2019-08-05 10:41 PDT, Michael Saboff
sbarati: review+
sbarati: commit-queue+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Samuel Groß 2019-07-22 02:44:04 PDT
The following JavaScript program crashes current JSC built in debug configurations:

function v0(v1, t) {
    let v5 = v1;
    for (let v10 = 0; v10 < 8; v10++) {
        const v11 = v1 instanceof Uint32Array;
        const v12 = v1[65537];
    }
    const v13 = v5[0];
}
const v15 = new Uint32Array(1024);
for (let v19 = 0; v19 < 10000; v19++) {
    const v20 = v0(v15);
}

Crashes with:

ASSERTION FAILED: node->arrayMode().alreadyChecked(m_jit.graph(), node, m_state.forNode(m_graph.varArgChild(node, 0)))
../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp(2966) : void JSC::DFG::SpeculativeJIT::compileGetByValOnIntTypedArray(JSC::DFG::Node *, JSC::TypedArrayType)
1   0x107f95d29 WTFCrash
2   0x10652311b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x1070bfc5f JSC::DFG::SpeculativeJIT::compileGetByValOnIntTypedArray(JSC::DFG::Node*, JSC::TypedArrayType)
4   0x1072711e1 JSC::DFG::SpeculativeJIT::compile(JSC::DFG::Node*)
5   0x1070b7f79 JSC::DFG::SpeculativeJIT::compileCurrentBlock()
6   0x1070b8d52 JSC::DFG::SpeculativeJIT::compile()
7   0x106f1d954 JSC::DFG::JITCompiler::compileBody()
8   0x106f211fa JSC::DFG::JITCompiler::compileFunction()
9   0x10705f813 JSC::DFG::Plan::compileInThreadImpl()
10  0x10705d256 JSC::DFG::Plan::compileInThread(JSC::DFG::ThreadData*)
11  0x107312c6b JSC::DFG::Worklist::ThreadBody::work()
12  0x107f9bf19 WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0::operator()() const
13  0x107f9bb59 WTF::Detail::CallableWrapper<WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0, void>::call()
14  0x1070ac5ea WTF::Function<void ()>::operator()() const
15  0x107fd8600 WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*)
16  0x10806d7d5 WTF::wtfThreadEntryPoint(void*)
17  0x7fff5b3d72eb _pthread_body
18  0x7fff5b3da249 _pthread_start
19  0x7fff5b3d640d thread_start

Roughly what seems to happen here is that during structure check hoisting, the structure check for the Uint32Array is moved to the beginning of the function and is replaced with a CheckStructureOrEmpty node (presumably due to the addition variable v5). Afterwards, AI types the argument as `Uint32Array | Empty`. This doesn't change until SpeculativeJIT lowering at which point the assertion triggers as the type of the input should just be `Uint32Array` but still is `Uint32Array | Empty`. However, I don't think the empty value can occur at this point in the program (an argument to a function). Moreover, even if it did this would probably only result in a nullpointer dereference. I'm reporting this as a security bug just in case I'm missing something here, but I do not believe this bug to be exploitable.
Comment 1 Radar WebKit Bug Importer 2019-07-22 02:44:16 PDT
<rdar://problem/53388642>
Comment 2 Saam Barati 2019-08-02 17:36:48 PDT
Thanks for the report.

This is not a security issue. Our static analysis in AI is conservative, the bug here is we're asserting that AI is precise. We shouldn't assert such things.
Comment 3 Michael Saboff 2019-08-05 10:41:48 PDT
Created attachment 375534 [details]
Patch
Comment 4 Yusuke Suzuki 2019-08-05 11:00:23 PDT
r=me too.
Comment 5 Darin Adler 2019-08-05 11:47:45 PDT
Since this is not a security issue, should we move it out of the security-sensitive component?
Comment 6 Michael Saboff 2019-08-05 13:02:53 PDT
Committed r248271: <https://trac.webkit.org/changeset/248271>