DEFCON is fun, finding a V8 bug is even more fun

This bug was found with @101010zyl and @Reset816, big shoutout to them. Here is a brief analysis of it.

One idea in Maglev is when we come across a deterministic deoptimization, the bytecode can be marked as dead, since it will always deopt back to interpreter. When marking bytecode as dead, the current block should be finished. However, in the following code,

ReduceResult MaglevGraphBuilder::BuildCheckMaps(
    ValueNode* object, base::Vector<const compiler::MapRef> maps,
    std::optional<ValueNode*> map,
    bool has_deprecated_map_without_migration_target,
    bool migration_done_outside) {
  // TODO(verwaest): Support other objects with possible known stable maps as
  // well.
  if (compiler::OptionalHeapObjectRef constant = TryGetConstant(object)) {
    // For constants with stable maps that match one of the desired maps, we
    // don't need to emit a map check, and can use the dependency -- we
    // can't do this for unstable maps because the constant could migrate
    // during compilation.
    compiler::MapRef constant_map = constant.value().map(broker());
    if (std::find(maps.begin(), maps.end(), constant_map) != maps.end()) {
      if (constant_map.is_stable()) {
        broker()->dependencies()->DependOnStableMap(constant_map);
        return ReduceResult::Done();
      }
      // TODO(verwaest): Reduce maps to the constant map.
    } else {
      // TODO(leszeks): Insert an unconditional deopt if the constant map
      // doesn't match the required map.
    }
  } else if (!object->is_tagged() && !object->is_holey_float64()) {
    // TODO(victorgomes): Implement the holey float64 case.
    auto heap_number_map =
        MakeRef(broker(), local_isolate()->factory()->heap_number_map());
    if (std::find(maps.begin(), maps.end(), heap_number_map) != maps.end()) {
      return ReduceResult::Done();
    }
    return ReduceResult::DoneWithAbort();
  }
  NodeInfo* known_info = GetOrCreateInfoFor(object);
  // Calculates if known maps are a subset of maps, their map intersection and
  // whether we should emit check with migration.
  KnownMapsMerger merger(broker(), zone(), maps);
  merger.IntersectWithKnownNodeAspects(object, known_node_aspects());
  if (IsEmptyNodeType(IntersectType(merger.node_type(), GetType(object)))) {
    return EmitUnconditionalDeopt(DeoptimizeReason::kWrongMap);
  }
  // If the known maps are the subset of the maps to check, we are done.
  if (merger.known_maps_are_subset_of_requested_maps()) {
    // The node type of known_info can get out of sync with the possible maps.
    // For instance after merging with an effectively dead branch (i.e., check
    // contradicting all possible maps).
    // TODO(olivf) Try to combine node_info and possible maps and ensure that
    // narrowing the type also clears impossible possible_maps.
    if (!NodeTypeIs(known_info->type(), merger.node_type())) {
      known_info->UnionType(merger.node_type());
    }
#ifdef DEBUG
    // Double check that, for every possible map, it's one of the maps we'd
    // want to check.
    for (compiler::MapRef possible_map :
         known_node_aspects().TryGetInfoFor(object)->possible_maps()) {
      DCHECK_NE(std::find(maps.begin(), maps.end(), possible_map), maps.end());
    }
#endif
    return ReduceResult::Done();
  }
  if (merger.intersect_set().is_empty()) {
    return EmitUnconditionalDeopt(DeoptimizeReason::kWrongMap);
  }
  // TODO(v8:7700): Check if the {maps} - {known_maps} size is smaller than
  // {maps} \intersect {known_maps}, we can emit CheckNotMaps instead.
  // Emit checks.
  if (merger.emit_check_with_migration() && !migration_done_outside) {
    AddNewNode<CheckMapsWithMigration>({object}, merger.intersect_set(),
                                       GetCheckType(known_info->type()));
  } else if (has_deprecated_map_without_migration_target &&
             !migration_done_outside) {
    AddNewNode<CheckMapsWithMigrationAndDeopt>(
        {object}, merger.intersect_set(), GetCheckType(known_info->type()));
  } else if (map) {
    AddNewNode<CheckMapsWithAlreadyLoadedMap>({object, *map},
                                              merger.intersect_set());
  } else {
    AddNewNode<CheckMaps>({object}, merger.intersect_set(),
                          GetCheckType(known_info->type()));
  }
  merger.UpdateKnownNodeAspects(object, known_node_aspects());
  return ReduceResult::Done();
}

When the check map is determined to be always false,

 else if (!object->is_tagged() && !object->is_holey_float64()) {
    // TODO(victorgomes): Implement the holey float64 case.
    auto heap_number_map =
        MakeRef(broker(), local_isolate()->factory()->heap_number_map());
    if (std::find(maps.begin(), maps.end(), heap_number_map) != maps.end()) {
      return ReduceResult::Done();
    }
    return ReduceResult::DoneWithAbort();
  }

however, there exists a path that calls BuildCheckMaps and without properly handling this case. Then when the ReduceResult is propagated back to BuildBody, a DCHECK will be triggered.

    if (VisitSingleBytecode().IsDoneWithAbort()) {
      MarkBytecodeDead();
    }

in TryBuildElementAccess,

 else {
      RETURN_IF_ABORT(BuildCheckMaps(
          object, base::VectorOf(access_info.lookup_start_object_maps())));
    }

the error is directly propagated back. To trigger it, we can craft the following code:

let arr = [];
function foo() {
    arr[0] *= 13.37;
}
foo();
foo();
arr++;
%OptimizeMaglevOnNextCall(foo);
foo();