Skip to content

Commit 9918dba

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 9918dba

File tree

3 files changed

+92
-16
lines changed

3 files changed

+92
-16
lines changed

secp256k1-sys/build.rs

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

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

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

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)