Skip to content

Commit

Permalink
Allow explicit padding (#2060)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ericseppanen committed Jul 16, 2021
1 parent 14a8d29 commit 67538b6
Show file tree
Hide file tree
Showing 6 changed files with 147 additions and 5 deletions.
8 changes: 8 additions & 0 deletions src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1797,6 +1797,14 @@ impl CodeGenerator for CompInfo {
(),
);
}
// Check whether an explicit padding field is needed
// at the end.
if let Some(comp_layout) = layout {
fields.extend(
struct_layout
.add_tail_padding(&canonical_name, comp_layout),
);
}
}

if is_opaque {
Expand Down
42 changes: 37 additions & 5 deletions src/codegen/struct_layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,11 @@ impl<'a> StructLayoutTracker<'a> {
let padding_layout = if self.is_packed || is_union {
None
} else {
let force_padding = self.ctx.options().force_explicit_padding;

// Otherwise the padding is useless.
let need_padding = padding_bytes >= field_layout.align ||
let need_padding = force_padding ||
padding_bytes >= field_layout.align ||
field_layout.align > MAX_GUARANTEED_ALIGN;

debug!(
Expand All @@ -236,11 +239,14 @@ impl<'a> StructLayoutTracker<'a> {
field_layout
);

let padding_align = if force_padding {
1
} else {
cmp::min(field_layout.align, MAX_GUARANTEED_ALIGN)
};

if need_padding && padding_bytes != 0 {
Some(Layout::new(
padding_bytes,
cmp::min(field_layout.align, MAX_GUARANTEED_ALIGN),
))
Some(Layout::new(padding_bytes, padding_align))
} else {
None
}
Expand All @@ -262,6 +268,32 @@ impl<'a> StructLayoutTracker<'a> {
padding_layout.map(|layout| self.padding_field(layout))
}

pub fn add_tail_padding(
&mut self,
comp_name: &str,
comp_layout: Layout,
) -> Option<proc_macro2::TokenStream> {
// Only emit an padding field at the end of a struct if the
// user configures explicit padding.
if !self.ctx.options().force_explicit_padding {
return None;
}

if self.latest_offset == comp_layout.size {
// This struct does not contain tail padding.
return None;
}

trace!(
"need a tail padding field for {}: offset {} -> size {}",
comp_name,
self.latest_offset,
comp_layout.size
);
let size = comp_layout.size - self.latest_offset;
Some(self.padding_field(Layout::new(size, 0)))
}

pub fn pad_struct(
&mut self,
layout: Layout,
Expand Down
19 changes: 19 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,10 @@ impl Builder {
output_vector.push("--c-naming".into());
}

if self.options.force_explicit_padding {
output_vector.push("--explicit-padding".into());
}

// Add clang arguments

output_vector.push("--".into());
Expand Down Expand Up @@ -1419,6 +1423,17 @@ impl Builder {
self
}

/// If true, always emit explicit padding fields.
///
/// 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 this should be enabled, as anything reading the padding bytes of
/// a struct may lead to Undefined Behavior.
pub fn explicit_padding(mut self, doit: bool) -> Self {
self.options.force_explicit_padding = doit;
self
}

/// Generate the Rust bindings using the options built up thus far.
pub fn generate(mut self) -> Result<Bindings, ()> {
// Add any extra arguments from the environment to the clang command line.
Expand Down Expand Up @@ -1937,6 +1952,9 @@ struct BindgenOptions {

/// Generate types with C style naming.
c_naming: bool,

/// Always output explicit padding fields
force_explicit_padding: bool,
}

/// TODO(emilio): This is sort of a lie (see the error message that results from
Expand Down Expand Up @@ -2079,6 +2097,7 @@ impl Default for BindgenOptions {
respect_cxx_access_specs: false,
translate_enum_integer_types: false,
c_naming: false,
force_explicit_padding: false,
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,9 @@ where
Arg::with_name("c-naming")
.long("c-naming")
.help("Generate types with C style naming."),
Arg::with_name("explicit-padding")
.long("explicit-padding")
.help("Always output explicit padding fields."),
]) // .args()
.get_matches_from(args);

Expand Down Expand Up @@ -960,6 +963,10 @@ where
builder = builder.c_naming(true);
}

if matches.is_present("explicit-padding") {
builder = builder.explicit_padding(true);
}

let verbose = matches.is_present("verbose");

Ok((builder, output, verbose))
Expand Down
65 changes: 65 additions & 0 deletions tests/expectations/tests/explicit-padding.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#![allow(
dead_code,
non_snake_case,
non_camel_case_types,
non_upper_case_globals
)]

#[repr(C)]
#[derive(Debug, Default, Copy, Clone)]
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],
}
#[test]
fn bindgen_test_layout_pad_me() {
assert_eq!(
::std::mem::size_of::<pad_me>(),
12usize,
concat!("Size of: ", stringify!(pad_me))
);
assert_eq!(
::std::mem::align_of::<pad_me>(),
4usize,
concat!("Alignment of ", stringify!(pad_me))
);
assert_eq!(
unsafe {
&(*(::std::ptr::null::<pad_me>())).first as *const _ as usize
},
0usize,
concat!(
"Offset of field: ",
stringify!(pad_me),
"::",
stringify!(first)
)
);
assert_eq!(
unsafe {
&(*(::std::ptr::null::<pad_me>())).second as *const _ as usize
},
4usize,
concat!(
"Offset of field: ",
stringify!(pad_me),
"::",
stringify!(second)
)
);
assert_eq!(
unsafe {
&(*(::std::ptr::null::<pad_me>())).third as *const _ as usize
},
8usize,
concat!(
"Offset of field: ",
stringify!(pad_me),
"::",
stringify!(third)
)
);
}
11 changes: 11 additions & 0 deletions tests/headers/explicit-padding.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// bindgen-flags: --explicit-padding

typedef unsigned char uint8_t;
typedef unsigned short uint16_t;
typedef unsigned int uint32_t;

struct pad_me {
uint8_t first;
uint32_t second;
uint16_t third;
};

0 comments on commit 67538b6

Please sign in to comment.