Skip to content

Commit

Permalink
Auto merge of #67676 - wesleywiser:lint_overflowing_int_casts, r=oli-obk
Browse files Browse the repository at this point in the history
Lint overflowing integer casts in const prop

This extends the invalid cases we catch in const prop to include
overflowing integer casts using the same machinery as the overflowing
binary and unary operation logic.

r? @oli-obk
  • Loading branch information
bors committed Jan 1, 2020
2 parents 1ed41b0 + e8c1c4c commit 0ec3706
Show file tree
Hide file tree
Showing 6 changed files with 219 additions and 63 deletions.
198 changes: 135 additions & 63 deletions src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ use rustc::mir::visit::{
MutVisitor, MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor,
};
use rustc::mir::{
read_only, AggregateKind, BasicBlock, BinOp, Body, BodyAndCache, ClearCrossCrate, Constant,
Local, LocalDecl, LocalKind, Location, Operand, Place, PlaceBase, ReadOnlyBodyAndCache, Rvalue,
SourceInfo, SourceScope, SourceScopeData, Statement, StatementKind, Terminator, TerminatorKind,
UnOp, RETURN_PLACE,
read_only, AggregateKind, BasicBlock, BinOp, Body, BodyAndCache, CastKind, ClearCrossCrate,
Constant, Local, LocalDecl, LocalKind, Location, Operand, Place, PlaceBase,
ReadOnlyBodyAndCache, Rvalue, SourceInfo, SourceScope, SourceScopeData, Statement,
StatementKind, Terminator, TerminatorKind, UnOp, RETURN_PLACE,
};
use rustc::ty::layout::{
HasDataLayout, HasTyCtxt, LayoutError, LayoutOf, Size, TargetDataLayout, TyLayout,
Expand All @@ -29,9 +29,9 @@ use syntax::ast::Mutability;

use crate::const_eval::error_to_const_error;
use crate::interpret::{
self, intern_const_alloc_recursive, AllocId, Allocation, Frame, ImmTy, Immediate, InterpCx,
LocalState, LocalValue, Memory, MemoryKind, OpTy, Operand as InterpOperand, PlaceTy, Pointer,
ScalarMaybeUndef, StackPopCleanup,
self, intern_const_alloc_recursive, truncate, AllocId, Allocation, Frame, ImmTy, Immediate,
InterpCx, LocalState, LocalValue, Memory, MemoryKind, OpTy, Operand as InterpOperand, PlaceTy,
Pointer, ScalarMaybeUndef, StackPopCleanup,
};
use crate::rustc::ty::subst::Subst;
use crate::transform::{MirPass, MirSource};
Expand Down Expand Up @@ -469,15 +469,134 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
}
}

fn check_unary_op(&mut self, arg: &Operand<'tcx>, source_info: SourceInfo) -> Option<()> {
self.use_ecx(source_info, |this| {
let ty = arg.ty(&this.local_decls, this.tcx);

if ty.is_integral() {
let arg = this.ecx.eval_operand(arg, None)?;
let prim = this.ecx.read_immediate(arg)?;
// Need to do overflow check here: For actual CTFE, MIR
// generation emits code that does this before calling the op.
if prim.to_bits()? == (1 << (prim.layout.size.bits() - 1)) {
throw_panic!(OverflowNeg)
}
}

Ok(())
})?;

Some(())
}

fn check_binary_op(
&mut self,
op: BinOp,
left: &Operand<'tcx>,
right: &Operand<'tcx>,
source_info: SourceInfo,
place_layout: TyLayout<'tcx>,
overflow_check: bool,
) -> Option<()> {
let r = self.use_ecx(source_info, |this| {
this.ecx.read_immediate(this.ecx.eval_operand(right, None)?)
})?;
if op == BinOp::Shr || op == BinOp::Shl {
let left_bits = place_layout.size.bits();
let right_size = r.layout.size;
let r_bits = r.to_scalar().and_then(|r| r.to_bits(right_size));
if r_bits.map_or(false, |b| b >= left_bits as u128) {
let lint_root = self.lint_root(source_info)?;
let dir = if op == BinOp::Shr { "right" } else { "left" };
self.tcx.lint_hir(
::rustc::lint::builtin::EXCEEDING_BITSHIFTS,
lint_root,
source_info.span,
&format!("attempt to shift {} with overflow", dir),
);
return None;
}
}

// If overflow checking is enabled (like in debug mode by default),
// then we'll already catch overflow when we evaluate the `Assert` statement
// in MIR. However, if overflow checking is disabled, then there won't be any
// `Assert` statement and so we have to do additional checking here.
if !overflow_check {
self.use_ecx(source_info, |this| {
let l = this.ecx.read_immediate(this.ecx.eval_operand(left, None)?)?;
let (_, overflow, _ty) = this.ecx.overflowing_binary_op(op, l, r)?;

if overflow {
let err = err_panic!(Overflow(op)).into();
return Err(err);
}

Ok(())
})?;
}

Some(())
}

