From 6b179adb8c05801bf069121b360531d135ee3f44 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Fri, 15 Sep 2023 15:04:21 -0700 Subject: [PATCH] gh-106213: Make Emscripten trampolines work with JSPI (GH-106219) There is a WIP proposal to enable webassembly stack switching which have been implemented in v8: https://github.com/WebAssembly/js-promise-integration It is not possible to switch stacks that contain JS frames so the Emscripten JS trampolines that allow calling functions with the wrong number of arguments don't work in this case. However, the js-promise-integration proposal requires the [type reflection for Wasm/JS API](https://github.com/WebAssembly/js-types) proposal, which allows us to actually count the number of arguments a function expects. For better compatibility with stack switching, this PR checks if type reflection is available, and if so we use a switch block to decide the appropriate signature. If type reflection is unavailable, we should use the current EMJS trampoline. We cache the function argument counts since when I didn't cache them performance was negatively affected. Co-authored-by: T. Wouters Co-authored-by: Brett Cannon --- .../internal/pycore_emscripten_trampoline.h | 81 ++++++++++++++++++ Include/internal/pycore_object.h | 1 + Include/internal/pycore_runtime.h | 7 ++ ...-06-29-09-42-56.gh-issue-106213.TCUgzM.rst | 2 + Objects/descrobject.c | 20 +---- Objects/methodobject.c | 7 -- Python/emscripten_trampoline.c | 82 +++++++++++++++++++ Python/pystate.c | 5 ++ configure | 4 +- configure.ac | 4 +- 10 files changed, 183 insertions(+), 30 deletions(-) create mode 100644 Include/internal/pycore_emscripten_trampoline.h create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-06-29-09-42-56.gh-issue-106213.TCUgzM.rst create mode 100644 Python/emscripten_trampoline.c diff --git a/Include/internal/pycore_emscripten_trampoline.h b/Include/internal/pycore_emscripten_trampoline.h new file mode 100644 index 00000000000..e519c99ad86 --- /dev/null +++ b/Include/internal/pycore_emscripten_trampoline.h @@ -0,0 +1,81 @@ +#ifndef Py_EMSCRIPTEN_TRAMPOLINE_H +#define Py_EMSCRIPTEN_TRAMPOLINE_H + +#include "pycore_runtime.h" // _PyRuntimeState + +/** + * C function call trampolines to mitigate bad function pointer casts. + * + * Section 6.3.2.3, paragraph 8 reads: + * + * A pointer to a function of one type may be converted to a pointer to a + * function of another type and back again; the result shall compare equal to + * the original pointer. If a converted pointer is used to call a function + * whose type is not compatible with the pointed-to type, the behavior is + * undefined. + * + * Typical native ABIs ignore additional arguments or fill in missing values + * with 0/NULL in function pointer cast. Compilers do not show warnings when a + * function pointer is explicitly casted to an incompatible type. + * + * Bad fpcasts are an issue in WebAssembly. WASM's indirect_call has strict + * function signature checks. Argument count, types, and return type must match. + * + * Third party code unintentionally rely on problematic fpcasts. The call + * trampoline mitigates common occurrences of bad fpcasts on Emscripten. + */ + +#if defined(__EMSCRIPTEN__) && defined(PY_CALL_TRAMPOLINE) + +void _Py_EmscriptenTrampoline_Init(_PyRuntimeState *runtime); + +PyObject* +_PyEM_TrampolineCall_JavaScript(PyCFunctionWithKeywords func, + PyObject* self, + PyObject* args, + PyObject* kw); + +PyObject* +_PyEM_TrampolineCall_Reflection(PyCFunctionWithKeywords func, + PyObject* self, + PyObject* args, + PyObject* kw); + +#define _PyEM_TrampolineCall(meth, self, args, kw) \ + ((_PyRuntime.wasm_type_reflection_available) ? \ + (_PyEM_TrampolineCall_Reflection((PyCFunctionWithKeywords)(meth), (self), (args), (kw))) : \ + (_PyEM_TrampolineCall_JavaScript((PyCFunctionWithKeywords)(meth), (self), (args), (kw)))) + +#define _PyCFunction_TrampolineCall(meth, self, args) \ + _PyEM_TrampolineCall( \ + (*(PyCFunctionWithKeywords)(void(*)(void))(meth)), (self), (args), NULL) + +#define _PyCFunctionWithKeywords_TrampolineCall(meth, self, args, kw) \ + _PyEM_TrampolineCall((meth), (self), (args), (kw)) + +#define descr_set_trampoline_call(set, obj, value, closure) \ + ((int)_PyEM_TrampolineCall((PyCFunctionWithKeywords)(set), (obj), (value), (PyObject*)(closure))) + +#define descr_get_trampoline_call(get, obj, closure) \ + _PyEM_TrampolineCall((PyCFunctionWithKeywords)(get), (obj), (PyObject*)(closure), NULL) + + +#else // defined(__EMSCRIPTEN__) && defined(PY_CALL_TRAMPOLINE) + +#define _Py_EmscriptenTrampoline_Init(runtime) + +#define _PyCFunction_TrampolineCall(meth, self, args) \ + (meth)((self), (args)) + +#define _PyCFunctionWithKeywords_TrampolineCall(meth, self, args, kw) \ + (meth)((self), (args), (kw)) + +#define descr_set_trampoline_call(set, obj, value, closure) \ + (set)((obj), (value), (closure)) + +#define descr_get_trampoline_call(get, obj, closure) \ + (get)((obj), (closure)) + +#endif // defined(__EMSCRIPTEN__) && defined(PY_CALL_TRAMPOLINE) + +#endif // ndef Py_EMSCRIPTEN_SIGNAL_H diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index daa06ebfbf9..2d50f42c9c6 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -10,6 +10,7 @@ extern "C" { #include #include "pycore_gc.h" // _PyObject_GC_IS_TRACKED() +#include "pycore_emscripten_trampoline.h" // _PyCFunction_TrampolineCall() #include "pycore_interp.h" // PyInterpreterState.gc #include "pycore_pystate.h" // _PyInterpreterState_GET() diff --git a/Include/internal/pycore_runtime.h b/Include/internal/pycore_runtime.h index 2ce46f3201d..0ddc405f221 100644 --- a/Include/internal/pycore_runtime.h +++ b/Include/internal/pycore_runtime.h @@ -267,6 +267,13 @@ typedef struct pyruntimestate { /* PyInterpreterState.interpreters.main */ PyInterpreterState _main_interpreter; + +#if defined(__EMSCRIPTEN__) && defined(PY_CALL_TRAMPOLINE) + // Used in "Python/emscripten_trampoline.c" to choose between type + // reflection trampoline and EM_JS trampoline. + bool wasm_type_reflection_available; +#endif + } _PyRuntimeState; diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-06-29-09-42-56.gh-issue-106213.TCUgzM.rst b/Misc/NEWS.d/next/Core and Builtins/2023-06-29-09-42-56.gh-issue-106213.TCUgzM.rst new file mode 100644 index 00000000000..431f9cc0e4b --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-06-29-09-42-56.gh-issue-106213.TCUgzM.rst @@ -0,0 +1,2 @@ +Changed the way that Emscripten call trampolines work for compatibility with +Wasm/JS Promise integration. diff --git a/Objects/descrobject.c b/Objects/descrobject.c index a744c3d1e58..56ce34f80ca 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -4,6 +4,7 @@ #include "pycore_abstract.h" // _PyObject_RealIsSubclass() #include "pycore_call.h" // _PyStack_AsDict() #include "pycore_ceval.h" // _Py_EnterRecursiveCallTstate() +#include "pycore_emscripten_trampoline.h" // descr_set_trampoline_call(), descr_get_trampoline_call() #include "pycore_descrobject.h" // _PyMethodWrapper_Type #include "pycore_object.h" // _PyObject_GC_UNTRACK() #include "pycore_pystate.h" // _PyThreadState_GET() @@ -16,25 +17,6 @@ class property "propertyobject *" "&PyProperty_Type" [clinic start generated code]*/ /*[clinic end generated code: output=da39a3ee5e6b4b0d input=556352653fd4c02e]*/ -// see pycore_object.h -#if defined(__EMSCRIPTEN__) && defined(PY_CALL_TRAMPOLINE) -#include -EM_JS(int, descr_set_trampoline_call, (setter set, PyObject *obj, PyObject *value, void *closure), { - return wasmTable.get(set)(obj, value, closure); -}); - -EM_JS(PyObject*, descr_get_trampoline_call, (getter get, PyObject *obj, void *closure), { - return wasmTable.get(get)(obj, closure); -}); -#else -#define descr_set_trampoline_call(set, obj, value, closure) \ - (set)((obj), (value), (closure)) - -#define descr_get_trampoline_call(get, obj, closure) \ - (get)((obj), (closure)) - -#endif // __EMSCRIPTEN__ && PY_CALL_TRAMPOLINE - static void descr_dealloc(PyDescrObject *descr) { diff --git a/Objects/methodobject.c b/Objects/methodobject.c index 521c9059770..b40b2821c38 100644 --- a/Objects/methodobject.c +++ b/Objects/methodobject.c @@ -553,10 +553,3 @@ cfunction_call(PyObject *func, PyObject *args, PyObject *kwargs) return _Py_CheckFunctionResult(tstate, func, result, NULL); } -#if defined(__EMSCRIPTEN__) && defined(PY_CALL_TRAMPOLINE) -#include - -EM_JS(PyObject*, _PyCFunctionWithKeywords_TrampolineCall, (PyCFunctionWithKeywords func, PyObject *self, PyObject *args, PyObject *kw), { - return wasmTable.get(func)(self, args, kw); -}); -#endif diff --git a/Python/emscripten_trampoline.c b/Python/emscripten_trampoline.c new file mode 100644 index 00000000000..2a80ec4f18d --- /dev/null +++ b/Python/emscripten_trampoline.c @@ -0,0 +1,82 @@ +#if defined(PY_CALL_TRAMPOLINE) + +#include // EM_JS +#include +#include "pycore_runtime.h" // _PyRuntime + + +/** + * This is the GoogleChromeLabs approved way to feature detect type-reflection: + * https://github.com/GoogleChromeLabs/wasm-feature-detect/blob/main/src/detectors/type-reflection/index.js + */ +EM_JS(int, _PyEM_detect_type_reflection, (), { + return "Function" in WebAssembly; +}); + +void +_Py_EmscriptenTrampoline_Init(_PyRuntimeState *runtime) +{ + runtime->wasm_type_reflection_available = _PyEM_detect_type_reflection(); +} + +/** + * Backwards compatible trampoline works with all JS runtimes + */ +EM_JS(PyObject*, +_PyEM_TrampolineCall_JavaScript, (PyCFunctionWithKeywords func, + PyObject *arg1, + PyObject *arg2, + PyObject *arg3), +{ + return wasmTable.get(func)(arg1, arg2, arg3); +} +); + +/** + * In runtimes with WebAssembly type reflection, count the number of parameters + * and cast to the appropriate signature + */ +EM_JS(int, _PyEM_CountFuncParams, (PyCFunctionWithKeywords func), +{ + let n = _PyEM_CountFuncParams.cache.get(func); + + if (n !== undefined) { + return n; + } + n = WebAssembly.Function.type(wasmTable.get(func)).parameters.length; + _PyEM_CountFuncParams.cache.set(func, n); + return n; +} +_PyEM_CountFuncParams.cache = new Map(); +) + + +typedef PyObject* (*zero_arg)(void); +typedef PyObject* (*one_arg)(PyObject*); +typedef PyObject* (*two_arg)(PyObject*, PyObject*); +typedef PyObject* (*three_arg)(PyObject*, PyObject*, PyObject*); + + +PyObject* +_PyEM_TrampolineCall_Reflection(PyCFunctionWithKeywords func, + PyObject* self, + PyObject* args, + PyObject* kw) +{ + switch (_PyEM_CountFuncParams(func)) { + case 0: + return ((zero_arg)func)(); + case 1: + return ((one_arg)func)(self); + case 2: + return ((two_arg)func)(self, args); + case 3: + return ((three_arg)func)(self, args, kw); + default: + PyErr_SetString(PyExc_SystemError, + "Handler takes too many arguments"); + return NULL; + } +} + +#endif diff --git a/Python/pystate.c b/Python/pystate.c index b5c4fd7fb50..08cf6f0bb7c 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -5,6 +5,7 @@ #include "pycore_ceval.h" #include "pycore_code.h" // stats #include "pycore_dtoa.h" // _dtoa_state_INIT() +#include "pycore_emscripten_trampoline.h" // _Py_EmscriptenTrampoline_Init() #include "pycore_frame.h" #include "pycore_initconfig.h" // _PyStatus_OK() #include "pycore_object.h" // _PyType_InitCache() @@ -449,6 +450,10 @@ init_runtime(_PyRuntimeState *runtime, runtime->unicode_state.ids.next_index = unicode_next_index; +#if defined(__EMSCRIPTEN__) && defined(PY_CALL_TRAMPOLINE) + _Py_EmscriptenTrampoline_Init(runtime); +#endif + runtime->_initialized = 1; } diff --git a/configure b/configure index 8326a1db06c..17b9e7f532a 100755 --- a/configure +++ b/configure @@ -16941,8 +16941,8 @@ PLATFORM_OBJS= case $ac_sys_system in #( Emscripten) : - as_fn_append PLATFORM_OBJS ' Python/emscripten_signal.o' - as_fn_append PLATFORM_HEADERS ' $(srcdir)/Include/internal/pycore_emscripten_signal.h' + as_fn_append PLATFORM_OBJS ' Python/emscripten_signal.o Python/emscripten_trampoline.o' + as_fn_append PLATFORM_HEADERS ' $(srcdir)/Include/internal/pycore_emscripten_signal.h $(srcdir)/Include/internal/pycore_emscripten_trampoline.h' ;; #( *) : ;; diff --git a/configure.ac b/configure.ac index 843f2b267a5..34958a1cdf1 100644 --- a/configure.ac +++ b/configure.ac @@ -4593,8 +4593,8 @@ PLATFORM_OBJS= AS_CASE([$ac_sys_system], [Emscripten], [ - AS_VAR_APPEND([PLATFORM_OBJS], [' Python/emscripten_signal.o']) - AS_VAR_APPEND([PLATFORM_HEADERS], [' $(srcdir)/Include/internal/pycore_emscripten_signal.h']) + AS_VAR_APPEND([PLATFORM_OBJS], [' Python/emscripten_signal.o Python/emscripten_trampoline.o']) + AS_VAR_APPEND([PLATFORM_HEADERS], [' $(srcdir)/Include/internal/pycore_emscripten_signal.h $(srcdir)/Include/internal/pycore_emscripten_trampoline.h']) ], ) AC_SUBST([PLATFORM_HEADERS])