Crash during deopt

[rmacnak-google]

The tests

corelib/string_fromcharcodes_test
lib/convert/json_utf8_chunk_test
vm/dart/string_equals_test

are failing on configurations

vm-reload-linux-debug-x64
vm-reload-linux-release-x64

--- Command "vm" (took 21.000694s):
DART_CONFIGURATION=DebugX64 out/DebugX64/dart --hot-reload-test-mode --sound-null-safety -Dtest_runner.configuration=vm-reload-linux-debug-x64 --ignore-unrecognized-flags --packages=/b/s/w/ir/.dart_tool/package_config.json /b/s/w/ir/tests/lib/convert/json_utf8_chunk_test.dart

exit code:
-6

stderr:
../../runtime/vm/deferred_objects.cc: 113: error: Can't locate continuation PC for deoptid 6 within dart:convert__JsonUtf8Parser@10003594_get_chunk

version=3.0.0-edge.5164ecd5ece2343bac7d7b01ded910a1025d891b (be) (Wed Feb 1 03:29:49 2023 +0000) on "linux_x64"
pid=14360, thread=14412, isolate_group=main(0x558365790000), isolate=main(0x558365791800)
os=linux, arch=x64, comp=no, sim=no
isolate_instructions=5583627cc300, vm_instructions=5583627cc300
fp=7fa20227d230, sp=7fa20227d0f8, pc=558362bebf9c
  pc 0x0000558362bebf9c fp 0x00007fa20227d230 dart::Profiler::DumpStackTrace(void*)+0x7c
  pc 0x00005583627cc4f4 fp 0x00007fa20227d310 dart::Assert::Fail(char const*, ...) const+0x84
  pc 0x0000558362a3e7ff fp 0x00007fa20227d370 dart::DeferredRetAddr::Materialize(dart::DeoptContext*)+0x25f
  pc 0x0000558362a43700 fp 0x00007fa20227d4c0 dart::DeoptContext::MaterializeDeferredObjects()+0x40
  pc 0x0000558362c66125 fp 0x00007fa20227dc10 dart::DRT_DeoptimizeMaterialize(dart::NativeArguments)+0x185
  pc 0x00007fa20d782f22 fp 0x00007fa20227dc58 Unknown symbol
  pc 0x00007fa20d783867 fp 0x00007fa20227dc80 Unknown symbol
  pc 0x0000000000000000 fp 0x00007fa20227dcd8 Unknown symbol
  pc 0x0000000000000000 fp 0x00007fa20227dd10 Unknown symbol
  pc 0x00007fa20ccb28c8 fp 0x00007fa20227dd80 Unknown symbol
  pc 0x00007fa20ccb24bd fp 0x00007fa20227ddd8 Unknown symbol
  pc 0x00007fa201c5648f fp 0x00007fa20227de38 Unknown symbol
  pc 0x00007fa20ccb052d fp 0x00007fa20227de78 Unknown symbol
  pc 0x00007fa20ccb00fd fp 0x00007fa20227dec8 Unknown symbol
  pc 0x00007fa201c5c23a fp 0x00007fa20227df00 Unknown symbol
  pc 0x00007fa201c5be20 fp 0x00007fa20227df78 Unknown symbol
  pc 0x00007fa201c5b3cf fp 0x00007fa20227dfc8 Unknown symbol
  pc 0x00007fa201c598eb fp 0x00007fa20227e008 Unknown symbol
  pc 0x00007fa20cca6363 fp 0x00007fa20227e030 Unknown symbol
  pc 0x00007fa20cca628b fp 0x00007fa20227e058 Unknown symbol
  pc 0x00007fa20cca61a1 fp 0x00007fa20227e0b8 Unknown symbol
  pc 0x00007fa20cca4efc fp 0x00007fa20227e0f8 Unknown symbol
  pc 0x00007fa20cca4c34 fp 0x00007fa20227e160 Unknown symbol
  pc 0x00007fa20cca4488 fp 0x00007fa20227e1b8 Unknown symbol
  pc 0x00007fa20d7833aa fp 0x00007fa20227e230 Unknown symbol
  pc 0x0000558362a25a1d fp 0x00007fa20227e2c0 dart::DartEntry::InvokeCode(dart::Code const&, unsigned long, dart::Array const&, dart::Array const&, dart::Thread*)+0xfd
  pc 0x0000558362a25659 fp 0x00007fa20227e330 dart::DartEntry::InvokeFunction(dart::Function const&, dart::Array const&, dart::Array const&, unsigned long)+0x159
  pc 0x0000558362a2878e fp 0x00007fa20227e390 dart::DartLibraryCalls::HandleMessage(long, dart::Instance const&)+0x10e
  pc 0x0000558362a5ee82 fp 0x00007fa20227eb30 dart::IsolateMessageHandler::HandleMessage(std::__2::unique_ptr<dart::Message, std::__2::default_delete<dart::Message>>)+0x3b2
  pc 0x0000558362aa1984 fp 0x00007fa20227ebc0 dart::MessageHandler::HandleMessages(dart::MonitorLocker*, bool, bool)+0x214
  pc 0x0000558362aa2708 fp 0x00007fa20227ec40 dart::MessageHandler::TaskCallback()+0x338
  pc 0x0000558362cb2570 fp 0x00007fa20227ecd0 dart::ThreadPool::WorkerLoop(dart::ThreadPool::Worker*)+0x1e0
  pc 0x0000558362cb2eea fp 0x00007fa20227ed20 dart::ThreadPool::Worker::Main(unsigned long)+0x14a
  pc 0x0000558362be4972 fp 0x00007fa20227ee70 out/DebugX64/dart+0x2606972