fn check_cast(
&mut self,
op: &Operand<'tcx>,
ty: Ty<'tcx>,
source_info: SourceInfo,
place_layout: TyLayout<'tcx>,
) -> Option<()> {
if !ty.is_integral() || !op.ty(&self.local_decls, self.tcx).is_integral() {
return Some(());
}

let value = self.use_ecx(source_info, |this| {
this.ecx.read_immediate(this.ecx.eval_operand(op, None)?)
})?;

// Do not try to read bits for ZSTs. This can occur when casting an enum with one variant
// to an integer. Such enums are represented as ZSTs but still have a discriminant value
// which can be casted.
if value.layout.is_zst() {
return Some(());
}

let value_size = value.layout.size;
let value_bits = value.to_scalar().and_then(|r| r.to_bits(value_size));
if let Ok(value_bits) = value_bits {
let truncated = truncate(value_bits, place_layout.size);
if truncated != value_bits {
let scope = source_info.scope;
let lint_root = match &self.source_scopes[scope].local_data {
ClearCrossCrate::Set(data) => data.lint_root,
ClearCrossCrate::Clear => return None,
};
self.tcx.lint_hir(
::rustc::lint::builtin::CONST_ERR,
lint_root,
source_info.span,
&format!(
"truncating cast: the value {} requires {} bits but the target type is \
only {} bits",
value_bits,
value_size.bits(),
place_layout.size.bits()
),
);
return None;
}
}

Some(())
}

fn const_prop(
&mut self,
rvalue: &Rvalue<'tcx>,
place_layout: TyLayout<'tcx>,
source_info: SourceInfo,
place: &Place<'tcx>,
) -> Option<()> {
let span = source_info.span;

// #66397: Don't try to eval into large places as that can cause an OOM
if place_layout.size >= Size::from_bytes(MAX_ALLOC_LIMIT) {
return None;
Expand All @@ -498,66 +617,14 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
// if an overflow would occur.
Rvalue::UnaryOp(UnOp::Neg, arg) if !overflow_check => {
trace!("checking UnaryOp(op = Neg, arg = {:?})", arg);

self.use_ecx(source_info, |this| {
let ty = arg.ty(&this.local_decls, this.tcx);

if ty.is_integral() {
let arg = this.ecx.eval_operand(arg, None)?;
let prim = this.ecx.read_immediate(arg)?;
// Need to do overflow check here: For actual CTFE, MIR
// generation emits code that does this before calling the op.
if prim.to_bits()? == (1 << (prim.layout.size.bits() - 1)) {
throw_panic!(OverflowNeg)
}
}

Ok(())
})?;
self.check_unary_op(arg, source_info)?;
}

// Additional checking: check for overflows on integer binary operations and report
// them to the user as lints.
Rvalue::BinaryOp(op, left, right) => {
trace!("checking BinaryOp(op = {:?}, left = {:?}, right = {:?})", op, left, right);

let r = self.use_ecx(source_info, |this| {
this.ecx.read_immediate(this.ecx.eval_operand(right, None)?)
})?;
if *op == BinOp::Shr || *op == BinOp::Shl {
let left_bits = place_layout.size.bits();
let right_size = r.layout.size;
let r_bits = r.to_scalar().and_then(|r| r.to_bits(right_size));
if r_bits.map_or(false, |b| b >= left_bits as u128) {
let lint_root = self.lint_root(source_info)?;
let dir = if *op == BinOp::Shr { "right" } else { "left" };
self.tcx.lint_hir(
::rustc::lint::builtin::EXCEEDING_BITSHIFTS,
lint_root,
span,
&format!("attempt to shift {} with overflow", dir),
);
return None;
}
}

// If overflow checking is enabled (like in debug mode by default),
// then we'll already catch overflow when we evaluate the `Assert` statement
// in MIR. However, if overflow checking is disabled, then there won't be any
// `Assert` statement and so we have to do additional checking here.
if !overflow_check {
self.use_ecx(source_info, |this| {
let l = this.ecx.read_immediate(this.ecx.eval_operand(left, None)?)?;
let (_, overflow, _ty) = this.ecx.overflowing_binary_op(*op, l, r)?;

if overflow {
let err = err_panic!(Overflow(*op)).into();
return Err(err);
}

Ok(())
})?;
}
self.check_binary_op(*op, left, right, source_info, place_layout, overflow_check)?;
}

// Work around: avoid ICE in miri. FIXME(wesleywiser)
Expand All @@ -584,6 +651,11 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
}
}

