Skip to content

Commit c9c2f34

Browse files
committed
[JSC] Store Wasm::Instance* in |codeBlock| slot
https://bugs.webkit.org/show_bug.cgi?id=250822 rdar://104410328 Reviewed by Mark Lam. This patch fixes wasm calling convention problems. The new one becomes much simpler and robust (and fixing existing bugs). 1. We remove vm.wasmContext.instance field. Previously, it was randomly configured and a lot code is missing the update of this thing (for example, wasm context switching inside wasm -> wasm call missed the update). We should not have this kind of "global" variable and instead we should query to CallFrame about Wasm::Instance* of the current CallFrame*. 2. Each wasm function stores Wasm::Instance* to the CallFrame. We use |codeBlock| slot for that purpose. Since it is next to |callee| slot, we can use storePairPtr in ARM64, no code size increase. Plus, because we are already writing |callee| slot, it does not add new performance problem. This change makes CallFrame::lexicalGlobalObjectFromWasmCallee super simple since we can just get this slot from CallFrame instead of getting it from vm.wasmContext.instance. And it also makes Interpreter::unwind much simpler since we no longer need to book-keep this variable in unwinding case. And this also contributes to code size reduction in Wasm IC since we no longer need to have store and load code for vm.wasmContext.instance. 3. We use |codeBlock| slot for Wasm::Instance*. However, Wasm LLInt already used it for Wasm::Callee* (wasm's codeblock). This patch allocates 2 internal slots for Wasm::LLInt code and use this slot for Wasm::Callee* instead. This slot is "WasmCodeBlock". 4. We revise the use of |this| slot for wasm functions. Previously, it was randomly used. But we use this slot to anchor JSWebAssemblyInstance from conservative GC root. While we are keeping Wasm::Instance* in the stack, it is not GC-managed cell. To keep Wasm functions alive while running, we need to anchor JSWebAssemblyInstance* from conservative GC root. We use this slot in three cases. (1) Calling wasm function from JS world / C++ world so that we need to keep wasm function alive. (2) We wasm-tail-call to a new function. Since tail-call can wipe the previous frame, if it is the entrance frame created by (1), we miss the anchor. Conservatively, we always store this cell in tail-call case. (3) And we do this when calling wasm function indirectly. This could switch to a new wasm instance, so we should keep a new instance anchored from the stack. This change wipes a lot of weird things in wasm and makes calling convention much simpler. * Source/JavaScriptCore/interpreter/Interpreter.cpp: (JSC::UnwindFunctor::operator() const): * Source/JavaScriptCore/interpreter/ProtoCallFrame.h: * Source/JavaScriptCore/llint/WebAssembly.asm: * Source/JavaScriptCore/wasm/WasmAirIRGeneratorBase.h: (JSC::Wasm::ExpressionType>::AirIRGeneratorBase): * Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp: (JSC::Wasm::B3IRGenerator::insertConstants): * Source/JavaScriptCore/wasm/WasmCallingConvention.h: * Source/JavaScriptCore/wasm/WasmIRGeneratorHelpers.h: (JSC::Wasm::emitCatchPrologueShared): * Source/JavaScriptCore/wasm/js/JSToWasm.cpp: (JSC::Wasm::createJSToWasmWrapper): * Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): Canonical link: https://commits.webkit.org/259139@main
1 parent cf1e882 commit c9c2f34

27 files changed

