ThrowErrorSlowPathCode does not generate deopt info unless inside try-catch

[mraleph]

It does the following:

void ThrowErrorSlowPathCode::EmitNativeCode(FlowGraphCompiler* compiler) {
  // ...
  if ((try_index_ != kInvalidTryIndex) ||
      (compiler->CurrentTryIndex() != kInvalidTryIndex)) {
    Environment* env =
        compiler->SlowPathEnvironmentFor(instruction(), num_args);
    if (FLAG_precompiled_mode) {
      compiler->RecordCatchEntryMoves(env, try_index_);
    } else if (env != nullptr) {
      compiler->AddSlowPathDeoptInfo(deopt_id, env);
    }
  } 
  // ...
}

This is not correct in the presence of debugger, which allows pausing on uncaught exceptions and inspecting frames.

You can use the following test:

// Copyright (c) 2020, 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.
// VMOptions=--deterministic --optimization-counter-threshold=1000

import 'dart:convert';

import 'package:observatory/service_io.dart';
import 'package:test/test.dart';
import 'test_helper.dart';
import 'service_test_common.dart';

class X {
  late String _y;

  @pragma('vm:never-inline')
  String get y => _y;
}

testeeMain() async {
  final x = X();
  x._y = "";
  for (var i = 0; i < 2000; i++) x.y;

  X().y;
}

var tests = <IsolateTest>[
  hasStoppedWithUnhandledException,
  (Isolate isolate) async {
    print("We stopped!");
    var stack = await isolate.getStack();
  }
];

main(args) => runIsolateTests(args, tests,
    pause_on_unhandled_exceptions: true,
    testeeConcurrent: testeeMain,
    extraArgs: extraDebuggingArgs);

Save it to runtime/observatory/tests/service/pause_on_exception_from_slow_path_test.dart then do:

$ tools/test.py -n dartk-weak-asserts-linux-release-x64 service/pause_on_exception_from_slow

The VM would crash instead of pausing. Marking as P0 because it severely impacts usability of NNBD release.

(Reported to me by @tvolkert)

[mit-mit]

cc @franklinyow

[alexmarkov]

Possible fix: https://dart-review.googlesource.com/c/sdk/+/173360

[franklinyow]

@alexmarkov
Since this is marked as P0, can you help give some context to the issues?
Is this new or regression?
How likely user is impacted?

[alexmarkov]

@franklinyow The bug was there for a while. I see the attempt to make ThrowErrorSlowPathCode work in JIT mode in 2019 (https://dart-review.googlesource.com/c/sdk/+/92783), where the current code for generating deopt info was introduced.

ThrowErrorSlowPathCode is rarely used in JIT mode, so this bug was unnoticed. With null safety it is now used more often - for late fields without initializer and for null check operator.

Also note that it doesn't seem to cause the incorrect behavior without a debugger. Only if you're debugging an application and pause on exception and the exception is thrown for the uninitialized late field or a null value in a null check, then you will hit that bug. So, users debugging applications which were migrated / started using new null safety features may be affected.