From 45cf139faf1bff26348934644b2012ba64e3461c Mon Sep 17 00:00:00 2001
From: Iain Ireland <iireland@mozilla.com>
Date: Tue, 30 Mar 2021 22:38:33 +0000
Subject: [PATCH] Bug 1701208: Specialize OSR-only phis before inserting
 conversions r=jandem

If we specialize OSR-only phis to `MIRType::Value` during conversion, then it is possible for `adjustPhiInputs` to see a phi that is still `MIRType::None`. On x86, this ends up with us crashing while trying to box a Value.

It's more robust to do a separate pass at the end of phi specialization, before we start converting.

Differential Revision: https://phabricator.services.mozilla.com/D109976
---
 js/src/jit-test/tests/warp/bug1701208.js | 18 +++++++++
 js/src/jit/IonAnalysis.cpp               | 48 +++++++++++++++++-------
 2 files changed, 52 insertions(+), 14 deletions(-)
 create mode 100644 js/src/jit-test/tests/warp/bug1701208.js

diff --git a/js/src/jit-test/tests/warp/bug1701208.js b/js/src/jit-test/tests/warp/bug1701208.js
new file mode 100644
index 000000000000..184c8dbf24b9
--- /dev/null
+++ b/js/src/jit-test/tests/warp/bug1701208.js
@@ -0,0 +1,18 @@
+// |jit-test| --fast-warmup; --no-threads
+
+function dummy() { with ({}) {} }
+
+function foo() {
+    dummy();
+    var x = [];
+    var y = [];
+    for (var i = 0; i < 10; i++) { }
+    for (var i = 0; i < 100; i++) {
+	var swap = x;
+	x = y;
+	y = swap;
+    }
+    return x;
+}
+
+foo();
diff --git a/js/src/jit/IonAnalysis.cpp b/js/src/jit/IonAnalysis.cpp
index 8b31182fab15..8e2ad14e4ff6 100644
--- a/js/src/jit/IonAnalysis.cpp
+++ b/js/src/jit/IonAnalysis.cpp
@@ -1356,6 +1356,7 @@ class TypeAnalyzer {
   bool respecialize(MPhi* phi, MIRType type);
   bool propagateSpecialization(MPhi* phi);
   bool specializePhis();
+  bool specializeOsrOnlyPhis();
   void replaceRedundantPhi(MPhi* phi);
   bool adjustPhiInputs(MPhi* phi);
   bool adjustInputs(MDefinition* def);
@@ -1593,6 +1594,34 @@ bool TypeAnalyzer::propagateAllPhiSpecializations() {
   return true;
 }
 
+// If branch pruning removes the path from the entry block to the OSR
+// preheader, we may have phis (or chains of phis) with no operands
+// other than OsrValues. These phis will still have MIRType::None.
+// Since we don't have any information about them, we specialize them
+// as MIRType::Value.
+bool TypeAnalyzer::specializeOsrOnlyPhis() {
+  MOZ_ASSERT(graph.osrBlock());
+  MOZ_ASSERT(graph.osrPreHeaderBlock()->numPredecessors() == 1);
+
+  for (PostorderIterator block(graph.poBegin()); block != graph.poEnd();
+       block++) {
+    if (mir->shouldCancel("Specialize osr-only phis (main loop)")) {
+      return false;
+    }
+
+    for (MPhiIterator phi(block->phisBegin()); phi != block->phisEnd(); phi++) {
+      if (mir->shouldCancel("Specialize osr-only phis (inner loop)")) {
+        return false;
+      }
+
+      if (phi->type() == MIRType::None) {
+        phi->specialize(MIRType::Value);
+      }
+    }
+  }
+  return true;
+}
+
 bool TypeAnalyzer::specializePhis() {
   for (PostorderIterator block(graph.poBegin()); block != graph.poEnd();
        block++) {
@@ -1631,9 +1660,12 @@ bool TypeAnalyzer::specializePhis() {
     MBasicBlock* header = preHeader->getSingleSuccessor();
 
     if (preHeader->numPredecessors() == 1) {
-      // Branch pruning has removed the path from the entry block
-      // to the preheader. There is nothing to do in this case.
       MOZ_ASSERT(preHeader->getPredecessor(0) == graph.osrBlock());
+      // Branch pruning has removed the path from the entry block
+      // to the preheader. Specialize any phis with no non-osr inputs.
+      if (!specializeOsrOnlyPhis()) {
+        return false;
+      }
     } else if (header->isLoopHeader()) {
       for (MPhiIterator phi(header->phisBegin()); phi != header->phisEnd();
            phi++) {
@@ -1851,18 +1883,6 @@ bool TypeAnalyzer::insertConversions() {
         replaceRedundantPhi(phi);
         block->discardPhi(phi);
       } else {
-        if (phi->type() == MIRType::None) {
-          MOZ_ASSERT(graph.osrBlock());
-          MOZ_ASSERT(graph.osrPreHeaderBlock()->numPredecessors() == 1);
-          // If branch pruning removes the path from the entry block
-          // to the OSR preheader, we may have phis (or chains of
-          // phis) with no operands other than OsrValues. These phis
-          // will still have MIRType::None. Since we don't have any
-          // information about them, we specialize them as
-          // MIRType::Value.
-          phi->specialize(MIRType::Value);
-        }
-
         if (!adjustPhiInputs(phi)) {
           return false;
         }