Skip to content

Commit dbe3a34

Browse files
committed
Retrieve the actual alignment of max_align_t
The code had a hard-coded maximum alignment of 16 (128 bits) which may have potentially wasted memory and cause memory unsafety if it was ever compiled with larger alignment (unlikely). This change implements a trick to retrieve the actual alignment from C. To achieve this a newly added C file defines a symbol initialized to `_Alignof(max_align_t)`. It is then compiled (as a separate object), the contents of the symbol is extracted and converted to decimal string which is injected into the `AlignedType` definition. The definition is generated as a separate `.rs` file in `OUT_DIR` and included into `types.rs`.
1 parent 5256139 commit dbe3a34

File tree

3 files changed

+93
-16
lines changed

3 files changed

+93
-16
lines changed

secp256k1-sys/build.rs

+85-7
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,77 @@ extern crate cc;
2525

2626
use std::env;
2727

28-
fn main() {
29-
// Actual build
28+
fn gen_max_align() {
29+
configured_cc()
30+
.file("depend/max_align.c")
31+
.cargo_metadata(false)
32+
.compile("max_align.o");
33+
let out_dir = std::path::PathBuf::from(std::env::var_os("OUT_DIR").expect("missing OUT_DIR"));
34+
let target_endian = std::env::var("CARGO_CFG_TARGET_ENDIAN")
35+
.expect("missing CARGO_CFG_TARGET_ENDIAN");
36+
let target_pointer_width_bytes = std::env::var("CARGO_CFG_TARGET_POINTER_WIDTH")
37+
.expect("missing CARGO_CFG_TARGET_POINTER_WIDTH")
38+
.parse::<usize>()
39+
.expect("malformed CARGO_CFG_TARGET_POINTER_WIDTH")
40+
// CARGO_CFG_TARGET_POINTER_WIDTH is in bits, we want bytes
41+
/ 8;
42+
let max_align_bin = out_dir.join("max_align.bin");
43+
// Note that this copies *whole* sections to a binary file.
44+
// It's a bit brittle because some other symbol could theoretically end up there.
45+
// Currently it only has one on my machine and we guard against unexpected changes by checking
46+
// the size - it must match the target pointer width.
47+
let objcopy = std::process::Command::new("objcopy")
48+
.args(&["-O", "binary"])
49+
// cc inserts depend - WTF
50+
.arg(out_dir.join("depend/max_align.o"))
51+
.arg(&max_align_bin)
52+
.spawn()
53+
.expect("failed to run objcopy")
54+
.wait()
55+
.expect("failed to wait for objcopy");
56+
assert!(objcopy.success(), "objcopy failed");
57+
let mut max_align_bytes = std::fs::read(max_align_bin).expect("failed to read max_align.bin");
58+
// The `usize` of target and host may not match so we need to do conversion.
59+
// Sensible alignments should be very small anyway but we don't want crappy `unsafe` code.
60+
// Little endian happens to be a bit easier to process so we convert into that.
61+
// If the type is smaller than `u64` we zero-pad it.
62+
// If the type is larger than `u6` but the number fits into `u64` it'll have
63+
// unused tail which is easy to cut-off.
64+
// If the number is larger than `u64::MAX` then bytes beyond `u64` size will
65+
// be non-zero.
66+
//
67+
// So as long as the max alignment fits into `u64` this can decode alignment
68+
// for any architecture on any architecture.
69+
assert_eq!(max_align_bytes.len(), target_pointer_width_bytes);
70+
if target_endian != "little" {
71+
max_align_bytes.reverse()
72+
}
73+
// copying like this auto-pads the number with zeroes
74+
let mut buf = [0; std::mem::size_of::<u64>()];
75+
let to_copy = buf.len().min(max_align_bytes.len());
76+
// Overflow check
77+
if max_align_bytes[to_copy..].iter().any(|b| *b != 0) {
78+
panic!("max alignment overflowed u64");
79+
}
80+
buf[..to_copy].copy_from_slice(&max_align_bytes[..to_copy]);
81+
let max_align = u64::from_le_bytes(buf);
82+
let src = format!(r#"
83+
/// A type that is as aligned as the biggest alignment for fundamental types in C.
84+
///
85+
/// Since C11 that means as aligned as `max_align_t` is.
86+
/// The exact size/alignment is unspecified.
87+
#[repr(align({}))]
88+
#[derive(Default, Copy, Clone)]
89+
pub struct AlignedType([u8; {}]);"#, max_align, max_align);
90+
std::fs::write(out_dir.join("aligned_type.rs"), src.as_bytes()).expect("failed to write aligned_type.rs");
91+
}
92+
93+
/// Returns CC builder configured with all defines but no C files.
94+
fn configured_cc() -> cc::Build {
95+
// While none of these currently affect max alignment we prefer to keep the "hygiene" so that
96+
// new code will be correct.
3097
let mut base_config = cc::Build::new();
31-
base_config.include("depend/secp256k1/")
32-
.include("depend/secp256k1/include")
33-
.include("depend/secp256k1/src")
34-
.flag_if_supported("-Wno-unused-function") // some ecmult stuff is defined but not used upstream
35-
.define("SECP256K1_API", Some(""))
98+
base_config.define("SECP256K1_API", Some(""))
3699
.define("ENABLE_MODULE_ECDH", Some("1"))
37100
.define("ENABLE_MODULE_SCHNORRSIG", Some("1"))
38101
.define("ENABLE_MODULE_EXTRAKEYS", Some("1"));
@@ -48,6 +111,17 @@ fn main() {
48111
#[cfg(feature = "recovery")]
49112
base_config.define("ENABLE_MODULE_RECOVERY", Some("1"));
50113

114+
base_config
115+
}
116+
117+
fn build_secp256k1() {
118+
let mut base_config = configured_cc();
119+
base_config.include("depend/secp256k1/")
120+
.include("depend/secp256k1/include")
121+
.include("depend/secp256k1/src")
122+
.flag_if_supported("-Wno-unused-function"); // some ecmult stuff is defined but not used upstream
123+
124+
51125
// WASM headers and size/align defines.
52126
if env::var("CARGO_CFG_TARGET_ARCH").unwrap() == "wasm32" {
53127
base_config.include("wasm/wasm-sysroot")
@@ -69,3 +143,7 @@ fn main() {
69143
}
70144
}
71145

146+
fn main() {
147+
gen_max_align();
148+
build_secp256k1();
149+
}

secp256k1-sys/depend/max_align.c

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
#include <stddef.h>
2+
3+
// Note that this symbol is NOT linked with the rest of the library.
4+
// The name is sort of unique in case it accidentally gets linked.
5+
const size_t rust_secp256k1_private_max_align = _Alignof(max_align_t);

secp256k1-sys/src/types.rs

+3-9
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,15 @@ pub type c_char = i8;
1111

1212
pub use core::ffi::c_void;
1313

14-
/// A type that is as aligned as the biggest alignment for fundamental types in C
15-
/// since C11 that means as aligned as `max_align_t` is.
16-
/// the exact size/alignment is unspecified.
17-
// 16 matches is as big as the biggest alignment in any arch that rust currently supports https://github.com/rust-lang/rust/blob/2c31b45ae878b821975c4ebd94cc1e49f6073fd0/library/std/src/sys_common/alloc.rs
18-
#[repr(align(16))]
19-
#[derive(Default, Copy, Clone)]
20-
pub struct AlignedType([u8; 16]);
14+
include!(concat!(env!("OUT_DIR"), "/aligned_type.rs"));
2115

2216
impl AlignedType {
2317
pub fn zeroed() -> Self {
24-
AlignedType([0u8; 16])
18+
Self::ZERO
2519
}
2620

2721
/// A static zeroed out AlignedType for use in static assignments of [AlignedType; _]
28-
pub const ZERO: AlignedType = AlignedType([0u8; 16]);
22+
pub const ZERO: AlignedType = AlignedType([0u8; core::mem::size_of::<AlignedType>()]);
2923
}
3024

3125
#[cfg(all(feature = "alloc", not(rust_secp_no_symbol_renaming)))]

0 commit comments

Comments
 (0)