Skip to content

Commit

Permalink
avoid MaybeUninit
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
icmccorm authored and benesch committed Nov 11, 2023
1 parent ece7d84 commit 5c866d6
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 23 deletions.
20 changes: 8 additions & 12 deletions dec/src/decimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,14 +497,13 @@ impl<const N: usize> Decimal<N> {
/// 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<N>) -> bool {
let mut d = MaybeUninit::<Decimal<N>>::uninit();
let d = unsafe {
let mut d = Decimal::<N>::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
Expand Down Expand Up @@ -1913,15 +1912,14 @@ impl<const N: usize> Context<Decimal<N>> {
rhs: &Decimal<M>,
) -> Option<Ordering> {
validate_n(N);
let mut d = MaybeUninit::<Decimal<N>>::uninit();
let d = unsafe {
let mut d = Decimal::<N>::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
Expand Down Expand Up @@ -2118,16 +2116,14 @@ impl<const N: usize> Context<Decimal<N>> {
rhs: &Decimal<M>,
) -> Ordering {
validate_n(N);

let mut d = MaybeUninit::<Decimal<N>>::uninit();
let d = unsafe {
let mut d = Decimal::<N>::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() {
Expand Down
2 changes: 1 addition & 1 deletion dec/src/decimal128.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion dec/src/decimal32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion dec/src/decimal64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion decnumber-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
32 changes: 25 additions & 7 deletions decnumber-sys/tests/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,34 @@ fn simple_addition() {
ctx.traps = 0;
ctx.digits = 3;

let mut a = MaybeUninit::<decNumber>::uninit();
let mut b = MaybeUninit::<decNumber>::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];
Expand Down

0 comments on commit 5c866d6

Please sign in to comment.