Skip to content

Commit

Permalink
Fix reproducible builds due to how derives were populated
Browse files Browse the repository at this point in the history
Previously `HashSet` was used to prevent duplicate entries in the
computed derive attributes for bindgen generated code. The default
hasher used in `HashSet` is randomly seeded on each process run to
prevent HashDos. This random seeding resulted in changing the ordering
of the resulted derive attributes. This changing order of the derive
attributes prevented reproducible builds as the derived implementations
would move around in the resultant binary. Now `BtreeSet` is used to
prevent duplicate derives during code generation. This provides a
consistent ordering of the derives.
  • Loading branch information
nick-mobilecoin committed Dec 11, 2023
1 parent 5123503 commit 46fde17
Showing 1 changed file with 52 additions and 15 deletions.
67 changes: 52 additions & 15 deletions core/build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use bindgen::{
callbacks::{IntKind, ParseCallbacks},
Builder, EnumVariation,
};
use std::collections::HashSet;
use std::collections::BTreeSet;
use std::{env, path::PathBuf};

static DEFAULT_SGX_SDK_PATH: &str = "/opt/intel/sgxsdk";
Expand Down Expand Up @@ -206,43 +206,47 @@ impl SgxParseCallbacks {
.extend(serialize_types.into_iter().map(ToString::to_string));
self
}
}

impl ParseCallbacks for SgxParseCallbacks {
fn item_name(&self, name: &str) -> Option<String> {
normalize_item_name(name)
}

fn add_derives(&self, info: &DeriveInfo<'_>) -> Vec<String> {
let mut attributes = HashSet::new();
let name = info.name;
/// Get the derives that should be applied to the provided named type.
fn derives(&self, type_name: &str) -> Vec<String> {
let mut attributes = BTreeSet::new();

if self.default_types.iter().any(|n| *n == name) {
if self.default_types.iter().any(|n| *n == type_name) {
attributes.insert("Default");
}

// The [enum_types] method adds enums to the [copyable_types]
if self.copyable_types.iter().any(|n| *n == name) {
if self.copyable_types.iter().any(|n| *n == type_name) {
attributes.insert("Copy");
}

if !self.dynamically_sized_types.iter().any(|n| *n == name) {
if !self.dynamically_sized_types.iter().any(|n| *n == type_name) {
// For dynamically sized types we don't even derive Debug, because
// they are often times packed and packed types can't derive Debug
// without deriving Copy, however by the dynamic nature one can't
// derive Copy
attributes.insert("Debug");
if !self.enum_types.iter().any(|n| *n == name) {
if !self.enum_types.iter().any(|n| *n == type_name) {
attributes.extend(["Eq", "PartialEq", "Hash", "Clone"]);
}
};

if self.serialize_types.iter().any(|n| *n == name) {
if self.serialize_types.iter().any(|n| *n == type_name) {
attributes.extend(["Serialize", "Deserialize"]);
}

attributes.into_iter().map(String::from).collect::<Vec<_>>()
}
}

impl ParseCallbacks for SgxParseCallbacks {
fn item_name(&self, name: &str) -> Option<String> {
normalize_item_name(name)
}

Check warning on line 245 in core/build/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

core/build/src/lib.rs#L243-L245

Added lines #L243 - L245 were not covered by tests

fn add_derives(&self, info: &DeriveInfo<'_>) -> Vec<String> {
self.derives(info.name)
}

Check warning on line 249 in core/build/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

core/build/src/lib.rs#L247-L249

Added lines #L247 - L249 were not covered by tests

fn int_macro(&self, name: &str, _value: i64) -> Option<IntKind> {
const USIZE_SUFFIXES: &[&str] = &["_SIZE", "_BYTES", "_IDX", "_COUNT"];
Expand Down Expand Up @@ -328,3 +332,36 @@ pub fn sgx_library_suffix() -> &'static str {
_ => "",
}
}

#[cfg(test)]
mod test {
use super::*;

#[test]
fn verify_consistency_of_derive_attributes() {
// This test prevents a regression where `HashSet` was used to prevent
// duplicates in the derives. Using `HashSet` resulted in the derives
// coming back in varying order based on seed of the default hasher.
// The seed of the default hasher is randomized per process run.
// This inconsistent ordering of the derives prevented reproducible
// builds as the derived implementations would move around in the
// resultant binary.
let mut callbacks = SgxParseCallbacks::default();
callbacks = callbacks.serialize_types(["foo"]);
let derives = callbacks.derives("foo");

let expected = [
"Clone",
"Debug",
"Deserialize",
"Eq",
"Hash",
"PartialEq",
"Serialize",
]
.into_iter()
.map(String::from)
.collect::<Vec<_>>();
assert_eq!(derives, expected);
}
}

0 comments on commit 46fde17

Please sign in to comment.