[vm/ffi] working with Pointer<NativeType> (where NativeType is not a concrete subtype of NativeType)

[sjindel-google]

Example:

import "dart:ffi";

main() {
  Pointer x = allocate<Int8>();
  Pointer<Pointer> y = allocate();
  y.store(x);
}

Crashes in Debug mode:

../../runtime/lib/ffi.cc: 462: error: expected: DartAndCTypeCorrespond(pointer_type_arg, arg_type)
version=2.3.3-edge.149ebf683bf1fd24e8c872fbdc5ad6732d5015a4 (Tue Jun 11 16:53:03 2019 +0000) on "linux_x64"
thread=120813, isolate=main(0x55d30ea8a100)
  pc 0x000055d30da169dc fp 0x00007f0a504fe480 dart::Profiler::DumpStackTrace(void*)
  pc 0x000055d30d645332 fp 0x00007f0a504fe560 dart::Assert::Fail(char const*, ...)
  pc 0x000055d30d84a8d4 fp 0x00007f0a504fe5f0 dart::BootstrapNatives::DN_Ffi_store(_Dart_NativeArguments*)
  pc 0x00007f0a5050134f fp 0x00007f0a504fe630 Unknown symbol
  pc 0x00007f0a4d82fa4e fp 0x00007f0a504fe678 Unknown symbol
  pc 0x00007f0a4d82f626 fp 0x00007f0a504fe6c0 Unknown symbol
  pc 0x00007f0a4d82f507 fp 0x00007f0a504fe700 Unknown symbol
  pc 0x00007f0a4d82f30f fp 0x00007f0a504fe738 Unknown symbol
  pc 0x00007f0a4d82e51f fp 0x00007f0a504fe780 Unknown symbol
  pc 0x00007f0a4d80a8f2 fp 0x00007f0a504fe7c0 Unknown symbol
  pc 0x00007f0a4d82e1ee fp 0x00007f0a504fe7f8 Unknown symbol
  pc 0x00007f0a505016ba fp 0x00007f0a504fe868 Unknown symbol
  pc 0x000055d30d8a5a75 fp 0x00007f0a504fe910 dart::DartEntry::InvokeFunction(dart::Function const&, dart::Array const&, dart::Array const&, unsigned long)
  pc 0x000055d30d8a8d31 fp 0x00007f0a504fe980 dart::DartLibraryCalls::HandleMessage(dart::Object const&, dart::Instance const&)
  pc 0x000055d30d8f1fe9 fp 0x00007f0a504feb80 dart::IsolateMessageHandler::HandleMessage(std::__2::unique_ptr<dart::Message, std::__2::default_delete<dart::Message> >)
  pc 0x000055d30d934300 fp 0x00007f0a504fec10 dart::MessageHandler::HandleMessages(dart::MonitorLocker*, bool, bool)
  pc 0x000055d30d935116 fp 0x00007f0a504fec90 dart::MessageHandler::TaskCallback()
  pc 0x000055d30dad298c fp 0x00007f0a504fece0 dart::ThreadPool::Worker::Loop()
  pc 0x000055d30dad2464 fp 0x00007f0a504fed30 dart::ThreadPool::Worker::Main(unsigned long)
  pc 0x000055d30da107c5 fp 0x00007f0a504fee70 out/DebugX64/dart+0x19f77c5
-- End of DumpStackTrace
Aborted

/cc @dcharkes

[sjindel-google]

@dcharkes : Want to look at this? There's no dependency on the API refactoring.

[dcharkes]

Looks like Pointer<Pointer> will be inferred as Pointer<Pointer<NativeType>>. Which means allocate will be inferred as Pointer<Pointer<NativeType>> allocate<Pointer<NativeType>>(). The frontend transformation should catch this, as it should have been a sub type of NativeType.

I'll conjure up a CL to fix this.

[dcharkes]

Or do we want to support working with Pointer<Pointer<NativeType>>? allocate does have enough information to allocate the right amount of bytes. Even store knows what to do in this example. (I guess that's why it works in release mode.)

So this is more an API design question.

extension on Pointer<Pointer<T>> {
  void store(Pointer<T> value);
  Pointer<T> load();
}

Pointer<Int8> innerTyped = allocate();
Pointer<NativeType> innerUntyped = allocate();
Pointer<Pointer<Int8>> outerTyped = allocate();
Pointer<Pointer<NativeType>> outerUntyped = allocate();

outerTyped.store(innerTyped);       // This will always work
innerTyped = outerTyped.load();     // This will always work

outerUntyped.store(innerUntyped);   // This should work  // currently rejected by VM assertion
innerUntyped = outerUntyped.load(); // This should work  // currently rejected by VM assertion

outerUntyped.store(innerTyped);     // This should work  // currently rejected by frontend transformation
innerTyped = outerUntyped.load();   // Implicit downcast, will fail at runtime now, will fail statically when NNBD lands

outerTyped.store(innerUntyped); ;   // Implicit downcast, will fail at runtime now, will fail statically when NNBD lands
innerUntyped = outerTyped.load();   // This should work // currently rejected by frontend transformation

Our frontend transformation currently partially supports untyped Pointers, while our assert in the VM does assume no support for it.

Edit: Thanks @lrhn for clarifying!

The // This should work-cases we could reject with Dart types if we get exact generics where you cannot have a subtype of the type argument:

extension on Pointer<Pointer<T>> {
  void store(Pointer<exactly T> value);
  Pointer<exactly T> load();
}

[dcharkes]

Including the differentiation between static and runtime types the following tables describe all possible scenarios:

// Notation used in following tables:
// * static_type//dynamic_type
// * P = Pointer
// * I = Int8
// * NT = NativeType
//
// Note #1: When NNBD is landed, implicit downcasts will be static errors.
//
// Note #2: When we switch to extension methods we will _only_ use the static
//          type of the container.
//
// ===== a.store(b) ======
//                  b     P<I>//P<I>   P<NT>//P<I>           P<NT>//P<NT>
// a
// P<P<I>>//P<P<I>>     1 ok         2 implicit downcast   3 implicit downcast
//                                     of argument: ok       of argument: fail
//
// P<P<NT>>//P<P<I>>    4 ok         5 ok                  6 dynamic type: fail
//                                                           static type:  ok
//
// P<P<NT>>//P<P<NT>>   7 ok         8 ok                  9 ok
//
// ====== final c = a.load() ======
// a                    c
// P<P<I>>//P<P<I>>     P<I>//P<I>
//
// P<P<NT>>//P<P<I>>    dynamic type: P<NT>//P<I>
//                      static type:  P<NT>//P<NT>
//
// P<P<NT>>//P<P<NT>>   P<NT>//P<NT>
//
// ====== b = a.load() ======
//                  b     P<I>                        P<NT>
// P<P<I>>//P<P<I>>     1 ok                        2 ok
//
// P<P<NT>>//P<P<I>>    3 implicit downcast         4 ok
//                        of returnvalue:
//                        * dynamic type: ok
//                        * static type: fail
//
// P<P<NT>>//P<P<NT>>   5 implicit downcast         6 ok
//                        of returnvalue: fail

I've added all of these in a regression test in https://dart-review.googlesource.com/c/sdk/+/107281.

In our current implementation, our frontend rejects many combinations. Moreover, we completely prohibit obtaining a Pointer<NativeType> currently.

edit: dynamic type means how this would work with instance methods of classes with generics, and static type means how this would work with top level functions or extension methods. Assuming we precisely follow the semantics of Dart. We could opt for going for the instance methods behavior, even when using extension methods in the future.