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.

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

[vm/ffi] Expose struct field offsets #41237

Open
dcharkes opened this issue Mar 27, 2020 · 7 comments
Open

[vm/ffi] Expose struct field offsets #41237

dcharkes opened this issue Mar 27, 2020 · 7 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. library-ffi

Comments

@dcharkes
Copy link
Contributor

dcharkes commented Mar 27, 2020

dart:ffi internally calculates the fields offsets of structs in order to enable loading/storing from/to those fields.

However, sometimes one would like to pass a pointer to a field inside a struct to C. For that use case we should expose the internal offsets.

One way we could do this, for example for

class VeryLargeStruct extends Struct {
@Int8()
int a;
@Int16()
int b;
@Int32()
int c;
@Int64()
int d;
@Uint8()
int e;
@Uint16()
int f;
@Uint32()
int g;
@Uint64()
int h;
@IntPtr()
int i;
@Double()
double j;
@Float()
double k;
Pointer<VeryLargeStruct> parent;
@IntPtr()
int numChildren;
Pointer<VeryLargeStruct> children;
@Int8()
int smallLastField;
}
would be

class VeryLargeStruct extends Struct { 
  @Int8() 
  int a;
  external static int get offsetOfA; // Will be filled in by the compiler.

  // ...
}

Or alternatively we could provide inner pointers:

class VeryLargeStruct extends Struct { 
  @Int8() 
  int a;
  external static Pointer<Int8> get pointerToA; // Will be filled in by the compiler.

  // ...
}

