diff --git a/Source/JavaScriptCore/heap/GCOwnedDataScope.cpp b/Source/JavaScriptCore/heap/GCOwnedDataScope.cpp index c49a6ed6c5b8..ec6aaa37c64b 100644 --- a/Source/JavaScriptCore/heap/GCOwnedDataScope.cpp +++ b/Source/JavaScriptCore/heap/GCOwnedDataScope.cpp @@ -32,10 +32,16 @@ namespace JSC { #if ASSERT_ENABLED +// Heap::m_topGCOwnedDataScope tracks the scopes on the mutator's stack, and it is not synchronized. +// Heap::clearConcurrentRetainedDataIfPossible asserts it is null before it drops the strings it kept +// alive for those scopes. Compiler and GC threads have their own guards there (no ongoing compilation, +// mutator not fenced), so a scope constructed on one of them must not touch the mutator's tracker. void setTopGCOwnedDataScopeIfNeeded(const JSCell* cell, const void* scope) { if (!cell) return; + if (isCompilationThread() || Thread::mayBeGCThread()) + return; if (!cell->vm().heap.m_topGCOwnedDataScope) cell->vm().heap.m_topGCOwnedDataScope = scope; } @@ -44,6 +50,8 @@ void clearTopGCOwnedDataScopeIfNeeded(const JSCell* cell, const void* scope) { if (!cell) return; + if (isCompilationThread() || Thread::mayBeGCThread()) + return; if (cell->vm().heap.m_topGCOwnedDataScope == scope) cell->vm().heap.m_topGCOwnedDataScope = nullptr; } diff --git a/Source/JavaScriptCore/jit/JITArithmetic.cpp b/Source/JavaScriptCore/jit/JITArithmetic.cpp index 65c62783179e..37a19878e914 100644 --- a/Source/JavaScriptCore/jit/JITArithmetic.cpp +++ b/Source/JavaScriptCore/jit/JITArithmetic.cpp @@ -234,17 +234,23 @@ ALWAYS_INLINE void JIT::emit_compareImpl(VirtualRegister op1, VirtualRegister op // - constant int immediate to int immediate // - int immediate to int immediate - constexpr bool disallowAllocation = false; auto handleConstantCharOperand = [&](VirtualRegister left, VirtualRegister right, RelationalCondition cond) { if (!isOperandConstantChar(left)) return false; + // This may run on a compiler thread, so read the StringImpl directly rather than through + // tryGetValue(), whose GCOwnedDataScope is only meant to be constructed on the mutator. + // Read the character at once: the mutator may atomize the constant at any time, and a GC + // during this compile then drops the old StringImpl. A single character constant is never a rope. + const StringImpl* impl = asString(getConstantOperand(left))->tryGetValueImpl(); + RELEASE_ASSERT(impl); + char16_t character = impl->at(0); emitGetVirtualRegister(right, jsRegT10); addSlowCase(branchIfNotCell(jsRegT10)); JumpList failures; emitLoadCharacterString(jsRegT10.payloadGPR(), jsRegT10.payloadGPR(), failures); addSlowCase(failures); - emitCompare(commute(cond), jsRegT10, Imm32(asString(getConstantOperand(left))->tryGetValue(disallowAllocation).data[0])); + emitCompare(commute(cond), jsRegT10, Imm32(character)); return true; }; diff --git a/Source/JavaScriptCore/lol/LOLJIT.cpp b/Source/JavaScriptCore/lol/LOLJIT.cpp index 8b5f990d0396..c716506e505f 100644 --- a/Source/JavaScriptCore/lol/LOLJIT.cpp +++ b/Source/JavaScriptCore/lol/LOLJIT.cpp @@ -961,17 +961,23 @@ ALWAYS_INLINE void LOLJIT::emitCompareImpl(VirtualRegister op1, JSValueRegs op1R // - constant int immediate to int immediate // - int immediate to int immediate - constexpr bool disallowAllocation = false; auto handleConstantCharOperand = [&](VirtualRegister left, JSValueRegs rightRegs, RelationalCondition cond) { if (!isOperandConstantChar(left)) return false; + // This may run on a compiler thread, so read the StringImpl directly rather than through + // tryGetValue(), whose GCOwnedDataScope is only meant to be constructed on the mutator. + // Read the character at once: the mutator may atomize the constant at any time, and a GC + // during this compile then drops the old StringImpl. A single character constant is never a rope. + const StringImpl* impl = asString(getConstantOperand(left))->tryGetValueImpl(); + RELEASE_ASSERT(impl); + char16_t character = impl->at(0); addSlowCase(branchIfNotCell(rightRegs)); JumpList failures; // FIXME: We could deduplicate the String's data load in emitLoadCharacterString if we had an extra scratch but we'd have to teach the register allocator about constants to do that unless we wanted to have the scratch in all cases, which doesn't seem worth it. emitLoadCharacterString(rightRegs.payloadGPR(), s_scratch, failures); addSlowCase(failures); - emitCompare(commute(cond), JSValueRegs { s_scratch }, Imm32(asString(getConstantOperand(left))->tryGetValue(disallowAllocation).data[0])); + emitCompare(commute(cond), JSValueRegs { s_scratch }, Imm32(character)); return true; };