[WebAssembly] Fix LLIntCallee's ownership
https://bugs.webkit.org/show_bug.cgi?id=204929
Reviewed by Saam Barati.
Currently, after the LLIntPlan finished generating bytecode, the Module takes ownership of the Vector
of LLIntCallee's and passes a pointer to the Vector's storage to the CodeBlock. However, while we're
tiering up, the module might be destroyed and we'll try to access the LLIntCallee after we finish
compiling through the pointer held by the CodeBlock, which is now stale, since the Vector was owned
by the Module. In order to fix this, we move the Vector into a reference counted wrapper class, LLIntCallees,
and both the Module and the CodeBlock hold references to the wrapper.
* wasm/WasmBBQPlan.cpp:
(JSC::Wasm::BBQPlan::work):
* wasm/WasmCallee.h:
(JSC::Wasm::LLIntCallees::create):
(JSC::Wasm::LLIntCallees::at const):
(JSC::Wasm::LLIntCallees::data const):
(JSC::Wasm::LLIntCallees::LLIntCallees):
* wasm/WasmCodeBlock.cpp:
(JSC::Wasm::CodeBlock::create):
(JSC::Wasm::CodeBlock::CodeBlock):
* wasm/WasmCodeBlock.h:
(JSC::Wasm::CodeBlock::wasmEntrypointCalleeFromFunctionIndexSpace):
* wasm/WasmModule.cpp:
(JSC::Wasm::Module::Module):
(JSC::Wasm::Module::getOrCreateCodeBlock):
* wasm/WasmModule.h:
* wasm/WasmOMGPlan.cpp:
(JSC::Wasm::OMGPlan::work):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@253188 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index e346f0f..5ca44b1 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,5 +1,38 @@
2019-12-05 Tadeu Zagallo <tzagallo@apple.com>
+ [WebAssembly] Fix LLIntCallee's ownership
+ https://bugs.webkit.org/show_bug.cgi?id=204929
+
+ Reviewed by Saam Barati.
+
+ Currently, after the LLIntPlan finished generating bytecode, the Module takes ownership of the Vector
+ of LLIntCallee's and passes a pointer to the Vector's storage to the CodeBlock. However, while we're
+ tiering up, the module might be destroyed and we'll try to access the LLIntCallee after we finish
+ compiling through the pointer held by the CodeBlock, which is now stale, since the Vector was owned
+ by the Module. In order to fix this, we move the Vector into a reference counted wrapper class, LLIntCallees,
+ and both the Module and the CodeBlock hold references to the wrapper.
+
+ * wasm/WasmBBQPlan.cpp:
+ (JSC::Wasm::BBQPlan::work):
+ * wasm/WasmCallee.h:
+ (JSC::Wasm::LLIntCallees::create):
+ (JSC::Wasm::LLIntCallees::at const):
+ (JSC::Wasm::LLIntCallees::data const):
+ (JSC::Wasm::LLIntCallees::LLIntCallees):
+ * wasm/WasmCodeBlock.cpp:
+ (JSC::Wasm::CodeBlock::create):
+ (JSC::Wasm::CodeBlock::CodeBlock):
+ * wasm/WasmCodeBlock.h:
+ (JSC::Wasm::CodeBlock::wasmEntrypointCalleeFromFunctionIndexSpace):
+ * wasm/WasmModule.cpp:
+ (JSC::Wasm::Module::Module):
+ (JSC::Wasm::Module::getOrCreateCodeBlock):
+ * wasm/WasmModule.h:
+ * wasm/WasmOMGPlan.cpp:
+ (JSC::Wasm::OMGPlan::work):
+
+2019-12-05 Tadeu Zagallo <tzagallo@apple.com>
+
REGRESSION(r253140): Wasm::FunctionParser needs to bounds check in SetLocal/TeeLocal
https://bugs.webkit.org/show_bug.cgi?id=204909
diff --git a/Source/JavaScriptCore/wasm/WasmBBQPlan.cpp b/Source/JavaScriptCore/wasm/WasmBBQPlan.cpp
index bc92b6d..b95f7ff 100644
--- a/Source/JavaScriptCore/wasm/WasmBBQPlan.cpp
+++ b/Source/JavaScriptCore/wasm/WasmBBQPlan.cpp
@@ -133,7 +133,7 @@
LockHolder holder(m_codeBlock->m_lock);
m_codeBlock->m_bbqCallees[m_functionIndex] = callee.copyRef();
{
- LLIntCallee& llintCallee = m_codeBlock->m_llintCallees[m_functionIndex].get();
+ LLIntCallee& llintCallee = m_codeBlock->m_llintCallees->at(m_functionIndex).get();
auto locker = holdLock(llintCallee.tierUpCounter().m_lock);
llintCallee.setReplacement(callee.copyRef());
llintCallee.tierUpCounter().m_compilationStatus = LLIntTierUpCounter::CompilationStatus::Compiled;
@@ -172,7 +172,7 @@
for (unsigned i = 0; i < m_codeBlock->m_wasmToWasmCallsites.size(); ++i) {
repatchCalls(m_codeBlock->m_wasmToWasmCallsites[i]);
if (m_codeBlock->m_llintCallees) {
- LLIntCallee& llintCallee = m_codeBlock->m_llintCallees[i].get();
+ LLIntCallee& llintCallee = m_codeBlock->m_llintCallees->at(i).get();
if (JITCallee* replacementCallee = llintCallee.replacement())
repatchCalls(replacementCallee->wasmToWasmCallsites());
if (OMGForOSREntryCallee* osrEntryCallee = llintCallee.osrEntryCallee())
diff --git a/Source/JavaScriptCore/wasm/WasmCallee.h b/Source/JavaScriptCore/wasm/WasmCallee.h
index e0d22e7..3a49c5f 100644
--- a/Source/JavaScriptCore/wasm/WasmCallee.h
+++ b/Source/JavaScriptCore/wasm/WasmCallee.h
@@ -215,6 +215,7 @@
: Callee(Wasm::CompilationMode::LLIntMode, index, WTFMove(name))
, m_codeBlock(WTFMove(codeBlock))
{
+ RELEASE_ASSERT(m_codeBlock);
}
RefPtr<JITCallee> m_replacement;
@@ -223,6 +224,32 @@
MacroAssemblerCodePtr<WasmEntryPtrTag> m_entrypoint;
};
+class LLIntCallees : public ThreadSafeRefCounted<LLIntCallees> {
+public:
+ static Ref<LLIntCallees> create(Vector<Ref<LLIntCallee>>&& llintCallees)
+ {
+ return adoptRef(*new LLIntCallees(WTFMove(llintCallees)));
+ }
+
+ const Ref<LLIntCallee>& at(unsigned i) const
+ {
+ return m_llintCallees.at(i);
+ }
+
+ const Ref<LLIntCallee>* data() const
+ {
+ return m_llintCallees.data();
+ }
+
+private:
+ LLIntCallees(Vector<Ref<LLIntCallee>>&& llintCallees)
+ : m_llintCallees(WTFMove(llintCallees))
+ {
+ }
+
+ Vector<Ref<LLIntCallee>> m_llintCallees;
+};
+
} } // namespace JSC::Wasm
#endif // ENABLE(WEBASSEMBLY)
diff --git a/Source/JavaScriptCore/wasm/WasmCodeBlock.cpp b/Source/JavaScriptCore/wasm/WasmCodeBlock.cpp
index 134da7d..e9771a7 100644
--- a/Source/JavaScriptCore/wasm/WasmCodeBlock.cpp
+++ b/Source/JavaScriptCore/wasm/WasmCodeBlock.cpp
@@ -36,13 +36,13 @@
namespace JSC { namespace Wasm {
-Ref<CodeBlock> CodeBlock::create(Context* context, MemoryMode mode, ModuleInformation& moduleInformation, const Ref<LLIntCallee>* llintCallees)
+Ref<CodeBlock> CodeBlock::create(Context* context, MemoryMode mode, ModuleInformation& moduleInformation, RefPtr<LLIntCallees> llintCallees)
{
auto* result = new (NotNull, fastMalloc(sizeof(CodeBlock))) CodeBlock(context, mode, moduleInformation, llintCallees);
return adoptRef(*result);
}
-CodeBlock::CodeBlock(Context* context, MemoryMode mode, ModuleInformation& moduleInformation, const Ref<LLIntCallee>* llintCallees)
+CodeBlock::CodeBlock(Context* context, MemoryMode mode, ModuleInformation& moduleInformation, RefPtr<LLIntCallees> llintCallees)
: m_calleeCount(moduleInformation.internalFunctionCount())
, m_mode(mode)
, m_llintCallees(llintCallees)
@@ -50,7 +50,7 @@
RefPtr<CodeBlock> protectedThis = this;
if (Options::useWasmLLInt()) {
- m_plan = adoptRef(*new LLIntPlan(context, makeRef(moduleInformation), m_llintCallees, createSharedTask<Plan::CallbackType>([this, protectedThis = WTFMove(protectedThis)] (Plan&) {
+ m_plan = adoptRef(*new LLIntPlan(context, makeRef(moduleInformation), m_llintCallees->data(), createSharedTask<Plan::CallbackType>([this, protectedThis = WTFMove(protectedThis)] (Plan&) {
auto locker = holdLock(m_lock);
if (m_plan->failed()) {
m_errorMessage = m_plan->errorMessage();
@@ -64,7 +64,7 @@
m_wasmIndirectCallEntryPoints.resize(m_calleeCount);
for (unsigned i = 0; i < m_calleeCount; ++i)
- m_wasmIndirectCallEntryPoints[i] = m_llintCallees[i]->entrypoint();
+ m_wasmIndirectCallEntryPoints[i] = m_llintCallees->at(i)->entrypoint();
m_wasmToWasmExitStubs = m_plan->takeWasmToWasmExitStubs();
m_wasmToWasmCallsites = m_plan->takeWasmToWasmCallsites();
diff --git a/Source/JavaScriptCore/wasm/WasmCodeBlock.h b/Source/JavaScriptCore/wasm/WasmCodeBlock.h
index 01547fd..d7ec06e 100644
--- a/Source/JavaScriptCore/wasm/WasmCodeBlock.h
+++ b/Source/JavaScriptCore/wasm/WasmCodeBlock.h
@@ -53,7 +53,7 @@
public:
typedef void CallbackType(Ref<CodeBlock>&&);
using AsyncCompilationCallback = RefPtr<WTF::SharedTask<CallbackType>>;
- static Ref<CodeBlock> create(Context*, MemoryMode, ModuleInformation&, const Ref<LLIntCallee>*);
+ static Ref<CodeBlock> create(Context*, MemoryMode, ModuleInformation&, RefPtr<LLIntCallees>);
void waitUntilFinished();
void compileAsync(Context*, AsyncCompilationCallback&&);
@@ -95,7 +95,7 @@
return *m_omgCallees[calleeIndex].get();
if (m_bbqCallees[calleeIndex])
return *m_bbqCallees[calleeIndex].get();
- return m_llintCallees[calleeIndex].get();
+ return m_llintCallees->at(calleeIndex).get();
}
BBQCallee& wasmBBQCalleeFromFunctionIndexSpace(unsigned functionIndexSpace)
@@ -128,13 +128,13 @@
friend class OMGPlan;
friend class OMGForOSREntryPlan;
- CodeBlock(Context*, MemoryMode, ModuleInformation&, const Ref<LLIntCallee>*);
+ CodeBlock(Context*, MemoryMode, ModuleInformation&, RefPtr<LLIntCallees>);
void setCompilationFinished();
unsigned m_calleeCount;
MemoryMode m_mode;
Vector<RefPtr<OMGCallee>> m_omgCallees;
Vector<RefPtr<BBQCallee>> m_bbqCallees;
- const Ref<LLIntCallee>* m_llintCallees;
+ RefPtr<LLIntCallees> m_llintCallees;
HashMap<uint32_t, RefPtr<EmbedderEntrypointCallee>, typename DefaultHash<uint32_t>::Hash, WTF::UnsignedWithZeroKeyHashTraits<uint32_t>> m_embedderCallees;
Vector<MacroAssemblerCodePtr<WasmEntryPtrTag>> m_wasmIndirectCallEntryPoints;
Vector<Vector<UnlinkedWasmToWasmCall>> m_wasmToWasmCallsites;
diff --git a/Source/JavaScriptCore/wasm/WasmModule.cpp b/Source/JavaScriptCore/wasm/WasmModule.cpp
index 3c8432e..db947f3 100644
--- a/Source/JavaScriptCore/wasm/WasmModule.cpp
+++ b/Source/JavaScriptCore/wasm/WasmModule.cpp
@@ -37,7 +37,7 @@
Module::Module(LLIntPlan& plan)
: m_moduleInformation(plan.takeModuleInformation())
- , m_llintCallees(plan.takeCallees())
+ , m_llintCallees(LLIntCallees::create(plan.takeCallees()))
, m_llintEntryThunks(plan.takeEntryThunks())
{
}
@@ -108,9 +108,9 @@
// FIXME: We might want to back off retrying at some point:
// https://bugs.webkit.org/show_bug.cgi?id=170607
if (!codeBlock || (codeBlock->compilationFinished() && !codeBlock->runnable())) {
- const Ref<LLIntCallee>* llintCallees = nullptr;
+ RefPtr<LLIntCallees> llintCallees = nullptr;
if (Options::useWasmLLInt())
- llintCallees = m_llintCallees.data();
+ llintCallees = m_llintCallees;
codeBlock = CodeBlock::create(context, mode, const_cast<ModuleInformation&>(moduleInformation()), llintCallees);
m_codeBlocks[static_cast<uint8_t>(mode)] = codeBlock;
}
diff --git a/Source/JavaScriptCore/wasm/WasmModule.h b/Source/JavaScriptCore/wasm/WasmModule.h
index 2dadb6a..4f63a5d 100644
--- a/Source/JavaScriptCore/wasm/WasmModule.h
+++ b/Source/JavaScriptCore/wasm/WasmModule.h
@@ -79,7 +79,7 @@
Module(LLIntPlan&);
Ref<ModuleInformation> m_moduleInformation;
RefPtr<CodeBlock> m_codeBlocks[Wasm::NumberOfMemoryModes];
- Vector<Ref<LLIntCallee>> m_llintCallees;
+ RefPtr<LLIntCallees> m_llintCallees;
MacroAssemblerCodeRef<B3CompilationPtrTag> m_llintEntryThunks;
Lock m_lock;
};
diff --git a/Source/JavaScriptCore/wasm/WasmOMGPlan.cpp b/Source/JavaScriptCore/wasm/WasmOMGPlan.cpp
index 5d49c3e..6e5ce71 100644
--- a/Source/JavaScriptCore/wasm/WasmOMGPlan.cpp
+++ b/Source/JavaScriptCore/wasm/WasmOMGPlan.cpp
@@ -123,7 +123,7 @@
bbqCallee->tierUpCount()->m_compilationStatusForOMG = TierUpCount::CompilationStatus::Compiled;
}
if (m_codeBlock->m_llintCallees) {
- LLIntCallee& llintCallee = m_codeBlock->m_llintCallees[m_functionIndex].get();
+ LLIntCallee& llintCallee = m_codeBlock->m_llintCallees->at(m_functionIndex).get();
auto locker = holdLock(llintCallee.tierUpCounter().m_lock);
llintCallee.setReplacement(callee.copyRef());
llintCallee.tierUpCounter().m_compilationStatus = LLIntTierUpCounter::CompilationStatus::Compiled;
@@ -163,7 +163,7 @@
for (unsigned i = 0; i < m_codeBlock->m_wasmToWasmCallsites.size(); ++i) {
repatchCalls(m_codeBlock->m_wasmToWasmCallsites[i]);
if (m_codeBlock->m_llintCallees) {
- LLIntCallee& llintCallee = m_codeBlock->m_llintCallees[i].get();
+ LLIntCallee& llintCallee = m_codeBlock->m_llintCallees->at(i).get();
if (JITCallee* replacementCallee = llintCallee.replacement())
repatchCalls(replacementCallee->wasmToWasmCallsites());
if (OMGForOSREntryCallee* osrEntryCallee = llintCallee.osrEntryCallee())
