VM crash with type mis-match

[jensjoha]

I have the following script:

main(List<String> arguments) async {
  final Iterator<Step> iterator =
      <Step>[new Step1(), new Step2(), new Step3()].iterator;

  Future doStep(dynamic input) async {
    if (iterator.moveNext()) {
      Step step = iterator.current;
      step.run(input).then((value) async {
        return doStep(value);
      });
    }
  }

  await doStep("Start");
}

abstract class Step<I, O> {
  Future<O> run(I input);
}

class Step1 extends Step<String, String> {
  Future<String> run(String start) async => "Hello";
}

class Step2 extends Step<String, int> {
  Future<int> run(String result) async => 0;
}

class Step3 extends Step<String, String> {
  Future<String> run(String result) async => result;
}

Notice how I and O is the input and output type of each step and how it doesn't match:

• Step 1 takes String and returns String
• Step 2 takes String and returns int
• Step 3 takes String and returns String

So when step 2 returns and int and tries to give it to step 3 wanting a string things will go wrong somehow.
This is what happens:

$ out/ReleaseX64/dart t.dart

===== CRASH =====
si_signo=Segmentation fault(11), si_code=1, si_addr=0xffffffffffffffff
version=2.10.0-edge.10de0ddc4c97251869dbf66b4cf015ee47277d04 (be) (Thu Sep 17 10:41:07 2020 +0200) on "linux_x64"
pid=127993, thread=128000, isolate_group=main(0x55b3e0e58580), isolate=main(0x55b3e0e40900)
isolate_instructions=55b3de407060, vm_instructions=55b3de407060
  pc 0x00007f48ca92eb0e fp 0x00007f48c49fe298 Unknown symbol
  pc 0x00007f48ca924d85 fp 0x00007f48c49fe338 Unknown symbol
  pc 0x00007f48ca92338d fp 0x00007f48c49fe378 Unknown symbol
  pc 0x00007f48ca924a2f fp 0x00007f48c49fe3d0 Unknown symbol
  pc 0x00007f48ca92cdf8 fp 0x00007f48c49fe470 Unknown symbol
  pc 0x00007f48ca92338d fp 0x00007f48c49fe4b0 Unknown symbol
  pc 0x00007f48ca92caef fp 0x00007f48c49fe508 Unknown symbol
  pc 0x00007f48ca92c719 fp 0x00007f48c49fe568 Unknown symbol
  pc 0x00007f48ca92c3fc fp 0x00007f48c49fe5b8 Unknown symbol
  pc 0x00007f48ca92c108 fp 0x00007f48c49fe620 Unknown symbol
  pc 0x00007f48ca92b484 fp 0x00007f48c49fe6a8 Unknown symbol
  pc 0x00007f48ca92a289 fp 0x00007f48c49fe6f0 Unknown symbol
  pc 0x00007f48ca92a093 fp 0x00007f48c49fe730 Unknown symbol
  pc 0x00007f48ca929e03 fp 0x00007f48c49fe778 Unknown symbol
  pc 0x00007f48ca929a64 fp 0x00007f48c49fe7b8 Unknown symbol
  pc 0x00007f48ca92997b fp 0x00007f48c49fe7e8 Unknown symbol
  pc 0x00007f48ca929833 fp 0x00007f48c49fe828 Unknown symbol
  pc 0x00007f48ca92100d fp 0x00007f48c49fe850 Unknown symbol
  pc 0x00007f48cb5026ff fp 0x00007f48c49fe8c8 Unknown symbol
  pc 0x000055b3de5a4872 fp 0x00007f48c49fe960 dart::DartEntry::InvokeCode(dart::Code const&, dart::Array const&, dart::Array const&, dart::Thread*)+0x112
  pc 0x000055b3de5a45d4 fp 0x00007f48c49fe9f0 dart::DartEntry::InvokeFunction(dart::Function const&, dart::Array const&, dart::Array const&, unsigned long)+0x2d4
  pc 0x000055b3de5a6d56 fp 0x00007f48c49fea40 dart::DartLibraryCalls::HandleMessage(dart::Object const&, dart::Instance const&)+0x1f6
  pc 0x000055b3de5dfbcc fp 0x00007f48c49fec30 dart::IsolateMessageHandler::HandleMessage(std::__2::unique_ptr<dart::Message, std::__2::default_delete<dart::Message> >)+0x4cc
  pc 0x000055b3de60d356 fp 0x00007f48c49feca0 dart::MessageHandler::HandleMessages(dart::MonitorLocker*, bool, bool)+0x146
  pc 0x000055b3de60da0a fp 0x00007f48c49fed00 dart::MessageHandler::TaskCallback()+0x1da
  pc 0x000055b3de71d878 fp 0x00007f48c49fed80 dart::ThreadPool::WorkerLoop(dart::ThreadPool::Worker*)+0x148
  pc 0x000055b3de71dd4c fp 0x00007f48c49fedb0 dart::ThreadPool::Worker::Main(unsigned long)+0x5c
  pc 0x000055b3de692f5d fp 0x00007f48c49fee70 out/ReleaseX64/dart+0x1c33f5d
