fourthTier: Recursive deadlock in DFG::ByteCodeParser
authoroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Jul 2013 04:02:20 +0000 (04:02 +0000)
committeroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 25 Jul 2013 04:02:20 +0000 (04:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=117376

Source/JavaScriptCore:

Reviewed by Mark Hahnenberg.

Leave the lock early to prevent a deadlock beneath get().

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):

Source/WTF:

Reviewed by Mark Hahnenberg.

I've often wanted to leave a lock early. Now I have that power!

* wtf/Locker.h:
(WTF::Locker::Locker):
(WTF::Locker::~Locker):
(Locker):
(WTF::Locker::unlockEarly):
(WTF::Locker::lock):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@153216 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/CodeBlock.cpp
Source/JavaScriptCore/bytecode/CodeBlock.h
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGCapabilities.h
Source/JavaScriptCore/dfg/DFGDriver.h
Source/JavaScriptCore/dfg/DFGJITCode.cpp
Source/JavaScriptCore/heap/DFGCodeBlocks.cpp
Source/JavaScriptCore/interpreter/CallFrame.h
Source/JavaScriptCore/interpreter/Interpreter.cpp
Source/JavaScriptCore/runtime/Executable.cpp
Source/JavaScriptCore/runtime/Executable.h
Source/JavaScriptCore/runtime/ExecutionHarness.h
Source/JavaScriptCore/runtime/VM.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/Locker.h

index 96f0050..f13c3b5 100644 (file)
@@ -1,3 +1,52 @@
+2013-06-09  Mark Lam  <mark.lam@apple.com>
+
+        Fix broken no-DFG build.
+        https://bugs.webkit.org/show_bug.cgi?id=117381.
+
+        Reviewed by Geoffrey Garen.
+
+        * bytecode/CodeBlock.cpp:
+        * bytecode/CodeBlock.h:
+        (CodeBlock):
+        (ProgramCodeBlock):
+        (EvalCodeBlock):
+        (FunctionCodeBlock):
+        * dfg/DFGCapabilities.h:
+        * dfg/DFGDriver.h:
+        (JSC::DFG::tryCompile):
+        (JSC::DFG::tryCompileFunction):
+        * dfg/DFGJITCode.cpp:
+        * dfg/DFGRepatch.h:
+        (JSC::DFG::dfgResetGetByID):
+        (JSC::DFG::dfgResetPutByID):
+        * heap/DFGCodeBlocks.cpp:
+        (JSC::DFGCodeBlocks::jettison):
+        * interpreter/CallFrame.h:
+        (ExecState):
+        (JSC::ExecState::trueCallFrame):
+        * interpreter/Interpreter.cpp:
+        (JSC::getCallerInfo):
+        * runtime/Executable.cpp:
+        * runtime/Executable.h:
+        (EvalExecutable):
+        (ProgramExecutable):
+        (FunctionExecutable):
+        * runtime/ExecutionHarness.h:
+        * runtime/VM.cpp:
+        (JSC::VM::~VM):
+
+2013-06-08  Filip Pizlo  <fpizlo@apple.com>
+
+        fourthTier: Recursive deadlock in DFG::ByteCodeParser
+        https://bugs.webkit.org/show_bug.cgi?id=117376
+
+        Reviewed by Mark Hahnenberg.
+        
+        Leave the lock early to prevent a deadlock beneath get().
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::parseBlock):
+
 2013-06-08  Mark Lam  <mark.lam@apple.com>
 
         Removed bogus assertion in CallFrame::setLocationAsBytecodeOffset().
index 9286f73..59168d5 100644 (file)
@@ -2875,6 +2875,7 @@ CodeBlock* FunctionCodeBlock::replacement()
     return &static_cast<FunctionExecutable*>(ownerExecutable())->generatedBytecodeFor(m_isConstructor ? CodeForConstruct : CodeForCall);
 }
 
