New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dart exceptions should restore the safestack stack pointer. #31356

Closed
apwilson opened this Issue Nov 10, 2017 · 13 comments

Comments

Projects
None yet
5 participants
@apwilson
Contributor

apwilson commented Nov 10, 2017

Not sure what you need to debug. This is with dart_debug=true as a gn argument.

start of symbolized stack:
#01: TransitionGeneratedToVM at ../../out/dashboard/../../third_party/dart/runtime/vm/safepoint.h:160
#02: dart::BootstrapNatives::DN_DateTime_currentTimeMicros(_Dart_NativeArguments*) at ../../out/dashboard/../../third_party/dart/runtime/lib/date.cc:44
#03: kDartVmSnapshotInstructions at ??:?
#04: kDartVmSnapshotInstructions at ??:?
#05: ?? ??:0
#06: ?? ??:0
#07: ?? ??:0
#08: ?? ??:0
#09: ?? ??:0
#10: ?? ??:0
#11: ?? ??:0
#12: ?? ??:0
#13: ?? ??:0
#14: ?? ??:0
#15: ?? ??:0
#16: ?? ??:0
#17: ?? ??:0
#18: kDartVmSnapshotInstructions at ??:?
#19: ?? ??:0
#20: ?? ??:0
#21: kDartVmSnapshotInstructions at ??:?
#22: kDartVmSnapshotInstructions at ??:?
#23: dart::DartEntry::InvokeFunction(dart::Function const&, dart::Array const&, dart::Array const&, unsigned long) at ../../out/dashboard/../../third_party/dart/runtime/vm/dart_entry.cc:126
#24: dart::DartEntry::InvokeFunction(dart::Function const&, dart::Array const&) at ../../out/dashboard/../../third_party/dart/runtime/vm/dart_entry.cc:29
#25: dart::DartLibraryCalls::HandleMessage(dart::Object const&, dart::Instance const&) at ../../out/dashboard/../../third_party/dart/runtime/vm/dart_entry.cc:585
#26: dart::IsolateMessageHandler::HandleMessage(dart::Message*) at ../../out/dashboard/../../third_party/dart/runtime/vm/isolate.cc:590
#27: dart::MessageHandler::HandleMessages(dart::MonitorLocker*, bool, bool) at ../../out/dashboard/../../third_party/dart/runtime/vm/message_handler.cc:212
#28: dart::MessageHandler::HandleNextMessage() at ../../out/dashboard/../../third_party/dart/runtime/vm/message_handler.cc:264
#29: Dart_HandleMessage at ../../out/dashboard/../../third_party/dart/runtime/vm/dart_api_impl.cc:1646
#30: tonic::DartMessageHandler::OnHandleMessage(tonic::DartState*) at ../../out/dashboard/../../topaz/lib/tonic/dart_message_handler.cc:89
#31: operator() at ../../out/dashboard/../../garnet/public/lib/fsl/tasks/message_loop.cc:215
#32: fbl::internal::HeapFunctionTarget<fsl::MessageLoop::TaskRecord::TaskRecord(unsigned long, std::__2::function<void ()>)::$_1, async_task_result_t, async_dispatcher*, int>::operator()(async_dispatcher*, int) const at ../../out/dashboard/../../zircon/system/ulib/fbl/include/fbl/function.h:97
#33: fbl::internal::Function<16ul, false, async_task_result_t (async_dispatcher*, int)>::operator()(async_dispatcher*, int) const at ../../out/dashboard/../../zircon/system/ulib/fbl/include/fbl/function.h:212
#34: async_loop_invoke_task_handler at ../../out/dashboard/../../zircon/system/ulib/async/loop.c:576
#35: async_loop_dispatch_tasks at ../../out/dashboard/../../zircon/system/ulib/async/loop.c:305
#36: async_loop_run_once at ../../out/dashboard/../../zircon/system/ulib/async/loop.c:217
#37: async_loop_run at ../../out/dashboard/../../zircon/system/ulib/async/loop.c:192
#38: fsl::MessageLoop::Run() at ../../out/dashboard/../../garnet/public/lib/fsl/tasks/message_loop.cc:164
#39: main at ../../out/dashboard/../../third_party/flutter/content_handler/main.cc:16
#40: start_main at ../../zircon/third_party/ulib/musl/src/env/__libc_start_main.c:57
#41: (unknown)
end of symbolized stack
@rmacnak-google

