Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Assertion failure when a struct contains a struct which contains bit fields spanning [17, 25) or [33, 57) bits. #4646

Closed
just-harry opened this issue May 5, 2024 · 2 comments · Fixed by #4708

Comments

@just-harry
Copy link
Contributor

A quick test case:

In a C file, define BitFlags like:

typedef struct BitFlags {
  unsigned int _00 : 1;
  unsigned int _01 : 1;
  unsigned int _02 : 1;
  unsigned int _03 : 1;
  unsigned int _04 : 1;
  unsigned int _05 : 1;
  unsigned int _06 : 1;
  unsigned int _07 : 1;
  unsigned int _08 : 1;
  unsigned int _09 : 1;
  unsigned int _10 : 1;
  unsigned int _11 : 1;
  unsigned int _12 : 1;
  unsigned int _13 : 1;
  unsigned int _14 : 1;
  unsigned int _15 : 1;
  unsigned int _16 : 1;
} BitFlags;

or like:

typedef struct BitFlags {
  unsigned long long _00 : 1;
  unsigned long long _01 : 1;
  unsigned long long _02 : 1;
  unsigned long long _03 : 1;
  unsigned long long _04 : 1;
  unsigned long long _05 : 1;
  unsigned long long _06 : 1;
  unsigned long long _07 : 1;
  unsigned long long _08 : 1;
  unsigned long long _09 : 1;
  unsigned long long _10 : 1;
  unsigned long long _11 : 1;
  unsigned long long _12 : 1;
  unsigned long long _13 : 1;
  unsigned long long _14 : 1;
  unsigned long long _15 : 1;
  unsigned long long _16 : 1;
  unsigned long long _17 : 1;
  unsigned long long _18 : 1;
  unsigned long long _19 : 1;
  unsigned long long _20 : 1;
  unsigned long long _21 : 1;
  unsigned long long _22 : 1;
  unsigned long long _23 : 1;
  unsigned long long _24 : 1;
  unsigned long long _25 : 1;
  unsigned long long _26 : 1;
  unsigned long long _27 : 1;
  unsigned long long _28 : 1;
  unsigned long long _29 : 1;
  unsigned long long _30 : 1;
  unsigned long long _31 : 1;
  unsigned long long _32 : 1;
} BitFlags;

then define a struct which contains BitFlags, e.g.

typedef struct Foo {
  BitFlags flags;
} Foo;

The following failure will occur upon compilation:

Assertion failed: fieldSize <= af.size, file D:\a\ldc\ldc\ir\irtypeaggr.cpp, line 219

As far as I can tell, this is the cause:
Within the context of a call to IrTypeStruct::get for the BitFlags struct, in AggrTypeBuilder::addAggregate when the bitfield-grouping's group.sizeInBytes (and thus the actual field's size) is 3, 5, 6, or 7 the LLVM type for that bitfield-grouping ends up being i24, i40, i48, or i56.

Then, because sd->structsize is 4 or 8, AggrTypeBuilder::addTailPadding ends up adding some bytes of padding after the bitfield's non-power-of-two-sized integer.

But as i24 has same ABI alignment as i32, and i40, i48, i56 the same as i64, they have their own implicit padding from LLVM's data-layout, so the tail-padding added by LDC ends up making the BitFlags struct bigger than it should be.

@kinke
Copy link
Member

kinke commented May 5, 2024

Thx for the report. Converted to D, compilable via -preview=bitfields:

struct BitField {
    uint _0 : 1;
    uint _1 : 1;
    uint _2 : 1;
    uint _3 : 1;
    uint _4 : 1;
    uint _5 : 1;
    uint _6 : 1;
    uint _7 : 1;
    uint _8 : 1;
    uint _9 : 1;
    uint _10 : 1;
    uint _11 : 1;
    uint _12 : 1;
    uint _13 : 1;
    uint _14 : 1;
    uint _15 : 1;
    uint _16 : 1;
}

struct Foo {
    BitField bf;
}

pragma(msg, BitField.sizeof, ", ", BitField.alignof); // 4, 4
pragma(msg, Foo.sizeof, ", ", Foo.alignof); // 4, 4

Compiling via -vv shows that the BitField IR type is an un-packed struct:

* * * * Building struct type test.BitField @ test.d(1)
* * * * * final struct type: %test.BitField = type { i24, [1 x i8] }

so that LLVM is indeed free to add some implicit padding. There might be a missing assertion checking the final aggregate size (IR vs. AST) too, catching this shouldn't require using the type as field of another aggregate.

@kinke
Copy link
Member

kinke commented May 5, 2024

Oh wait, the type-alloc size of a packed <{ i24, [1 x i8] }> is apparently 5 bytes too! Edit: Well, it is 8 if unpacked.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants