Skip to content

Commit f397e48

Browse files
authored
Fix miscompilation in transmute_unchecked introduced by bug in LLVM (#25)
Currently some casts from byte primitives to two element tuple lead to miscompilation on **release** builds on Rust `>=1.70.0`: - `castaway::cast!(123_u8, (u8, u8))` unexpectedly returns `Ok(...)` that leads to **UB**. - `castaway::cast!(false, (bool, u16))` leads to `SIGILL: illegal instruction` runtime error. Upstream issues: - Rust: rust-lang/rust#127286 - LLVM: llvm/llvm-project#97702 I suggest considering adding a safe "workaround" to fix the issue in this crate without having to wait for the upstream fixes. This way we will have this fixed in older Rust versions as well. This PR adds size eq `assert` to `transmute_unchecked`. This workaround was found while preparing an MRE for an upstream issue. Checked locally with `cargo test --release` for Rust `1.38`, `1.68.0`, `1.69.0`, `1.70.0`, `1.71.0`, `1.72.0`, `stable`, `beta`, `nightly`. Generated assembly for other tests cases for the release build seems the same (checks and casts are optimized away). Btw: it might also be a good idea to run tests in `--release` mode as well since the crate relies heavily on optimizing the casts to zero-cost.
1 parent 7e15c46 commit f397e48

File tree

2 files changed

+25
-1
lines changed

2 files changed

+25
-1
lines changed

src/lib.rs

+10
Original file line numberDiff line numberDiff line change
@@ -512,5 +512,15 @@ mod tests {
512512
0u8 => Err(0u8),
513513
Some(42u8) => Ok(Some(42u8)),
514514
}
515+
516+
// See https://github.com/rust-lang/rust/issues/127286 for details.
517+
for (u8, u8) as TupleU8U8 {
518+
(1u8, 2u8) => Ok((1u8, 2u8)),
519+
1u8 => Err(1u8),
520+
}
521+
for (bool, u16) as TupleBoolU16 {
522+
(false, 2u16) => Ok((false, 2u16)),
523+
true => Err(true),
524+
}
515525
}
516526
}

src/utils.rs

+15-1
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,29 @@ fn non_static_type_id<T: ?Sized>() -> TypeId {
6464
/// size and layout and that it is safe to do this conversion. Which it probably
6565
/// isn't, unless `T` and `U` are identical.
6666
///
67+
/// # Panics
68+
///
69+
/// This function panics if `T` and `U` have different sizes.
70+
///
6771
/// # Safety
6872
///
6973
/// It is up to the caller to uphold the following invariants:
7074
///
71-
/// - `T` must have the same size as `U`
7275
/// - `T` must have the same alignment as `U`
7376
/// - `T` must be safe to transmute into `U`
7477
#[inline(always)]
7578
pub(crate) unsafe fn transmute_unchecked<T, U>(value: T) -> U {
79+
// Assert is necessary to avoid miscompilation caused by a bug in LLVM.
80+
// Without it `castaway::cast!(123_u8, (u8, u8))` returns `Ok(...)` on
81+
// release build profile. `assert` shouldn't be replaced by `assert_eq`
82+
// because with `assert_eq` Rust 1.70 and 1.71 will still miscompile it.
83+
//
84+
// See https://github.com/rust-lang/rust/issues/127286 for details.
85+
assert!(
86+
mem::size_of::<T>() == mem::size_of::<U>(),
87+
"cannot transmute_unchecked if Dst and Src have different size"
88+
);
89+
7690
let dest = ptr::read(&value as *const T as *const U);
7791
mem::forget(value);
7892
dest

0 commit comments

Comments
 (0)