ldc icon indicating copy to clipboard operation
ldc copied to clipboard

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

Open just-harry opened this issue 1 year ago • 2 comments

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.

just-harry avatar May 05 '24 03:05 just-harry

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 avatar May 05 '24 11:05 kinke

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.

kinke avatar May 05 '24 12:05 kinke