From a28947f7bb010ab46abb86811e385ba6d17bedef Mon Sep 17 00:00:00 2001 From: Thibaud Michaud Date: Wed, 8 Jun 2022 14:09:16 +0200 Subject: [PATCH] [wasm][eh] Wasm exceptions are not JS errors Context: https://github.com/WebAssembly/exception-handling/pull/197 This change removes the wasm exception -> JS Error inheritance. R=jkummerow@chromium.org Bug: v8:8091 Change-Id: I479f16fe03d4d77d2ecd8409e96f9a3c063912b5 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3688401 Commit-Queue: Thibaud Michaud Reviewed-by: Jakob Kummerow Cr-Commit-Position: refs/heads/main@{#80997} --- src/compiler/types.cc | 1 + src/diagnostics/objects-debug.cc | 8 +++++++ src/diagnostics/objects-printer.cc | 8 +++++++ src/heap/factory.cc | 1 - src/init/bootstrapper.cc | 4 ---- src/objects/contexts.h | 2 -- src/objects/js-objects.cc | 2 ++ src/objects/map.cc | 1 + src/objects/objects-body-descriptors-inl.h | 2 ++ src/objects/objects-inl.h | 8 ------- src/objects/objects.h | 1 + src/wasm/wasm-js.cc | 22 +++----------------- src/wasm/wasm-objects.cc | 5 +++-- src/wasm/wasm-objects.h | 2 ++ src/wasm/wasm-objects.tq | 2 ++ test/message/fail/wasm-exception-rethrow.out | 5 +---- test/message/fail/wasm-exception-throw.out | 5 +---- test/mjsunit/regress/wasm/regress-8094.js | 2 +- test/mjsunit/wasm/exceptions-api.js | 3 ++- test/wasm-js/wasm-js.status | 1 - tools/v8heapconst.py | 17 ++++++++------- 21 files changed, 47 insertions(+), 55 deletions(-) diff --git a/src/compiler/types.cc b/src/compiler/types.cc index 25b8facda6..f97934daab 100644 --- a/src/compiler/types.cc +++ b/src/compiler/types.cc @@ -285,6 +285,7 @@ Type::bitset BitsetType::Lub(const MapRefLike& map) { case WASM_SUSPENDER_OBJECT_TYPE: case WASM_TABLE_OBJECT_TYPE: case WASM_TAG_OBJECT_TYPE: + case WASM_EXCEPTION_PACKAGE_TYPE: case WASM_VALUE_OBJECT_TYPE: #endif // V8_ENABLE_WEBASSEMBLY case WEAK_CELL_TYPE: diff --git a/src/diagnostics/objects-debug.cc b/src/diagnostics/objects-debug.cc index 1512e8cee4..1c0609247a 100644 --- a/src/diagnostics/objects-debug.cc +++ b/src/diagnostics/objects-debug.cc @@ -263,6 +263,9 @@ void HeapObject::HeapObjectVerify(Isolate* isolate) { case WASM_VALUE_OBJECT_TYPE: WasmValueObject::cast(*this).WasmValueObjectVerify(isolate); break; + case WASM_EXCEPTION_PACKAGE_TYPE: + WasmExceptionPackage::cast(*this).WasmExceptionPackageVerify(isolate); + break; #endif // V8_ENABLE_WEBASSEMBLY case JS_SET_KEY_VALUE_ITERATOR_TYPE: case JS_SET_VALUE_ITERATOR_TYPE: @@ -1793,6 +1796,11 @@ void WasmValueObject::WasmValueObjectVerify(Isolate* isolate) { CHECK(IsWasmValueObject()); } +void WasmExceptionPackage::WasmExceptionPackageVerify(Isolate* isolate) { + JSObjectVerify(isolate); + CHECK(IsWasmExceptionPackage()); +} + void WasmExportedFunctionData::WasmExportedFunctionDataVerify( Isolate* isolate) { TorqueGeneratedClassVerifiers::WasmExportedFunctionDataVerify(*this, isolate); diff --git a/src/diagnostics/objects-printer.cc b/src/diagnostics/objects-printer.cc index 5e297ef16d..d355243502 100644 --- a/src/diagnostics/objects-printer.cc +++ b/src/diagnostics/objects-printer.cc @@ -205,6 +205,9 @@ void HeapObject::HeapObjectPrint(std::ostream& os) { case WASM_VALUE_OBJECT_TYPE: WasmValueObject::cast(*this).WasmValueObjectPrint(os); break; + case WASM_EXCEPTION_PACKAGE_TYPE: + WasmExceptionPackage::cast(*this).WasmExceptionPackagePrint(os); + break; #endif // V8_ENABLE_WEBASSEMBLY case CODE_TYPE: Code::cast(*this).CodePrint(os); @@ -2059,6 +2062,11 @@ void WasmCapiFunctionData::WasmCapiFunctionDataPrint(std::ostream& os) { os << "\n"; } +void WasmExceptionPackage::WasmExceptionPackagePrint(std::ostream& os) { + PrintHeader(os, "WasmExceptionPackage"); + os << "\n"; +} + void WasmModuleObject::WasmModuleObjectPrint(std::ostream& os) { PrintHeader(os, "WasmModuleObject"); os << "\n - module: " << module(); diff --git a/src/heap/factory.cc b/src/heap/factory.cc index 4465dcd03c..1966f811ff 100644 --- a/src/heap/factory.cc +++ b/src/heap/factory.cc @@ -2294,7 +2294,6 @@ DEFINE_ERROR(TypeError, type_error) DEFINE_ERROR(WasmCompileError, wasm_compile_error) DEFINE_ERROR(WasmLinkError, wasm_link_error) DEFINE_ERROR(WasmRuntimeError, wasm_runtime_error) -DEFINE_ERROR(WasmExceptionError, wasm_exception_error) #undef DEFINE_ERROR Handle Factory::NewFunctionPrototype(Handle function) { diff --git a/src/init/bootstrapper.cc b/src/init/bootstrapper.cc index 9040e95202..fbbe58e7da 100644 --- a/src/init/bootstrapper.cc +++ b/src/init/bootstrapper.cc @@ -2758,10 +2758,6 @@ void Genesis::InitializeGlobal(Handle global_object, // -- R u n t i m e E r r o r InstallError(isolate_, dummy, factory->RuntimeError_string(), Context::WASM_RUNTIME_ERROR_FUNCTION_INDEX); - - // -- W e b A s s e m b l y . E x c e p t i o n - InstallError(isolate_, dummy, factory->WebAssemblyException_string(), - Context::WASM_EXCEPTION_ERROR_FUNCTION_INDEX); } // Initialize the embedder data slot. diff --git a/src/objects/contexts.h b/src/objects/contexts.h index ba14db0495..afe6c133cc 100644 --- a/src/objects/contexts.h +++ b/src/objects/contexts.h @@ -361,8 +361,6 @@ enum ContextLookupFlags { V(WASM_LINK_ERROR_FUNCTION_INDEX, JSFunction, wasm_link_error_function) \ V(WASM_RUNTIME_ERROR_FUNCTION_INDEX, JSFunction, \ wasm_runtime_error_function) \ - V(WASM_EXCEPTION_ERROR_FUNCTION_INDEX, JSFunction, \ - wasm_exception_error_function) \ V(WEAKMAP_SET_INDEX, JSFunction, weakmap_set) \ V(WEAKMAP_GET_INDEX, JSFunction, weakmap_get) \ V(WEAKMAP_DELETE_INDEX, JSFunction, weakmap_delete) \ diff --git a/src/objects/js-objects.cc b/src/objects/js-objects.cc index 917285b03b..4ee20157dd 100644 --- a/src/objects/js-objects.cc +++ b/src/objects/js-objects.cc @@ -2528,6 +2528,8 @@ int JSObject::GetHeaderSize(InstanceType type, return WasmValueObject::kHeaderSize; case WASM_TAG_OBJECT_TYPE: return WasmTagObject::kHeaderSize; + case WASM_EXCEPTION_PACKAGE_TYPE: + return WasmExceptionPackage::kHeaderSize; #endif // V8_ENABLE_WEBASSEMBLY default: { // Special type check for API Objects because they are in a large variable diff --git a/src/objects/map.cc b/src/objects/map.cc index 967ba60519..ab44c4658d 100644 --- a/src/objects/map.cc +++ b/src/objects/map.cc @@ -311,6 +311,7 @@ VisitorId Map::GetVisitorId(Map map) { #endif // V8_INTL_SUPPORT #if V8_ENABLE_WEBASSEMBLY case WASM_TAG_OBJECT_TYPE: + case WASM_EXCEPTION_PACKAGE_TYPE: case WASM_GLOBAL_OBJECT_TYPE: case WASM_MEMORY_OBJECT_TYPE: case WASM_MODULE_OBJECT_TYPE: diff --git a/src/objects/objects-body-descriptors-inl.h b/src/objects/objects-body-descriptors-inl.h index 4f60662e81..86e9f252b6 100644 --- a/src/objects/objects-body-descriptors-inl.h +++ b/src/objects/objects-body-descriptors-inl.h @@ -1151,6 +1151,8 @@ auto BodyDescriptorApply(InstanceType type, Args&&... args) { return CALL_APPLY(WasmArray); case WASM_CAPI_FUNCTION_DATA_TYPE: return CALL_APPLY(WasmCapiFunctionData); + case WASM_EXCEPTION_PACKAGE_TYPE: + return CALL_APPLY(WasmExceptionPackage); case WASM_EXPORTED_FUNCTION_DATA_TYPE: return CALL_APPLY(WasmExportedFunctionData); case WASM_INTERNAL_FUNCTION_TYPE: diff --git a/src/objects/objects-inl.h b/src/objects/objects-inl.h index 4dfde12920..c76386315c 100644 --- a/src/objects/objects-inl.h +++ b/src/objects/objects-inl.h @@ -400,14 +400,6 @@ DEF_GETTER(HeapObject, IsObjectHashTable, bool) { DEF_GETTER(HeapObject, IsHashTableBase, bool) { return IsHashTable(cage_base); } -#if V8_ENABLE_WEBASSEMBLY -DEF_GETTER(HeapObject, IsWasmExceptionPackage, bool) { - // It is not possible to check for the existence of certain properties on the - // underlying {JSReceiver} here because that requires calling handlified code. - return IsJSReceiver(cage_base); -} -#endif // V8_ENABLE_WEBASSEMBLY - bool Object::IsPrimitive() const { if (IsSmi()) return true; HeapObject this_heap_object = HeapObject::cast(*this); diff --git a/src/objects/objects.h b/src/objects/objects.h index 1e4353f2a8..104a524dc4 100644 --- a/src/objects/objects.h +++ b/src/objects/objects.h @@ -95,6 +95,7 @@ // - JSSegments // If V8_INTL_SUPPORT enabled. // - JSSegmentIterator // If V8_INTL_SUPPORT enabled. // - JSV8BreakIterator // If V8_INTL_SUPPORT enabled. +// - WasmExceptionPackage // - WasmTagObject // - WasmGlobalObject // - WasmInstanceObject diff --git a/src/wasm/wasm-js.cc b/src/wasm/wasm-js.cc index 686dcf4000..d015330ec4 100644 --- a/src/wasm/wasm-js.cc +++ b/src/wasm/wasm-js.cc @@ -2304,10 +2304,6 @@ void WebAssemblyExceptionGetArg( auto this_tag = i::WasmExceptionPackage::GetExceptionTag(i_isolate, exception); - if (this_tag->IsUndefined()) { - thrower.TypeError("Expected a WebAssembly.Exception object"); - return; - } DCHECK(this_tag->IsWasmExceptionTag()); if (tag->tag() != *this_tag) { thrower.TypeError("First argument does not match the exception tag"); @@ -2441,10 +2437,6 @@ void WebAssemblyExceptionIs(const v8::FunctionCallbackInfo& args) { if (thrower.error()) return; auto tag = i::WasmExceptionPackage::GetExceptionTag(i_isolate, exception); - if (tag->IsUndefined()) { - thrower.TypeError("Expected a WebAssembly.Exception object"); - return; - } DCHECK(tag->IsWasmExceptionTag()); auto maybe_tag = GetFirstArgumentAsTag(args, &thrower); @@ -3023,21 +3015,13 @@ void WasmJs::Install(Isolate* isolate, bool exposed_on_global_object) { Handle exception_constructor = InstallConstructorFunc( isolate, webassembly, "Exception", WebAssemblyException); SetDummyInstanceTemplate(isolate, exception_constructor); - Handle exception_map(isolate->native_context() - ->wasm_exception_error_function() - .initial_map(), - isolate); - Handle exception_proto( - JSObject::cast(isolate->native_context() - ->wasm_exception_error_function() - .instance_prototype()), - isolate); + Handle exception_proto = SetupConstructor( + isolate, exception_constructor, i::WASM_EXCEPTION_PACKAGE_TYPE, + WasmExceptionPackage::kHeaderSize, "WebAssembly.Exception"); InstallFunc(isolate, exception_proto, "getArg", WebAssemblyExceptionGetArg, 2); InstallFunc(isolate, exception_proto, "is", WebAssemblyExceptionIs, 1); context->set_wasm_exception_constructor(*exception_constructor); - JSFunction::SetInitialMap(isolate, exception_constructor, exception_map, - exception_proto); } // Setup Suspender. diff --git a/src/wasm/wasm-objects.cc b/src/wasm/wasm-objects.cc index e82ab59b7e..ee1da9ec46 100644 --- a/src/wasm/wasm-objects.cc +++ b/src/wasm/wasm-objects.cc @@ -1692,8 +1692,9 @@ Handle WasmExceptionPackage::New( Handle WasmExceptionPackage::New( Isolate* isolate, Handle exception_tag, Handle values) { - Handle exception = isolate->factory()->NewWasmExceptionError( - MessageTemplate::kWasmExceptionError); + Handle exception_cons( + isolate->native_context()->wasm_exception_constructor(), isolate); + Handle exception = isolate->factory()->NewJSObject(exception_cons); CHECK(!Object::SetProperty(isolate, exception, isolate->factory()->wasm_exception_tag_symbol(), exception_tag, StoreOrigin::kMaybeKeyed, diff --git a/src/wasm/wasm-objects.h b/src/wasm/wasm-objects.h index 3c71a27497..b5d12831fe 100644 --- a/src/wasm/wasm-objects.h +++ b/src/wasm/wasm-objects.h @@ -570,6 +570,8 @@ class V8_EXPORT_PRIVATE WasmExceptionPackage : public JSObject { static uint32_t GetEncodedSize(const wasm::WasmTag* tag); DECL_CAST(WasmExceptionPackage) + DECL_PRINTER(WasmExceptionPackage) + DECL_VERIFIER(WasmExceptionPackage) OBJECT_CONSTRUCTORS(WasmExceptionPackage, JSObject); }; diff --git a/src/wasm/wasm-objects.tq b/src/wasm/wasm-objects.tq index daab1aede4..7e90091d87 100644 --- a/src/wasm/wasm-objects.tq +++ b/src/wasm/wasm-objects.tq @@ -114,6 +114,8 @@ extern class WasmExceptionTag extends Struct { index: Smi; } +extern class WasmExceptionPackage extends JSObject; + extern class WasmModuleObject extends JSObject { managed_native_module: ManagedWasmNativeModule; export_wrappers: FixedArray; diff --git a/test/message/fail/wasm-exception-rethrow.out b/test/message/fail/wasm-exception-rethrow.out index 657c4279f1..7f06420dc0 100644 --- a/test/message/fail/wasm-exception-rethrow.out +++ b/test/message/fail/wasm-exception-rethrow.out @@ -1,4 +1 @@ -wasm-function[0]:0x32: WebAssembly.Exception: wasm exception -WebAssembly.Exception: wasm exception - at rethrow0 (wasm://wasm/f019909a:wasm-function[0]:0x32) - at *%(basename)s:21:18 +undefined:0: [object WebAssembly.Exception] diff --git a/test/message/fail/wasm-exception-throw.out b/test/message/fail/wasm-exception-throw.out index eb1f260a7b..7f06420dc0 100644 --- a/test/message/fail/wasm-exception-throw.out +++ b/test/message/fail/wasm-exception-throw.out @@ -1,4 +1 @@ -wasm-function[0]:0x2e: WebAssembly.Exception: wasm exception -WebAssembly.Exception: wasm exception - at throw0 (wasm://wasm/e4cabeba:wasm-function[0]:0x2e) - at *%(basename)s:17:18 +undefined:0: [object WebAssembly.Exception] diff --git a/test/mjsunit/regress/wasm/regress-8094.js b/test/mjsunit/regress/wasm/regress-8094.js index 212f75774b..7438aab529 100644 --- a/test/mjsunit/regress/wasm/regress-8094.js +++ b/test/mjsunit/regress/wasm/regress-8094.js @@ -26,4 +26,4 @@ try { // that no extraneous properties exist. Setting such properties could be // observable by JavaScript and could break compatibility. assertInstanceof(exception, WebAssembly.Exception); -assertArrayEquals(["stack", "message"], Object.getOwnPropertyNames(exception)); +assertArrayEquals([], Object.getOwnPropertyNames(exception)); diff --git a/test/mjsunit/wasm/exceptions-api.js b/test/mjsunit/wasm/exceptions-api.js index 41e81b6a9f..18701576cf 100644 --- a/test/mjsunit/wasm/exceptions-api.js +++ b/test/mjsunit/wasm/exceptions-api.js @@ -75,6 +75,7 @@ load("test/mjsunit/wasm/wasm-module-builder.js"); // Check prototype. assertSame(WebAssembly.Exception.prototype, js_exception.__proto__); assertTrue(js_exception instanceof WebAssembly.Exception); + assertFalse(js_exception instanceof Error); // Check prototype of a thrown exception. let builder = new WasmModuleBuilder(); @@ -236,5 +237,5 @@ function TestGetArgHelper(types_str, types, values) { assertFalse(exception.is(tag2)); assertThrows(() => exception.is.apply({}, tag1), TypeError, - /Expected a WebAssembly.Exception object/); + /Receiver is not a WebAssembly.Exception/); })(); diff --git a/test/wasm-js/wasm-js.status b/test/wasm-js/wasm-js.status index 9a0efa1f7d..798a0cf9cd 100644 --- a/test/wasm-js/wasm-js.status +++ b/test/wasm-js/wasm-js.status @@ -9,7 +9,6 @@ 'wpt/idlharness': [SKIP], # Failing WPT tests 'wpt/exception/getArg.tentative': [FAIL], - 'wpt/exception/toString.tentative': [FAIL], 'wpt/exception/type.tentative': [FAIL], 'wpt/function/constructor.tentative': [FAIL], 'wpt/function/table.tentative': [FAIL], diff --git a/tools/v8heapconst.py b/tools/v8heapconst.py index 993785ec03..6c2e5b0208 100644 --- a/tools/v8heapconst.py +++ b/tools/v8heapconst.py @@ -264,14 +264,15 @@ INSTANCE_TYPES = { 2139: "JS_TEMPORAL_ZONED_DATE_TIME_TYPE", 2140: "JS_V8_BREAK_ITERATOR_TYPE", 2141: "JS_WEAK_REF_TYPE", - 2142: "WASM_GLOBAL_OBJECT_TYPE", - 2143: "WASM_INSTANCE_OBJECT_TYPE", - 2144: "WASM_MEMORY_OBJECT_TYPE", - 2145: "WASM_MODULE_OBJECT_TYPE", - 2146: "WASM_SUSPENDER_OBJECT_TYPE", - 2147: "WASM_TABLE_OBJECT_TYPE", - 2148: "WASM_TAG_OBJECT_TYPE", - 2149: "WASM_VALUE_OBJECT_TYPE", + 2142: "WASM_EXCEPTION_PACKAGE_TYPE", + 2143: "WASM_GLOBAL_OBJECT_TYPE", + 2144: "WASM_INSTANCE_OBJECT_TYPE", + 2145: "WASM_MEMORY_OBJECT_TYPE", + 2146: "WASM_MODULE_OBJECT_TYPE", + 2147: "WASM_SUSPENDER_OBJECT_TYPE", + 2148: "WASM_TABLE_OBJECT_TYPE", + 2149: "WASM_TAG_OBJECT_TYPE", + 2150: "WASM_VALUE_OBJECT_TYPE", } # List of known V8 maps.