dlang-community / D-Scanner

Swiss-army knife for D source code
Boost Software License 1.0
242 stars 80 forks source link

Bitfields support #955

Open rikkimax opened 4 months ago

rikkimax commented 4 months ago

With the upcoming bitfields feature to D, we've been trying to understand Walter's position on why he will not accept it to have a predictable layout.

Our conclusion has been that as long as you do not start a bit field in one memory location and end in another it is predictable regardless of the types used.

So what dscanner will need to warn for the following code:

struct Foo {
    int a:31;
    int b:2; // Warning: `b` starts in an earlier memory position and will not have a predictable bit layout. Wrap in a `struct { ... }` or add padding to flesh out `a` to be 32 bits wide before `b`.
}

From C23 specification:

An implementation may allocate any addressable storage unit large enough to hold a bit-field. If enough space remains, a bit-field that immediately follows another bit-field in a structure shall be packed into adjacent bits of the same unit. If insufficient space remains, whether a bit-field that does not fit is put into the next unit or overlaps adjacent units is implementation-defined. The order of allocation of bit-fields within a unit (high-order to low-order or low-order to high-order) is implementation-defined. The alignment of the addressable storage unit is unspecified.

struct Foo {
    int a:31;
    char b:1;
};

int main() {
    struct Foo foo;
    foo.a = 1;
    foo.b = 0;

    return 0;
}
main:
 push   rbp
 mov    rbp,rsp
 mov    DWORD PTR [rbp-0x4],0x0
 mov    eax,DWORD PTR [rbp-0x8]
 and    eax,0x80000000
 or     eax,0x1
 mov    DWORD PTR [rbp-0x8],eax
 mov    eax,DWORD PTR [rbp-0x8]
 and    eax,0x7fffffff
 or     eax,0x0
 mov    DWORD PTR [rbp-0x8],eax
 xor    eax,eax
 pop    rbp
 ret
rikkimax commented 4 months ago

When I wrote this ticket I had assumed that the size of the memory address was dependent upon the first entries type, this isn't the case.

It is whatever number of bytes it fits inside of at a minimum that is guaranteed.

But from a practical standpoint, we can use a power of 2 up to a pointer in size to assume what it'll be. With a default of 8 bytes (since all CPU's today are 64bit).

It'll also need to take into account alignment and that'll drop the previous value down to the minimum number of bytes.

Oh how I hate the need for this mitigation.

rikkimax commented 4 months ago

Anddd we are back at not negatable.

struct Foo {
     unsigned short x;
     unsigned int a : 12;
     unsigned int b : 12;
     unsigned int c : 8;

     //void* next;
};

int main() {
    struct Foo foo;
    foo.a = 1;
    foo.b = 0;

    return 0;
}
rikkimax commented 4 months ago

No wait, if you chuck it into an anonymous struct it's fine.

struct Foo {
      unsigned short x;

      struct {
         unsigned int a : 12;
         unsigned int b : 12;
         unsigned int c : 8;
      };

      //void* next;
};

sigh