Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[analyzer/ffi] FfiNatives don't check whether the native types are native and correspond to the Dart types #49412

Closed
dcharkes opened this issue Jul 7, 2022 · 0 comments
Closed

[analyzer/ffi] FfiNatives don't check whether the native types are native and correspond to the Dart types #49412

dcharkes opened this issue Jul 7, 2022 · 0 comments
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. library-ffi P2 A bug or feature request we're likely to work on

Comments

@dcharkes
Copy link
Contributor

dcharkes commented Jul 7, 2022 

@FfiNative<IntPtr Function(Double)>('doesntmatter') //# 14: compile-time error
external int wrongFfiParameter(int v); //# 14: compile-time error

@FfiNative<IntPtr Function(IntPtr)>('doesntmatter') //# 15: compile-time error
external double wrongFfiReturnType(int v); //# 15: compile-time error

@FfiNative<int Function(Double)>('doesntmatter') //# 16: compile-time error
external int nonFfiParameter(int v); //# 16: compile-time error

@FfiNative<double Function(IntPtr)>('doesntmatter') //# 17: compile-time error
external double nonFfiReturnType(int v); //# 17: compile-time error

Are not marked as error by the analyzer.

We need to apply the same logic to these as we do to lookupFunction and asFunction.
@dcharkes dcharkes added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. and removed area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. labels Jul 7, 2022
@scheglov scheglov added the P2 A bug or feature request we're likely to work on label Jul 7, 2022
copybara-service bot pushed a commit that referenced this issue Jul 8, 2022
@dcharkes
[cfe/ffi] Transform FfiNatives in a single pass
aa6abbf 
Before this CL, `FfiNative`s were first transformed to `asFunction`
calls, which were then immediately transformed to `_asFunctionInternal`
calls.

This caused the the static checks to be done in two steps, the second
step happening after the first transform. It is cleaner to first do all
checks.

This refactoring enables implementing `_asFunctionInternal` variants
for `FfiNative`s that don't use a `Pointer` for the address.

Besides the transform change, this CL
- moves shared logic over to pkg/vm/lib/transformations/ffi/common.dart,
- splits up the ffi-native tests in to positive and negative tests, and
- adds negative tests for mismatches between Dart and native types.

These new tests do _not yet_ pass on the analyzer. This is tracked in:
#49412

TEST=tests/ffi/ffi_native_test.dart
TEST=tests/ffi/vmspecific_static_checks_ffinative_test.dart

Closes: #49413
Change-Id: I5baded43eab7ff1dc1ffb16550b2a638e4b7a34e
Cq-Include-Trybots: luci.dart.try:analyzer-linux-release-try,analyzer-mac-release-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/250843
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
@dcharkes dcharkes mentioned this issue Jul 23, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. library-ffi P2 A bug or feature request we're likely to work on
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@scheglov @dcharkes