Skip to content

Allow access to global variables in ffi static binding #50551

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

Closed
Tracked by #50565
mkustermann opened this issue Nov 25, 2022 · 15 comments
Closed
Tracked by #50565

Allow access to global variables in ffi static binding #50551

mkustermann opened this issue Nov 25, 2022 · 15 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi

Comments

@mkustermann
Copy link
Member

mkustermann commented Nov 25, 2022

We're moving towards a direction where all native symbols will be declaratively specified. We have support for declaratively specifying native C functions (see #43889).

Though we'd also want a way to declaratively specify global variables - which can then be accessed from Dart.

@dcharkes
Copy link
Contributor

Possible syntax:

// Global var in native asset, similar to `@Native external` function.
@NativeSymbol(/* optional symbol and asset */); // No type argument, as the Pointer already has it.
external Pointer<Uint8> get greeting;

@simolus3
Copy link
Contributor

Is someone currently working on this? I need this feature to use native assets, so I'm happy to give this a shot myself otherwise.

Possible syntax:

Given that @Natives translate to direct Dart functions, wouldn't a more consistent API be something that does the load and stores implicitly?

@NativeSymbol<Uint8>(/* optional symbol and asset */);
external int greeting; // Field loads and assignments lowered to calls on the pointer.

A downside is that the #50552 might need special consideration (but detecting addressOf(greeting) in the CFE and lowering it should be possible too).

@dcharkes
Copy link
Contributor

Is someone currently working on this? I need this feature to use native assets, so I'm happy to give this a shot myself otherwise.

Possible syntax:

Given that @Natives translate to direct Dart functions, wouldn't a more consistent API be something that does the load and stores implicitly?

@NativeSymbol<Uint8>(/* optional symbol and asset */);
external int greeting; // Field loads and assignments lowered to calls on the pointer.

A downside is that the #50552 might need special consideration (but detecting addressOf(greeting) in the CFE and lowering it should be possible too).

The way that this should be implemented is going to be dlopen+dlsym in C and then conceptually .value in Dart. So I suspect we might want to do addressOf in the same PR.

https://dart-review.googlesource.com/c/sdk/+/284300 completely changes how the dlopen+dlsym happen in the Dart SDK for @Native external functions. So it might be worth waiting until that lands. It changes the storage of the address into the pool rather than a static field in Dart. I'd prefer to avoid landing an implementation the CFE for fields now and then throwing it away in 2 months again. (Sorry for sitting so long on 284300, I've been preoccupied with getting the native assets to work in Flutter.)

@dcharkes
Copy link
Contributor

dcharkes commented Oct 11, 2023

We should be able to merge both functions and non-function symbols:

// Global var in native asset, similar to `@Native external` function.
@Native<Int64>(/* optional symbol and asset */);
external int get greeting;
external set greeting(int value);

final greetingAddress = addressOf<Int64>(greeting); // Unfortunately no inference, requires extra custom check.

@dcharkes
Copy link
Contributor

cc @lrhn for any bright API ideas.

We could possibly make the global vars nicer by exposing the Pointer instead and using .value as suggested above. However, we don't want to do that with the functions (using Pointer and then asFunction), because we want to do static linking. And it's probably best to keep functions and non-functions consistent.

@simolus3
Copy link
Contributor

We should be able to merge both functions and non-function symbols:

I have seen existing code using @FfiNative to link unary C functions as getters on members . And even though it's probably not intentional, I think the current @Native implementation also allows linking nullary C functions by annotating getters. So there's a question of whether we want to break this.

We could make a distinction by saying that getters/setters are functions in C, whereas annotated fields would be symbols. Or we just break the existing (probably rare?) usages of getters linking functions.

@dcharkes
Copy link
Contributor

We could probably make both nullary functions and non-functions getters (and unary functions and non functions setters).

@lrhn
Copy link
Member

lrhn commented Oct 11, 2023

The closer the syntax is to how functions are accessed, the better.

@Native<Int64>(/* optional symbol and asset */);
external int greeting;

seems optimal, if @Native<Int64 Function(Int64)>(...) external int foo(int x); is what we do for functions.

If the variable declaration is final, no setter is generated, as normal.
Don't know if we can handle late. Probably not.

We can, and probably should, deliberately allow variables to be treated as nullary getter functions/unary setter functions in Dart, and vice versa.

@Native<Int64 Function()>("something.getFoo")
external int get foo;
@Native<Void Function(Int64)>("something.setFoo")
external set foo(int value);

@Native<Int64>("something.foo") // native variable reference, reading method signature.
external int fetchFoo();
@Native<Int64>("something.foo") // same native variable reference, writing method signature.
external void pushFoo(int value);

Nothing more complicated than this, if at all possible.

(Don't know how we deal with immutable variables, but I guess writing to immutable memory will crash you through a setter the same way it would writing through a pointer. If we can somehow detect that the variable is not writable, we can just give a better error message.)

@dcharkes
Copy link
Contributor

If we can somehow detect that the variable is not writable, we can just give a better error message.

I think in FFIgen we will just not generate a setter if we can detect that something is const char* for example. (Not sure of libclang exposes that. cc @mannprerak2)

@mkustermann
Copy link
Member Author

I think in FFIgen we will just not generate a setter if we can detect that something is const char* for example.

const char* global; is a pointer to characters that are constant. The global can be assigned a new value just fine, can't it?

@mannprerak2
Copy link

mannprerak2 commented Oct 12, 2023

...if we can detect that something is const char* for example. (Not sure of libclang exposes that. cc @mannprerak2)

The only way I found is to use the type spelling (clang_getTypeSpelling) to determine this (which should be something like const char *.). And to handle any typedef references we can do - clang_getCanonicalType(cursor.type())

@simolus3
Copy link
Contributor

simolus3 commented Nov 2, 2023

I'm still happy to work on this now that e16bb21 has landed, I just want to make sure I understand the changes correctly.

@dcharkes mentioned it may be easier to implement this and addressOf in the same PR. So I'm wondering if this could be implemented on top of a addressOf operator directly? So, could a transformation look like this:

const a = Native<Int64>(...);

// input:
@a
external int foo;

// transformed:
int get foo {
  final ptr = ffi._addressOfAnnotation(a);
  return ffi._loadInt32(ptr);
}

set foo(int value) {
  final ptr = ffi._addressOfAnnotation(a);
  ffi._storeInt32(ptr, value);
}

Or should a field be lowered into an external getter/setter pair with the body constructed in the VM, similar to what's now being done with @Native methods? That looks like more work since (AFAIK) all loads and stores are currently done with recognized Dart methods?

@dcharkes
Copy link
Contributor

dcharkes commented Nov 3, 2023

addressOf must be implemented in the VM once we do static linking, because we would want to emit an unresolved symbol. (Since we don't have static linking yet, we could do it with it with a CFE transform + to the _ffi_resolver_function temporarily. However, we would undo that in a later CL.)

We could probably lower the fields as you propose, I believe then the optimizer should optimize the Pointer object away (and if not, we should make it do that).

P.S. Unfortunately we had to revert e16bb21.

@simolus3
Copy link
Contributor

I've started working on this in https://dart-review.googlesource.com/c/sdk/+/338020.

@mkustermann
Copy link
Member Author

@Native<Int64>(/* optional symbol and asset */);
external int greeting;

One strange thing about using the re-existing @Native<>() annotation (which was designed for functions originally) is that the Native class has a bool isLeaf property - this has no meaning when accessing fields.

Though @Native is shorter than @NativeField.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi
Projects
None yet
Development

No branches or pull requests

5 participants