[wasm-gc][turbofan] Allow incompatible mutability in CsaLoadElimination

Loading from/storing to the same field with incompatible mutabilities
is possible in unreachable code, specifically when a value is cast to
two different types with incompatible mutability for the same field
offset. Therefore, we allow this pattern in CsaLoadElimination.
When we detect it, we emit an Unreachable node to immediately crash the
program in case this unreachable code is somehow executed.

Bug: v8:7748, v8:12874
Change-Id: Ieb359d3e1b9f7bc4a91c556af2bba0507526d20e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3644806
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80587}

src/compiler/csa-load-elimination.cc

@@ -328,6 +328,19 @@ void CsaLoadElimination::HalfState::Print() const {
   Print(arbitrary_unknown_entries_);
 }
 
+// We may encounter a mutable/immutable inconsistency if the same field offset
+// is loaded/stored from the same object both as mutable and immutable. This can
+// only happen in code where the object has been cast to two different
+// incompatible types, i.e. in unreachable code. For safety, we introduce an
+// Unreachable node before the load/store.
+Reduction CsaLoadElimination::AssertUnreachable(Node* node) {
+  Node* effect = NodeProperties::GetEffectInput(node);
+  Node* control = NodeProperties::GetControlInput(node);
+  Node* unreachable =
+      graph()->NewNode(jsgraph()->common()->Unreachable(), effect, control);
+  return Replace(unreachable);
+}
+
 Reduction CsaLoadElimination::ReduceLoadFromObject(Node* node,
                                                    ObjectAccess const& access) {
   DCHECK(node->opcode() == IrOpcode::kLoadFromObject ||
@@ -338,10 +351,12 @@ Reduction CsaLoadElimination::ReduceLoadFromObject(Node* node,
   AbstractState const* state = node_states_.Get(effect);
   if (state == nullptr) return NoChange();
   bool is_mutable = node->opcode() == IrOpcode::kLoadFromObject;
-  // We should never find a field in the wrong half-state.
-  DCHECK((is_mutable ? &state->immutable_state : &state->mutable_state)
-             ->Lookup(object, offset)
-             .IsEmpty());
+  // We can only find the field in the wrong half-state in unreachable code.
+  if (!(is_mutable ? &state->immutable_state : &state->mutable_state)
+           ->Lookup(object, offset)
+           .IsEmpty()) {
+    return AssertUnreachable(node);
+  }
   HalfState const* half_state =
       is_mutable ? &state->mutable_state : &state->immutable_state;
 
@@ -384,8 +399,10 @@ Reduction CsaLoadElimination::ReduceStoreToObject(Node* node,
   if (state == nullptr) return NoChange();
   MachineRepresentation repr = access.machine_type.representation();
   if (node->opcode() == IrOpcode::kStoreToObject) {
-    // We should not find the field in the wrong half-state.
-    DCHECK(state->immutable_state.Lookup(object, offset).IsEmpty());
+    // We can only find the field in the wrong half-state in unreachable code.
+    if (!(state->immutable_state.Lookup(object, offset).IsEmpty())) {
+      return AssertUnreachable(node);
+    }
     HalfState const* mutable_state =
         state->mutable_state.KillField(object, offset, repr);
     mutable_state = mutable_state->AddField(object, offset, value, repr);
@@ -393,8 +410,10 @@ Reduction CsaLoadElimination::ReduceStoreToObject(Node* node,
         zone()->New<AbstractState>(*mutable_state, state->immutable_state);
     return UpdateState(node, new_state);
   } else {
-    // We should not find the field in the wrong half-state.
-    DCHECK(state->mutable_state.Lookup(object, offset).IsEmpty());
+    // We can only find the field in the wrong half-state in unreachable code.
+    if (!(state->mutable_state.Lookup(object, offset).IsEmpty())) {
+      return AssertUnreachable(node);
+    }
     // We should not initialize the same immutable field twice.
     DCHECK(state->immutable_state.Lookup(object, offset).IsEmpty());
     HalfState const* immutable_state =

src/compiler/csa-load-elimination.h

@@ -165,6 +165,7 @@ class V8_EXPORT_PRIVATE CsaLoadElimination final
                                         AbstractState const* state) const;
   Node* TruncateAndExtend(Node* node, MachineRepresentation from,
                           MachineType to);
+  Reduction AssertUnreachable(Node* node);
 
   CommonOperatorBuilder* common() const;
   MachineOperatorBuilder* machine() const;

test/mjsunit/regress/wasm/regress-12874.js

+// Copyright 2022 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --experimental-wasm-gc --no-liftoff --no-wasm-inlining
+
+d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
+
+var builder = new WasmModuleBuilder();
+
+var sig_index = builder.addType({params: [kWasmDataRef], results: [kWasmI32]});
+
+var sub1 = builder.addStruct([makeField(kWasmI32, true)]);
+var sub2 = builder.addStruct([makeField(kWasmI32, false)]);
+
+builder.addFunction('producer', makeSig([], [kWasmDataRef]))
+  .addBody([
+    kExprI32Const, 10,
+    kGCPrefix, kExprStructNew, sub1])
+  .exportFunc();
+
+builder.addFunction('main', sig_index)
+  .addBody([
+    // Cast to sub1 and write field 0.
+    kExprLocalGet, 0,
+    kGCPrefix, kExprRefCastStatic, sub1,
+    kExprI32Const, 42,
+    kGCPrefix, kExprStructSet, sub1, 0,
+    // Cast to sub2 and read field 0.
+    kExprLocalGet, 0,
+    kGCPrefix, kExprRefCastStatic, sub2,
+    kGCPrefix, kExprStructGet, sub2, 0])
+  .exportFunc();
+
+var instance = builder.instantiate();
+
+try {
+  instance.exports.main(instance.exports.producer());
+  console.log(`Failure: expected trap`);
+} catch (e) {
+  console.log(`Success: trapped: ${e}`);
+}