From 325291ab87de8fbce838da7caa8e49cf429eb9d9 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Thu, 25 Aug 2022 14:41:39 -0700 Subject: [PATCH] Use upstream C++ error formatting support (#2828) The logic that we are using to format C++ exceptions has been upstreamed. This will also work correctly with wasm exceptions. --- Makefile | 4 -- Makefile.envs | 1 + src/core/error_handling.ts | 102 +++++++------------------------- src/core/error_handling_cpp.cpp | 26 -------- 4 files changed, 24 insertions(+), 109 deletions(-) delete mode 100644 src/core/error_handling_cpp.cpp diff --git a/Makefile b/Makefile index 81882409b..cb8e45510 100644 --- a/Makefile +++ b/Makefile @@ -29,7 +29,6 @@ dist/pyodide_py.tar: $(wildcard src/py/pyodide/*.py) $(wildcard src/py/_pyodide dist/pyodide.asm.js: \ src/core/docstring.o \ src/core/error_handling.o \ - src/core/error_handling_cpp.o \ src/core/hiwire.o \ src/core/js2python.o \ src/core/jsproxy.o \ @@ -168,9 +167,6 @@ clean-all: clean make -C emsdk clean make -C cpython clean-all -src/core/error_handling_cpp.o: src/core/error_handling_cpp.cpp - $(CXX) -o $@ -c $< $(MAIN_MODULE_CFLAGS) -Isrc/core/ - %.o: %.c $(CPYTHONLIB) $(wildcard src/core/*.h src/core/*.js) $(CC) -o $@ -c $< $(MAIN_MODULE_CFLAGS) -Isrc/core/ diff --git a/Makefile.envs b/Makefile.envs index de6d3a7e3..c55cb7d21 100644 --- a/Makefile.envs +++ b/Makefile.envs @@ -98,6 +98,7 @@ export SIDE_MODULE_LDFLAGS= $(LDFLAGS_BASE) -s SIDE_MODULE=1 export MAIN_MODULE_LDFLAGS= $(LDFLAGS_BASE) \ -s MAIN_MODULE=1 \ -s EXPORT_NAME="'_createPyodideModule'" \ + -s EXPORT_EXCEPTION_HANDLING_HELPERS \ -s EXCEPTION_CATCHING_ALLOWED=['we only want to allow exception handling in side modules'] \ -s DEMANGLE_SUPPORT=1 \ -s USE_ZLIB \ diff --git a/src/core/error_handling.ts b/src/core/error_handling.ts index 4ee178a39..c957a2053 100644 --- a/src/core/error_handling.ts +++ b/src/core/error_handling.ts @@ -36,6 +36,28 @@ function ensureCaughtObjectIsError(e: any): Error { return e; } +class CppException extends Error { + ty: string; + constructor(ty: string, msg: string | undefined, ptr: number) { + if (!msg) { + msg = `The exception is an object of type ${ty} at address ${ptr} which does not inherit from std::exception`; + } + super(msg); + this.ty = ty; + } +} +Object.defineProperty(CppException.prototype, "name", { + get() { + return `${this.constructor.name} ${this.ty}`; + }, +}); + +function convertCppException(e: number) { + let [ty, msg]: [string, string] = Module.getExceptionMessage(e); + return new CppException(ty, msg, e); +} +Tests.convertCppException = convertCppException; + let fatal_error_occurred = false; /** * Signal a fatal error. @@ -59,7 +81,7 @@ API.fatal_error = function (e: any) { return; } if (typeof e === "number") { - // Hopefully a C++ exception? Have to do some conversion work. + // Hopefully a C++ exception? e = convertCppException(e); } else { e = ensureCaughtObjectIsError(e); @@ -105,84 +127,6 @@ API.fatal_error = function (e: any) { throw e; }; -class CppException extends Error { - ty: string; - constructor(ty: string, msg: string) { - super(msg); - this.ty = ty; - } -} -Object.defineProperty(CppException.prototype, "name", { - get() { - return `${this.constructor.name} ${this.ty}`; - }, -}); - -/** - * - * Return the type name, whether the pointer inherits from exception, and the - * vtable pointer for the type. - * - * This code is based on imitating: - * 1. the implementation of __cxa_find_matching_catch - * 2. the disassembly from: - * ```C++ - * try { - * ... - * } catch(exception e){ - * ... - * } - * - * @param ptr - * @returns - * exc_type_name : the type name of the exception, as would be reported by - * `typeid(type).name()` but also demangled. - * - * is_exception_subclass : true if the object is a subclass of exception. In - * this case we will use `exc.what()` to get an error message. - * - * adjusted_ptr : The adjusted vtable pointer for the exception to use to invoke - * exc.what(). - * - * @private - */ -function cppExceptionInfo(ptr: number): [string, boolean, number] { - const base_exception_type = Module._exc_type(); - const ei = new Module.ExceptionInfo(ptr); - const caught_exception_type = ei.get_type(); - const stackTop = Module.stackSave(); - const exceptionThrowBuf = Module.stackAlloc(4); - Module.HEAP32[exceptionThrowBuf / 4] = ptr; - const exc_type_name = Module.demangle( - Module.UTF8ToString(Module._exc_typename(caught_exception_type)) - ); - const is_exception_subclass = !!Module.___cxa_can_catch( - base_exception_type, - caught_exception_type, - exceptionThrowBuf - ); - const adjusted_ptr = Module.HEAP32[exceptionThrowBuf / 4]; - Module.stackRestore(stackTop); - return [exc_type_name, is_exception_subclass, adjusted_ptr]; -} - -function convertCppException(ptr: number): CppException { - const [exc_type_name, is_exception_subclass, adjusted_ptr] = - cppExceptionInfo(ptr); - let msg; - if (is_exception_subclass) { - // If the ptr inherits from exception, we can use exception.what() to - // generate a message - const msgPtr = Module._exc_what(adjusted_ptr); - msg = Module.UTF8ToString(msgPtr); - } else { - msg = `The exception is an object of type ${exc_type_name} at address ${ptr} which does not inherit from std::exception`; - } - return new CppException(exc_type_name, msg); -} -// Expose for testing -Tests.convertCppException = convertCppException; - function isPyodideFrame(frame: ErrorStackParser.StackFrame): boolean { if (!frame) { return false; diff --git a/src/core/error_handling_cpp.cpp b/src/core/error_handling_cpp.cpp deleted file mode 100644 index a87230d79..000000000 --- a/src/core/error_handling_cpp.cpp +++ /dev/null @@ -1,26 +0,0 @@ -#include -#include -#include -using namespace std; - -extern "C" -{ - - EMSCRIPTEN_KEEPALIVE - const char* exc_what(exception& e) - { - return e.what(); - } - - EMSCRIPTEN_KEEPALIVE - const std::type_info* exc_type() - { - return &typeid(exception); - } - - EMSCRIPTEN_KEEPALIVE - const char* exc_typename(std::type_info* type) - { - return type->name(); - } -}