From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Olivier=20Fl=C3=BCckiger?= <olivf@chromium.org>
Date: Mon, 24 Jun 2024 16:22:09 +0200
Subject: Fix skipped smi check due to phi hoisting
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Given we have a phi (29) assumed to be smi at graph building time, we
must not untag it's input phis (10,12) to float64.

    ╭─────►Block b2
    │       10: φᵀ r0 (n4, n29) (compressed) → (x), 3 uses
    │       12: φᵀ r2 (n6, n39) (compressed) → (x), 6 uses
        ...
    │       13: CheckedSmiUntag [n10:(x)] → (x), 2 uses
    │       14: CheckedSmiUntag [n12:(x)] → (x), 1 uses
        ...
    │╭──────17: BranchIfToBooleanTrue [n16:(x)] b3 b9
        ...
    ││ │    29: φᵀ <accumulator> (n10, n12) (compressed) → (x), 4 uses
        ...
    ││ │    33: UnsafeSmiUntag [n29:(x)] → (x), 1 uses

Doing so could invalidate the `UnsafeSmiUntag` instruction.

This can only happen when hoisting the untagging out of the loop, as
this will remove the original `CheckedSmiUntag` instruction.

Fixed: 348567825
Change-Id: I2d7f2c3b544f991be9850d44bf11c3f632a0bb46
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5645901
Reviewed-by: Darius Mercadier <dmercadier@chromium.org>
Commit-Queue: Darius Mercadier <dmercadier@chromium.org>
Auto-Submit: Olivier Flückiger <olivf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#94615}

diff --git a/src/maglev/maglev-phi-representation-selector.cc b/src/maglev/maglev-phi-representation-selector.cc
index 02e030bcb543287433c944e5f298998cdc5b7ce9..cabe3a29383b111fc0f1568a40d439e1513bc154 100644
--- a/src/maglev/maglev-phi-representation-selector.cc
+++ b/src/maglev/maglev-phi-representation-selector.cc
@@ -285,6 +285,14 @@ MaglevPhiRepresentationSelector::ProcessPhi(Phi* node) {
                                ValueRepresentation::kHoleyFloat64};
   }
 
+  // When hoisting we must ensure that we don't turn a tagged flowing into
+  // CheckedSmiUntag into a float64. This would cause us to loose the smi check
+  // which in turn can invalidate assumptions on aliasing values.
+  if (hoist_untagging.size() && node->uses_require_31_bit_value()) {
+    allowed_inputs_for_uses.Remove(
+        {ValueRepresentation::kFloat64, ValueRepresentation::kHoleyFloat64});
+  }
+
   auto intersection = possible_inputs & allowed_inputs_for_uses;
 
   TRACE_UNTAGGING("  + intersection reprs: " << intersection);
@@ -615,6 +623,7 @@ void MaglevPhiRepresentationSelector::ConvertTaggedPhiTo(
                                    TaggedToFloat64ConversionType::kOnlyNumber),
                                block, NewNodePosition::kEnd);
           } else {
+            DCHECK(!phi->uses_require_31_bit_value());
             untagged = AddNode(NodeBase::New<CheckedNumberOrOddballToFloat64>(
                                    builder_->zone(), {input},
                                    TaggedToFloat64ConversionType::kOnlyNumber),