Skip to content

Commit 0ec3706

Browse files
committed
Auto merge of #67676 - wesleywiser:lint_overflowing_int_casts, r=oli-obk
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
2 parents 1ed41b0 + e8c1c4c commit 0ec3706

File tree

6 files changed

+219
-63
lines changed

6 files changed

+219
-63
lines changed

src/librustc_mir/transform/const_prop.rs

+135-63
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ use rustc::mir::visit::{
1212
MutVisitor, MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor,
1313
};
1414
use rustc::mir::{
15-
read_only, AggregateKind, BasicBlock, BinOp, Body, BodyAndCache, ClearCrossCrate, Constant,
16-
Local, LocalDecl, LocalKind, Location, Operand, Place, PlaceBase, ReadOnlyBodyAndCache, Rvalue,
17-
SourceInfo, SourceScope, SourceScopeData, Statement, StatementKind, Terminator, TerminatorKind,
18-
UnOp, RETURN_PLACE,
15+
read_only, AggregateKind, BasicBlock, BinOp, Body, BodyAndCache, CastKind, ClearCrossCrate,
16+
Constant, Local, LocalDecl, LocalKind, Location, Operand, Place, PlaceBase,
17+
ReadOnlyBodyAndCache, Rvalue, SourceInfo, SourceScope, SourceScopeData, Statement,
18+
StatementKind, Terminator, TerminatorKind, UnOp, RETURN_PLACE,
1919
};
2020
use rustc::ty::layout::{
2121
HasDataLayout, HasTyCtxt, LayoutError, LayoutOf, Size, TargetDataLayout, TyLayout,
@@ -29,9 +29,9 @@ use syntax::ast::Mutability;
2929

3030
use crate::const_eval::error_to_const_error;
3131
use crate::interpret::{
32-
self, intern_const_alloc_recursive, AllocId, Allocation, Frame, ImmTy, Immediate, InterpCx,
33-
LocalState, LocalValue, Memory, MemoryKind, OpTy, Operand as InterpOperand, PlaceTy, Pointer,
34-
ScalarMaybeUndef, StackPopCleanup,
32+
self, intern_const_alloc_recursive, truncate, AllocId, Allocation, Frame, ImmTy, Immediate,
33+
InterpCx, LocalState, LocalValue, Memory, MemoryKind, OpTy, Operand as InterpOperand, PlaceTy,
34+
Pointer, ScalarMaybeUndef, StackPopCleanup,
3535
};
3636
use crate::rustc::ty::subst::Subst;
3737
use crate::transform::{MirPass, MirSource};
@@ -469,15 +469,134 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
469469
}
470470
}
471471

