From 5c866d6e4e67c73f35c24257558e420f5b609bc7 Mon Sep 17 00:00:00 2001 From: Ian McCormack Date: Sun, 5 Nov 2023 12:54:33 -0500 Subject: [PATCH] avoid `MaybeUninit` The use of `MaybeUninit` was invalid in `quantum_matches`, `partial_cmp`, `total_cmp`. The underlying decNumber library was *not* guaranteed to zero-initialize the entire `Decimal` struct--in particular it would often leave unneeded bytes in `lsu` uninitialized--but Rust requires that the entire struct be initialized before calling `assume_init`. So just zero initialize in Rust and avoid `MaybeUninit` entirely. Note that we do think that the use of `MaybeUninit` to avoid zero-initializing the entire string formatting buffer in the `fmt::Display` impls was valid, but to avoid a false positive with @icmccorm's automatic verification tool, we're switching to zero initialization there as well. The performance hit should be negligible, and we can always switch back to `MaybeUninit` there in the future if it becomes important. --- dec/src/decimal.rs | 20 ++++++++------------ dec/src/decimal128.rs | 2 +- dec/src/decimal32.rs | 2 +- dec/src/decimal64.rs | 2 +- decnumber-sys/src/lib.rs | 2 +- decnumber-sys/tests/basic.rs | 32 +++++++++++++++++++++++++------- 6 files changed, 37 insertions(+), 23 deletions(-) diff --git a/dec/src/decimal.rs b/dec/src/decimal.rs index a67bf32..5334d27 100644 --- a/dec/src/decimal.rs +++ b/dec/src/decimal.rs @@ -497,14 +497,13 @@ impl Decimal { /// Quantums are considered to match if the numbers have the same exponent, /// are both NaNs, or both infinite. pub fn quantum_matches(&self, rhs: &Decimal) -> bool { - let mut d = MaybeUninit::>::uninit(); - let d = unsafe { + let mut d = Decimal::::zero(); + unsafe { decnumber_sys::decNumberSameQuantum( d.as_mut_ptr() as *mut decnumber_sys::decNumber, self.as_ptr(), rhs.as_ptr(), ); - d.assume_init() }; if d.is_zero() { false @@ -1913,15 +1912,14 @@ impl Context> { rhs: &Decimal, ) -> Option { validate_n(N); - let mut d = MaybeUninit::>::uninit(); - let d = unsafe { + let mut d = Decimal::::zero(); + unsafe { decnumber_sys::decNumberCompare( d.as_mut_ptr() as *mut decnumber_sys::decNumber, lhs.as_ptr(), rhs.as_ptr(), &mut self.inner, - ); - d.assume_init() + ) }; if d.is_nan() { None @@ -2118,16 +2116,14 @@ impl Context> { rhs: &Decimal, ) -> Ordering { validate_n(N); - - let mut d = MaybeUninit::>::uninit(); - let d = unsafe { + let mut d = Decimal::::zero(); + unsafe { decnumber_sys::decNumberCompareTotal( d.as_mut_ptr() as *mut decnumber_sys::decNumber, lhs.as_ptr(), rhs.as_ptr(), &mut self.inner, - ); - d.assume_init() + ) }; debug_assert!(!d.is_special()); if d.is_negative() { diff --git a/dec/src/decimal128.rs b/dec/src/decimal128.rs index ea36b72..98083ea 100644 --- a/dec/src/decimal128.rs +++ b/dec/src/decimal128.rs @@ -443,7 +443,7 @@ impl fmt::Debug for Decimal128 { impl fmt::Display for Decimal128 { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let mut buf = MaybeUninit::<[c_char; decnumber_sys::DECQUAD_String]>::uninit(); + let mut buf = ['\0'; decnumber_sys::DECQUAD_String]; let c_str = unsafe { if f.alternate() { decnumber_sys::decQuadToEngString(&self.inner, buf.as_mut_ptr() as *mut c_char); diff --git a/dec/src/decimal32.rs b/dec/src/decimal32.rs index 49bc8e4..567c4aa 100644 --- a/dec/src/decimal32.rs +++ b/dec/src/decimal32.rs @@ -167,7 +167,7 @@ impl fmt::Debug for Decimal32 { impl fmt::Display for Decimal32 { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let mut buf = MaybeUninit::<[c_char; decnumber_sys::DECDOUBLE_String]>::uninit(); + let mut buf = ['\0'; decnumber_sys::DECDOUBLE_String]; let c_str = unsafe { if f.alternate() { decnumber_sys::decSingleToEngString(&self.inner, buf.as_mut_ptr() as *mut c_char); diff --git a/dec/src/decimal64.rs b/dec/src/decimal64.rs index 1a21b7b..065cc55 100644 --- a/dec/src/decimal64.rs +++ b/dec/src/decimal64.rs @@ -403,7 +403,7 @@ impl fmt::Debug for Decimal64 { impl fmt::Display for Decimal64 { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let mut buf = MaybeUninit::<[c_char; decnumber_sys::DECDOUBLE_String]>::uninit(); + let mut buf = ['\0'; decnumber_sys::DECDOUBLE_String]; let c_str = unsafe { if f.alternate() { decnumber_sys::decDoubleToEngString(&self.inner, buf.as_mut_ptr() as *mut c_char); diff --git a/decnumber-sys/src/lib.rs b/decnumber-sys/src/lib.rs index e2de892..1db188c 100644 --- a/decnumber-sys/src/lib.rs +++ b/decnumber-sys/src/lib.rs @@ -108,7 +108,7 @@ pub const DECSPECIAL: u8 = DECINF | DECNAN | DECSNAN; pub const DECDPUN: usize = 3; #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Copy, Clone, Default)] pub struct decNumber { pub digits: i32, pub exponent: i32, diff --git a/decnumber-sys/tests/basic.rs b/decnumber-sys/tests/basic.rs index aa0598b..cfd1df2 100644 --- a/decnumber-sys/tests/basic.rs +++ b/decnumber-sys/tests/basic.rs @@ -16,16 +16,34 @@ fn simple_addition() { ctx.traps = 0; ctx.digits = 3; - let mut a = MaybeUninit::::uninit(); - let mut b = MaybeUninit::::uninit(); - let (mut a, mut b) = unsafe { - decnumber_sys::decNumberFromString(a.as_mut_ptr(), c_str!("1.23").as_ptr(), &mut ctx); - decnumber_sys::decNumberFromString(b.as_mut_ptr(), c_str!("4.71").as_ptr(), &mut ctx); - (a.assume_init(), b.assume_init()) + let mut a = decNumber { + digits: 1, + exponent: 0, + bits: 0, + lsu: [0; 12], + }; + let mut b = decNumber { + digits: 1, + exponent: 0, + bits: 0, + lsu: [0; 12], }; unsafe { - decnumber_sys::decNumberAdd(&mut a, &mut a, &mut b, &mut ctx); + decnumber_sys::decNumberFromString( + &mut a as *mut decNumber, + c_str!("1.23").as_ptr(), + &mut ctx, + ); + decnumber_sys::decNumberFromString( + &mut b as *mut decNumber, + c_str!("4.71").as_ptr(), + &mut ctx, + ); + } + let a_as_ptr = &mut a as *mut decNumber; + unsafe { + decnumber_sys::decNumberAdd(a_as_ptr, a_as_ptr, &mut b, &mut ctx); } let mut buf = vec![0_u8; 5];