[vm/compiler] Prune SSA during construction using liveness

Only insert Phi at JoinEntry and Parameter at CatchBlockEntry if
corresponding variable is alive into the block. This keeps insertion
consistent with environment pruning logic employed by RenameRecursive:
which prunes dead variables out environments when entering the block
based on contents of the live_in set.

Previously we would insert Parameter instructions for dead variables
which lead to an incorrect (unsound) set of catch entry moves
generated by the backend due to an inconsistency between Parameter
insertion and environment pruning logic and the fact that
catch entry moves generator would simply ignore all moves if the
source of the move was an optimized_out (aka constant_dead) marker.

The following situation was possible: if the variable was assigned
inside the try and partially alive inside of it (e.g. we had
blocks where it was alive and blocks where it was not) then we
would end up inserting Parameter for it, but some of the exceptional
edges (corresponding to blocks where the variable was dead) would
carry optimized_out value into that Parameter. However
`CatchEntryMoveFor` would not record these moves leading to
uninitialized garbage arriving on those exceptional edges instead.

Note that Phis were not susceptible to the same issue as Parameters
because regalloc / backend would not treat optimized_out value
specially.

This change converts silent treatment of optimized_out in the
catch entry moves generation into a release assert instead.

Reviewing other uses of constant_dead in the compiler revealed
a bug in canonicalization rule for `o.runtimeType` which
did not take exceptional edges into account. We add a
regression test for that and fix that issue as well.

Fixes #59822

TEST=vm/dart/regress_59822,vm/dart/regress_runtime_type_in_trycatch

Cq-Include-Trybots: luci.dart.try:vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-linux-product-x64-try,vm-aot-linux-release-arm64-try,vm-aot-linux-release-simarm_x64-try,vm-aot-linux-release-x64-try
Change-Id: I8dc397cb98d84048a1791b9dd6baa7586a2688c6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403583
Reviewed-by: Alexander Aprelev <[email protected]>
Commit-Queue: Slava Egorov <[email protected]>

Author: mraleph

runtime/tests/vm/dart/regress_59822_test.dart

@@ -0,0 +1,38 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+//
+// Regression test for crash https://github.com/dart-lang/sdk/issues/59822
+//
+// VMOptions= --optimization_counter_threshold=1 --deterministic
+
+@pragma('vm:never-inline')
+void use(Object? v) {}
+
+@pragma('vm:never-inline')
+void foo(bool a, bool b) {
+  String v = "a";
+  try {
+    if (a) {
+      if (b) {
+        // Live assignment which flows out of the block.
+        v = "b";
+      }
+      // Last use of `v` the variable is dead after this point.
+      use(v); // (1)
+    }
+    use(0); // (2)
+  } catch (e) {
+    // `v` is not live into the catch. Notice that different values of `v`
+    // will flow on exception edges: from (1) phi("a", "b") flows but from
+    // (2) optimized out value flows because `v` is dead at that point and
+    // pruned from the environment. This means TCO pass should not be able
+    // to remove Parameter corresponding to v by itself - but such
+    // Parameter should not be inserted by SSA construction either.
+  }
+}
+
+void main() {
+  foo(true, true);
+  foo(true, true);
+}

runtime/tests/vm/dart/regress_runtime_type_in_trycatch_test.dart

@@ -0,0 +1,29 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+//
+// Regression test which verifies that we don't accidentally eliminate
+// runtimeType invocation which flows into a catch block entry parameter.
+
+import 'package:expect/expect.dart';
+
+class A {}
+
+@pragma('vm:never-inline')
+void foo(Object? v) {
+  Type t = String;
+  try {
+    t = v.runtimeType;
+    if (v is A) {
+      throw 'goto catch';
+    }
+  } catch (e) {
+    Expect.equals(A, t);
+    return;
+  }
+}
+
+void main() {
+  foo('hi');
+  foo(A());
+}

runtime/vm/compiler/backend/flow_graph.cc

