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] add alignmentOf<T> (besides just having sizeOf<T>) #39964

Open
dcharkes opened this issue Jan 2, 2020 · 2 comments
Open

[vm/ffi] add alignmentOf<T> (besides just having sizeOf<T>) #39964

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

Comments

@dcharkes
Copy link
Contributor

dcharkes commented Jan 2, 2020

Allocation in package:ffi currently only relies on the sizeOf an FFI type. However, when allocating more than count: 1, it should also take into account the required alignment.

For example, the size of of a struct might be 28 bytes, while the required alignment should be 8 bytes. This means that when allocating count: 2, the second value should be at an offset of 32 bytes, rather than 28 bytes.

Proposed Solution

Add the alignmentOf<T>:

/// Number of bytes used by native type T.
///
/// Includes padding and alignment of struct fields.
external int sizeOf<T extends NativeType>();

/// Alignment in number of bytes of native type T.
///
/// Includes padding and alignment of structs.
external int alignmentOf<T extends NativeType>();

Note that we might want to remove the generic use of sizeOf<T> (#38721 (comment)), the same would be true for alignmentOfT<T>.

Original Example

class KeyboardInput extends Struct {
  @Uint32() int type;
  @Uint16() int virtualCode;
  @Uint16() int scanCode;
  @Uint32() int flags;
  @Uint32() int time;
  Pointer dwExtraInfo; // Is 8 bytes long and 8 byte aligned on many 64 bit platforms.
  @Uint32() int padding;
}

I don't understand why sizeOf<KeyboardInput>() gives me size of 32 instead of 28 (but with padding field removed it correctly says 24).

Originally posted by @marad in #37271 (comment)

Thanks for reporting this @marad.

cc @mkustermann

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

dcharkes commented Jan 2, 2020

We should probably do this together with #36730, they both need the same info.

@safasofuoglu
Copy link

@dcharkes is it possible this gets attention anytime soon?

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

2 participants