This comment has been minimized.

Show comment
Hide comment
@rmacnak-google

rmacnak-google Nov 11, 2017

Member

Which application? How do I provoke the crash?

Member

rmacnak-google commented Nov 11, 2017

Which application? How do I provoke the crash?

@apwilson

This comment has been minimized.

Show comment
Hide comment
@apwilson

apwilson Nov 16, 2017

Contributor

I'm not sure if this is the same crash but here's a repro:

  1. Apply this patch to fuchsia's topaz layer: https://fuchsia-review.googlesource.com/c/topaz/+/89461
  2. Build debug, and boot.
  3. Login as guest.
  4. Type 'das'. Tap the first suggestion (don't hit enter as that triggers a different bug in flutter).
  5. The dashboard should come up and start spinning. It will continutally say its timing out, that's fine.
  6. After a few minutes of this the dashboard will crash in the dart VM.

An example crash:

start of symbolized stack:
#02: (unknown)
#01: TransitionGeneratedToVM at ../../out/dashboard/../../third_party/dart/runtime/vm/safepoint.h:160
#02: dart::BootstrapNatives::DN_Object_instanceOf(_Dart_NativeArguments*) at ../../out/dashboard/../../third_party/dart/runtime/lib/object.cc:122
#03: kDartVmSnapshotInstructions at ??:?
#04: ?? ??:0
#05: ?? ??:0
#06: ?? ??:0
#07: ?? ??:0
#08: ?? ??:0
#09: ?? ??:0
#10: ?? ??:0
#11: ?? ??:0
#12: ?? ??:0
#13: ?? ??:0
#14: kDartVmSnapshotInstructions at ??:?
#15: ?? ??:0
#16: kDartVmSnapshotInstructions at ??:?
#17: ?? ??:0
#18: ?? ??:0
#19: kDartVmSnapshotInstructions at ??:?
#20: kDartVmSnapshotInstructions at ??:?
#21: dart::DartEntry::InvokeFunction(dart::Function const&, dart::Array const&, dart::Array const&, unsigned long) at ../../out/dashboard/../../third_party/dart/runtime/vm/dart_entry.cc:126
#22: dart::DartEntry::InvokeFunction(dart::Function const&, dart::Array const&) at ../../out/dashboard/../../third_party/dart/runtime/vm/dart_entry.cc:29
#23: dart::DartLibraryCalls::HandleMessage(dart::Object const&, dart::Instance const&) at ../../out/dashboard/../../third_party/dart/runtime/vm/dart_entry.cc:585
#24: dart::IsolateMessageHandler::HandleMessage(dart::Message*) at ../../out/dashboard/../../third_party/dart/runtime/vm/isolate.cc:590
#25: dart::MessageHandler::HandleMessages(dart::MonitorLocker*, bool, bool) at ../../out/dashboard/../../third_party/dart/runtime/vm/message_handler.cc:212
#26: dart::MessageHandler::HandleNextMessage() at ../../out/dashboard/../../third_party/dart/runtime/vm/message_handler.cc:264
#27: Dart_HandleMessage at ../../out/dashboard/../../third_party/dart/runtime/vm/dart_api_impl.cc:1646
#28: tonic::DartMessageHandler::OnHandleMessage(tonic::DartState*) at ../../out/dashboard/../../topaz/lib/tonic/dart_message_handler.cc:89
#29: operator() at ../../out/dashboard/../../garnet/public/lib/fsl/tasks/message_loop.cc:215
#30: fbl::internal::HeapFunctionTarget<fsl::MessageLoop::TaskRecord::TaskRecord(unsigned long, std::__2::function<void ()>)::$_1, async_task_result_t, async_dispatcher*, int>::operator()(async_dispatcher*, int) const at ../../out/dashboard/../../zircon/system/ulib/fbl/include/fbl/function.h:97
#31: fbl::internal::Function<16ul, false, async_task_result_t (async_dispatcher*, int)>::operator()(async_dispatcher*, int) const at ../../out/dashboard/../../zircon/system/ulib/fbl/include/fbl/function.h:212
#32: async_loop_invoke_task_handler at ../../out/dashboard/../../zircon/system/ulib/async/loop.c:576
#33: async_loop_dispatch_tasks at ../../out/dashboard/../../zircon/system/ulib/async/loop.c:305
#34: async_loop_run_once at ../../out/dashboard/../../zircon/system/ulib/async/loop.c:217
#35: async_loop_run at ../../out/dashboard/../../zircon/system/ulib/async/loop.c:192
#36: fsl::MessageLoop::Run() at ../../out/dashboard/../../garnet/public/lib/fsl/tasks/message_loop.cc:164
#37: main at ../../out/dashboard/../../third_party/flutter/content_handler/main.cc:16
#38: start_main at ../../zircon/third_party/ulib/musl/src/env/__libc_start_main.c:57
#39: (unknown)
end of symbolized stack

Contributor

apwilson commented Nov 16, 2017

I'm not sure if this is the same crash but here's a repro:

  1. Apply this patch to fuchsia's topaz layer: https://fuchsia-review.googlesource.com/c/topaz/+/89461
  2. Build debug, and boot.
  3. Login as guest.
  4. Type 'das'. Tap the first suggestion (don't hit enter as that triggers a different bug in flutter).
  5. The dashboard should come up and start spinning. It will continutally say its timing out, that's fine.
  6. After a few minutes of this the dashboard will crash in the dart VM.

An example crash:

start of symbolized stack:
#02: (unknown)
#01: TransitionGeneratedToVM at ../../out/dashboard/../../third_party/dart/runtime/vm/safepoint.h:160
#02: dart::BootstrapNatives::DN_Object_instanceOf(_Dart_NativeArguments*) at ../../out/dashboard/../../third_party/dart/runtime/lib/object.cc:122
#03: kDartVmSnapshotInstructions at ??:?
#04: ?? ??:0
#05: ?? ??:0
#06: ?? ??:0
#07: ?? ??:0
#08: ?? ??:0
#09: ?? ??:0
#10: ?? ??:0
#11: ?? ??:0
#12: ?? ??:0
#13: ?? ??:0
#14: kDartVmSnapshotInstructions at ??:?
#15: ?? ??:0
#16: kDartVmSnapshotInstructions at ??:?
#17: ?? ??:0
#18: ?? ??:0
#19: kDartVmSnapshotInstructions at ??:?
#20: kDartVmSnapshotInstructions at ??:?
#21: dart::DartEntry::InvokeFunction(dart::Function const&, dart::Array const&, dart::Array const&, unsigned long) at ../../out/dashboard/../../third_party/dart/runtime/vm/dart_entry.cc:126
#22: dart::DartEntry::InvokeFunction(dart::Function const&, dart::Array const&) at ../../out/dashboard/../../third_party/dart/runtime/vm/dart_entry.cc:29
#23: dart::DartLibraryCalls::HandleMessage(dart::Object const&, dart::Instance const&) at ../../out/dashboard/../../third_party/dart/runtime/vm/dart_entry.cc:585
#24: dart::IsolateMessageHandler::HandleMessage(dart::Message*) at ../../out/dashboard/../../third_party/dart/runtime/vm/isolate.cc:590
#25: dart::MessageHandler::HandleMessages(dart::MonitorLocker*, bool, bool) at ../../out/dashboard/../../third_party/dart/runtime/vm/message_handler.cc:212
#26: dart::MessageHandler::HandleNextMessage() at ../../out/dashboard/../../third_party/dart/runtime/vm/message_handler.cc:264
#27: Dart_HandleMessage at ../../out/dashboard/../../third_party/dart/runtime/vm/dart_api_impl.cc:1646
#28: tonic::DartMessageHandler::OnHandleMessage(tonic::DartState*) at ../../out/dashboard/../../topaz/lib/tonic/dart_message_handler.cc:89
#29: operator() at ../../out/dashboard/../../garnet/public/lib/fsl/tasks/message_loop.cc:215
#30: fbl::internal::HeapFunctionTarget<fsl::MessageLoop::TaskRecord::TaskRecord(unsigned long, std::__2::function<void ()>)::$_1, async_task_result_t, async_dispatcher*, int>::operator()(async_dispatcher*, int) const at ../../out/dashboard/../../zircon/system/ulib/fbl/include/fbl/function.h:97
#31: fbl::internal::Function<16ul, false, async_task_result_t (async_dispatcher*, int)>::operator()(async_dispatcher*, int) const at ../../out/dashboard/../../zircon/system/ulib/fbl/include/fbl/function.h:212
#32: async_loop_invoke_task_handler at ../../out/dashboard/../../zircon/system/ulib/async/loop.c:576
#33: async_loop_dispatch_tasks at ../../out/dashboard/../../zircon/system/ulib/async/loop.c:305
#34: async_loop_run_once at ../../out/dashboard/../../zircon/system/ulib/async/loop.c:217
#35: async_loop_run at ../../out/dashboard/../../zircon/system/ulib/async/loop.c:192
#36: fsl::MessageLoop::Run() at ../../out/dashboard/../../garnet/public/lib/fsl/tasks/message_loop.cc:164
#37: main at ../../out/dashboard/../../third_party/flutter/content_handler/main.cc:16
#38: start_main at ../../zircon/third_party/ulib/musl/src/env/__libc_start_main.c:57
#39: (unknown)
end of symbolized stack

@zanderso

This comment has been minimized.

Show comment
Hide comment
@zanderso

zanderso Nov 17, 2017

Member

It looks like we're crashing in a function prologue. The crashing instruction is 1cab4e1. Maybe this is safestack setup? @frobtech

_ZN4dart23TransitionGeneratedToVMC2EPNS_6ThreadE:
 1cab4b0:       55      pushq   %rbp
 1cab4b1:       48 89 e5        movq    %rsp, %rbp
 1cab4b4:       41 57   pushq   %r15
 1cab4b6:       41 56   pushq   %r14
 1cab4b8:       41 55   pushq   %r13
 1cab4ba:       41 54   pushq   %r12
 1cab4bc:       53      pushq   %rbx
 1cab4bd:       50      pushq   %rax
 1cab4be:       64 4c 8b 24 25 18 00 00 00      movq    %fs:24, %r12
 1cab4c7:       49 8d 44 24 a0  leaq    -96(%r12), %rax
 1cab4cc:       64 48 89 04 25 18 00 00 00      movq    %rax, %fs:24
 1cab4d5:       49 89 ff        movq    %rdi, %r15
 1cab4d8:       64 4c 8b 2c 25 10 00 00 00      movq    %fs:16, %r13
 1cab4e1:       4d 89 6c 24 f8  movq    %r13, -8(%r12)
 1cab4e6:       48 8d 05 e3 58 9e 00    leaq    10377443(%rip), %rax
 1cab4ed:       48 83 c0 10     addq    $16, %rax
 1cab4f1:       49 89 07        movq    %rax, (%r15)
 1cab4f4:       49 89 f6        movq    %rsi, %r14
 1cab4f7:       0f 57 c0        xorps   %xmm0, %xmm0
 1cab4fa:       41 0f 11 47 08  movups  %xmm0, 8(%r15)
Member

zanderso commented Nov 17, 2017

It looks like we're crashing in a function prologue. The crashing instruction is 1cab4e1. Maybe this is safestack setup? @frobtech

_ZN4dart23TransitionGeneratedToVMC2EPNS_6ThreadE:
 1cab4b0:       55      pushq   %rbp
 1cab4b1:       48 89 e5        movq    %rsp, %rbp
 1cab4b4:       41 57   pushq   %r15
 1cab4b6:       41 56   pushq   %r14
 1cab4b8:       41 55   pushq   %r13
 1cab4ba:       41 54   pushq   %r12
 1cab4bc:       53      pushq   %rbx
 1cab4bd:       50      pushq   %rax
 1cab4be:       64 4c 8b 24 25 18 00 00 00      movq    %fs:24, %r12
 1cab4c7:       49 8d 44 24 a0  leaq    -96(%r12), %rax
 1cab4cc:       64 48 89 04 25 18 00 00 00      movq    %rax, %fs:24
 1cab4d5:       49 89 ff        movq    %rdi, %r15
 1cab4d8:       64 4c 8b 2c 25 10 00 00 00      movq    %fs:16, %r13
 1cab4e1:       4d 89 6c 24 f8  movq    %r13, -8(%r12)
 1cab4e6:       48 8d 05 e3 58 9e 00    leaq    10377443(%rip), %rax
 1cab4ed:       48 83 c0 10     addq    $16, %rax
 1cab4f1:       49 89 07        movq    %rax, (%r15)
 1cab4f4:       49 89 f6        movq    %rsi, %r14
 1cab4f7:       0f 57 c0        xorps   %xmm0, %xmm0
 1cab4fa:       41 0f 11 47 08  movups  %xmm0, 8(%r15)
@zanderso

This comment has been minimized.

Show comment
Hide comment
@zanderso

zanderso Nov 17, 2017

Member

The crash is due to a page fault and the stack is 41 frames deep.

Member

zanderso commented Nov 17, 2017

The crash is due to a page fault and the stack is 41 frames deep.

@zanderso

This comment has been minimized.

Show comment
Hide comment
@zanderso

zanderso Nov 17, 2017

Member

I disabled safestacks in the Dart VM build, and the crash has not happened after running the dashboard for ~35 minutes.

Member

zanderso commented Nov 17, 2017

I disabled safestacks in the Dart VM build, and the crash has not happened after running the dashboard for ~35 minutes.

@zanderso

This comment has been minimized.

Show comment
Hide comment
@zanderso

zanderso Nov 17, 2017

Member

Still going after an hour. It seems that this crash is indeed related to safestack. Is there any information I can gather that would help track this down? Should I disable it for the Dart VM in the meantime? @petrhosek

Member

zanderso commented Nov 17, 2017

Still going after an hour. It seems that this crash is indeed related to safestack. Is there any information I can gather that would help track this down? Should I disable it for the Dart VM in the meantime? @petrhosek

@frobtech

This comment has been minimized.

Show comment
Hide comment
@frobtech

frobtech Nov 18, 2017

Contributor

Does the Dart VM use a conservative GC that wants to scan thread stacks?

Contributor

frobtech commented Nov 18, 2017

Does the Dart VM use a conservative GC that wants to scan thread stacks?

@zanderso

This comment has been minimized.

Show comment
Hide comment
@zanderso

zanderso Nov 18, 2017

Member

Yes. GC iterates over Dart frames, but not C++ frames since there are no raw pointers into the Dart heap in C++ frames. We find Dart frames by recording the frame pointer before calling from Dart into C++, for example here: https://github.com/dart-lang/sdk/blob/master/runtime/vm/stub_code_x64.cc#L219.

Member

zanderso commented Nov 18, 2017

Yes. GC iterates over Dart frames, but not C++ frames since there are no raw pointers into the Dart heap in C++ frames. We find Dart frames by recording the frame pointer before calling from Dart into C++, for example here: https://github.com/dart-lang/sdk/blob/master/runtime/vm/stub_code_x64.cc#L219.

@mraleph

This comment has been minimized.

Show comment
Hide comment
@mraleph

mraleph Nov 18, 2017

Contributor

I have a tangentially related question: here it says that:

New code implementing some new kind of non-local exit or context switch will need to handle the unsafe stack pointer similarly to how it handles the traditional machine stack pointer register.

However when I look at Exceptions::JumpToFrame or StubCode::GenerateJumpToFrameStub I don't see any code updating internal safestack pointer. We might be throwing exception through C++ frames so I would expect that it needs to be updated.

I think this indicates that our exceptions leave safestack in the inconsistent state.

Contributor

mraleph commented Nov 18, 2017

I have a tangentially related question: here it says that:

New code implementing some new kind of non-local exit or context switch will need to handle the unsafe stack pointer similarly to how it handles the traditional machine stack pointer register.

However when I look at Exceptions::JumpToFrame or StubCode::GenerateJumpToFrameStub I don't see any code updating internal safestack pointer. We might be throwing exception through C++ frames so I would expect that it needs to be updated.

I think this indicates that our exceptions leave safestack in the inconsistent state.

@zanderso

This comment has been minimized.

Show comment
Hide comment
@zanderso

zanderso Nov 19, 2017

Member

Ah, yeah, I think you're right Slava. Andrew's patch to the dashboard causes lots of exceptions to be thrown, so I bet we're overflowing the safestack stack, but not the regular stack, which we are still handling correctly. I guess we'll have to stash the safestack stack pointer in the Dart VM's Thread object on invoking Dart code, and manually restore it in the JumpToFrame stub.

Slava, do you know if our x64 assembler can already handle moves to/from segment registers? On arm64, we'll have to add the mrs instruction.

Member

zanderso commented Nov 19, 2017

Ah, yeah, I think you're right Slava. Andrew's patch to the dashboard causes lots of exceptions to be thrown, so I bet we're overflowing the safestack stack, but not the regular stack, which we are still handling correctly. I guess we'll have to stash the safestack stack pointer in the Dart VM's Thread object on invoking Dart code, and manually restore it in the JumpToFrame stub.

Slava, do you know if our x64 assembler can already handle moves to/from segment registers? On arm64, we'll have to add the mrs instruction.

@zanderso zanderso changed the title from Dart crash running a flutter app on fuchsia to Dart exceptions should restore the safestack stack pointer. Nov 19, 2017

@mraleph

This comment has been minimized.

Show comment
Hide comment
@mraleph

mraleph Nov 19, 2017

Contributor

Slava, do you know if our x64 assembler can already handle moves to/from segment registers?

I don't think we have support for this in x64 assembler. However because safestack is a clang specific extension I think it can be okay to simply use inline assembly instead of putting this code into stubs.

It's a pity that safestack does not provide APIs for updating the stack pointer manually. You can read it with __builtin___get_unsafe_stack_ptr() but there is nothing in documentation about how are you supposed to write it :-(

Contributor

mraleph commented Nov 19, 2017

Slava, do you know if our x64 assembler can already handle moves to/from segment registers?

I don't think we have support for this in x64 assembler. However because safestack is a clang specific extension I think it can be okay to simply use inline assembly instead of putting this code into stubs.

It's a pity that safestack does not provide APIs for updating the stack pointer manually. You can read it with __builtin___get_unsafe_stack_ptr() but there is nothing in documentation about how are you supposed to write it :-(

@zanderso

This comment has been minimized.

Show comment
Hide comment

@whesse whesse closed this in f97e11f Nov 22, 2017

@zanderso

This comment has been minimized.

Show comment
Hide comment
@zanderso

zanderso Nov 22, 2017

Member

The above fix has also now rolled into Fuchsia.

Member

zanderso commented Nov 22, 2017

The above fix has also now rolled into Fuchsia.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment