|
| 1 | +The following JavaScript program, found by Fuzzilli and slightly modified, crashes JavaScriptCore built from HEAD and the current stable release (/System/Library/Frameworks/JavaScriptCore.framework/Resources/jsc): |
| 2 | + |
| 3 | + let notAGetterSetter = {whatever: 42}; |
| 4 | + |
| 5 | + function v2(v5) { |
| 6 | + const v10 = Object(); |
| 7 | + if (v5) { |
| 8 | + const v12 = {set:Array}; |
| 9 | + const v14 = Object.defineProperty(v10,"length",v12); |
| 10 | + const v15 = (140899729)[140899729]; |
| 11 | + } else { |
| 12 | + v10.length = notAGetterSetter; |
| 13 | + } |
| 14 | + const v18 = new Uint8ClampedArray(49415); |
| 15 | + v18[1] = v10; |
| 16 | + const v19 = v10.length; |
| 17 | + let v20 = 0; |
| 18 | + while (v20 < 100000) { |
| 19 | + v20++; |
| 20 | + } |
| 21 | + } |
| 22 | + const v26 = v2(); |
| 23 | + for (let v32 = 0; v32 < 1000; v32++) { |
| 24 | + const v33 = v2(true); |
| 25 | + } |
| 26 | + |
| 27 | + /* |
| 28 | + Crashes with: |
| 29 | + ASSERTION FAILED: from.isCell() && from.asCell()->JSCell::inherits(*from.asCell()->vm(), std::remove_pointer<To>::type::info()) |
| 30 | + ../../Source/JavaScriptCore/runtime/JSCast.h(44) : To JSC::jsCast(JSC::JSValue) [To = JSC::GetterSetter *] |
| 31 | + 1 0x1111ada79 WTFCrash |
| 32 | + 2 0x1111ada99 WTFCrashWithSecurityImplication |
| 33 | + 3 0x10ffb8f55 JSC::GetterSetter* JSC::jsCast<JSC::GetterSetter*>(JSC::JSValue) |
| 34 | + 4 0x10ffaf820 JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::executeEffects(unsigned int, JSC::DFG::Node*) |
| 35 | + 5 0x10ff9f37b JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::execute(unsigned int) |
| 36 | + 6 0x10ff9def2 JSC::DFG::CFAPhase::performBlockCFA(JSC::DFG::BasicBlock*) |
| 37 | + 7 0x10ff9d957 JSC::DFG::CFAPhase::performForwardCFA() |
| 38 | + 8 0x10ff9d647 JSC::DFG::CFAPhase::run() |
| 39 | + 9 0x10ff9cc61 bool JSC::DFG::runAndLog<JSC::DFG::CFAPhase>(JSC::DFG::CFAPhase&) |
| 40 | + 10 0x10ff6c65b bool JSC::DFG::runPhase<JSC::DFG::CFAPhase>(JSC::DFG::Graph&) |
| 41 | + 11 0x10ff6c625 JSC::DFG::performCFA(JSC::DFG::Graph&) |
| 42 | + 12 0x110279031 JSC::DFG::Plan::compileInThreadImpl() |
| 43 | + 13 0x110274fa6 JSC::DFG::Plan::compileInThread(JSC::DFG::ThreadData*) |
| 44 | + 14 0x11052a9bb JSC::DFG::Worklist::ThreadBody::work() |
| 45 | + 15 0x1111b3c69 WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0::operator()() const |
| 46 | + 16 0x1111b38a9 WTF::Detail::CallableWrapper<WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0, void>::call() |
| 47 | + 17 0x1102c433a WTF::Function<void ()>::operator()() const |
| 48 | + 18 0x1111f0350 WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) |
| 49 | + 19 0x111285525 WTF::wtfThreadEntryPoint(void*) |
| 50 | + 20 0x7fff5a7262eb _pthread_body |
| 51 | + 21 0x7fff5a729249 _pthread_start |
| 52 | + 22 0x7fff5a72540d thread_start |
| 53 | + */ |
| 54 | + |
| 55 | +The assertion indicates that a JSCell is incorrectly downcasted to a GetterSetter [1] (a pseudo object used to implement property getters/setter). In non debug builds, a type confusion then follows. |
| 56 | + |
| 57 | +Below is my preliminary analysis of the cause of the bug. |
| 58 | + |
| 59 | +The function v2 is eventually JIT compiled by the FTL JIT compiler. Initially, it will create the following (pseudo) DFG IR for it: |
| 60 | + |
| 61 | +# Block 0 (before if-else): |
| 62 | + 44: NewObject(...) |
| 63 | + <jump to block 1 or 2 depending on v5> |
| 64 | + |
| 65 | +# Block 1 (the if part): |
| 66 | + ... <install .length property on @44> |
| 67 | + // Code for const v15 = (140899729)[140899729]; |
| 68 | + ForceOSRExit |
| 69 | + Unreachable |
| 70 | + |
| 71 | +# Block 2 (the else part) |
| 72 | + PutByOffset @44, notAGetterSetter |
| 73 | + PutStructure |
| 74 | + |
| 75 | +# Block 3 (after the if-else): |
| 76 | + ... |
| 77 | + // Code for v10.length. Due to feedback from previous executions, DFG |
| 78 | + // JIT speculates that the if branch will be taken and that it will see |
| 79 | + // v10 with a GetterSetter for .length here |
| 80 | + CheckStructure @44, structureWithLengthBeingAGetterSetter |
| 81 | + 166: GetGetterSetterByOffset @44, .length // Load the GetterSetter object from @44 |
| 82 | + 167: GetGetter @166 // Load the getter function from the GetterSetter |
| 83 | + ... |
| 84 | + |
| 85 | + |
| 86 | +Here, the end of block 1 has already been marked as unreachable due to the element load from a number which will always cause a bailout. |
| 87 | + |
| 88 | +Later, the global subexpression elimination phase [2] runs and does the following (which can be seen by enabling verbose CSE [3]): |
| 89 | + |
| 90 | +* It determines that the GetGetterSetterByOffset node loads the named property from the object @44 |
| 91 | +* It determines that this property slot is assigned in block 2 (the else block) and that this block strictly dominates the current block (meaning that the current block can only be reached through block 2) |
| 92 | + * This is now the case as block 1 does a bailout, so block 3 can never be reached from block 1 |
| 93 | +* As such, CSE replaces the GetGetterSetterByOffset operation with the constant for |notAGetterSetter| (as that is what is assigned in block 2). |
| 94 | + |
| 95 | +At this point the IR is incorrect as the input to a GetGetter operation is expected to be a GetterSetter object, but in this case it is not. During later optimizations, e.g. the AbstractInterpreter relies on that invariant and casts the input to a GetterSetter object [4]. At that point JSC crashes in debug builds with the above assertion. It might also be possible to trigger the type confusion at runtime instead of at compile time but I have not attempted that. |
| 96 | + |
| 97 | + |
| 98 | + |
| 99 | +[1] https://github.com/WebKit/webkit/blob/87064d847a0f1b22a9bb400647647fe4004a4ccd/Source/JavaScriptCore/runtime/GetterSetter.h#L43 |
| 100 | +[2] https://github.com/WebKit/webkit/blob/87064d847a0f1b22a9bb400647647fe4004a4ccd/Source/JavaScriptCore/dfg/DFGCSEPhase.h#L49 |
| 101 | +[3] https://github.com/WebKit/webkit/blob/87064d847a0f1b22a9bb400647647fe4004a4ccd/Source/JavaScriptCore/dfg/DFGCSEPhase.cpp#L51 |
| 102 | +[4] https://github.com/WebKit/webkit/blob/87064d847a0f1b22a9bb400647647fe4004a4ccd/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h#L2811 |
0 commit comments