@@ -1013,10 +1013,9 @@ void FlowGraph::ComputeSSA(
 
   GrowableArray<PhiInstr*> live_phis;
 
-  InsertCatchBlockParams(preorder_, variable_liveness.ComputeAssignedVars());
+  InsertCatchBlockParams(preorder_, variable_liveness);
   // Inserted catch block parameters add to the set of assigned variables.
-  InsertPhis(preorder_, variable_liveness.ComputeAssignedVars(),
-             dominance_frontier, &live_phis);
+  InsertPhis(preorder_, variable_liveness, dominance_frontier, &live_phis);
 
   // Rename uses to reference inserted phis where appropriate.
   // Collect phis that reach a non-environment use.
@@ -1144,9 +1143,11 @@ void FlowGraph::CompressPath(intptr_t start_index,
 }
 
 void FlowGraph::InsertPhis(const GrowableArray<BlockEntryInstr*>& preorder,
-                           const GrowableArray<BitVector*>& assigned_vars,
+                           VariableLivenessAnalysis& variable_liveness,
                            const GrowableArray<BitVector*>& dom_frontier,
                            GrowableArray<PhiInstr*>* live_phis) {
+  const auto& assigned_vars = variable_liveness.ComputeAssignedVars();
+
   const intptr_t block_count = preorder.length();
   // Map preorder block number to the highest variable index that has a phi
   // in that block.  Use it to avoid inserting multiple phis for the same
@@ -1183,6 +1184,14 @@ void FlowGraph::InsertPhis(const GrowableArray<BlockEntryInstr*>& preorder,
            !it.Done(); it.Advance()) {
         int index = it.Current();
         if (has_already[index] < var_index) {
+          if (!always_live && !variable_liveness.GetLiveInSet(preorder[index])
+                                   ->Contains(var_index)) {
+            // Avoid inserting Phis for variables which are not
+            // live into the join block. Such Phis instructions can not
+            // have real uses: they correspond to dead variables which can
+            // be pruned from the environment (e.g. by RenameRecursive).
+            continue;
+          }
           JoinEntryInstr* join = preorder[index]->AsJoinEntry();
           ASSERT(join != nullptr);
           PhiInstr* phi = join->InsertPhi(
@@ -1243,25 +1252,80 @@ void FlowGraph::AddCatchEntryParameter(intptr_t var_index,
 
 void FlowGraph::InsertCatchBlockParams(
     const GrowableArray<BlockEntryInstr*>& preorder,
-    const GrowableArray<BitVector*>& assigned_vars) {
+    VariableLivenessAnalysis& variable_liveness) {
+  // Parameters at CatchBlockEntry serve the same function as Phis at Joins.
+  //
+  // They represent a merge between versions of a variable which arrive on
+  // different control flow edges. For Joins these are explicit control flow
+  // edges between blocks, for CatchBlockEntry these are implicit edges
+  // from throwing instructions covered by this catch block.
+  // Consider two blocks A and B. If the block A has an assignment into a
+  // variable V and there is a path from A to B: B0 = A, B1, B2, ..., Bn = B
+  // such that B{i+1} is a successor of B{i} and B{i} does not dominate B{i+1}
+  // then B needs a Phi function or Parameter corresponding
+  // to the variable V. (for the purposes of shortening the definition we
+  // consider catch entry to be an immediate successor of a block
+  // that it covers).
+  //
+  // Now let us consider two distinct cases:
+  // 1) normal flow graph for a function
+  // 2) OSR relinked flow graph.
+  //
+  // In the first case we have the property that if a path B0, ..., Bn ends up
+  // in the CatchEntry and none of Bi is CatchEntry then this path either fully
+  // contained within the corresponding try block or crosses into the try
+  // block via TryEntry. This property means that if a block A contains an
+  // assignment to a variable V then this assignment can only induce
+  // a Parameter instruction in the surrounding catches (if any).
+  //
+  // This property does hold for the OSR relinked graph because all TryEntry
+  // blocks are lifted up to the graph entry. That means there can exist paths
+  // from a block with an assignment to a variable which itself is not
+  // covered by a try/catch to the catch block which do not cross through
+  // the try entry. This means when inserting Parameter instructions for OSR
+  // relinked graph we can't limit insertion using a list of variables assigned
+  // inside blocks covered by catch and instead we should insert parameters for
+  // all live variables.
+  //
+  // Note: if we computed dominance frontier treating catch block entries as
+  // successors to blocks they cover then we could combine Parameter and Phi
+  // insertion into a single pass algorithm and remove distinction between
+  // OSR and non-OSR case.
   intptr_t fixed_slot_count = Utils::Maximum(
       graph_entry_->fixed_slot_count(),
       (IsCompiledForOsr() ? osr_variable_count() : variable_count()) -
           num_direct_parameters());
   if (IsCompiledForOsr()) {
-    for (intptr_t i = 0; i < try_entries_.length(); i++) {
-      if (try_entries_[i] == nullptr) {
+    for (auto try_entry : try_entries_) {
+      if (try_entry == nullptr) {
         continue;
       }
-      CatchBlockEntryInstr* catch_entry = try_entries_[i]->catch_target();
-      // Will insert parameters for all variables
-      for (intptr_t i = 0, n = variable_count(); i < n; ++i) {
-        // Local variables will arrive on the stack while exception and
-        // stack trace will be passed in fixed registers.
-        AddCatchEntryParameter(i, catch_entry);
+      auto catch_entry = try_entry->catch_target();
+      const auto exception_var_index =
+          EnvIndex(catch_entry->raw_exception_var());
+      const auto stacktrace_var_index =
+          EnvIndex(catch_entry->raw_stacktrace_var());
+      for (intptr_t var_index = 0, n = variable_count(); var_index < n;
+           ++var_index) {
+        const bool always_live = !FLAG_prune_dead_locals ||
+                                 IsImmortalVariable(var_index) ||
+                                 var_index == exception_var_index ||
+                                 var_index == stacktrace_var_index;
+
+        if (!always_live &&
+            !variable_liveness.GetLiveInSet(catch_entry)->Contains(var_index)) {
+          // Avoid inserting Parameter for variables which are not
+          // live into the catch block. Such Parameter instructions can not
+          // have real uses: they correspond to dead variables which can
+          // be pruned from the environment (e.g. by RenameRecursive).
+          continue;
+        }
+        AddCatchEntryParameter(var_index, catch_entry);
       }
     }
   } else {
+    const auto& assigned_vars = variable_liveness.ComputeAssignedVars();
+
     const intptr_t block_count = preorder.length();
     // Map preorder block number to the highest variable index that has an
     // assignment in that block.  Use it to avoid inserting multiple parameters
@@ -1276,11 +1340,11 @@ void FlowGraph::InsertCatchBlockParams(
     has_already.EnsureLength(block_count, -1);
     work.EnsureLength(block_count, -1);
 
-    for (intptr_t i = 0; i < try_entries_.length(); i++) {
-      if (try_entries_[i] == nullptr) {
+    for (auto try_entry : try_entries_) {
+      if (try_entry == nullptr) {
         continue;
       }
-      CatchBlockEntryInstr* catch_block = try_entries_[i]->catch_target();
+      auto catch_block = try_entry->catch_target();
 
       AddCatchEntryParameter(EnvIndex(catch_block->raw_exception_var()),
                              catch_block);
@@ -1295,6 +1359,9 @@ void FlowGraph::InsertCatchBlockParams(
     GrowableArray<BlockEntryInstr*> worklist;
     // Look at every variable in turn.
     for (intptr_t var_index = 0; var_index < variable_count(); ++var_index) {
+      const bool always_live =
+          !FLAG_prune_dead_locals || IsImmortalVariable(var_index);
+
       // Add to the worklist each block containing an assignment to this
       // variable.
       for (intptr_t block_index = 0; block_index < block_count; ++block_index) {
@@ -1312,6 +1379,14 @@ void FlowGraph::InsertCatchBlockParams(
               GetCatchBlockByTryIndex(try_index);
           intptr_t catch_entry_index = catch_entry->preorder_number();
           if (has_already[catch_entry_index] < var_index) {
+            if (!always_live && !variable_liveness.GetLiveInSet(catch_entry)
+                                     ->Contains(var_index)) {
+              // Avoid inserting Parameter for variables which are not
+              // live into the catch block. Such Parameter instructions can not
+              // have real uses: they correspond to dead variables which can
+              // be pruned from the environment (e.g. by RenameRecursive).
+              continue;
+            }
             AddCatchEntryParameter(var_index, catch_entry);
             has_already[catch_entry_index] = var_index;
             if (work[catch_entry_index] < var_index) {
@@ -1590,17 +1665,18 @@ void FlowGraph::RenameRecursive(
     PopulateEnvironmentFromCatchEntry(catch_entry, env);
   }
 
-  if (!block_entry->IsGraphEntry() &&
-      !block_entry->IsBlockEntryWithInitialDefs()) {
-    // Prune non-live variables at block entry by replacing their environment
-    // slots with null.
-    BitVector* live_in = variable_liveness->GetLiveInSet(block_entry);
-    for (intptr_t i = 0; i < variable_count(); i++) {
-      // TODO(fschneider): Make sure that live_in always contains the
-      // CurrentContext variable to avoid the special case here.
-      if (FLAG_prune_dead_locals && !live_in->Contains(i) &&
-          !IsImmortalVariable(i)) {
-        (*env)[i] = constant_dead();
+  if (FLAG_prune_dead_locals) {
+    if (!block_entry->IsBlockEntryWithInitialDefs() ||
+        block_entry->IsCatchBlockEntry()) {
+      // Prune non-live variables at block entry by replacing their environment
+      // slots with null.
+      BitVector* live_in = variable_liveness->GetLiveInSet(block_entry);
+      for (intptr_t i = 0; i < variable_count(); i++) {
+        // TODO(fschneider): Make sure that live_in always contains the
+        // CurrentContext variable to avoid the special case here.
+        if (!live_in->Contains(i) && !IsImmortalVariable(i)) {
+          (*env)[i] = constant_dead();
+        }
       }
     }
   }

runtime/vm/compiler/backend/flow_graph.h

@@ -690,13 +690,13 @@ class FlowGraph : public ZoneAllocated {
   void AttachEnvironment(Instruction* instr, GrowableArray<Definition*>* env);
 
   void InsertPhis(const GrowableArray<BlockEntryInstr*>& preorder,
-                  const GrowableArray<BitVector*>& assigned_vars,
+                  VariableLivenessAnalysis& variable_liveness,
                   const GrowableArray<BitVector*>& dom_frontier,
                   GrowableArray<PhiInstr*>* live_phis);
   void AddCatchEntryParameter(intptr_t var_index,
                               CatchBlockEntryInstr* catch_entry);
   void InsertCatchBlockParams(const GrowableArray<BlockEntryInstr*>& preorder,
-                              const GrowableArray<BitVector*>& assigned_vars);
+                              VariableLivenessAnalysis& variable_liveness);
 
   void RemoveDeadPhis(GrowableArray<PhiInstr*>* live_phis);
 

runtime/vm/compiler/backend/flow_graph_compiler.cc

@@ -365,10 +365,7 @@ static CatchEntryMove CatchEntryMoveFor(compiler::Assembler* assembler,
                                         const Location& src,
                                         intptr_t dst_index) {
   if (src.IsConstant()) {
-    // Skip dead locations.
-    if (src.constant().ptr() == Object::optimized_out().ptr()) {
-      return CatchEntryMove();
-    }
+    RELEASE_ASSERT(src.constant().ptr() != Object::optimized_out().ptr());
     const intptr_t pool_index =
         assembler->object_pool_builder().FindObject(src.constant());
     return CatchEntryMove::FromSlot(CatchEntryMove::SourceKind::kConstant,

runtime/vm/compiler/backend/il.cc

@@ -5721,6 +5721,31 @@ static Definition* CanonicalizeStringInterpolateSingle(StaticCallInstr* call,
   return call;
 }
 
+static bool CanFlowIntoCatch(FlowGraph* flow_graph, Definition* defn) {
+  if (flow_graph->try_entries().is_empty()) {
+    // No try/catch blocks.
+    return false;
+  }
+
+  if (defn->env_use_list() == nullptr) {
+    // No uses in environments.
+    return false;
+  }
+
+  for (auto use : defn->environment_uses()) {
+    if (use->instruction()->MayThrow() &&
+        use->instruction()->GetBlock()->InsideTryBlock()) {
+      // Conservatively assume that this value might end up in the
+      // corresponding catch. Ideally we would like to check if
+      // there is a corresponding catch parameter, but there is no
+      // straightforward way to do that.
+      return true;
+    }
+  }
+
+  return false;
+}
+
 Definition* StaticCallInstr::Canonicalize(FlowGraph* flow_graph) {
   auto& compiler_state = CompilerState::Current();
 
@@ -5760,9 +5785,9 @@ Definition* StaticCallInstr::Canonicalize(FlowGraph* flow_graph) {
   }
 
   if (kind == MethodRecognizer::kObjectRuntimeType) {
-    if (input_use_list() == nullptr) {
+    if (input_use_list() == nullptr && !CanFlowIntoCatch(flow_graph, this)) {
       // This function has only environment uses. In precompiled mode it is
-      // fine to remove it - because we will never deoptimize.
+      // fine to remove it if the value can't flow into the catch block entry.
       return flow_graph->constant_dead();
     }
   }

runtime/vm/compiler/backend/il.h

@@ -2653,6 +2653,10 @@ class Definition : public Instruction {
     return ValueListIterable(input_use_list_);
   }
 
+  ValueListIterable environment_uses() const {
+    return ValueListIterable(env_use_list_);
+  }
+
   void AddInputUse(Value* value) { Value::AddToList(value, &input_use_list_); }
   void AddEnvUse(Value* value) { Value::AddToList(value, &env_use_list_); }