commit d9c442bce8d13bb4db977e738d795b753ce84168
author Daco Harkes <dacoharkes@google.com> Fri Sep 30 13:29:51 2022 +0000
committer Commit Queue <dart-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Sep 30 13:29:51 2022 +0000
tree 57baa7556c35ebe156ee5eaf4b6d7dfd49218615
parent 175385825a81b935402d8cf7ba28ba477ed7bfca

[vm/ffi] Throw on returning `Error` in `Handle`

Makes `Dart_Handle` FFI returns behave as the following snippet:

```
Dart_Handle ExampleSnippet() {
  Dart_Handle result;
  if (Dart_IsError(result)) {
    Dart_PropagateError(result);
  }
  return result;
}
```

Also makes FFI consistent with Dart_NativeFunctions, which will
automatically throw upon return if Dart_SetReturnValue set the result
to an error.

`UnhandledExceptions` cannot flow out into Dart generated code. So,
the implementation needs to be in `FfiCallInstr::EmitNativeCode`.

Using `Dart_IsError` is slow compared to a machine code class id
check. So, we should do the handle unwrapping and class id check in
machine code.

Unwrapping Handles in machine code is only safe when the GC is
guaranteed to not run: Either (1) in `kThreadInGenerated`, or (2) in
`kThreadInNative`, but only when transitioned into safepoint. So, the
handle cannot be unwrapped immediately after the FFI call in machine code. We first need to transition back to generated.

This means we need to transition again to native to do the actual
`Dart_PropagateError` call. We can do so without the stub in JIT
because we never return with normal control flow.

Performance impact of this change is within benchmark noise in both
JIT and AOT.
Size impact is 42 bytes on x64, which is 10% in AOT and 12% in JIT.

For more numbers see: go/dart-ffi-handle-error

TEST=runtime/bin/ffi_test/ffi_test_functions_vmspecific.cc
TEST=tests/ffi/vmspecific_handle_test.dart

Closes: https://github.com/dart-lang/sdk/issues/49936
Change-Id: Ie8fabeb6d53bc80689541bc4470cb37ee2200581
Cq-Include-Trybots: luci.dart.try:vm-canary-linux-debug-try,vm-ffi-android-debug-arm64c-try,vm-ffi-android-debug-arm-try,vm-kernel-gcc-linux-try,vm-kernel-linux-debug-x64-try,vm-kernel-linux-debug-x64c-try,vm-kernel-msvc-windows-try,vm-kernel-optcounter-threshold-linux-release-x64-try,vm-kernel-precomp-asan-linux-release-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-reload-linux-debug-x64-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-tsan-linux-release-x64-try,vm-kernel-win-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-precomp-ffi-qemu-linux-release-arm-try,vm-precomp-ffi-qemu-linux-release-riscv64-try,vm-kernel-linux-debug-ia32-try,vm-kernel-mac-release-arm64-try,vm-kernel-precomp-win-debug-x64c-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/261603
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>