472+
fn check_unary_op(&mut self, arg: &Operand<'tcx>, source_info: SourceInfo) -> Option<()> {
473+
self.use_ecx(source_info, |this| {
474+
let ty = arg.ty(&this.local_decls, this.tcx);
475+
476+
if ty.is_integral() {
477+
let arg = this.ecx.eval_operand(arg, None)?;
478+
let prim = this.ecx.read_immediate(arg)?;
479+
// Need to do overflow check here: For actual CTFE, MIR
480+
// generation emits code that does this before calling the op.
481+
if prim.to_bits()? == (1 << (prim.layout.size.bits() - 1)) {
482+
throw_panic!(OverflowNeg)
483+
}
484+
}
485+
486+
Ok(())
487+
})?;
488+
489+
Some(())
490+
}
491+
492+
fn check_binary_op(
493+
&mut self,
494+
op: BinOp,
495+
left: &Operand<'tcx>,
496+
right: &Operand<'tcx>,
497+
source_info: SourceInfo,
498+
place_layout: TyLayout<'tcx>,
499+
overflow_check: bool,
500+
) -> Option<()> {
501+
let r = self.use_ecx(source_info, |this| {
502+
this.ecx.read_immediate(this.ecx.eval_operand(right, None)?)
503+
})?;
504+
if op == BinOp::Shr || op == BinOp::Shl {
505+
let left_bits = place_layout.size.bits();
506+
let right_size = r.layout.size;
507+
let r_bits = r.to_scalar().and_then(|r| r.to_bits(right_size));
508+
if r_bits.map_or(false, |b| b >= left_bits as u128) {
509+
let lint_root = self.lint_root(source_info)?;
510+
let dir = if op == BinOp::Shr { "right" } else { "left" };
511+
self.tcx.lint_hir(
512+
::rustc::lint::builtin::EXCEEDING_BITSHIFTS,
513+
lint_root,
514+
source_info.span,
515+
&format!("attempt to shift {} with overflow", dir),
516+
);
517+
return None;
518+
}
519+
}
520+
521+
// If overflow checking is enabled (like in debug mode by default),
522+
// then we'll already catch overflow when we evaluate the `Assert` statement
523+
// in MIR. However, if overflow checking is disabled, then there won't be any
524+
// `Assert` statement and so we have to do additional checking here.
525+
if !overflow_check {
526+
self.use_ecx(source_info, |this| {
527+
let l = this.ecx.read_immediate(this.ecx.eval_operand(left, None)?)?;
528+
let (_, overflow, _ty) = this.ecx.overflowing_binary_op(op, l, r)?;
529+
530+
if overflow {
531+
let err = err_panic!(Overflow(op)).into();
532+
return Err(err);
533+
}
534+
535+
Ok(())
536+
})?;
537+
}
538+
539+
Some(())
540+
}
541+
542+
fn check_cast(
543+
&mut self,
544+
op: &Operand<'tcx>,
545+
ty: Ty<'tcx>,
546+
source_info: SourceInfo,
547+
place_layout: TyLayout<'tcx>,
548+
) -> Option<()> {
549+
if !ty.is_integral() || !op.ty(&self.local_decls, self.tcx).is_integral() {
550+
return Some(());
551+
}
552+
553+
let value = self.use_ecx(source_info, |this| {
554+
this.ecx.read_immediate(this.ecx.eval_operand(op, None)?)
555+
})?;
556+
557+
// Do not try to read bits for ZSTs. This can occur when casting an enum with one variant
558+
// to an integer. Such enums are represented as ZSTs but still have a discriminant value
559+
// which can be casted.
560+
if value.layout.is_zst() {
561+
return Some(());
562+
}
563+
564+
let value_size = value.layout.size;
565+
let value_bits = value.to_scalar().and_then(|r| r.to_bits(value_size));
566+
if let Ok(value_bits) = value_bits {
567+
let truncated = truncate(value_bits, place_layout.size);
568+
if truncated != value_bits {
569+
let scope = source_info.scope;
570+
let lint_root = match &self.source_scopes[scope].local_data {
571+
ClearCrossCrate::Set(data) => data.lint_root,
572+
ClearCrossCrate::Clear => return None,
573+
};
574+
self.tcx.lint_hir(
575+
::rustc::lint::builtin::CONST_ERR,
576+
lint_root,
577+
source_info.span,
578+
&format!(
579+
"truncating cast: the value {} requires {} bits but the target type is \
580+
only {} bits",
581+
value_bits,
582+
value_size.bits(),
583+
place_layout.size.bits()
584+
),
585+
);
586+
return None;
587+
}
588+
}
589+
590+
Some(())
591+
}
592+
472593
fn const_prop(
473594
&mut self,
474595
rvalue: &Rvalue<'tcx>,
475596
place_layout: TyLayout<'tcx>,
476597
source_info: SourceInfo,
477598
place: &Place<'tcx>,
478599
) -> Option<()> {
479-
let span = source_info.span;
480-
481600
// #66397: Don't try to eval into large places as that can cause an OOM
482601
if place_layout.size >= Size::from_bytes(MAX_ALLOC_LIMIT) {
483602
return None;
@@ -498,66 +617,14 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
498617
// if an overflow would occur.
499618
Rvalue::UnaryOp(UnOp::Neg, arg) if !overflow_check => {
500619
trace!("checking UnaryOp(op = Neg, arg = {:?})", arg);
501-
502-
self.use_ecx(source_info, |this| {
503-
let ty = arg.ty(&this.local_decls, this.tcx);
504-
505-
if ty.is_integral() {
506-
let arg = this.ecx.eval_operand(arg, None)?;
507-
let prim = this.ecx.read_immediate(arg)?;
508-
// Need to do overflow check here: For actual CTFE, MIR
509-
// generation emits code that does this before calling the op.
510-
if prim.to_bits()? == (1 << (prim.layout.size.bits() - 1)) {
511-
throw_panic!(OverflowNeg)
512-
}
513-
}
514-
515-
Ok(())
516-
})?;
620+
self.check_unary_op(arg, source_info)?;
517621
}
518622

519623
// Additional checking: check for overflows on integer binary operations and report
520624
// them to the user as lints.
521625
Rvalue::BinaryOp(op, left, right) => {
522626
trace!("checking BinaryOp(op = {:?}, left = {:?}, right = {:?})", op, left, right);
523-
524-
let r = self.use_ecx(source_info, |this| {
525-
this.ecx.read_immediate(this.ecx.eval_operand(right, None)?)
526-
})?;
527-
if *op == BinOp::Shr || *op == BinOp::Shl {
528-
let left_bits = place_layout.size.bits();
529-
let right_size = r.layout.size;
530-
let r_bits = r.to_scalar().and_then(|r| r.to_bits(right_size));
531-
if r_bits.map_or(false, |b| b >= left_bits as u128) {
532-
let lint_root = self.lint_root(source_info)?;
533-
let dir = if *op == BinOp::Shr { "right" } else { "left" };
534-
self.tcx.lint_hir(
535-
::rustc::lint::builtin::EXCEEDING_BITSHIFTS,
536-
lint_root,
537-
span,
538-
&format!("attempt to shift {} with overflow", dir),
539-
);
540-
return None;
541-
}
542-
}
543-
544-
// If overflow checking is enabled (like in debug mode by default),
545-
// then we'll already catch overflow when we evaluate the `Assert` statement
546-
// in MIR. However, if overflow checking is disabled, then there won't be any
547-
// `Assert` statement and so we have to do additional checking here.
548-
if !overflow_check {
549-
self.use_ecx(source_info, |this| {
550-
let l = this.ecx.read_immediate(this.ecx.eval_operand(left, None)?)?;
551-
let (_, overflow, _ty) = this.ecx.overflowing_binary_op(*op, l, r)?;
552-
553-
if overflow {
554-
let err = err_panic!(Overflow(*op)).into();
555-
return Err(err);
556-
}
557-
558-
Ok(())
559-
})?;
560-
}
627+
self.check_binary_op(*op, left, right, source_info, place_layout, overflow_check)?;
561628
}
562629

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

654+
Rvalue::Cast(CastKind::Misc, op, ty) => {
655+
trace!("checking Cast(Misc, {:?}, {:?})", op, ty);
656+
self.check_cast(op, ty, source_info, place_layout)?;
657+
}
658+
587659
_ => {}
588660
}
589661

src/test/mir-opt/const_prop/cast.rs

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
fn main() {
2+
let x = 42u8 as u32;
3+
4+
let y = 42u32 as u8;
5+
}
6+
7+
// END RUST SOURCE
8+
// START rustc.main.ConstProp.before.mir
9+
// let mut _0: ();
10+
// let _1: u32;
11+
// scope 1 {
12+
// debug x => _1;
13+
// let _2: u8;
14+
// scope 2 {
15+
// debug y => _2;
16+
// }
17+
// }
18+
// bb0: {
19+
// StorageLive(_1);
20+
// _1 = const 42u8 as u32 (Misc);
21+
// StorageLive(_2);
22+
// _2 = const 42u32 as u8 (Misc);
23+
// _0 = ();
24+
// StorageDead(_2);
25+
// StorageDead(_1);
26+
// return;
27+
// }
28+
// END rustc.main.ConstProp.before.mir
29+
// START rustc.main.ConstProp.after.mir
30+
// let mut _0: ();
31+
// let _1: u32;
32+
// scope 1 {
33+
// debug x => _1;
34+
// let _2: u8;
35+
// scope 2 {
36+
// debug y => _2;
37+
// }
38+
// }
39+
// bb0: {
40+
// StorageLive(_1);
41+
// _1 = const 42u32;
42+
// StorageLive(_2);
43+
// _2 = const 42u8;
44+
// _0 = ();
45+
// StorageDead(_2);
46+
// StorageDead(_1);
47+
// return;
48+
// }
49+
// END rustc.main.ConstProp.after.mir
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// build-fail
2+
// ignore-tidy-linelength
3+
4+
fn main() {
5+
let _ = 0u8 as u32;
6+
let _ = (1u32 << 31) as u16; //~ ERROR truncating cast: the value 2147483648 requires 32 bits but the target type is only 16 bits
7+
let _ = (1u16 << 15) as u8; //~ ERROR truncating cast: the value 32768 requires 16 bits but the target type is only 8 bits
8+
let _ = (!0u16) as u8; //~ ERROR truncating cast: the value 65535 requires 16 bits but the target type is only 8 bits
9+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
error: truncating cast: the value 2147483648 requires 32 bits but the target type is only 16 bits
2+
--> $DIR/const-prop-overflowing-casts.rs:6:13
3+
|
4+
LL | let _ = (1u32 << 31) as u16;
5+
| ^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `#[deny(const_err)]` on by default
8+
9+
error: truncating cast: the value 32768 requires 16 bits but the target type is only 8 bits
10+
--> $DIR/const-prop-overflowing-casts.rs:7:13
11+
|
12+
LL | let _ = (1u16 << 15) as u8;
13+
| ^^^^^^^^^^^^^^^^^^
14+
15+
error: truncating cast: the value 65535 requires 16 bits but the target type is only 8 bits
16+
--> $DIR/const-prop-overflowing-casts.rs:8:13
17+
|
18+
LL | let _ = (!0u16) as u8;
19+
| ^^^^^^^^^^^^^
20+
21+
error: aborting due to 3 previous errors
22+

src/test/ui/simd/simd-intrinsic-generic-cast.rs

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

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

89
extern crate test;
910

src/test/ui/wrapping-int-api.rs

+3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
// run-pass
22
// Test inherent wrapping_* methods for {i,u}{size,8,16,32,64}.
33

4+
// Don't warn about overflowing ops on 32-bit platforms
5+
#![cfg_attr(target_pointer_width = "32", allow(const_err))]
6+
47
use std::{i8, i16, i32, i64, isize};
58
use std::{u8, u16, u32, u64, usize};
69

0 commit comments

Comments
 (0)