Lines changed: 218 additions & 307 deletions

Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,7 +1220,6 @@
12201220
7311FA32240DB1D3003D48DB /* DeleteByVariant.h in Headers */ = {isa = PBXBuildFile; fileRef = 7311FA31240DB1D3003D48DB /* DeleteByVariant.h */; };
12211221
734B655523F5C10400A069D1 /* DeletePropertySlot.h in Headers */ = {isa = PBXBuildFile; fileRef = 734B655423F4A33100A069D1 /* DeletePropertySlot.h */; settings = {ATTRIBUTES = (Private, ); }; };
12221222
73AD062923FF662600F53593 /* DeleteByStatus.h in Headers */ = {isa = PBXBuildFile; fileRef = 73AD062823FF662600F53593 /* DeleteByStatus.h */; };
1223-
7593C898BE714A64BE93A6E7 /* WasmContextInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = A27958D7FA1142B0AC9E364D /* WasmContextInlines.h */; settings = {ATTRIBUTES = (Private, ); }; };
12241223
790081391E95A8EC0052D7CD /* WasmModule.h in Headers */ = {isa = PBXBuildFile; fileRef = 790081371E95A8EC0052D7CD /* WasmModule.h */; settings = {ATTRIBUTES = (Private, ); }; };
12251224
7905BB691D12050E0019FE57 /* InlineAccess.h in Headers */ = {isa = PBXBuildFile; fileRef = 7905BB671D12050E0019FE57 /* InlineAccess.h */; settings = {ATTRIBUTES = (Private, ); }; };
12261225
79160DBE1C8E3EC8008C085A /* ProxyRevoke.h in Headers */ = {isa = PBXBuildFile; fileRef = 79160DBC1C8E3EC8008C085A /* ProxyRevoke.h */; settings = {ATTRIBUTES = (Private, ); }; };
@@ -4718,7 +4717,6 @@
47184717
A1D792FB1B43864B004516F5 /* IntlNumberFormatPrototype.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = IntlNumberFormatPrototype.h; sourceTree = "<group>"; };
47194718
A1E0451B1C25B4B100BB663C /* StringPrototype.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = StringPrototype.js; sourceTree = "<group>"; };
47204719
A1FE1EB01C2C537E00A289FF /* DatePrototype.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = DatePrototype.js; sourceTree = "<group>"; };
4721-
A27958D7FA1142B0AC9E364D /* WasmContextInlines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WasmContextInlines.h; sourceTree = "<group>"; };
47224720
A321AA6C2626359B0023ADA2 /* IntlWorkaround.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = IntlWorkaround.h; sourceTree = "<group>"; };
47234721
A37619402625127C00CBCBA9 /* IntlWorkaround.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = IntlWorkaround.cpp; sourceTree = "<group>"; };
47244722
A38120AE28ADAD4A008064D5 /* TemporalPlainDateTimeConstructor.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = TemporalPlainDateTimeConstructor.cpp; sourceTree = "<group>"; };
@@ -7501,7 +7499,6 @@
75017499
E3BD2B7522F275020011765C /* WasmCompilationMode.h */,
75027500
E3915C062309682900CB2561 /* WasmContext.cpp */,
75037501
AD412B321E7B2E8A008AF157 /* WasmContext.h */,
7504-
A27958D7FA1142B0AC9E364D /* WasmContextInlines.h */,
75057502
E36CC9462086314F0051FFD6 /* WasmCreationMode.h */,
75067503
AD5C36DC1F688B5F000BCAAF /* WasmEmbedder.h */,
75077504
14AB0C91231747B6000250BC /* WasmEntryPlan.cpp */,
@@ -11396,7 +11393,6 @@
1139611393
E337B967224324EA0093A820 /* WasmCapabilities.h in Headers */,
1139711394
E3BD2B7622F275020011765C /* WasmCompilationMode.h in Headers */,
1139811395
AD412B341E7B2E9E008AF157 /* WasmContext.h in Headers */,
11399-
7593C898BE714A64BE93A6E7 /* WasmContextInlines.h in Headers */,
1140011396
E36CC9472086314F0051FFD6 /* WasmCreationMode.h in Headers */,
1140111397
AD5C36DD1F688B65000BCAAF /* WasmEmbedder.h in Headers */,
1140211398
79DAE27A1E03C82200B526AA /* WasmExceptionType.h in Headers */,

Source/JavaScriptCore/interpreter/CallFrame.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
#include "LLIntPCRanges.h"
3535
#include "VMEntryRecord.h"
3636
#include "VMEntryScope.h"
37-
#include "WasmContextInlines.h"
37+
#include "WasmContext.h"
3838
#include "WasmInstance.h"
3939
#include <wtf/StringPrintStream.h>
4040

@@ -351,9 +351,9 @@ void CallFrame::convertToStackOverflowFrame(VM& vm, CodeBlock* codeBlockToKeepAl
351351
}
352352

353353
#if ENABLE(WEBASSEMBLY)
354-
JSGlobalObject* CallFrame::lexicalGlobalObjectFromWasmCallee(VM& vm) const
354+
JSGlobalObject* CallFrame::lexicalGlobalObjectFromWasmCallee(VM&) const
355355
{
356-
return vm.wasmContext.load()->owner<JSWebAssemblyInstance>()->globalObject();
356+
return wasmInstance()->owner<JSWebAssemblyInstance>()->globalObject();
357357
}
358358
#endif
359359

Source/JavaScriptCore/interpreter/CallFrame.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@
3131
#include <wtf/EnumClassOperatorOverloads.h>
3232

3333
namespace JSC {
34+
namespace Wasm {
35+
class Instance;
36+
}
3437

3538
template<typename> struct BaseInstruction;
3639
struct JSOpcodeTraits;
@@ -217,6 +220,11 @@ using JSInstruction = BaseInstruction<JSOpcodeTraits>;
217220
unsigned unsafeCallSiteAsRawBits() const;
218221
CallSiteIndex callSiteIndex() const;
219222
CallSiteIndex unsafeCallSiteIndex() const;
223+
224+
#if ENABLE(WEBASSEMBLY)
225+
Wasm::Instance* wasmInstance() const;
226+
#endif
227+
220228
private:
221229
unsigned callSiteBitsAsBytecodeOffset() const;
222230
#if ENABLE(WEBASSEMBLY)

Source/JavaScriptCore/interpreter/CallFrameInlines.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,14 @@ inline JSGlobalObject* CallFrame::lexicalGlobalObject(VM& vm) const
7878
return jsCallee()->globalObject();
7979
}
8080