Rvalue::Cast(CastKind::Misc, op, ty) => {
trace!("checking Cast(Misc, {:?}, {:?})", op, ty);
self.check_cast(op, ty, source_info, place_layout)?;
}

_ => {}
}

Expand Down
49 changes: 49 additions & 0 deletions src/test/mir-opt/const_prop/cast.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
fn main() {
let x = 42u8 as u32;

let y = 42u32 as u8;
}

// END RUST SOURCE
// START rustc.main.ConstProp.before.mir
// let mut _0: ();
// let _1: u32;
// scope 1 {
// debug x => _1;
// let _2: u8;
// scope 2 {
// debug y => _2;
// }
// }
// bb0: {
// StorageLive(_1);
// _1 = const 42u8 as u32 (Misc);
// StorageLive(_2);
// _2 = const 42u32 as u8 (Misc);
// _0 = ();
// StorageDead(_2);
// StorageDead(_1);
// return;
// }
// END rustc.main.ConstProp.before.mir
// START rustc.main.ConstProp.after.mir
// let mut _0: ();
// let _1: u32;
// scope 1 {
// debug x => _1;
// let _2: u8;
// scope 2 {
// debug y => _2;
// }
// }
// bb0: {
// StorageLive(_1);
// _1 = const 42u32;
// StorageLive(_2);
// _2 = const 42u8;
// _0 = ();
// StorageDead(_2);
// StorageDead(_1);
// return;
// }
// END rustc.main.ConstProp.after.mir
9 changes: 9 additions & 0 deletions src/test/ui/consts/const-prop-overflowing-casts.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// build-fail
// ignore-tidy-linelength

fn main() {
let _ = 0u8 as u32;
let _ = (1u32 << 31) as u16; //~ ERROR truncating cast: the value 2147483648 requires 32 bits but the target type is only 16 bits
let _ = (1u16 << 15) as u8; //~ ERROR truncating cast: the value 32768 requires 16 bits but the target type is only 8 bits
let _ = (!0u16) as u8; //~ ERROR truncating cast: the value 65535 requires 16 bits but the target type is only 8 bits
}
22 changes: 22 additions & 0 deletions src/test/ui/consts/const-prop-overflowing-casts.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error: truncating cast: the value 2147483648 requires 32 bits but the target type is only 16 bits
--> $DIR/const-prop-overflowing-casts.rs:6:13
|
LL | let _ = (1u32 << 31) as u16;
| ^^^^^^^^^^^^^^^^^^^
|
= note: `#[deny(const_err)]` on by default

error: truncating cast: the value 32768 requires 16 bits but the target type is only 8 bits
--> $DIR/const-prop-overflowing-casts.rs:7:13
|
LL | let _ = (1u16 << 15) as u8;
| ^^^^^^^^^^^^^^^^^^

error: truncating cast: the value 65535 requires 16 bits but the target type is only 8 bits
--> $DIR/const-prop-overflowing-casts.rs:8:13
|
LL | let _ = (!0u16) as u8;
| ^^^^^^^^^^^^^

error: aborting due to 3 previous errors

1 change: 1 addition & 0 deletions src/test/ui/simd/simd-intrinsic-generic-cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#![feature(repr_simd, platform_intrinsics, concat_idents, test)]
#![allow(non_camel_case_types)]
#![allow(const_err)] // the test macro casts i32s to i8 and u8 which causes lots of warnings

extern crate test;

Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/wrapping-int-api.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// run-pass
// Test inherent wrapping_* methods for {i,u}{size,8,16,32,64}.

// Don't warn about overflowing ops on 32-bit platforms
#![cfg_attr(target_pointer_width = "32", allow(const_err))]

use std::{i8, i16, i32, i64, isize};
use std::{u8, u16, u32, u64, usize};

Expand Down

0 comments on commit 0ec3706

Please sign in to comment.