[stringrefs] Expand domain of string.eq to include null values

See https://github.com/WebAssembly/stringref/pull/32.

Bug: v8:12868
Change-Id: Ia804809b01eb06163c4477b3f06ac09e016b6277
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3747875Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Andy Wingo <wingo@igalia.com>
Cr-Commit-Position: refs/heads/main@{#81572}

src/compiler/wasm-compiler.cc

@@ -5846,11 +5846,15 @@ Node* WasmGraphBuilder::StringConcat(Node* head, CheckForNull head_null_check,
 Node* WasmGraphBuilder::StringEqual(Node* a, CheckForNull a_null_check, Node* b,
                                     CheckForNull b_null_check,
                                     wasm::WasmCodePosition position) {
-  if (a_null_check == kWithNullCheck) a = AssertNotNull(a, position);
-  if (b_null_check == kWithNullCheck) b = AssertNotNull(b, position);
-
   auto done = gasm_->MakeLabel(MachineRepresentation::kWord32);
+  // Covers "identical string pointer" and "both are null" cases.
   gasm_->GotoIf(gasm_->TaggedEqual(a, b), &done, Int32Constant(1));
+  if (a_null_check == kWithNullCheck) {
+    gasm_->GotoIf(gasm_->IsNull(a), &done, Int32Constant(0));
+  }
+  if (b_null_check == kWithNullCheck) {
+    gasm_->GotoIf(gasm_->IsNull(b), &done, Int32Constant(0));
+  }
   gasm_->Goto(&done, gasm_->CallBuiltin(Builtin::kWasmStringEqual,
                                         Operator::kNoDeopt, a, b));
   gasm_->Bind(&done);

src/wasm/baseline/liftoff-compiler.cc

@@ -6458,16 +6458,51 @@ class LiftoffCompiler {
 
   void StringEq(FullDecoder* decoder, const Value& a, const Value& b,
                 Value* result) {
-    LiftoffRegList pinned;
+    LiftoffRegister result_reg(kReturnRegister0);
+    LiftoffRegList pinned{result_reg};
+    LiftoffRegister b_reg = pinned.set(__ PopToModifiableRegister(pinned));
+    LiftoffRegister a_reg = pinned.set(__ PopToModifiableRegister(pinned));
 
-    LiftoffRegister b_reg = pinned.set(__ PopToRegister(pinned));
-    MaybeEmitNullCheck(decoder, b_reg.gp(), pinned, b.type);
-    LiftoffAssembler::VarState b_var(kRef, b_reg, 0);
+    __ SpillAllRegisters();
 
-    LiftoffRegister a_reg = pinned.set(__ PopToRegister(pinned));
-    MaybeEmitNullCheck(decoder, a_reg.gp(), pinned, a.type);
-    LiftoffAssembler::VarState a_var(kRef, a_reg, 0);
+    Label done;
+
+    {
+      LiftoffRegister null = pinned.set(__ GetUnusedRegister(kGpReg, pinned));
+      bool check_for_null = a.type.is_nullable() || b.type.is_nullable();
+      if (check_for_null) {
+        LoadNullValue(null.gp(), pinned);
+      }
+
+      FREEZE_STATE(frozen);
 
+      // If values pointer-equal, result is 1.
+      __ LoadConstant(result_reg, WasmValue(int32_t{1}));
+      __ emit_cond_jump(LiftoffCondition::kEqual, &done, kRefNull, a_reg.gp(),
+                        b_reg.gp(), frozen);
+
+      // Otherwise if either operand is null, result is 0.
+      if (check_for_null) {
+        __ LoadConstant(result_reg, WasmValue(int32_t{0}));
+        if (a.type.is_nullable()) {
+          __ emit_cond_jump(LiftoffCondition::kEqual, &done, kRefNull,
+                            a_reg.gp(), null.gp(), frozen);
+        }
+        if (b.type.is_nullable()) {
+          __ emit_cond_jump(LiftoffCondition::kEqual, &done, kRefNull,
+                            b_reg.gp(), null.gp(), frozen);
+        }
+      }
+
+      // Ending the frozen state here is fine, because we already spilled the
+      // rest of the cache, and the subsequent runtime call will reset the cache
+      // state anyway.
+    }
+
+    // Operands are pointer-distinct and neither is null; call out to the
+    // runtime.
+    LiftoffAssembler::VarState a_var(kRef, a_reg, 0);
+    LiftoffAssembler::VarState b_var(kRef, b_reg, 0);
     CallRuntimeStub(WasmCode::kWasmStringEqual,
                     MakeSig::Returns(kI32).Params(kRef, kRef),
                     {
@@ -6477,7 +6512,8 @@ class LiftoffCompiler {
                     decoder->position());
     RegisterDebugSideTableEntry(decoder, DebugSideTableBuilder::kDidSpill);
 
-    LiftoffRegister result_reg(kReturnRegister0);
+    __ bind(&done);
+
     __ PushRegister(kI32, result_reg);
   }
 

test/mjsunit/wasm/stringrefs-exec.js

@@ -561,6 +561,13 @@ function makeWtf16TestDataSegment() {
       kExprRefNull, kStringRefCode,
       kGCPrefix, kExprStringEq
     ]);
+  builder.addFunction("eq_both_null", kSig_i_v)
+    .exportFunc()
+    .addBody([
+      kExprRefNull, kStringRefCode,
+      kExprRefNull, kStringRefCode,
+      kGCPrefix, kExprStringEq
+    ]);
 
   let instance = builder.instantiate();
 
@@ -570,12 +577,11 @@ function makeWtf16TestDataSegment() {
       assertEquals(result, instance.exports.eq(head, tail));
       assertEquals(result, instance.exports.eq(head + head, tail + tail));
     }
+    assertEquals(0, instance.exports.eq_null_a(head))
+    assertEquals(0, instance.exports.eq_null_b(head))
   }
 
-  assertThrows(() => instance.exports.eq_null_a("hey"),
-               WebAssembly.RuntimeError, "dereferencing a null pointer");
-  assertThrows(() => instance.exports.eq_null_b("hey"),
-               WebAssembly.RuntimeError, "dereferencing a null pointer");
+  assertEquals(1, instance.exports.eq_both_null());
 })();
 
 (function TestStringIsUSVSequence() {