(Though, I like exposing offset better. That is a better indication you're doing your own pointer arithmetic.)

Edit: we can't expose Pointers because the struct might be backed by TypedData. But we could generate extension methods for the struct pointers in package:ffigen: dart-lang/native#331.

cc @timsneath

@dcharkes dcharkes added area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. library-ffi labels Mar 27, 2020
@ds84182
Copy link
Contributor

ds84182 commented Jul 20, 2021

This would be very nice to have, but I'd imagine we could only expose field offsets because of TypedData-backed structs?

Also, to reduce the need for having to use the name of the field in the offset name, maybe this should use an annotation?

class MyStruct extends Struct {
  @Int32()
  external int foo;
  @Int64()
  external int bar;

  @OffsetOf(#foo)
  external static int offsetOfFoo; // Will be filled in by the compiler.
  @OffsetOf(#bar)
  external static int offsetOfBar; // Will be filled in by the compiler.
}

@dcharkes
Copy link
Contributor Author

If we use the offsetOf prefix, it would be less typing, but it could conflict with with an existing field name.

@ds84182 I presume you meant making the external consts static? Because these offsets do not belong to a specific offset. Also, they cant be const because the actual offset depends on the architecture we're running on (imagine having an intptr_t field before another field). So the compiler generates a branch pick one of the possible offsets at runtime.

@lrhn any API suggestions besides the one by @ds84182 and me?

@timsneath
Copy link
Contributor

Looking further at the suggestion of generated static ints, this suffers from the requirement that every potential needed offset needs to be identified and chosen. In the real-world scenario that I have in mind for this (wrapping Win32 structs), it's the caller who will likely decide that they need a pointer to the location, so unless I include an offset for every struct field, I can't guarantee a complete offering.

The example I used in a private mail was this:

class PAINTSTRUCT extends Struct {
  @IntPtr()
  external int hdc;

  @Int32()
  external int fErase;

  external RECT rcPaint;

  @Int32()
  external int fRestore;

  @Int32()
  external int fIncUpdate;

  @Array(32)
  external Array<Uint8> rgbReserved;
}

I need a pointer to the memory location of rcPaint to pass to the FillRect() API, which takes a Pointer<RECT>. With this scenario, the caller could theoretically add an extension to find the offset:

extension PSOffset on PAINTSTRUCT {
  @OffsetOf(#rcPaint)
  external static int rcPaintOffset; 
}

and then add

  final ps = calloc<PAINTSTRUCT>();
  final hdc = BeginPaint(hwnd, ps);
  final psRect = Pointer<RECT>.fromAddress(ps.address + ps.ref.rcPaintOffset);
  ...

but that seems rather unwieldy. Hopefully there's a better way?

@dcharkes
Copy link
Contributor Author

dcharkes commented May 2, 2022

The caller could theoretically add an extension to find the offset, but that seems rather unwieldy. Hopefully there's a better way?

Agreed.

As a general approach, we would like to add the low level building blocks to dart:ffi and have the higher level things built on top by for example package:ffigen. In that case you'd have to generate these in your win32 generator rather than have the user write them. (Side note: We should consider checking if it would be possible to have the win32 header parser as a frontend for package:ffigen, so that all features added to package:ffigen can benefit package:win32s generated API as well. But I image package:win32's generator is quite involved by now already.)

@timsneath Inner pointers seem to only be useful when nested structs are involved, correct? So for adding something to package:ffigen and the win32 generator we could probably generate the extension methods for the nested structs but not for all primitive type fields.

@ds84182
Copy link
Contributor

ds84182 commented May 8, 2023

Coming back to this, how about (similar to sizeOf, alignOf):

inline class FieldOffset<T extends Struct> {
  final int offset;
  const FieldOffset(this.offset);
}

external FieldOffset<T> offsetOf<T extends Struct>(Symbol field);

extension<T extends Struct> StructPointer on Pointer<T> {
  external Pointer<F> offsetTo<F extends NativeType>(FieldOffset<T> offset);

  external Pointer<F> addressOf<F extends NativeType>(Symbol symbol);
}

final psRect = ps.offsetTo<RECT>(offsetOf(#rcPaint));
// OR
final psRect = ps.addressOf<RECT>(#rcPaint);

@dcharkes
Copy link
Contributor Author

dcharkes commented May 9, 2023

We already have sizeOf<T extends NativeType>(), and it would make sense to add alignmentOf<T>() #39964. I'm thinking the most natural fit would be to have int offsetIn<S extends Struct>(Symbol field).

Both offsetIn and alignmentOf will have the same restrictions as sizeOf, the (type) arguments all need to be compile-time constants.

I'm not sure about the inline classes yet, we do not have an inline class wrapper for Pointer.address to reason about addresses either. So adding an inline class now would make our pointer arithmetic half with and half without inline classes. And I presume making the current pointer arithmetic inline classes would be a big change. Thanks for the suggestion! I've filed #52320 to discuss this.

extension<T extends Struct> StructPointer on Pointer<T> {
  external Pointer<F> addressOf<F extends NativeType>(Symbol symbol);
}

Are you proposing this to be in the SDK? It would have the same requirements: statically known arguments. We might also want to add type checking of F against symbols type.

However, maybe it would be better to just add offsetIn and let package:ffigen generate this instead. In FFIgen we could generate:

class Foo extends Struct {
  @Int8()
  int a;

  @Int8()
  int b;
}

extension FooPointer on Pointer<Foo> {
  Pointer<Int8> a => Pointer.fromAddress(address + offsetIn<Foo>(#a));

  Pointer<Int8> b => Pointer.fromAddress(address + offsetIn<Foo>(#b));
}

dart:ffi would supply the offsetIn building block, FFIgen generates the nice API.

@FeodorFitsner
Copy link

Is there a timeframe when this feature could be implemented? Looks like it's currently impossible to call a native method if it expects a pointer to a struct member as a parameter. Is there any way to calculate the offset "manually" and create a pointer from it?

copybara-service bot pushed a commit that referenced this issue Feb 9, 2024
For getting the address of `Struct` fields and `Array` elements
it would be useful to access the `_typedDataBase` field in these
'wrappers' in a unified way.

This CL makes `Array` extend `_Compound`.
Since `Array` is not a `SizedNativeType`, the implements clauses
for `Struct` and `Union` are moved to these classes.

Moreover, any references to 'compound' which only apply to struct or
union are updated.

TEST=test/ffi

Bug: #44589
Bug: #54739
Bug: #41237

CoreLibraryReviewExempt: No API change, only refactoring.
Change-Id: Ib9d8bccd4872df04bcc67731e4052f826ab70af4
Cq-Include-Trybots: luci.dart.try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-aot-win-debug-x64-try,vm-appjit-linux-debug-x64-try,vm-asan-linux-release-x64-try,vm-checked-mac-release-arm64-try,vm-eager-optimization-linux-release-ia32-try,vm-eager-optimization-linux-release-x64-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-try,vm-fuchsia-release-x64-try,vm-kernel-linux-debug-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-linux-debug-ia32-try,vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-mac-debug-arm64-try,vm-mac-debug-x64-try,vm-msan-linux-release-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-ubsan-linux-release-x64-try,vm-win-debug-arm64-try,vm-win-debug-x64-try,vm-win-release-ia32-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/350960
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Liam Appelbe <liama@google.com>
copybara-service bot pushed a commit that referenced this issue Mar 6, 2024
And use these new getters inside field reads/writes.
This outlines this currently inlined code into calls to a helper
methods. This may make the kernel a little smaller and easier to
read. Also, it removes the need to compute the (now outlined)
expression on arbitrary call sites in the FFI transformer

TEST=test/ffi
TEST=all the expect files

Bug: #44589
Bug: #41237
Change-Id: I53d69778ee50186944229550f89f10c22452e13f
Cq-Include-Trybots: luci.dart.try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-appjit-linux-debug-x64-try,vm-asan-linux-release-x64-try,vm-checked-mac-release-arm64-try,vm-eager-optimization-linux-release-ia32-try,vm-eager-optimization-linux-release-x64-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-try,vm-fuchsia-release-x64-try,vm-kernel-linux-debug-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-linux-debug-ia32-try,vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-mac-debug-arm64-try,vm-mac-debug-x64-try,vm-msan-linux-release-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-ubsan-linux-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/353261
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
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, FFI, and the AOT and JIT backends. library-ffi
Projects
None yet
Development

No branches or pull requests

4 participants