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

Support for bitfields in structs #406

Open
mannprerak2 opened this issue Aug 19, 2020 · 10 comments
Open

Support for bitfields in structs #406

mannprerak2 opened this issue Aug 19, 2020 · 10 comments
Labels
package:ffigen type-enhancement A request for a change that isn't a bug waiting-on-dart-ffi Issues which are blocked by lacking suppport in dart:ffi

Comments

@mannprerak2
Copy link
Contributor

Bitfields are not directly supported in dart:ffi but it may be possible to generate code to work with it. See dart-lang/sdk#38954.

@dcharkes
Copy link
Collaborator

This is a great feature to add to package:ffigen indeed.

@dcharkes dcharkes added type-enhancement A request for a change that isn't a bug v1.0 labels Aug 24, 2020
@mannprerak2
Copy link
Contributor Author

mannprerak2 commented Aug 29, 2020

It may not be possible to generate bitfields for structs, because of padding issues.

Consider a struct S -

typedef struct
{
    int *a;                             // Can be 4 or 8 bytes
    long long b:35;             // Takes 5 bytes
    char c;                           // Takes 1 byte
} A;

The memory layout for this should be similar to this, The offset of char c is 13 on both 64 and 32-bit compilers, you can test it out here - (https://godbolt.org/z/4e3ET8) -
Screenshot 2020-08-29 at 2 33 54 PM

This 64-bit layout can be confirmed by clang's output

$ clang -Xclang -fdump-record-layouts test.c

*** Dumping AST Record Layout
         0 | A
         0 |   int * a
    8:0-34 |   long long b
        13 |   char c
           | [sizeof=16, align=8]

For generating 32-bit layout on my 64-bit machine, I simply replaced int *a with int a (I suppose it doesn't make a difference)

clang -Xclang -fdump-record-layouts test.c     

*** Dumping AST Record Layout
         0 | A
         0 |   int a
    8:0-34 |   long long b
        13 |   char c
           | [sizeof=16, align=8]

Here's why we cannot generate platform-agnostic bindings for this struct with bitfield -

for representing long long b:35

  • If we use an Int64 this will also end up including the char c inside it.
  • If we use a combination of ints less (in size) than Int64 It will be placed immediately after int *a, and because int *a can end anywhere depending on architecture, this won't be platform-agnostic.

Also, it's possible to pack the structure tightly (using __attribute__((packed)), in which case again, any generated bindings will be useless. See related issue for this -> dart-lang/sdk#43247 dart-lang/sdk#38158

Considering all that, it seems more viable to have bitfields in dart:ffi and let dart VM handle the padding.
We should reopen -> dart-lang/sdk#38954 in that case.

@dcharkes
Copy link
Collaborator

dcharkes commented Aug 31, 2020

Good find. A good middle ground solution would be to add some alignment directives to dart:ffi:

class A extends Struct {
  Pointer<Int> a;

  @AlignedTo(8)
  Int8()
  int _b0;
  Int8()
  int _b1;
  // ...

  Int8()
  int c;
}

I filed dart-lang/sdk#43257 to support that.

@mannprerak2
Copy link
Contributor Author

I am not sure if we will be able to tell which alignment to use.
For example this struct,

typedef struct
{
    char a;
    long long b:30;
} A;

has the following layout

*** Dumping AST Record Layout
         0 | A
         0 |   char a
    1:0-29 |   long long b
           | [sizeof=8, align=8]

I would have expected long long b:30 to start from the 4-byte boundary, but the compiler is basically trying to use minimum space while making sure that those fields can be accessed with minimum read cycles.

Here we must use @AlignedTo(1) but

  • If we replace char a with int *a we will have to use @AlignedTo(4).
  • If we had 5 chars before long long b:30 we would again need to use AlignedTo(4)

@dcharkes dcharkes removed the v1.0 label Aug 31, 2020
@mannprerak2 mannprerak2 added the waiting-on-dart-ffi Issues which are blocked by lacking suppport in dart:ffi label Jan 24, 2021
@bvoq
Copy link

bvoq commented Jan 11, 2023

I would suggest omitting the bitfield information when generating the code.
If I understood the documentation correctly this would still be compliant C code (specifically, it must not be greater than size of the type with the bit information omitted).
https://stackoverflow.com/questions/32677235/bit-fields-memory-management-in-c

That's how I've called my C code with bitfield information and it worked well.

unsigned int sid      : 8;
unsigned int mlength  : 31;

would be interpreted as

unsigned int sid;
unsigned int mlength;

@liamappelbe liamappelbe transferred this issue from dart-archive/ffigen Nov 15, 2023
@MarlonJD
Copy link

MarlonJD commented Jun 18, 2024

Any news about this?

@bvoq
Copy link

bvoq commented Jun 19, 2024

Ignoring the bitfields would fix a lot of codes and it is perfectly compatible with the C standard.
I had to manually replace all bitfields in a C implementation of mbedTLS just to get it working with Dart. After this, everything was working as expected.
It should be an easy fix to ignore the annotations and makes the code cross-platform compatible. What do you think?

@dcharkes
Copy link
Collaborator

As @mannprerak2 mentioned above, it will not be working in all cases.

However, I think it would be fine to add what you're asking for under a config flag, given that it will work for a (large?) subset of all bitfields. That way you can just put the flag in your FFIgen config instead of replace all bitfields in your source header. Feel free to submit a PR!

@mannprerak2
Copy link
Contributor Author

If there is no general solution, I would suggest custom writing the Structs yourself and using type-map and library-import to replace references to that struct's usage across ffigen's generated bindings.

@bvoq
Copy link

bvoq commented Jun 20, 2024

As long as the C/C++ compiler itself is not compiled with bitfields it should work in all cases. Is there a compiler flag for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:ffigen type-enhancement A request for a change that isn't a bug waiting-on-dart-ffi Issues which are blocked by lacking suppport in dart:ffi
Projects
None yet
Development

No branches or pull requests

5 participants