From e083273ec77b96a17f317d83fa3fed8ab205964a Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 12 Dec 2019 22:03:32 -0500 Subject: [PATCH 1/5] 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. --- src/librustc_mir/transform/const_prop.rs | 53 ++++++++++++++++--- src/test/mir-opt/const_prop/cast.rs | 49 +++++++++++++++++ .../ui/consts/const-prop-overflowing-casts.rs | 9 ++++ .../const-prop-overflowing-casts.stderr | 22 ++++++++ .../ui/simd/simd-intrinsic-generic-cast.rs | 1 + 5 files changed, 127 insertions(+), 7 deletions(-) create mode 100644 src/test/mir-opt/const_prop/cast.rs create mode 100644 src/test/ui/consts/const-prop-overflowing-casts.rs create mode 100644 src/test/ui/consts/const-prop-overflowing-casts.stderr diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index c36f793511553..8de85aab6e380 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -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, @@ -29,9 +29,9 @@ use syntax_pos::{Span, DUMMY_SP}; 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}; @@ -584,6 +584,45 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } } + Rvalue::Cast(CastKind::Misc, op, ty) => { + trace!("checking Cast(Misc, {:?}, {:?})", op, ty); + + if ty.is_integral() && op.ty(&self.local_decls, self.tcx).is_integral() { + 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 + if !value.layout.is_zst() { + 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, + 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; + } + } + } + } + } + _ => {} } diff --git a/src/test/mir-opt/const_prop/cast.rs b/src/test/mir-opt/const_prop/cast.rs new file mode 100644 index 0000000000000..9cfbfebdcc3df --- /dev/null +++ b/src/test/mir-opt/const_prop/cast.rs @@ -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 diff --git a/src/test/ui/consts/const-prop-overflowing-casts.rs b/src/test/ui/consts/const-prop-overflowing-casts.rs new file mode 100644 index 0000000000000..11a04611487ba --- /dev/null +++ b/src/test/ui/consts/const-prop-overflowing-casts.rs @@ -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 +} diff --git a/src/test/ui/consts/const-prop-overflowing-casts.stderr b/src/test/ui/consts/const-prop-overflowing-casts.stderr new file mode 100644 index 0000000000000..af4e2c7005afb --- /dev/null +++ b/src/test/ui/consts/const-prop-overflowing-casts.stderr @@ -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 + diff --git a/src/test/ui/simd/simd-intrinsic-generic-cast.rs b/src/test/ui/simd/simd-intrinsic-generic-cast.rs index 15f232e2c0f70..b81a76851d3c3 100644 --- a/src/test/ui/simd/simd-intrinsic-generic-cast.rs +++ b/src/test/ui/simd/simd-intrinsic-generic-cast.rs @@ -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; From 397a2fd744c489b3ad656fc4ca5767593fd8a7eb Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Sat, 28 Dec 2019 16:43:39 -0500 Subject: [PATCH 2/5] [const-prop] Extract some functions out of `_const_prop` --- src/librustc_mir/transform/const_prop.rs | 209 +++++++++++++---------- 1 file changed, 118 insertions(+), 91 deletions(-) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 8de85aab6e380..2ba18359e02cd 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -469,6 +469,121 @@ 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() { + 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 + if !value.layout.is_zst() { + 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>, @@ -476,8 +591,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, '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; @@ -498,66 +611,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) @@ -586,41 +647,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { Rvalue::Cast(CastKind::Misc, op, ty) => { trace!("checking Cast(Misc, {:?}, {:?})", op, ty); - - if ty.is_integral() && op.ty(&self.local_decls, self.tcx).is_integral() { - 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 - if !value.layout.is_zst() { - 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, - 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; - } - } - } - } + self.check_cast(op, ty, source_info, place_layout)?; } _ => {} From 18869853809b022346c00f46f4f1ce0726c67499 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Sat, 28 Dec 2019 16:51:13 -0500 Subject: [PATCH 3/5] [const-prop] Clean up `check_cast()` a bit --- src/librustc_mir/transform/const_prop.rs | 66 +++++++++++++----------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 2ba18359e02cd..9f474d97afb12 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -546,38 +546,42 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { source_info: SourceInfo, place_layout: TyLayout<'tcx>, ) -> Option<()> { - if ty.is_integral() && op.ty(&self.local_decls, self.tcx).is_integral() { - let value = self.use_ecx(source_info, |this| { - this.ecx.read_immediate(this.ecx.eval_operand(op, None)?) - })?; + if !ty.is_integral() || !op.ty(&self.local_decls, self.tcx).is_integral() { + return Some(()); + } - // Do not try to read bits for ZSTs - if !value.layout.is_zst() { - 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; - } - } + 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 + 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; } } From 001cea4ac46d05ccfea8b4239ed2730edd724a7f Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Sun, 29 Dec 2019 10:34:09 -0500 Subject: [PATCH 4/5] [const-prop] Expand comment about casting ZST enums --- src/librustc_mir/transform/const_prop.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 9f474d97afb12..723ca79fa99ae 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -554,7 +554,9 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { this.ecx.read_immediate(this.ecx.eval_operand(op, None)?) })?; - // Do not try to read bits for ZSTs + // 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(()); } From e8c1c4cd5bc0d370f59d0399ae2458e872a51622 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Wed, 1 Jan 2020 12:30:17 -0500 Subject: [PATCH 5/5] Ignore overflow lint on 32-bit platform --- src/test/ui/wrapping-int-api.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/ui/wrapping-int-api.rs b/src/test/ui/wrapping-int-api.rs index 2a5baad8b786f..ecdd742fb4e08 100644 --- a/src/test/ui/wrapping-int-api.rs +++ b/src/test/ui/wrapping-int-api.rs @@ -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};