bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

interpret: make overflowing binops just normal binops

Browse source
This commit is contained in:
Ralf Jung 2024-05-21 12:17:34 +02:00
parent 9cb6bb8599
commit c0b4b454c3
40 changed files with 323 additions and 349 deletions
Download patch file Download diff file Expand all files Collapse all files

31
compiler/rustc_mir_transform/src/dataflow_const_prop.rs
View file

@ -165,9 +165,7 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> {
}
}
}
Rvalue::BinaryOp(overflowing_op, box (left, right))
if let Some(op) = overflowing_op.overflowing_to_wrapping() =>
{
Rvalue::BinaryOp(op, box (left, right)) if op.is_overflowing() => {
// Flood everything now, so we can use `insert_value_idx` directly later.
state.flood(target.as_ref(), self.map());
@ -177,7 +175,7 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> {
let overflow_target = self.map().apply(target, TrackElem::Field(1_u32.into()));
if value_target.is_some() || overflow_target.is_some() {
let (val, overflow) = self.binary_op(state, op, left, right);
let (val, overflow) = self.binary_op(state, *op, left, right);
if let Some(value_target) = value_target {
// We have flooded `target` earlier.
@ -186,7 +184,7 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> {
if let Some(overflow_target) = overflow_target {
let overflow = match overflow {
FlatSet::Top => FlatSet::Top,
FlatSet::Elem(overflow) => FlatSet::Elem(Scalar::from_bool(overflow)),
FlatSet::Elem(overflow) => FlatSet::Elem(overflow),
FlatSet::Bottom => FlatSet::Bottom,
};
// We have flooded `target` earlier.
@ -266,15 +264,16 @@ impl<'tcx> ValueAnalysis<'tcx> for ConstAnalysis<'_, 'tcx> {
FlatSet::Top => FlatSet::Top,
}
}
Rvalue::BinaryOp(op, box (left, right)) => {
Rvalue::BinaryOp(op, box (left, right)) if !op.is_overflowing() => {
// Overflows must be ignored here.
// The overflowing operators are handled in `handle_assign`.
let (val, _overflow) = self.binary_op(state, *op, left, right);
val
}
Rvalue::UnaryOp(op, operand) => match self.eval_operand(operand, state) {
FlatSet::Elem(value) => self
.ecx
.wrapping_unary_op(*op, &value)
.unary_op(*op, &value)
.map_or(FlatSet::Top, |val| self.wrap_immediate(*val)),
FlatSet::Bottom => FlatSet::Bottom,
FlatSet::Top => FlatSet::Top,
@ -439,7 +438,7 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> {
op: BinOp,
left: &Operand<'tcx>,
right: &Operand<'tcx>,
) -> (FlatSet<Scalar>, FlatSet<bool>) {
) -> (FlatSet<Scalar>, FlatSet<Scalar>) {
let left = self.eval_operand(left, state);
let right = self.eval_operand(right, state);
@ -447,9 +446,17 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> {
(FlatSet::Bottom, _) | (_, FlatSet::Bottom) => (FlatSet::Bottom, FlatSet::Bottom),
// Both sides are known, do the actual computation.
(FlatSet::Elem(left), FlatSet::Elem(right)) => {
match self.ecx.overflowing_binary_op(op, &left, &right) {
Ok((val, overflow)) => {
(FlatSet::Elem(val.to_scalar()), FlatSet::Elem(overflow))
match self.ecx.binary_op(op, &left, &right) {
// Ideally this would return an Immediate, since it's sometimes
// a pair and sometimes not. But as a hack we always return a pair
// and just make the 2nd component `Bottom` when it does not exist.
Ok(val) => {
if matches!(val.layout.abi, Abi::ScalarPair(..)) {
let (val, overflow) = val.to_scalar_pair();
(FlatSet::Elem(val), FlatSet::Elem(overflow))
} else {
(FlatSet::Elem(val.to_scalar()), FlatSet::Bottom)
}
}
_ => (FlatSet::Top, FlatSet::Top),
}
@ -475,7 +482,7 @@ impl<'a, 'tcx> ConstAnalysis<'a, 'tcx> {
(FlatSet::Elem(arg_scalar), FlatSet::Bottom)
}
BinOp::Mul if layout.ty.is_integral() && arg_value == 0 => {
(FlatSet::Elem(arg_scalar), FlatSet::Elem(false))
(FlatSet::Elem(arg_scalar), FlatSet::Elem(Scalar::from_bool(false)))
}
_ => (FlatSet::Top, FlatSet::Top),
}

19
compiler/rustc_mir_transform/src/gvn.rs
View file

@ -223,7 +223,7 @@ enum Value<'tcx> {
NullaryOp(NullOp<'tcx>, Ty<'tcx>),
UnaryOp(UnOp, VnIndex),
BinaryOp(BinOp, VnIndex, VnIndex),
CheckedBinaryOp(BinOp, VnIndex, VnIndex),
CheckedBinaryOp(BinOp, VnIndex, VnIndex), // FIXME get rid of this, work like MIR instead
Cast {
kind: CastKind,
value: VnIndex,
@ -497,7 +497,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
UnaryOp(un_op, operand) => {
let operand = self.evaluated[operand].as_ref()?;
let operand = self.ecx.read_immediate(operand).ok()?;
let (val, _) = self.ecx.overflowing_unary_op(un_op, &operand).ok()?;
let val = self.ecx.unary_op(un_op, &operand).ok()?;
val.into()
}
BinaryOp(bin_op, lhs, rhs) => {
@ -505,7 +505,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
let lhs = self.ecx.read_immediate(lhs).ok()?;
let rhs = self.evaluated[rhs].as_ref()?;
let rhs = self.ecx.read_immediate(rhs).ok()?;
let (val, _) = self.ecx.overflowing_binary_op(bin_op, &lhs, &rhs).ok()?;
let val = self.ecx.binary_op(bin_op, &lhs, &rhs).ok()?;
val.into()
}
CheckedBinaryOp(bin_op, lhs, rhs) => {
@ -513,14 +513,11 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
let lhs = self.ecx.read_immediate(lhs).ok()?;
let rhs = self.evaluated[rhs].as_ref()?;
let rhs = self.ecx.read_immediate(rhs).ok()?;
let (val, overflowed) = self.ecx.overflowing_binary_op(bin_op, &lhs, &rhs).ok()?;
let tuple = Ty::new_tup_from_iter(
self.tcx,
[val.layout.ty, self.tcx.types.bool].into_iter(),
);
let tuple = self.ecx.layout_of(tuple).ok()?;
ImmTy::from_scalar_pair(val.to_scalar(), Scalar::from_bool(overflowed), tuple)
.into()
let val = self
.ecx
.binary_op(bin_op.wrapping_to_overflowing().unwrap(), &lhs, &rhs)
.ok()?;
val.into()
}
Cast { kind, value, from: _, to } => match kind {
CastKind::IntToInt | CastKind::IntToFloat => {

67
compiler/rustc_mir_transform/src/known_panics_lint.rs
View file

@ -304,20 +304,25 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
fn check_unary_op(&mut self, op: UnOp, arg: &Operand<'tcx>, location: Location) -> Option<()> {
let arg = self.eval_operand(arg)?;
if let (val, true) = self.use_ecx(|this| {
let val = this.ecx.read_immediate(&arg)?;
let (_res, overflow) = this.ecx.overflowing_unary_op(op, &val)?;
Ok((val, overflow))
})? {
// `AssertKind` only has an `OverflowNeg` variant, so make sure that is
// appropriate to use.
assert_eq!(op, UnOp::Neg, "Neg is the only UnOp that can overflow");
self.report_assert_as_lint(
location,
AssertLintKind::ArithmeticOverflow,
AssertKind::OverflowNeg(val.to_const_int()),
);
return None;
// The only operator that can overflow is `Neg`.
if op == UnOp::Neg && arg.layout.ty.is_integral() {
// Compute this as `0 - arg` so we can use `SubWithOverflow` to check for overflow.
let (arg, overflow) = self.use_ecx(|this| {
let arg = this.ecx.read_immediate(&arg)?;
let (_res, overflow) = this
.ecx
.binary_op(BinOp::SubWithOverflow, &ImmTy::from_int(0, arg.layout), &arg)?
.to_scalar_pair();
Ok((arg, overflow.to_bool()?))
})?;
if overflow {
self.report_assert_as_lint(
location,
AssertLintKind::ArithmeticOverflow,
AssertKind::OverflowNeg(arg.to_const_int()),
);
return None;
}
}
Some(())
@ -363,11 +368,20 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
}
}
if let (Some(l), Some(r)) = (l, r) {
// The remaining operators are handled through `overflowing_binary_op`.
// Div/Rem are handled via the assertions they trigger.
// But for Add/Sub/Mul, those assertions only exist in debug builds, and we want to
// lint in release builds as well, so we check on the operation instead.
// So normalize to the "overflowing" operator, and then ensure that it
// actually is an overflowing operator.
let op = op.wrapping_to_overflowing().unwrap_or(op);
// The remaining operators are handled through `wrapping_to_overflowing`.
if let (Some(l), Some(r)) = (l, r)
&& l.layout.ty.is_integral()
&& op.is_overflowing()
{
if self.use_ecx(|this| {
let (_res, overflow) = this.ecx.overflowing_binary_op(op, &l, &r)?;
Ok(overflow)
let (_res, overflow) = this.ecx.binary_op(op, &l, &r)?.to_scalar_pair();
overflow.to_bool()
})? {
self.report_assert_as_lint(
location,
@ -399,8 +413,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
}
Rvalue::BinaryOp(op, box (left, right)) => {
trace!("checking BinaryOp(op = {:?}, left = {:?}, right = {:?})", op, left, right);
let op = op.overflowing_to_wrapping().unwrap_or(*op);
self.check_binary_op(op, left, right, location)?;
self.check_binary_op(*op, left, right, location)?;
}
// Do not try creating references (#67862)
@ -547,17 +560,15 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
let right = self.eval_operand(right)?;
let right = self.use_ecx(|this| this.ecx.read_immediate(&right))?;
if let Some(bin_op) = bin_op.overflowing_to_wrapping() {
let (val, overflowed) =
self.use_ecx(|this| this.ecx.overflowing_binary_op(bin_op, &left, &right))?;
let overflowed = ImmTy::from_bool(overflowed, self.tcx);
let val = self.use_ecx(|this| this.ecx.binary_op(bin_op, &left, &right))?;
if matches!(val.layout.abi, Abi::ScalarPair(..)) {
// FIXME `Value` should properly support pairs in `Immediate`... but currently it does not.
let (val, overflow) = val.to_pair(&self.ecx);
Value::Aggregate {
variant: VariantIdx::ZERO,
fields: [Value::from(val), overflowed.into()].into_iter().collect(),
fields: [val.into(), overflow.into()].into_iter().collect(),
}
} else {
let val =
self.use_ecx(|this| this.ecx.wrapping_binary_op(bin_op, &left, &right))?;
val.into()
}
}
@ -566,7 +577,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
let operand = self.eval_operand(operand)?;
let val = self.use_ecx(|this| this.ecx.read_immediate(&operand))?;
let val = self.use_ecx(|this| this.ecx.wrapping_unary_op(un_op, &val))?;
let val = self.use_ecx(|this| this.ecx.unary_op(un_op, &val))?;
val.into()
}