-- End of DumpStackTrace
Aborted

It's not supposed to go wrong like that.

Also notice that simple changes can make the crash go away:

• Change the iterator to final Iterator<Step> iterator = <Step>[new Step1(), new Step2(), new Step1()].iterator; gives Unhandled exception: type 'int' is not a subtype of type 'String' of 'start' (though if one in step1 instead returns "Hello $start" we get the crash again).
• Change Step3 to look like Step1 (i.e. return "Hello" instead of result (or seemingly any plain string)) gives Unhandled exception: type 'int' is not a subtype of type 'String' of 'result'
• If one removes all the async and future stuff one also gets Unhandled exception: type 'int' is not a subtype of type 'String' of 'result'

/cc @mraleph

[mraleph]

/cc @mkustermann

[mkustermann]

This happens due to an optimized write-barrier when saving the original argument into the newly allocated context. It thinks the value it stores into the context cannot be a Smi (write barrier is based on LoadLocalInstr::ComputeType(), which would return nullable String as type).

The creation and population of context happens before a possible AssertAssignable and it can therefore not make the assumption that the argument is String.

(Due to overriding Steps Future<O> run(I input) the argument in Step3s Future<String> run(String result) should be marked covariant and needs therefore to be checked by the callee before we can rely on it's type.)

@sstrickl @askeksa-google Could one of you take a look at this?

[askeksa-google]

https://dart-review.googlesource.com/c/sdk/+/149063 is the first crashing commit.

[mraleph]

@askeksa-google I don't think so, that CL just improved the precision. Try something like (note String replaced with a "sealed" class A):

class A {}

abstract class B<T> {
  dynamic foo(T a);
}

class C extends B<A> {
  dynamic foo(A a) {
    return () => a;
  }
}


main() {
  print((C().foo as dynamic)(1));
}

The actual issue is somewhere else.

[mkustermann]

The bug is that LoadLocalInstr(<parameter-var>)::ComputeType() cannot rely on the static type of <parameter-var> (String in this case) if the parameter is generic-covariant-impl before we actually check it's type (we use it in the prologue when storing it's value into the context, because it's captured -- this happens before any AssertAssignable)

[a-siva]

I am not convinced that the change https://dart-review.googlesource.com/c/sdk/+/164500 made to fix this bugs needs to be cherry picked into 2.10 stable release (actually the release has already been done and so it would only go into 2.10.1 which means flutter stable will not pick it up unless another dot release of flutter is done).
The issue was reported internally, is not a known regression and from the description in the CL that fixes it only effects unoptimized code.
please comment if anybody deems this critical enough to require a 2.10.1 stable release with this cherry pick.

[mkustermann]

@a-siva The actual bug can affect flutter developers during development mode.

I think the best course of action for bug fixes that are small and well understood is to file a cherry-pick request and let the committee decide whether they want to actually pick it. Maybe the answer is no, but we should still file the CP.

[a-siva]

I think we should target this for the 2.10 hot fix (similar to #43601)

[a-siva]

@askeksa-google looks like the CL for fixing this issue has not landed yet (https://dart-review.googlesource.com/c/sdk/+/164500) in order to be considered for a hot fix into the stable channel (2.10).