-- End of DumpStackTrace
  pc 0x0000000000000000 fp 0x00007fa20227dc58 sp 0x0000000000000000 [Stub] CallToRuntime
  pc 0x00007fa20d783867 fp 0x00007fa20227dc80 sp 0x00007fa20227dc68 [Stub] Deoptimize
  pc 0x0000000000000000 fp 0x00007fa20227dcd8 sp 0x00007fa20227dc90 [Stub] FrameAwaitingMaterialization
  pc 0x0000000000000000 fp 0x00007fa20227dd10 sp 0x00007fa20227dce8 [Stub] FrameAwaitingMaterialization
  pc 0x00007fa20ccb28c8 fp 0x00007fa20227dd80 sp 0x00007fa20227dd20 [Unoptimized] _ChunkedJsonParser@10003594.parsePartialKeyword
  pc 0x00007fa20ccb24bd fp 0x00007fa20227ddd8 sp 0x00007fa20227dd90 [Unoptimized] _ChunkedJsonParser@10003594.parsePartial
  pc 0x00007fa201c5648f fp 0x00007fa20227de38 sp 0x00007fa20227dde8 [Optimized] _ChunkedJsonParser@10003594.parse
  pc 0x00007fa20ccb052d fp 0x00007fa20227de78 sp 0x00007fa20227de48 [Unoptimized] _JsonUtf8DecoderSink@10003594._addChunk@10003594
  pc 0x00007fa20ccb00fd fp 0x00007fa20227dec8 sp 0x00007fa20227de88 [Unoptimized] _JsonUtf8DecoderSink@10003594.add
  pc 0x00007fa201c5c23a fp 0x00007fa20227df00 sp 0x00007fa20227ded8 [Unoptimized] jsonMalformedTest.<anonymous closure>
  pc 0x00007fa201c5be20 fp 0x00007fa20227df78 sp 0x00007fa20227df10 [Unoptimized] jsonMalformedTest.test
  pc 0x00007fa201c5b3cf fp 0x00007fa20227dfc8 sp 0x00007fa20227df88 [Unoptimized] jsonMalformedTest
  pc 0x00007fa201c598eb fp 0x00007fa20227e008 sp 0x00007fa20227dfd8 [Unoptimized] testMalformed
  pc 0x00007fa20cca6363 fp 0x00007fa20227e030 sp 0x00007fa20227e018 [Unoptimized] main
  pc 0x00007fa20cca628b fp 0x00007fa20227e058 sp 0x00007fa20227e040 [Unoptimized] main
  pc 0x00007fa20cca61a1 fp 0x00007fa20227e0b8 sp 0x00007fa20227e068 [Unoptimized] _Closure@0150898.dyn:call
  pc 0x00007fa20cca4efc fp 0x00007fa20227e0f8 sp 0x00007fa20227e0c8 [Unoptimized] _delayEntrypointInvocation@1026248.<anonymous closure>
  pc 0x00007fa20cca4c34 fp 0x00007fa20227e160 sp 0x00007fa20227e108 [Unoptimized] _Closure@0150898.dyn:call
  pc 0x00007fa20cca4488 fp 0x00007fa20227e1b8 sp 0x00007fa20227e170 [Unoptimized] _RawReceivePort@1026248._handleMessage@1026248
  pc 0x00007fa20d7833aa fp 0x00007fa20227e230 sp 0x00007fa20227e1c8 [Stub] InvokeDartCode

--- Re-run this test:
python3 tools/test.py -n vm-reload-linux-debug-x64 lib/convert/json_utf8_chunk_test

[alexmarkov]

The crash is flaky. I was able to reproduce the following crash on vm/dart/string_equals_test:

