Allow explicit padding #2060
Allow explicit padding #2060
Conversation
|
|
src/codegen/struct_layout.rs
Outdated
| @@ -236,11 +239,14 @@ impl<'a> StructLayoutTracker<'a> { | |||
| field_layout | |||
| ); | |||
|
|
|||
| let padding_align = if force_padding { | |||
| 0 | |||
There was a problem hiding this comment.
Using an alignment of zero seems wrong, shouldn't it be one?
There was a problem hiding this comment.
Yes, sorry. Will fix.
There was a problem hiding this comment.
Actually, thinking a bit more about it, shouldn't the padding alignment be the default if it's mandatory padding? Otherwise we might not fulfill the alignment requirements.
There was a problem hiding this comment.
I'm afraid I don't understand. The padding I see emitted (with my change enabled) is an array of u8s, e.g.
struct pad_me {
uint8_t first;
uint32_t second;
uint16_t third;
};pub struct pad_me {
pub first: u8,
pub __bindgen_padding_0: [u8; 3usize],
pub second: u32,
pub third: u16,
pub __bindgen_padding_1: [u8; 2usize],
}I don't understand why any __bindgen_padding_X field would ever have an alignment requirement-- that sounds like something that would re-introduce gaps between fields, which is exactly what I wanted to eliminate.
There was a problem hiding this comment.
I may be confused by the fact that I don't actually know what the existing padding code is for!
My desire is to get a struct that has no gaps-- every byte is accounted for by a named field. The previous code is kind of a mystery to me; I don't know what problem it's solving.
If a struct needs to be serialized in its native format (padding bytes and all), for example writing it to a file or sending it on the network, then explicit padding fields are necessary, as anything reading the padding bytes of a struct may lead to Undefined Behavior.
ericseppanen
force-pushed
the
explicit_padding
branch
from
89795f8 to
9281810
Compare
Padding fields are an array of bytes; specify 1-byte alignment.
|
Test failures seem to be due to |
If a struct needs to be serialized in its native format (padding bytes and all), for example writing it to a file or sending it on the network, then explicit padding fields are necessary, as anything reading the padding bytes of a struct may lead to Undefined Behavior.
This allows structs with padding to be compatible with
zerocopy::AsBytes, as well as the more dangerous practice oftransmuteto a byte array.Who would want such a crazy thing? Well, it turns out that there are a bunch of data structures in the Postgres code base that are written to files in their native memory format, non-portable padding and all. To be able to read and write those files we can use
serdeorzerocopy, but in either case we need the padding fields to be explicit.This change is a bit more complicated than just "always turn on the padding bindgen could already compute" because previously, there was no code to generate padding fields at the end of a struct.
That leaves the question of what should happen in the cases where bindgen was emitting padding already-- is the missing tail padding a bug? I'm not sure how to trigger the existing "bindgen decided on its own to emit padding fields" mode, so I left that alone so far.