Throw a WasmErrorObject rather than an integer.

Fixes the implementation of wasm exceptions to use a WasmRuntimeError
object, and set the exception tag value as a property of the
object. This guarantees that an uncaught wasm exception is treated
like all other runtime errors.

Bug: v8:6577
Change-Id: I0ab0130444e745178e86c23b3bc9fc9f385c8d05
Reviewed-on: https://chromium-review.googlesource.com/611124Reviewed-by: Bill Budge <bbudge@chromium.org>
Reviewed-by: Eric Holk <eholk@chromium.org>
Commit-Queue: Karl Schimpf <kschimpf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47346}

src/compiler/wasm-compiler.cc

@@ -1809,23 +1809,7 @@ Node* WasmGraphBuilder::GrowMemory(Node* input) {
 
 Node* WasmGraphBuilder::Throw(Node* input) {
   SetNeedsStackCheck();
-  MachineOperatorBuilder* machine = jsgraph()->machine();
-
-  // Pass the thrown value as two SMIs:
-  //
-  // upper = static_cast<uint32_t>(input) >> 16;
-  // lower = input & 0xFFFF;
-  //
-  // This is needed because we can't safely call BuildChangeInt32ToTagged from
-  // this method.
-  //
-  // TODO(wasm): figure out how to properly pass this to the runtime function.
-  Node* upper = BuildChangeInt32ToSmi(
-      graph()->NewNode(machine->Word32Shr(), input, Int32Constant(16)));
-  Node* lower = BuildChangeInt32ToSmi(
-      graph()->NewNode(machine->Word32And(), input, Int32Constant(0xFFFFu)));
-
-  Node* parameters[] = {lower, upper};  // thrown value
+  Node* parameters[] = {BuildChangeInt32ToSmi(input)};
   return BuildCallToRuntime(Runtime::kWasmThrow, parameters,
                             arraysize(parameters));
 }
@@ -1838,45 +1822,13 @@ Node* WasmGraphBuilder::Rethrow() {
 
 Node* WasmGraphBuilder::Catch(Node* input, wasm::WasmCodePosition position) {
   SetNeedsStackCheck();
-  CommonOperatorBuilder* common = jsgraph()->common();
-
   Node* parameters[] = {input};  // caught value
   Node* value = BuildCallToRuntime(Runtime::kWasmSetCaughtExceptionValue,
                                    parameters, arraysize(parameters));
-
-  Node* is_smi;
-  Node* is_heap;
-  BranchExpectFalse(BuildTestNotSmi(value), &is_heap, &is_smi);
-
-  // is_smi
-  Node* smi_i32 = BuildChangeSmiToInt32(value);
-  Node* is_smi_effect = *effect_;
-
-  // is_heap
-  *control_ = is_heap;
-  Node* heap_f64 = BuildLoadHeapNumberValue(value, is_heap);
-
-  // *control_ needs to point to the current control dependency (is_heap) in
-  // case BuildI32SConvertF64 needs to insert nodes that depend on the "current"
-  // control node.
-  Node* heap_i32 = BuildI32SConvertF64(heap_f64, position);
-  // *control_ contains the control node that should be used when merging the
-  // result for the catch clause. It may be different than *control_ because
-  // BuildI32SConvertF64 may introduce a new control node (used for trapping if
-  // heap_f64 cannot be converted to an i32.
-  is_heap = *control_;
-  Node* is_heap_effect = *effect_;
-
-  Node* merge = graph()->NewNode(common->Merge(2), is_heap, is_smi);
-  Node* effect_merge = graph()->NewNode(common->EffectPhi(2), is_heap_effect,
-                                        is_smi_effect, merge);
-
-  Node* value_i32 = graph()->NewNode(
-      common->Phi(MachineRepresentation::kWord32, 2), heap_i32, smi_i32, merge);
-
-  *control_ = merge;
-  *effect_ = effect_merge;
-  return value_i32;
+  parameters[0] = value;
+  value = BuildCallToRuntime(Runtime::kWasmGetExceptionTag, parameters,
+                             arraysize(parameters));
+  return BuildChangeSmiToInt32(value);
 }
 
 Node* WasmGraphBuilder::BuildI32DivS(Node* left, Node* right,

src/heap-symbols.h

@@ -203,7 +203,8 @@
   V(will_handle_string, "willHandle")                              \
   V(writable_string, "writable")                                   \
   V(year_string, "year")                                           \
-  V(zero_string, "0")
+  V(zero_string, "0")                                              \
+  V(WasmExceptionTag_string, "WasmExceptionTag")
 
 #define PRIVATE_SYMBOL_LIST(V)              \
   V(array_iteration_kind_symbol)            \

src/isolate-inl.h

@@ -88,8 +88,13 @@ bool Isolate::is_catchable_by_javascript(Object* exception) {
 }
 
 bool Isolate::is_catchable_by_wasm(Object* exception) {
-  return is_catchable_by_javascript(exception) &&
-         (exception->IsNumber() || exception->IsSmi());
+  if (!is_catchable_by_javascript(exception) || !exception->IsJSError())
+    return false;
+  HandleScope scope(this);
+  Handle<Object> exception_handle(exception, this);
+  return JSReceiver::HasProperty(Handle<JSReceiver>::cast(exception_handle),
+                                 factory()->WasmExceptionTag_string())
+      .IsJust();
 }
 
 void Isolate::FireBeforeCallEnteredCallback() {

src/messages.h

@@ -703,6 +703,7 @@ class ErrorUtils : public AllStatic {
   T(WasmTrapFuncSigMismatch, "function signature mismatch")                    \
   T(WasmTrapInvalidIndex, "invalid index into function table")                 \
   T(WasmTrapTypeError, "invalid type")                                         \
+  T(WasmExceptionError, "wasm exception")                                      \
   /* Asm.js validation related */                                              \
   T(AsmJsInvalid, "Invalid asm.js: %")                                         \
   T(AsmJsCompiled, "Converted asm.js to WebAssembly: %")                       \

src/runtime/runtime-wasm.cc

@@ -23,6 +23,9 @@ namespace v8 {
 namespace internal {
 
 namespace {
+
+constexpr int kInvalidExceptionTag = -1;
+
 WasmInstanceObject* GetWasmInstanceOnStackTop(Isolate* isolate) {
   DisallowHeapAllocation no_allocation;
   const Address entry = Isolate::c_entry_fp(isolate->thread_local_top());
@@ -142,20 +145,20 @@ RUNTIME_FUNCTION(Runtime_WasmThrowTypeError) {
 }
 
 RUNTIME_FUNCTION(Runtime_WasmThrow) {
-  // TODO(kschimpf): Change this to build a runtime exception with
-  // wasm properties, instead of just an integer.
   HandleScope scope(isolate);
-  DCHECK_EQ(2, args.length());
-  CONVERT_SMI_ARG_CHECKED(lower, 0);
-  CONVERT_SMI_ARG_CHECKED(upper, 1);
-
-  const int32_t thrown_value = (upper << 16) | lower;
-
-  // Set the current isolate's context.
   DCHECK_NULL(isolate->context());
   isolate->set_context(GetWasmContextOnStackTop(isolate));
 
-  return isolate->Throw(*isolate->factory()->NewNumberFromInt(thrown_value));
+  DCHECK_EQ(1, args.length());
+  Handle<Object> tag = args.at(0);
+  Handle<Object> except = isolate->factory()->NewWasmRuntimeError(
+      static_cast<MessageTemplate::Template>(
+          MessageTemplate::kWasmExceptionError));
+  DCHECK(tag->IsSmi());
+  CHECK(!JSReceiver::SetProperty(
+             except, isolate->factory()->WasmExceptionTag_string(), tag, STRICT)
+             .is_null());
+  return isolate->Throw(*except);
 }
 
 RUNTIME_FUNCTION(Runtime_WasmRethrow) {
@@ -166,14 +169,28 @@ RUNTIME_FUNCTION(Runtime_WasmRethrow) {
   return isolate->Throw(exception);
 }
 
+RUNTIME_FUNCTION(Runtime_WasmGetExceptionTag) {
+  HandleScope scope(isolate);
+  DCHECK_EQ(1, args.length());
+  Object* exception = args[0];
+  DCHECK(isolate->is_catchable_by_wasm(exception));
+  Handle<Object> exception_handle(exception, isolate);
+  Handle<Object> tag_handle;
+  if (JSReceiver::GetProperty(Handle<JSReceiver>::cast(exception_handle),
+                              isolate->factory()->WasmExceptionTag_string())
+          .ToHandle(&tag_handle)) {
+    if (tag_handle->IsSmi()) return *tag_handle;
+  }
+  return Smi::FromInt(kInvalidExceptionTag);
+}
+
 RUNTIME_FUNCTION(Runtime_WasmSetCaughtExceptionValue) {
+  // TODO(kschimpf): Implement stack of caught exceptions, rather than
+  // just innermost.
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());
   Object* exception = args[0];
-  // The unwinder will only deliver exceptions to wasm if the exception is a
-  // Number or a Smi (which we have just converted to a Number.) This logic
-  // lives in Isolate::is_catchable_by_wasm(Object*).
-  CHECK(exception->IsNumber());
+  DCHECK(isolate->is_catchable_by_wasm(exception));
   isolate->set_wasm_caught_exception(exception);
   return exception;
 }

src/runtime/runtime.h

@@ -636,8 +636,9 @@ namespace internal {
   F(ThrowWasmErrorFromTrapIf, 1, 1)    \
   F(ThrowWasmStackOverflow, 0, 1)      \
   F(WasmThrowTypeError, 0, 1)          \
-  F(WasmThrow, 2, 1)                   \
+  F(WasmThrow, 1, 1)                   \
   F(WasmRethrow, 0, 1)                 \
+  F(WasmGetExceptionTag, 1, 1)         \
   F(WasmSetCaughtExceptionValue, 1, 1) \
   F(WasmRunInterpreter, 3, 1)          \
   F(WasmStackGuard, 0, 1)              \

test/mjsunit/wasm/wasm-constants.js

@@ -385,6 +385,9 @@ function assertWasmThrows(values, code) {
       eval(code);
     }
   } catch (e) {
+    assertTrue(e instanceof WebAssembly.RuntimeError);
+    assertNotEquals(e['WasmExceptionTag'], undefined);
+    assertTrue(Number.isInteger(e['WasmExceptionTag']));
     // TODO(kschimpf): Extract values from the exception.
     let e_values = [];
     assertEquals(values, e_values);