81+
#if ENABLE(WEBASSEMBLY)
82+
inline Wasm::Instance* CallFrame::wasmInstance() const
83+
{
84+
ASSERT(callee().isWasm());
85+
return bitwise_cast<Wasm::Instance*>(const_cast<CallFrame*>(this)->uncheckedR(CallFrameSlot::codeBlock).asanUnsafePointer());
86+
}
87+
#endif
88+
8189
inline bool CallFrame::isStackOverflowFrame() const
8290
{
8391
if (callee().isWasm())

Source/JavaScriptCore/interpreter/Interpreter.cpp

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@
7979

8080
#if ENABLE(WEBASSEMBLY)
8181
#include "JSWebAssemblyInstance.h"
82-
#include "WasmContextInlines.h"
82+
#include "WasmContext.h"
8383
#include "WebAssemblyFunction.h"
8484
#endif
8585

@@ -605,9 +605,9 @@ class UnwindFunctor {
605605
if (m_catchableFromWasm && callee.isWasm()) {
606606
Wasm::Callee* wasmCallee = callee.asWasmCallee();
607607
if (wasmCallee->hasExceptionHandlers()) {
608-
JSWebAssemblyInstance* jsInstance = jsCast<JSWebAssemblyInstance*>(m_callFrame->thisValue());
608+
Wasm::Instance* instance = m_callFrame->wasmInstance();
609609
unsigned exceptionHandlerIndex = m_callFrame->callSiteIndex().bits();
610-
m_handler = { wasmCallee->handlerForIndex(jsInstance->instance(), exceptionHandlerIndex, m_wasmTag), wasmCallee };
610+
m_handler = { wasmCallee->handlerForIndex(*instance, exceptionHandlerIndex, m_wasmTag), wasmCallee };
611611
if (m_handler.m_valid)
612612
return IterationStatus::Done;
613613
}
@@ -622,14 +622,6 @@ class UnwindFunctor {
622622

623623
notifyDebuggerOfUnwinding(m_vm, m_callFrame);
624624

625-
#if ENABLE(WEBASSEMBLY)
626-
if (callee.isWasm()) {
627-
Wasm::Callee* wasmCallee = callee.asWasmCallee();
628-
if (wasmCallee->compilationMode() == Wasm::CompilationMode::JSToWasmICMode)
629-
m_vm.wasmContext.store(static_cast<Wasm::JSToWasmICCallee*>(wasmCallee)->previousInstance(m_callFrame));
630-
}
631-
#endif
632-
633625
copyCalleeSavesToEntryFrameCalleeSavesBuffer(visitor);
634626

635627
bool shouldStopUnwinding = visitor->callerIsEntryFrame();

Source/JavaScriptCore/interpreter/ProtoCallFrame.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,13 @@ struct JS_EXPORT_PRIVATE ProtoCallFrame {
6969
JSValue thisValue() const { return thisArg.Register::jsValue(); }
7070
void setThisValue(JSValue value) { thisArg = value; }
7171

72+
#if ENABLE(WEBASSEMBLY)
73+
void setWasmInstance(Wasm::Instance* instance)
74+
{
75+
codeBlockValue = bitwise_cast<CallFrame*>(instance);
76+
}
77+
#endif
78+
7279
bool needArityCheck() { return hasArityMismatch; }
7380

7481
JSValue argument(size_t argumentIndex)

Source/JavaScriptCore/jit/AssemblyHelpers.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@
4141
#include "UnlinkedCodeBlock.h"
4242

4343
#if ENABLE(WEBASSEMBLY)
44+
#include "WasmContext.h"
4445
#include "WasmMemoryInformation.h"
45-
#include "WasmContextInlines.h"
4646
#include "WasmInstance.h"
4747
#endif
4848

Source/JavaScriptCore/jit/GPRInfo.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,7 @@ class GPRInfo {
585585
static constexpr GPRReg returnValueGPR2 = ARMRegisters::r1; // regT1
586586
static constexpr GPRReg nonPreservedNonReturnGPR = ARMRegisters::r5;
587587
static constexpr GPRReg nonPreservedNonArgumentGPR0 = ARMRegisters::r5;
588+
static constexpr GPRReg nonPreservedNonArgumentGPR1 = InvalidGPRReg;
588589

589590
static GPRReg toRegister(unsigned index)
590591
{

Source/JavaScriptCore/llint/LLIntExceptions.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
#include "LLIntCommon.h"
3030
#include "LLIntData.h"
3131
#include "LLIntThunks.h"
32-
#include "WasmContextInlines.h"
32+
#include "WasmContext.h"
3333

3434
#if LLINT_TRACING
3535
#include "CatchScope.h"

Source/JavaScriptCore/llint/LLIntThunks.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
#include "LinkBuffer.h"
3434
#include "VMEntryRecord.h"
3535
#include "WasmCallingConvention.h"
36-
#include "WasmContextInlines.h"
36+
#include "WasmContext.h"
3737
#include <wtf/NeverDestroyed.h>
3838

3939
namespace JSC {

0 commit comments

Comments
 (0)