== Deoptimizing code for 'dart:math__Random@11383281_nextInt', code & frame, 
Eager deopt fp=00007f382387df28 pc=00007f382fedc7f9
Deoptimizing (reason 3 'CheckSmi') at pc=00007f382fedc7f9 fp=00007f382387df28 'dart:math__Random@11383281_nextInt' (count 0)
*0. [0x7f382387de70] 0x0 [pp(20)]
*1. [0x7f382387de78] 0x7f3830b10f81 [pc(17)]
*2. [0x7f382387de80] 0x7f382387ded8 [callerfp]
*3. [0x7f382387de88] 0x0 [retaddr(20, 6)]
*4. [0x7f382387de90] 0x7f3830b08081 [const(17)]
*5. [0x7f382387de98] 0x7f3830b08081 [const(17)]
*6. [0x7f382387dea0] 0x7f38308622d1 [const(3)]
*7. [0x7f382387dea8] 0x7f3826135719 [word(rax)]
*8. [0x7f382387deb0] 0x7f3826135719 [word(rax)]
*9. [0x7f382387deb8] 0x7f3830b08081 [const(17)]
*10. [0x7f382387dec0] 0x7f3830b08081 [const(17)]
*11. [0x7f382387dec8] 0x0 [pp(16)]
*12. [0x7f382387ded0] 0x7f3830b14e31 [pc(20)]
*13. [0x7f382387ded8] 0x7f382387df28 [callerfp]
*14. [0x7f382387dee0] 0x0 [retaddr(16, 83)]
*15. [0x7f382387dee8] 0x7f3830b08581 [const(19)]
*16. [0x7f382387def0] 0x7f3830b08581 [const(19)]
*17. [0x7f382387def8] 0x7f3830b08581 [const(19)]
*18. [0x7f382387df00] 0x7f3830b08581 [const(19)]
*19. [0x7f382387df08] 0x7f3830b08581 [const(19)]
*20. [0x7f382387df10] 0x7f3830b08081 [const(17)]
*21. [0x7f382387df18] 0x7f3824ae1d11 [callerpp]
*22. [0x7f382387df20] 0x7f3830b14e31 [pc(16)]
*23. [0x7f382387df28] 0x7f382387df60 [callerfp]
*24. [0x7f382387df30] 0x7f382fedd111 [callerpc]
*25. [0x7f382387df38] 0x800 [word(s1)]
*26. [0x7f382387df40] 0x7f3830b08581 [const(19)]
materializing pp at 0x7f382387de70: 0x7f382671d7b1
../../runtime/vm/deferred_objects.cc: 113: error: Can't locate continuation PC for deoptid 6 within dart:math__Random@11383281_get__state@11383281

dart:math__Random@11383281_get__state@11383281 was compiled before hot reload happened and _Random._state field didn't need load guard (field.needs_load_guard() returned false). So this implicit getter doesn't have a code to check the type and doesn't have deopt_id 6.

Later, after the hot reload, dart:math__Random@11383281_nextInt was compiled and when inlining a getter, field.needs_load_guard() returned true and a check for the result of the getter was added, with deopt_id 6. That check transformed into CheckSmi and deoptimization happened. But deoptimization was not able to find deopt_id 6 in the getter.

It looks like the mechanism for adding extra type checks if field.needs_load_guard() makes deopt_ids unstable, so we should add those type checks in a different way. Maybe invalidate/recompile implicit getters when setting field.needs_load_guard()? Or maybe include dead code for type check into unoptimized implicit getters, just in case?

@rmacnak-google @mkustermann @mraleph WDYT?

[mraleph]

Nice catch @alexmarkov (this means we finally have a repro for flutter/flutter#103268).

I am a bit puzzled with the description: in general I would not expect deopt stability over hot reload (i.e. if the source changes deopt ids will get skewed as well, so you can't take opt code after hot reload and deopt to some function compiled before hot reload). The way you describe it it seems we have not correctly discarded the code for implicit getter - my guess adding this logic would solve the problem.

Maybe invalidate/recompile implicit getters when setting field.needs_load_guard()?

Yep, I think this is the way. I am puzzled why it does not happen already - all functions in dirty libraries though have their code dropped.

[rmacnak-google]

I would not expect deopt stability over hot reload

Changes in hot reload introduce new functions rather than changing functions in place. I.e., we should never deopt from the old version of a function to the new version of a function. Any old functions captured by the stack or closures should deopt to themselves.

Maybe invalidate/recompile implicit getters when setting field.needs_load_guard()?

This should effectively be happening already. Reload marks every frame as needing to deopt, every function as needing to recompile, then sets needs_load_guard for some fields, then resumes execution. The lazy deopts will recompile the corresponding unoptimized code, but now it different deopt ids from the extra load guard.

We could consume an extra deopt id when the field doesn't need a load guard, which IIRC is how we dealt with the analogous issue with deferred library load checks back when deferred libraries really were deferred.

[mkustermann]

Reload marks every frame as needing to deopt ... then sets needs_load_guard for some fields, then resumes execution.

When reload happens with an active stack then stable deopt numbering before/after reload does matter for old optimized functions that deopt to old unoptimized functions. Allocating deopt ids eagerly (initially unused) for that case should address the issue. It's unclear if we are aware of all cases that may require that (load guards are one, maybe there are other situations as well?)

With the new implementation for async/await, our SuspendState objects may be considered as part of the stack, since those have effectively a copy of an optimized stack frame and rely on the same lazy deopt mechanism to go back to unoptimized state.

[alexmarkov]

For some reason, unoptimized code of implicit getter is not recompiled. So reserving deopt_id (by generating a fragment of flow-graph but not linking it) doesn't really work - the deopt_id is now present, but type checking code is not. So resumption PC within getter is pointing to a somewhat random location which expects a different expression stack state.

I'll try to figure out why the implicit getter is not recompiled during hot reload. Maybe we should explicitly force recompilation of implicit getters for fields which were marked as needs_load_guard.

[alexmarkov]

Attempt to fix: https://dart-review.googlesource.com/c/sdk/+/280401.