Remove overflow panic from divrem

Includes some remarks in intrinsics.rs,
generated while auditing the interface for remaining UB.
This commit is contained in:
Jubilee Young 2022-02-07 21:24:21 -08:00
parent 4910274686
commit 672bfebfd8
3 changed files with 45 additions and 27 deletions

View File

@ -17,9 +17,15 @@ extern "platform-intrinsic" {
pub(crate) fn simd_mul<T>(x: T, y: T) -> T; pub(crate) fn simd_mul<T>(x: T, y: T) -> T;
/// udiv/sdiv/fdiv /// udiv/sdiv/fdiv
/// ints and uints: {s,u}div incur UB if division by zero occurs.
/// ints: sdiv is UB for int::MIN / -1.
/// floats: fdiv is never UB, but may create NaNs or infinities.
pub(crate) fn simd_div<T>(x: T, y: T) -> T; pub(crate) fn simd_div<T>(x: T, y: T) -> T;
/// urem/srem/frem /// urem/srem/frem
/// ints and uints: {s,u}rem incur UB if division by zero occurs.
/// ints: srem is UB for int::MIN / -1.
/// floats: frem is equivalent to libm::fmod in the "default" floating point environment, sans errno.
pub(crate) fn simd_rem<T>(x: T, y: T) -> T; pub(crate) fn simd_rem<T>(x: T, y: T) -> T;
/// shl /// shl
@ -45,6 +51,9 @@ extern "platform-intrinsic" {
pub(crate) fn simd_as<T, U>(x: T) -> U; pub(crate) fn simd_as<T, U>(x: T) -> U;
/// neg/fneg /// neg/fneg
/// ints: ultimately becomes a call to cg_ssa's BuilderMethods::neg. cg_llvm equates this to `simd_sub(Simd::splat(0), x)`.
/// floats: LLVM's fneg, which changes the floating point sign bit. Some arches have instructions for it.
/// Rust panics for Neg::neg(int::MIN) due to overflow, but it is not UB in LLVM without `nsw`.
pub(crate) fn simd_neg<T>(x: T) -> T; pub(crate) fn simd_neg<T>(x: T) -> T;
/// fabs /// fabs

View File

@ -57,29 +57,40 @@ macro_rules! wrap_bitshift {
}; };
} }
// Division by zero is poison, according to LLVM. /// SAFETY: This macro must only be used to impl Div or Rem and given the matching intrinsic.
// So is dividing the MIN value of a signed integer by -1, /// It guards against LLVM's UB conditions for integer div or rem using masks and selects,
// since that would return MAX + 1. /// thus guaranteeing a Rust value returns instead.
// FIXME: Rust allows <SInt>::MIN / -1, ///
// so we should probably figure out how to make that safe. /// | | LLVM | Rust
/// | :--------------: | :--- | :----------
/// | N {/,%} 0 | UB | panic!()
/// | <$int>::MIN / -1 | UB | <$int>::MIN
/// | <$int>::MIN % -1 | UB | 0
///
macro_rules! int_divrem_guard { macro_rules! int_divrem_guard {
( $lhs:ident, ( $lhs:ident,
$rhs:ident, $rhs:ident,
{ const PANIC_ZERO: &'static str = $zero:literal; { const PANIC_ZERO: &'static str = $zero:literal;
const PANIC_OVERFLOW: &'static str = $overflow:literal;
$simd_call:ident $simd_call:ident
}, },
$int:ident ) => { $int:ident ) => {
if $rhs.lanes_eq(Simd::splat(0)).any() { if $rhs.lanes_eq(Simd::splat(0)).any() {
panic!($zero); panic!($zero);
} else if <$int>::MIN != 0
&& ($lhs.lanes_eq(Simd::splat(<$int>::MIN))
// type inference can break here, so cut an SInt to size
& $rhs.lanes_eq(Simd::splat(-1i64 as _))).any()
{
panic!($overflow);
} else { } else {
unsafe { $crate::simd::intrinsics::$simd_call($lhs, $rhs) } // Prevent otherwise-UB overflow on the MIN / -1 case.
let rhs = if <$int>::MIN != 0 {
// This should, at worst, optimize to a few branchless logical ops
// Ideally, this entire conditional should evaporate
// Fire LLVM and implement those manually if it doesn't get the hint
($lhs.lanes_eq(Simd::splat(<$int>::MIN))
// type inference can break here, so cut an SInt to size
& $rhs.lanes_eq(Simd::splat(-1i64 as _)))
.select(Simd::splat(1), $rhs)
} else {
// Nice base case to make it easy to const-fold away the other branch.
$rhs
};
unsafe { $crate::simd::intrinsics::$simd_call($lhs, rhs) }
} }
}; };
} }
@ -183,7 +194,6 @@ for_base_ops! {
impl Div::div { impl Div::div {
int_divrem_guard { int_divrem_guard {
const PANIC_ZERO: &'static str = "attempt to divide by zero"; const PANIC_ZERO: &'static str = "attempt to divide by zero";
const PANIC_OVERFLOW: &'static str = "attempt to divide with overflow";
simd_div simd_div
} }
} }
@ -191,7 +201,6 @@ for_base_ops! {
impl Rem::rem { impl Rem::rem {
int_divrem_guard { int_divrem_guard {
const PANIC_ZERO: &'static str = "attempt to calculate the remainder with a divisor of zero"; const PANIC_ZERO: &'static str = "attempt to calculate the remainder with a divisor of zero";
const PANIC_OVERFLOW: &'static str = "attempt to calculate the remainder with overflow";
simd_rem simd_rem
} }
} }

View File

@ -210,15 +210,21 @@ macro_rules! impl_signed_tests {
) )
} }
fn div_min_may_overflow<const LANES: usize>() {
let a = Vector::<LANES>::splat(Scalar::MIN);
let b = Vector::<LANES>::splat(-1);
assert_eq!(a / b, a / (b * b));
}
fn rem_min_may_overflow<const LANES: usize>() {
let a = Vector::<LANES>::splat(Scalar::MIN);
let b = Vector::<LANES>::splat(-1);
assert_eq!(a % b, a % (b * b));
}
} }
test_helpers::test_lanes_panic! { test_helpers::test_lanes_panic! {
fn div_min_overflow_panics<const LANES: usize>() {
let a = Vector::<LANES>::splat(Scalar::MIN);
let b = Vector::<LANES>::splat(-1);
let _ = a / b;
}
fn div_by_all_zeros_panics<const LANES: usize>() { fn div_by_all_zeros_panics<const LANES: usize>() {
let a = Vector::<LANES>::splat(42); let a = Vector::<LANES>::splat(42);
let b = Vector::<LANES>::splat(0); let b = Vector::<LANES>::splat(0);
@ -232,12 +238,6 @@ macro_rules! impl_signed_tests {
let _ = a / b; let _ = a / b;
} }
fn rem_min_overflow_panic<const LANES: usize>() {
let a = Vector::<LANES>::splat(Scalar::MIN);
let b = Vector::<LANES>::splat(-1);
let _ = a % b;
}
fn rem_zero_panic<const LANES: usize>() { fn rem_zero_panic<const LANES: usize>() {
let a = Vector::<LANES>::splat(42); let a = Vector::<LANES>::splat(42);
let b = Vector::<LANES>::splat(0); let b = Vector::<LANES>::splat(0);