+#if ENABLE(DFG_JIT)
 JSObject* ProgramCodeBlock::compileOptimized(ExecState* exec, JSScope* scope, CompilationResult& result, unsigned bytecodeIndex)
 {
     if (JITCode::isHigherTier(replacement()->jitType(), jitType())) {
@@ -2919,6 +2920,7 @@ CompilationResult FunctionCodeBlock::replaceWithDeferredOptimizedCode(PassRefPtr
 {
     return static_cast<FunctionExecutable*>(ownerExecutable())->replaceWithDeferredOptimizedCodeFor(plan, m_isConstructor ? CodeForConstruct : CodeForCall);
 }
+#endif // ENABLE(DFG_JIT)
 
 DFG::CapabilityLevel ProgramCodeBlock::capabilityLevelInternal()
 {
index b9656c4..a3b3a34 100644 (file)
@@ -287,8 +287,10 @@ public:
     {
         return jitType() == JITCode::BaselineJIT;
     }
+#if ENABLE(DFG_JIT)
     virtual JSObject* compileOptimized(ExecState*, JSScope*, CompilationResult&, unsigned bytecodeIndex) = 0;
     virtual CompilationResult replaceWithDeferredOptimizedCode(PassRefPtr<DFG::Plan>) = 0;
+#endif // ENABLE(DFG_JIT)
     void jettison();
     CompilationResult jitCompile(ExecState* exec)
     {
@@ -1154,8 +1156,11 @@ public:
 
 #if ENABLE(JIT)
 protected:
+#if ENABLE(DFG_JIT)
     virtual JSObject* compileOptimized(ExecState*, JSScope*, CompilationResult&, unsigned bytecodeIndex);
     virtual CompilationResult replaceWithDeferredOptimizedCode(PassRefPtr<DFG::Plan>);
+#endif // ENABLE(DFG_JIT)
+
     virtual void jettisonImpl();
     virtual CompilationResult jitCompileImpl(ExecState*);
     virtual CodeBlock* replacement();
@@ -1180,8 +1185,11 @@ public:
     
 #if ENABLE(JIT)
 protected:
+#if ENABLE(DFG_JIT)
     virtual JSObject* compileOptimized(ExecState*, JSScope*, CompilationResult&, unsigned bytecodeIndex);
     virtual CompilationResult replaceWithDeferredOptimizedCode(PassRefPtr<DFG::Plan>);
+#endif // ENABLE(DFG_JIT)
+
     virtual void jettisonImpl();
     virtual CompilationResult jitCompileImpl(ExecState*);
     virtual CodeBlock* replacement();
@@ -1206,8 +1214,11 @@ public:
     
 #if ENABLE(JIT)
 protected:
+#if ENABLE(DFG_JIT)
     virtual JSObject* compileOptimized(ExecState*, JSScope*, CompilationResult&, unsigned bytecodeIndex);
     virtual CompilationResult replaceWithDeferredOptimizedCode(PassRefPtr<DFG::Plan>);
+#endif // ENABLE(DFG_JIT)
+
     virtual void jettisonImpl();
     virtual CompilationResult jitCompileImpl(ExecState*);
     virtual CodeBlock* replacement();
index cd9a891..c000d82 100644 (file)
@@ -3123,6 +3123,7 @@ bool ByteCodeParser::parseBlock(unsigned limit)
                 ConcurrentJITLocker locker(m_inlineStackTop->m_profiledBlock->m_lock);
                 
                 if (!putToBase->m_ready) {
+                    locker.unlockEarly();
                     addToGraph(ForceOSRExit);
                     addToGraph(Phantom, get(base));
                     addToGraph(Phantom, get(value));
index 928a178..5bd80c5 100644 (file)
 #ifndef DFGCapabilities_h
 #define DFGCapabilities_h
 
-#include "Intrinsic.h"
+#include "CodeBlock.h"
 #include "DFGCommon.h"
 #include "DFGNode.h"
 #include "Executable.h"
-#include "Options.h"
 #include "Interpreter.h"
+#include "Intrinsic.h"
+#include "Options.h"
 #include <wtf/Platform.h>
 
 namespace JSC { namespace DFG {
index 478ef98..cc7a92d 100644 (file)
@@ -46,8 +46,8 @@ CompilationResult tryCompile(ExecState*, CodeBlock*, RefPtr<JSC::JITCode>&, unsi
 CompilationResult tryCompileFunction(ExecState*, CodeBlock*, RefPtr<JSC::JITCode>&, MacroAssemblerCodePtr& jitCodeWithArityCheck, unsigned bytecodeIndex);
 CompilationResult tryFinalizePlan(PassRefPtr<Plan>, RefPtr<JSC::JITCode>&, MacroAssemblerCodePtr* jitCodeWithArityCheck);
 #else
-inline CompilationResult tryCompile(ExecState*, CodeBlock*, RefPtr<JSC::JITCode>&, unsigned) { return false; }
-inline CompilationResult tryCompileFunction(ExecState*, CodeBlock*, RefPtr<JSC::JITCode>&, MacroAssemblerCodePtr&, unsigned) { return false; }
+inline CompilationResult tryCompile(ExecState*, CodeBlock*, RefPtr<JSC::JITCode>&, unsigned) { return CompilationFailed; }
+inline CompilationResult tryCompileFunction(ExecState*, CodeBlock*, RefPtr<JSC::JITCode>&, MacroAssemblerCodePtr&, unsigned) { return CompilationFailed; }
 inline CompilationResult tryFinalizePlan(PassRefPtr<Plan>, RefPtr<JSC::JITCode>&, MacroAssemblerCodePtr*)
 {
     UNREACHABLE_FOR_PLATFORM();
index c9dbc5a..ba80553 100644 (file)
@@ -26,6 +26,8 @@
 #include "config.h"
 #include "DFGJITCode.h"
 
+#if ENABLE(DFG_JIT)
+
 namespace JSC { namespace DFG {
 
 JITCode::JITCode()
@@ -59,3 +61,4 @@ void JITCode::shrinkToFit()
 
 } } // namespace JSC::DFG
 
+#endif // ENABLE(DFG_JIT)
index 8ec1882..4871231 100644 (file)
@@ -91,7 +91,7 @@ void DFGCodeBlocks::traceMarkedCodeBlocks(SlotVisitor& visitor)
 
 #else // ENABLE(DFG_JIT)
 
-void DFGCodeBlocks::jettison(PassOwnPtr<CodeBlock>)
+void DFGCodeBlocks::jettison(PassRefPtr<CodeBlock>)
 {
 }
 
index 8c45875..9f51f39 100644 (file)
@@ -162,7 +162,9 @@ namespace JSC  {
         void setLocationAsRawBits(unsigned);
         void setLocationAsBytecodeOffset(unsigned);
 
+#if ENABLE(DFG_JIT)
         unsigned bytecodeOffsetFromCodeOriginIndex();
+#endif
 
         Register* frameExtent()
         {
@@ -282,7 +284,7 @@ namespace JSC  {
         // HostCallFrameFlag stuff.
         CallFrame* trueCallerFrame();
 #else
-        CallFrame* trueCallFrame(AbstractPC) { return this; }
+        CallFrame* trueCallFrame() { return this; }
         CallFrame* trueCallerFrame() { return callerFrame()->removeHostCallFrameFlag(); }
 #endif
         CallFrame* callerFrameNoFlags() { return callerFrame()->removeHostCallFrameFlag(); }
index ff5b8b4..79fa7cf 100644 (file)
@@ -494,9 +494,11 @@ static CallFrame* getCallerInfo(VM* vm, CallFrame* callFrame, unsigned& bytecode
     }
     
     CodeBlock* callerCodeBlock = trueCallerFrame->codeBlock();
+#if ENABLE(DFG_JIT)
     if (trueCallerFrame->hasLocationAsCodeOriginIndex())
         bytecodeOffset = trueCallerFrame->bytecodeOffsetFromCodeOriginIndex();
     else
+#endif // ENABLE(DFG_JIT)
         bytecodeOffset = trueCallerFrame->locationAsBytecodeOffset();
 
     caller = callerCodeBlock;
index 2a440db..fc26caa 100644 (file)
@@ -157,6 +157,7 @@ void FunctionExecutable::destroy(JSCell* cell)
     static_cast<FunctionExecutable*>(cell)->FunctionExecutable::~FunctionExecutable();
 }
 
+#if ENABLE(DFG_JIT)
 JSObject* EvalExecutable::compileOptimized(ExecState* exec, JSScope* scope, CompilationResult& result, unsigned bytecodeIndex)
 {
     ASSERT(exec->vm().dynamicGlobalObject);
@@ -169,6 +170,7 @@ JSObject* EvalExecutable::compileOptimized(ExecState* exec, JSScope* scope, Comp
     ASSERT(!!m_evalCodeBlock);
     return error;
 }
+#endif // ENABLE(DFG_JIT)
 
 #if ENABLE(JIT)
 CompilationResult EvalExecutable::jitCompile(ExecState* exec)
@@ -238,11 +240,13 @@ JSObject* EvalExecutable::compileInternal(ExecState* exec, JSScope* scope, JITCo
     return 0;
 }
 
+#if ENABLE(DFG_JIT)
 CompilationResult EvalExecutable::replaceWithDeferredOptimizedCode(PassRefPtr<DFG::Plan> plan)
 {
     return JSC::replaceWithDeferredOptimizedCode(
         plan, m_evalCodeBlock, m_jitCodeForCall, 0, 0);
 }
+#endif // ENABLE(DFG_JIT)
 
 #if ENABLE(JIT)
 void EvalExecutable::jettisonOptimizedCode(VM& vm)
@@ -294,6 +298,7 @@ JSObject* ProgramExecutable::checkSyntax(ExecState* exec)
     return error.toErrorObject(lexicalGlobalObject, m_source);
 }
 
+#if ENABLE(DFG_JIT)
 JSObject* ProgramExecutable::compileOptimized(ExecState* exec, JSScope* scope, CompilationResult& result, unsigned bytecodeIndex)
 {
     RELEASE_ASSERT(exec->vm().dynamicGlobalObject);
@@ -306,6 +311,7 @@ JSObject* ProgramExecutable::compileOptimized(ExecState* exec, JSScope* scope, C
     ASSERT(!!m_programCodeBlock);
     return error;
 }
+#endif // ENABLE(DFG_JIT)
 
 #if ENABLE(JIT)
 CompilationResult ProgramExecutable::jitCompile(ExecState* exec)
@@ -344,11 +350,13 @@ JSObject* ProgramExecutable::compileInternal(ExecState* exec, JSScope* scope, JI
     return 0;
 }
 
+#if ENABLE(DFG_JIT)
 CompilationResult ProgramExecutable::replaceWithDeferredOptimizedCode(PassRefPtr<DFG::Plan> plan)
 {
     return JSC::replaceWithDeferredOptimizedCode(
         plan, m_programCodeBlock, m_jitCodeForCall, 0, 0);
 }
+#endif // ENABLE(DFG_JIT)
 
 #if ENABLE(JIT)
 void ProgramExecutable::jettisonOptimizedCode(VM& vm)
@@ -468,6 +476,7 @@ FunctionCodeBlock* FunctionExecutable::baselineCodeBlockFor(CodeSpecializationKi
     return result;
 }
 
+#if ENABLE(DFG_JIT)
 JSObject* FunctionExecutable::compileOptimizedForCall(ExecState* exec, JSScope* scope, CompilationResult& result, unsigned bytecodeIndex)
 {
     RELEASE_ASSERT(exec->vm().dynamicGlobalObject);
@@ -493,6 +502,7 @@ JSObject* FunctionExecutable::compileOptimizedForConstruct(ExecState* exec, JSSc
     ASSERT(!!m_codeBlockForConstruct);
     return error;
 }
+#endif // ENABLE(DFG_JIT)
 
 #if ENABLE(JIT)
 CompilationResult FunctionExecutable::jitCompileForCall(ExecState* exec)
@@ -566,12 +576,14 @@ JSObject* FunctionExecutable::compileForCallInternal(ExecState* exec, JSScope* s
     return 0;
 }
 
+#if ENABLE(DFG_JIT)
 CompilationResult FunctionExecutable::replaceWithDeferredOptimizedCodeForCall(PassRefPtr<DFG::Plan> plan)
 {
     return JSC::replaceWithDeferredOptimizedCode(
         plan, m_codeBlockForCall, m_jitCodeForCall, &m_jitCodeForCallWithArityCheck,
         &m_numParametersForCall);
 }
+#endif // ENABLE(DFG_JIT)
 
 JSObject* FunctionExecutable::compileForConstructInternal(ExecState* exec, JSScope* scope, JITCode::JITType jitType, CompilationResult* result, unsigned bytecodeIndex)
 {
@@ -601,12 +613,14 @@ JSObject* FunctionExecutable::compileForConstructInternal(ExecState* exec, JSSco
     return 0;
 }
 
+#if ENABLE(DFG_JIT)
 CompilationResult FunctionExecutable::replaceWithDeferredOptimizedCodeForConstruct(PassRefPtr<DFG::Plan> plan)
 {
     return JSC::replaceWithDeferredOptimizedCode(
         plan, m_codeBlockForConstruct, m_jitCodeForConstruct,
         &m_jitCodeForConstructWithArityCheck, &m_numParametersForConstruct);
 }
+#endif // ENABLE(DFG_JIT)
 
 #if ENABLE(JIT)
 void FunctionExecutable::jettisonOptimizedCodeForCall(VM& vm)
index 8afc2a5..a38fa54 100644 (file)
@@ -442,8 +442,10 @@ namespace JSC {
             return error;
         }
         
+#if ENABLE(DFG_JIT)
         JSObject* compileOptimized(ExecState*, JSScope*, CompilationResult&, unsigned bytecodeIndex);
         CompilationResult replaceWithDeferredOptimizedCode(PassRefPtr<DFG::Plan>);
+#endif // ENABLE(DFG_JIT)
         
 #if ENABLE(JIT)
         void jettisonOptimizedCode(VM&);
@@ -521,8 +523,10 @@ namespace JSC {
             return error;
         }
 
+#if ENABLE(DFG_JIT)
         JSObject* compileOptimized(ExecState*, JSScope*, CompilationResult&, unsigned bytecodeIndex);
         CompilationResult replaceWithDeferredOptimizedCode(PassRefPtr<DFG::Plan>);
+#endif // ENABLE(DFG_JIT)
         
 #if ENABLE(JIT)
         void jettisonOptimizedCode(VM&);
@@ -617,8 +621,10 @@ namespace JSC {
             return error;
         }
 
+#if ENABLE(DFG_JIT)
         JSObject* compileOptimizedForCall(ExecState*, JSScope*, CompilationResult&, unsigned bytecodeIndex);
         CompilationResult replaceWithDeferredOptimizedCodeForCall(PassRefPtr<DFG::Plan>);
+#endif // ENABLE(DFG_JIT)
         
 #if ENABLE(JIT)
         void jettisonOptimizedCodeForCall(VM&);
@@ -646,8 +652,10 @@ namespace JSC {
             return error;
         }
 
+#if ENABLE(DFG_JIT)
         JSObject* compileOptimizedForConstruct(ExecState*, JSScope*, CompilationResult&, unsigned bytecodeIndex);
         CompilationResult replaceWithDeferredOptimizedCodeForConstruct(PassRefPtr<DFG::Plan>);
+#endif // ENABLE(DFG_JIT)
         
 #if ENABLE(JIT)
         void jettisonOptimizedCodeForConstruct(VM&);
@@ -677,6 +685,7 @@ namespace JSC {
             return compileForConstruct(exec, scope);
         }
         
+#if ENABLE(DFG_JIT)
         JSObject* compileOptimizedFor(ExecState* exec, JSScope* scope, CompilationResult& result, unsigned bytecodeIndex, CodeSpecializationKind kind)
         {
             ASSERT(exec->callee());
@@ -695,6 +704,7 @@ namespace JSC {
                 return replaceWithDeferredOptimizedCodeForCall(plan);
             return replaceWithDeferredOptimizedCodeForConstruct(plan);
         }
+#endif // ENABLE(DFG_JIT)
 
 #if ENABLE(JIT)
         void jettisonOptimizedCodeFor(VM& vm, CodeSpecializationKind kind)
index 78e7330..ebe0244 100644 (file)
@@ -139,6 +139,7 @@ inline CompilationResult prepareFunctionForExecution(
         sink, codeBlock, jitCode, jitCodeWithArityCheck, &numParameters);
 }
 
+#if ENABLE(DFG_JIT)
 template<typename CodeBlockType>
 inline CompilationResult replaceWithDeferredOptimizedCode(
     PassRefPtr<DFG::Plan> passedPlan, RefPtr<CodeBlockType>& sink, RefPtr<JITCode>& jitCode,
@@ -158,6 +159,7 @@ inline CompilationResult replaceWithDeferredOptimizedCode(
         jitCodeWithArityCheck ? *jitCodeWithArityCheck : MacroAssemblerCodePtr(),
         numParameters);
 }
+#endif // ENABLE(DFG_JIT)
 
 } // namespace JSC
 
index bcf959a..3a0a4d0 100644 (file)
@@ -266,12 +266,14 @@ VM::VM(VMType vmType, HeapType heapType)
 
 VM::~VM()
 {
+#if ENABLE(DFG_JIT)
     // Make sure concurrent compilations are done, but don't install them, since installing
     // them might cause a GC. We don't want to GC right now.
     if (worklist) {
         worklist->waitUntilAllPlansForVMAreReady(*this);
         worklist->removeAllReadyPlansForVM(*this);
     }
+#endif // ENABLE(DFG_JIT)
     
     // Clear this first to ensure that nobody tries to remove themselves from it.
     m_perBytecodeProfiler.clear();
index 85b9a32..dc3a5ea 100644 (file)
@@ -1,3 +1,19 @@
+2013-06-08  Filip Pizlo  <fpizlo@apple.com>
+
+        fourthTier: Recursive deadlock in DFG::ByteCodeParser
+        https://bugs.webkit.org/show_bug.cgi?id=117376
+
+        Reviewed by Mark Hahnenberg.
+        
+        I've often wanted to leave a lock early. Now I have that power!
+
+        * wtf/Locker.h:
+        (WTF::Locker::Locker):
+        (WTF::Locker::~Locker):
+        (Locker):
+        (WTF::Locker::unlockEarly):
+        (WTF::Locker::lock):
+
 2013-05-27  Filip Pizlo  <fpizlo@apple.com>
 
         It should be possible to record heap operations (both FastMalloc and JSC GC)
index c465b99..ad88546 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2008, 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -35,10 +35,27 @@ namespace WTF {
 template <typename T> class Locker {
     WTF_MAKE_NONCOPYABLE(Locker);
 public:
-    Locker(T& lockable) : m_lockable(lockable) { m_lockable.lock(); }
-    ~Locker() { m_lockable.unlock(); }
+    explicit Locker(T& lockable) : m_lockable(&lockable) { lock(); }
+    explicit Locker(T* lockable) : m_lockable(lockable) { lock(); }
+    ~Locker()
+    {
+        if (m_lockable)
+            m_lockable->unlock();
+    }
+    
+    void unlockEarly()
+    {
+        m_lockable->unlock();
+        m_lockable = 0;
+    }
 private:
-    T& m_lockable;
+    void lock()
+    {
+        if (m_lockable)
+            m_lockable->lock();
+    }
+    
+    T* m_lockable;
 };
 
 }