Crystal: C structure generation broken for UInt64

[Papierkorb]

Hello,

I found a bug which drove me almost insane.

C struct generation is broken. I don't know if in the following example code it's the mixture of UInt64 with UInt32, or if UInt64 is broken all the time.

Note: I tried the same code on play.crystal-lang.org, where the code gives the expected results. Could it be that the play instance is running on a x86? Link: https://play.crystal-lang.org/#/r/1a39

Expected behaviour: All prints give the "same" output (Save for the 2x UInt32 vs 1x UInt64)
Observed behaviour: The UInt64 field values are completely wrong, it looks like it's padding the first UInt32 to 8 Byte or something?

Platform

• Machine: x64 ArchLinux - Everything installed via pacman
• Crystal: Crystal 0.19.1 (2016-09-09). I also tried ad83832f1f5afa8e0cda7544e542dd4d48e3bb70 with same results.
• GCC: gcc (GCC) 6.2.1 20160830
• ld: GNU ld (GNU Binutils) 2.27 in case it does something goofy
• LLVM: 3.8.1

Example code

lib MemoryMap
  struct Good
    size : UInt32

    base_address_lo : UInt32 # Split the UInt64s into low and high parts
    base_address_hi : UInt32
    length_lo : UInt32
    length_hi : UInt32

    block_type : UInt32
  end

  struct Bad
    size : UInt32

    base_address : UInt64
    length : UInt64

    block_type : UInt32
  end
end

data = StaticArray[ #=> Show expected values on a little-endian machine
  0x14u8, 0x00u8, 0x00u8, 0x00u8, #=> 0x14u32
  0x00u8, 0x00u8, 0x00u8, 0x00u8, 0x00u8, 0x00u8, 0x00u8, 0x00u8, #=> 0u64
  0x00u8, 0xFCu8, 0x09u8, 0x00u8, 0x00u8, 0x00u8, 0x00u8, 0x00u8, #=> 0x09FC00u64
  0x01u8, 0x00u8, 0x00u8, 0x00u8 #=> 1u32
]

good = data.to_unsafe.as(MemoryMap::Good*)
bad = data.to_unsafe.as(MemoryMap::Bad*)

p sizeof(MemoryMap::Good) #=> 24 (Correct!)
p sizeof(MemoryMap::Bad)  #=> 32 - Wrong, expected 24 here
p data.size               #=> 24

p bad.value               #=> Garbage
# MemoryMap::Bad(@size=20, @base_address=2810351720595456, @length=4294967296, @block_type=0)

p good.value              #=> This is fine
# MemoryMap::Good(@size=20, @base_address_lo=0, @base_address_hi=0, @length_lo=654336, @length_hi=0, @block_type=1)

Thanks.

[asterite]

Welcome to the C world :-)

C ABI requires some padding... I can't remember where the docs are, searching "c abi x86_64" might yield some results, in particular a PDF explaining all the details.

Try this C program:

#include <stdio.h>

struct Good {
  int size;
  int base_address_lo;
  int base_address_hi;
  int length_lo;
  int length_hi;
  int block_type;
};

struct Bad {
  int size;
  long base_address;
  long length;
  int block_type;
};

int main(int argc, char** argv) {
  printf("%lu\n", sizeof(struct Good));
  printf("%lu\n", sizeof(struct Bad));

  return 0;
}

On my machine (64 bits) I get:

32

[asterite]

However, you can annotate C structs with @[Packed]. I think this is what you want/need :-)

[lbguilherme]

Just to give more details on why this happens: Every C type has a memory alignment. UInt32 must be aligned to a multiple of 4, UInt64 must be aligned to a multiple of 8. And the alignment of a struct must be in such a way to satisfy the alignment of every field in it. Padding may also be added to force this alignment.

lib MemoryMap
  struct Good  # Size = 24, Alignment = 4
    size : UInt32
    base_address_lo : UInt32
    base_address_hi : UInt32
    length_lo : UInt32
    length_hi : UInt32
    block_type : UInt32
  end

  struct Bad # Size = 32, Alignment = 8
    size : UInt32
    # Padding of 4 bytes
    base_address : UInt64
    length : UInt64
    block_type : UInt32
    # Padding of 4 bytes
  end
end

The first padding is there so that the UInt64 fields get aligned to 8 bytes when the struct itself is aligned to 8 bytes. The second padding happens to ensure the struct still remains aligned even when put in a StaticArray.

Two ways to fix:

lib MemoryMap
  @[Packed]
  struct NotBad1 # No padding added
    size : UInt32
    base_address : UInt64
    length : UInt64
    block_type : UInt32
  end

  struct NotBad2 # No padding added
    size : UInt32
    block_type : UInt32
    base_address : UInt64
    length : UInt64
  end
end

[Papierkorb]

@asterite Aha, I assumed that "C structures", as far Crystal is concerned, are always packed. That clears things up. Thanks

[ysbaddaden]

Thanks for the detailed explanation @lbguilherme!