Bug: not 128bit aligned with repr-c feature

[Erigara]

I've noticed that while docs claim that Decimal is 128bit aligned it's not the case which could lead to UB (playground run with miri).

Either docs should be edited of fix implemented.

I can work on the fix by introducing new ZST field _align: [u128; 0] into Decimal which would force proper alignment of the type to be equal to u128.

#[cfg_attr(feature = "c-repr", repr(C))]
pub struct Decimal {
    // Bits 0-15: unused
    // Bits 16-23: Contains "e", a value between 0-28 that indicates the scale
    // Bits 24-30: unused
    // Bit 31: the sign of the Decimal value, 0 meaning positive and 1 meaning negative.
    flags: u32,
    // The lo, mid, hi, and flags fields contain the representation of the
    // Decimal value as a 96-bit integer.
    hi: u32,
    lo: u32,
    mid: u32,
    // Enforce proper alignment
    #[cfg(feature = "c-repr")]
    _align: [u128; 0],
}

[Tony-Samuels]

I would suggest the approach should be to update the docs to remove this guarantee. The main problem is that whilst you're correct that the current implementation is not aligned to 128 bits, your proposed implementation won't be either!

You can see this in your playground link if you run it normally (i.e. not with miri) where the error if the alignments don't match displays that u128 is 8 byte aligned (i.e. only 64 bits).

So we would have two options for fixing the alignment, but both break the principle of least surprise, imo:

1. repr(c, align(16))
   • + Aligns to 128 bits
   • - The alignment on some platforms will be greater than than of u128
2. _align: [u128; 0]
   • + The alignment across all platforms will match u128's
   • - The alignment is not guaranteed to be 128 bits

[Tony-Samuels]

A separate issue to removing the docs is still whether the maintainer wishes to change the alignment to match that of u128, which wouldn't be unreasonable. However I think the doc need to be updated to fix this issue regardless.

[Erigara]

You can see this in your playground link if you run it normally (i.e. not with miri) where the error if the alignments don't match displays that u128 is 8 byte aligned (i.e. only 64 bits).

Yeah, i've tried to use repr(c, align(16)) initially to align to 128bits, but i forgot that u128 alignment is tricky.