Bug 151892 – CRASH: CodeBlock::setOptimizationThresholdBasedOnCompilationResult + 567

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Michael Saboff]

Backtrace like:
   1 com.apple.JavaScriptCore       0x113ea02ce WTFCrash + 0x3e (Assertions.cpp:321)
>  2 com.apple.JavaScriptCore       0x1138acbb7 JSC::CodeBlock::setOptimizationThresholdBasedOnCompilationResult(JSC::CompilationResult) + 0x237 (DataLog.h:45)
   3 com.apple.JavaScriptCore       0x113c16e4c JSC::JITToDFGDeferredCompilationCallback::compilationDidComplete(JSC::CodeBlock*, JSC::CompilationResult) + 0x8c (JITToDFGDeferredCompilationCallback.cpp:69)
   4 com.apple.JavaScriptCore       0x113aac7d4 JSC::DFG::Worklist::completeAllReadyPlansForVM(JSC::VM&, JSC::DFG::CompilationKey) + 0x104 (DFGCompilationKey.h:74)
   5 com.apple.JavaScriptCore       0x113aaca0e JSC::DFG::Worklist::completeAllPlansForVM(JSC::VM&) + 0x3e (HeapInlines.h:308)
   6 com.apple.JavaScriptCore       0x113aadde8 JSC::DFG::completeAllPlansForVM(JSC::VM&) + 0x38 (DFGWorklist.cpp:453)
   7 com.apple.JavaScriptCore       0x1138ea488 JSC::Debugger::exception(JSC::ExecState*, JSC::JSValue, bool) + 0x68 (JSCInlines.h:66)
   8 com.apple.JavaScriptCore       0x113bc0cb5 JSC::Interpreter::unwind(void*&, JSC::ExecState*&, JSC::Exception*) + 0x155 (Interpreter.cpp:691)
   9 com.apple.JavaScriptCore       0x113bde07a JSC::genericUnwind(JSC::VM*, JSC::ExecState*) + 0x5a (JITExceptions.cpp:57)
  10                                0x00004e6b5a801060 0 + 86222986809440
  11 com.apple.JavaScriptCore       0x113d1ef10 llint_entry + 0x58c7
  12 com.apple.JavaScriptCore       0x113d1ef10 llint_entry + 0x58c7
  13 com.apple.JavaScriptCore       0x113d19439 vmEntryToJavaScript + 0x146
  14 com.apple.JavaScriptCore       0x113bdc7d9 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 0xa9 (JITCode.cpp:82)
  15 com.apple.JavaScriptCore       0x113bc397a JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 0x1ba (Interpreter.cpp:946)
  16 com.apple.JavaScriptCore       0x1138947a7 JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) + 0x47 (MarkedBlock.h:250)
  17 com.apple.WebCore              0x114a31dda WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext*, WebCore::Event*) + 0x3ea (JSMainThreadExecState.h:56)
  18 com.apple.WebCore              0x11467607b WebCore::EventTarget::fireEventListeners(WebCore::Event*, WebCore::EventTargetData*, WTF::Vector<WebCore::RegisteredEventListener, 1ul, WTF::CrashOnOverflow, 16ul>&) + 0x27b (InspectorInstrumentation.h:283)
  19 com.apple.WebCore              0x114675d40 WebCore::EventTarget::fireEventListeners(WebCore::Event*) + 0xe0 (EventTarget.cpp:208)
  20 com.apple.WebCore              0x114675c3d WebCore::EventTarget::dispatchEvent(WTF::PassRefPtr<WebCore::Event>) + 0x5d (PassRefPtr.h:68)
...

[Michael Saboff]

Created attachment 266679 [details]
Patch

[Mark Lam]

Comment on attachment 266679 [details]
Patch

r=me

[Michael Saboff]

Created attachment 266682 [details]
Patch for Landing

Changed the order of the "if" predicates to make it a little more defensive.  That is don't check the replacement CodeBlock until first checking that the compilation was successful.

[Michael Saboff]

I'll land without the change for https://bugs.webkit.org/show_bug.cgi?id=151893 that was inadvertently added in the last patch.

[Joseph Pecoraro]

Comment on attachment 266682 [details]
Patch for Landing

Err back to r?. I am fine with the inspector test. A JSC reviewer should review the CodeBlock changes!

[Joseph Pecoraro]

Oh, I think I accidentally stomped Mark Lam's r+ on this!

[Joseph Pecoraro]

Comment on attachment 266682 [details]
Patch for Landing

Restoring Mark Lam's r+

[Michael Saboff]

Committed r193491: <http://trac.webkit.org/changeset/193491>

[Geoffrey Garen]

I don't love this fix. A lot of things can go wrong if we throw away code while compiling. I think it would be better to wait for all compilation to terminate. I thought that's how things already worked.

[Michael Saboff]

(In reply to comment #9)
> I don't love this fix. A lot of things can go wrong if we throw away code
> while compiling. I think it would be better to wait for all compilation to
> terminate. I thought that's how things already worked.

I don't understand your concern.

We are in the process of waiting for all compilations to complete as per the backtrace frame 6.  In this case, we aren't throwing away any code, just not updating the optimization threshold.

[Michael Saboff]

After discussing with Geoff, going to take a less confusion approach to fixing this problem.

[Michael Saboff]

Created attachment 266821 [details]
Updated patch for landing

[Michael Saboff]

Committed r193674: <http://trac.webkit.org/changeset/193674>

[Michael Saboff]

*** Bug 131771 has been marked as a duplicate of this bug. ***