[wasm] Make rethrow trap on nullptr

The behaviour was clarified in the spec:
https://github.com/WebAssembly/exception-handling/pull/97

br_on_exn (which was done in another CL) and also rethrow should trap on
nullptr. This CL implements this by an explicit check in the builtin
called for rethrow.

R=jkummerow@chromium.org
CC=aheejin@chromium.org

Bug: v8:10128
Change-Id: Icb0f4e54991b3385917bf183efa825048db4cb82
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2115430
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66827}

src/builtins/builtins-definitions.h

@@ -879,6 +879,7 @@ namespace internal {
   TFS(ThrowWasmTrapElemSegmentDropped)                                         \
   TFS(ThrowWasmTrapTableOutOfBounds)                                           \
   TFS(ThrowWasmTrapBrOnExnNullRef)                                             \
+  TFS(ThrowWasmTrapRethrowNullRef)                                             \
                                                                                \
   /* WeakMap */                                                                \
   TFJ(WeakMapConstructor, kDontAdaptArgumentsSentinel)                         \

src/builtins/builtins-wasm-gen.cc

@@ -60,7 +60,16 @@ TF_BUILTIN(WasmRethrow, WasmBuiltinsAssembler) {
   TNode<Object> exception = CAST(Parameter(Descriptor::kException));
   TNode<WasmInstanceObject> instance = LoadInstanceFromFrame();
   TNode<Context> context = LoadContextFromInstance(instance);
+
+  Label nullref(this, Label::kDeferred);
+  GotoIf(TaggedEqual(NullConstant(), exception), &nullref);
+
   TailCallRuntime(Runtime::kReThrow, context, exception);
+
+  BIND(&nullref);
+  MessageTemplate message_id = MessageTemplate::kWasmTrapRethrowNullRef;
+  TailCallRuntime(Runtime::kThrowWasmError, context,
+                  SmiConstant(static_cast<int>(message_id)));
 }
 
 TF_BUILTIN(WasmTraceMemory, WasmBuiltinsAssembler) {

src/common/globals.h

@@ -1597,7 +1597,8 @@ enum class LoadSensitivity {
   V(TrapDataSegmentDropped)        \
   V(TrapElemSegmentDropped)        \
   V(TrapTableOutOfBounds)          \
-  V(TrapBrOnExnNullRef)
+  V(TrapBrOnExnNullRef)            \
+  V(TrapRethrowNullRef)
 
 enum KeyedAccessLoadMode {
   STANDARD_LOAD,

src/common/message-template.h

@@ -552,6 +552,7 @@ namespace internal {
   T(WasmTrapElemSegmentDropped, "element segment has been dropped")            \
   T(WasmTrapTableOutOfBounds, "table access out of bounds")                    \
   T(WasmTrapBrOnExnNullRef, "br_on_exn on nullref value")                      \
+  T(WasmTrapRethrowNullRef, "rethrowing nullref value")                        \
   T(WasmExceptionError, "wasm exception")                                      \
   /* Asm.js validation related */                                              \
   T(AsmJsInvalid, "Invalid asm.js: %")                                         \

src/compiler/wasm-compiler.cc

@@ -2121,7 +2121,6 @@ Node* WasmGraphBuilder::BuildDecodeException64BitValue(Node* values_array,
 }
 
 Node* WasmGraphBuilder::Rethrow(Node* except_obj) {
-  needs_stack_check_ = true;
   // TODO(v8:8091): Currently the message of the original exception is not being
   // preserved when rethrown to the console. The pending message will need to be
   // saved when caught and restored here while being rethrown.
@@ -2132,9 +2131,7 @@ Node* WasmGraphBuilder::Rethrow(Node* except_obj) {
       Operator::kNoProperties, StubCallMode::kCallWasmRuntimeStub);
   Node* call_target = mcgraph()->RelocatableIntPtrConstant(
       wasm::WasmCode::kWasmRethrow, RelocInfo::WASM_STUB_CALL);
-  return SetEffectControl(
-      graph()->NewNode(mcgraph()->common()->Call(call_descriptor), call_target,
-                       except_obj, effect(), control()));
+  return gasm_->Call(call_descriptor, call_target, except_obj);
 }
 
 Node* WasmGraphBuilder::ExceptionTagEqual(Node* caught_tag,

src/wasm/wasm-interpreter.cc

@@ -3121,6 +3121,7 @@ class ThreadImpl {
         case kExprRethrow: {
           HandleScope handle_scope(isolate_);  // Avoid leaking handles.
           WasmValue ex = Pop();
+          if (ex.to_anyref()->IsNull()) return DoTrap(kTrapRethrowNullRef, pc);
           CommitPc(pc);  // Needed for local unwinding.
           if (!DoRethrowException(ex)) return;
           ReloadFromFrameOnException(&decoder, &code, &pc, &limit);

test/mjsunit/wasm/exceptions-anyref.js

@@ -232,3 +232,48 @@ load("test/mjsunit/wasm/exceptions-utils.js");
   assertTraps(kTrapBrOnExnNullRef, () => instance.exports.call_import(2));
   assertEquals(kNoMatch, instance.exports.call_import(3));
 })();
+
+// 'rethrow' on a null-ref value should trap.
+(function TestRethrowNullRefSimple() {
+  print(arguments.callee.name);
+  let builder = new WasmModuleBuilder();
+  let except = builder.addException(kSig_v_r);
+  builder.addFunction('rethrow_nullref', kSig_v_v)
+      .addBody([
+        kExprRefNull,
+        kExprRethrow
+      ]).exportFunc();
+  let instance = builder.instantiate();
+
+  assertTraps(kTrapRethrowNullRef, () => instance.exports.rethrow_nullref());
+})();
+
+(function TestRethrowNullRefFromJS() {
+  print(arguments.callee.name);
+  let builder = new WasmModuleBuilder();
+  let except = builder.addException(kSig_v_i);
+  let imp = builder.addImport('imp', 'ort', kSig_i_i);
+  let kSuccess = 11;
+  builder.addFunction('call_import', kSig_i_i)
+      .addBody([
+        kExprTry, kWasmI32,
+          kExprLocalGet, 0,
+          kExprCallFunction, imp,
+        kExprCatch,
+          kExprRethrow,
+        kExprEnd
+      ]).exportFunc();
+  let instance;
+  function js_import(i) {
+    if (i == 0) return kSuccess;       // Will return kSuccess.
+    if (i == 1) throw new Error('1');  // Will rethrow.
+    if (i == 2) throw null;            // Will trap.
+    throw undefined;                   // Will rethrow.
+  }
+  instance = builder.instantiate({imp: {ort: js_import}});
+
+  assertEquals(kSuccess, instance.exports.call_import(0));
+  assertThrows(() => instance.exports.call_import(1), Error, '1');
+  assertTraps(kTrapRethrowNullRef, () => instance.exports.call_import(2));
+  assertThrowsEquals(() => instance.exports.call_import(3), undefined);
+})();

test/mjsunit/wasm/wasm-module-builder.js

@@ -502,6 +502,7 @@ let kTrapDataSegmentDropped   = 10;
 let kTrapElemSegmentDropped   = 11;
 let kTrapTableOutOfBounds     = 12;
 let kTrapBrOnExnNullRef       = 13;
+let kTrapRethrowNullRef       = 14;
 
 let kTrapMsgs = [
   "unreachable",
@@ -517,7 +518,8 @@ let kTrapMsgs = [
   "data segment has been dropped",
   "element segment has been dropped",
   "table access out of bounds",
-  "br_on_exn on nullref value"
+  "br_on_exn on nullref value",
+  "rethrowing nullref value"
 ];
 
 function assertTraps(trap, code) {