rollup merge of #23536: pnkfelix/arith-oflo-shifts

overflow-checking for rhs of shift operators

Subtask of #22020 ([RFC 560](https://github.com/rust-lang/rfcs/blob/master/text/0560-integer-overflow.md))
This commit is contained in:
Alex Crichton 2015-03-23 15:08:17 -07:00
commit c77af69a37
13 changed files with 449 additions and 30 deletions

View File

@ -756,7 +756,7 @@ pub fn iter_structural_ty<'blk, 'tcx, F>(cx: Block<'blk, 'tcx>,
}
pub fn cast_shift_expr_rhs(cx: Block,
op: ast::BinOp,
op: ast::BinOp_,
lhs: ValueRef,
rhs: ValueRef)
-> ValueRef {
@ -765,24 +765,24 @@ pub fn cast_shift_expr_rhs(cx: Block,
|a,b| ZExt(cx, a, b))
}
pub fn cast_shift_const_rhs(op: ast::BinOp,
pub fn cast_shift_const_rhs(op: ast::BinOp_,
lhs: ValueRef, rhs: ValueRef) -> ValueRef {
cast_shift_rhs(op, lhs, rhs,
|a, b| unsafe { llvm::LLVMConstTrunc(a, b.to_ref()) },
|a, b| unsafe { llvm::LLVMConstZExt(a, b.to_ref()) })
}
pub fn cast_shift_rhs<F, G>(op: ast::BinOp,
lhs: ValueRef,
rhs: ValueRef,
trunc: F,
zext: G)
-> ValueRef where
fn cast_shift_rhs<F, G>(op: ast::BinOp_,
lhs: ValueRef,
rhs: ValueRef,
trunc: F,
zext: G)
-> ValueRef where
F: FnOnce(ValueRef, Type) -> ValueRef,
G: FnOnce(ValueRef, Type) -> ValueRef,
{
// Shifts may have any size int on the rhs
if ast_util::is_shift_binop(op.node) {
if ast_util::is_shift_binop(op) {
let mut rhs_llty = val_ty(rhs);
let mut lhs_llty = val_ty(lhs);
if rhs_llty.kind() == Vector { rhs_llty = rhs_llty.element_type() }

View File

@ -376,7 +376,7 @@ fn const_expr_unadjusted<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
let signed = ty::type_is_signed(intype);
let (te2, _) = const_expr(cx, &**e2, param_substs);
let te2 = base::cast_shift_const_rhs(b, te1, te2);
let te2 = base::cast_shift_const_rhs(b.node, te1, te2);
match b.node {
ast::BiAdd => {

View File

@ -1770,7 +1770,6 @@ fn trans_eager_binop<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
};
let is_float = ty::type_is_fp(intype);
let is_signed = ty::type_is_signed(intype);
let rhs = base::cast_shift_expr_rhs(bcx, op, lhs, rhs);
let info = expr_info(binop_expr);
let binop_debug_loc = binop_expr.debug_loc();
@ -1843,13 +1842,17 @@ fn trans_eager_binop<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
ast::BiBitOr => Or(bcx, lhs, rhs, binop_debug_loc),
ast::BiBitAnd => And(bcx, lhs, rhs, binop_debug_loc),
ast::BiBitXor => Xor(bcx, lhs, rhs, binop_debug_loc),
ast::BiShl => Shl(bcx, lhs, rhs, binop_debug_loc),
ast::BiShl => {
let (newbcx, res) = with_overflow_check(
bcx, OverflowOp::Shl, info, lhs_t, lhs, rhs, binop_debug_loc);
bcx = newbcx;
res
}
ast::BiShr => {
if is_signed {
AShr(bcx, lhs, rhs, binop_debug_loc)
} else {
LShr(bcx, lhs, rhs, binop_debug_loc)
}
let (newbcx, res) = with_overflow_check(
bcx, OverflowOp::Shr, info, lhs_t, lhs, rhs, binop_debug_loc);
bcx = newbcx;
res
}
ast::BiEq | ast::BiNe | ast::BiLt | ast::BiGe | ast::BiLe | ast::BiGt => {
if is_simd {
@ -2389,9 +2392,38 @@ enum OverflowOp {
Add,
Sub,
Mul,
Shl,
Shr,
}
impl OverflowOp {
fn codegen_strategy(&self) -> OverflowCodegen {
use self::OverflowCodegen::{ViaIntrinsic, ViaInputCheck};
match *self {
OverflowOp::Add => ViaIntrinsic(OverflowOpViaIntrinsic::Add),
OverflowOp::Sub => ViaIntrinsic(OverflowOpViaIntrinsic::Sub),
OverflowOp::Mul => ViaIntrinsic(OverflowOpViaIntrinsic::Mul),
OverflowOp::Shl => ViaInputCheck(OverflowOpViaInputCheck::Shl),
OverflowOp::Shr => ViaInputCheck(OverflowOpViaInputCheck::Shr),
}
}
}
enum OverflowCodegen {
ViaIntrinsic(OverflowOpViaIntrinsic),
ViaInputCheck(OverflowOpViaInputCheck),
}
enum OverflowOpViaInputCheck { Shl, Shr, }
enum OverflowOpViaIntrinsic { Add, Sub, Mul, }
impl OverflowOpViaIntrinsic {
fn to_intrinsic<'blk, 'tcx>(&self, bcx: Block<'blk, 'tcx>, lhs_ty: Ty) -> ValueRef {
let name = self.to_intrinsic_name(bcx.tcx(), lhs_ty);
bcx.ccx().get_intrinsic(&name)
}
fn to_intrinsic_name(&self, tcx: &ty::ctxt, ty: Ty) -> &'static str {
use syntax::ast::IntTy::*;
use syntax::ast::UintTy::*;
@ -2413,7 +2445,7 @@ impl OverflowOp {
};
match *self {
OverflowOp::Add => match new_sty {
OverflowOpViaIntrinsic::Add => match new_sty {
ty_int(TyI8) => "llvm.sadd.with.overflow.i8",
ty_int(TyI16) => "llvm.sadd.with.overflow.i16",
ty_int(TyI32) => "llvm.sadd.with.overflow.i32",
@ -2426,7 +2458,7 @@ impl OverflowOp {
_ => unreachable!(),
},
OverflowOp::Sub => match new_sty {
OverflowOpViaIntrinsic::Sub => match new_sty {
ty_int(TyI8) => "llvm.ssub.with.overflow.i8",
ty_int(TyI16) => "llvm.ssub.with.overflow.i16",
ty_int(TyI32) => "llvm.ssub.with.overflow.i32",
@ -2439,7 +2471,7 @@ impl OverflowOp {
_ => unreachable!(),
},
OverflowOp::Mul => match new_sty {
OverflowOpViaIntrinsic::Mul => match new_sty {
ty_int(TyI8) => "llvm.smul.with.overflow.i8",
ty_int(TyI16) => "llvm.smul.with.overflow.i16",
ty_int(TyI32) => "llvm.smul.with.overflow.i32",
@ -2454,16 +2486,14 @@ impl OverflowOp {
},
}
}
}
fn with_overflow_check<'a, 'b>(bcx: Block<'a, 'b>, oop: OverflowOp, info: NodeIdAndSpan,
lhs_t: Ty, lhs: ValueRef, rhs: ValueRef, binop_debug_loc: DebugLoc)
-> (Block<'a, 'b>, ValueRef) {
if bcx.unreachable.get() { return (bcx, _Undef(lhs)); }
if bcx.ccx().check_overflow() {
let name = oop.to_intrinsic_name(bcx.tcx(), lhs_t);
let llfn = bcx.ccx().get_intrinsic(&name);
fn build_intrinsic_call<'blk, 'tcx>(&self, bcx: Block<'blk, 'tcx>,
info: NodeIdAndSpan,
lhs_t: Ty<'tcx>, lhs: ValueRef,
rhs: ValueRef,
binop_debug_loc: DebugLoc)
-> (Block<'blk, 'tcx>, ValueRef) {
let llfn = self.to_intrinsic(bcx, lhs_t);
let val = Call(bcx, llfn, &[lhs, rhs], None, binop_debug_loc);
let result = ExtractValue(bcx, val, 0); // iN operation result
@ -2482,11 +2512,118 @@ fn with_overflow_check<'a, 'b>(bcx: Block<'a, 'b>, oop: OverflowOp, info: NodeId
InternedString::new("arithmetic operation overflowed")));
(bcx, result)
}
}
impl OverflowOpViaInputCheck {
fn build_with_input_check<'blk, 'tcx>(&self,
bcx: Block<'blk, 'tcx>,
info: NodeIdAndSpan,
lhs_t: Ty<'tcx>,
lhs: ValueRef,
rhs: ValueRef,
binop_debug_loc: DebugLoc)
-> (Block<'blk, 'tcx>, ValueRef)
{
let lhs_llty = val_ty(lhs);
let rhs_llty = val_ty(rhs);
// Panic if any bits are set outside of bits that we always
// mask in.
//
// Note that the mask's value is derived from the LHS type
// (since that is where the 32/64 distinction is relevant) but
// the mask's type must match the RHS type (since they will
// both be fed into a and-binop)
let invert_mask = !shift_mask_val(lhs_llty);
let invert_mask = C_integral(rhs_llty, invert_mask, true);
let outer_bits = And(bcx, rhs, invert_mask, binop_debug_loc);
let cond = ICmp(bcx, llvm::IntNE, outer_bits,
C_integral(rhs_llty, 0, false), binop_debug_loc);
let result = match *self {
OverflowOpViaInputCheck::Shl =>
build_unchecked_lshift(bcx, lhs, rhs, binop_debug_loc),
OverflowOpViaInputCheck::Shr =>
build_unchecked_rshift(bcx, lhs_t, lhs, rhs, binop_debug_loc),
};
let bcx =
base::with_cond(bcx, cond, |bcx|
controlflow::trans_fail(bcx, info,
InternedString::new("shift operation overflowed")));
(bcx, result)
}
}
fn shift_mask_val(llty: Type) -> u64 {
// i8/u8 can shift by at most 7, i16/u16 by at most 15, etc.
llty.int_width() - 1
}
// To avoid UB from LLVM, these two functions mask RHS with an
// appropriate mask unconditionally (i.e. the fallback behavior for
// all shifts). For 32- and 64-bit types, this matches the semantics
// of Java. (See related discussion on #1877 and #10183.)
fn build_unchecked_lshift<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
lhs: ValueRef,
rhs: ValueRef,
binop_debug_loc: DebugLoc) -> ValueRef {
let rhs = base::cast_shift_expr_rhs(bcx, ast::BinOp_::BiShl, lhs, rhs);
// #1877, #10183: Ensure that input is always valid
let rhs = shift_mask_rhs(bcx, rhs, binop_debug_loc);
Shl(bcx, lhs, rhs, binop_debug_loc)
}
fn build_unchecked_rshift<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
lhs_t: Ty<'tcx>,
lhs: ValueRef,
rhs: ValueRef,
binop_debug_loc: DebugLoc) -> ValueRef {
let rhs = base::cast_shift_expr_rhs(bcx, ast::BinOp_::BiShr, lhs, rhs);
// #1877, #10183: Ensure that input is always valid
let rhs = shift_mask_rhs(bcx, rhs, binop_debug_loc);
let is_signed = ty::type_is_signed(lhs_t);
if is_signed {
AShr(bcx, lhs, rhs, binop_debug_loc)
} else {
LShr(bcx, lhs, rhs, binop_debug_loc)
}
}
fn shift_mask_rhs<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
rhs: ValueRef,
debug_loc: DebugLoc) -> ValueRef {
let rhs_llty = val_ty(rhs);
let mask = shift_mask_val(rhs_llty);
And(bcx, rhs, C_integral(rhs_llty, mask, false), debug_loc)
}
fn with_overflow_check<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, oop: OverflowOp, info: NodeIdAndSpan,
lhs_t: Ty<'tcx>, lhs: ValueRef,
rhs: ValueRef,
binop_debug_loc: DebugLoc)
-> (Block<'blk, 'tcx>, ValueRef) {
if bcx.unreachable.get() { return (bcx, _Undef(lhs)); }
if bcx.ccx().check_overflow() {
match oop.codegen_strategy() {
OverflowCodegen::ViaIntrinsic(oop) =>
oop.build_intrinsic_call(bcx, info, lhs_t, lhs, rhs, binop_debug_loc),
OverflowCodegen::ViaInputCheck(oop) =>
oop.build_with_input_check(bcx, info, lhs_t, lhs, rhs, binop_debug_loc),
}
} else {
let res = match oop {
OverflowOp::Add => Add(bcx, lhs, rhs, binop_debug_loc),
OverflowOp::Sub => Sub(bcx, lhs, rhs, binop_debug_loc),
OverflowOp::Mul => Mul(bcx, lhs, rhs, binop_debug_loc),
OverflowOp::Shl =>
build_unchecked_lshift(bcx, lhs, rhs, binop_debug_loc),
OverflowOp::Shr =>
build_unchecked_rshift(bcx, lhs_t, lhs, rhs, binop_debug_loc),
};
(bcx, res)
}

View File

@ -0,0 +1,19 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// error-pattern:thread '<main>' panicked at 'shift operation overflowed'
// compile-flags: -C debug-assertions
// (Work around constant-evaluation)
fn id<T>(x: T) -> T { x }
fn main() {
let _x = 1_i32 << id(32);
}

View File

@ -0,0 +1,19 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// error-pattern:thread '<main>' panicked at 'shift operation overflowed'
// compile-flags: -C debug-assertions
// (Work around constant-evaluation)
fn id<T>(x: T) -> T { x }
fn main() {
let _x = 1 << id(-1);
}

View File

@ -0,0 +1,19 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// error-pattern:thread '<main>' panicked at 'shift operation overflowed'
// compile-flags: -C debug-assertions
// (Work around constant-evaluation)
fn id<T>(x: T) -> T { x }
fn main() {
let _x = 1_u64 << id(64);
}

View File

@ -0,0 +1,34 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// error-pattern:thread '<main>' panicked at 'shift operation overflowed'
// compile-flags: -C debug-assertions
// This function is checking that our automatic truncation does not
// sidestep the overflow checking.
// (Work around constant-evaluation)
fn id<T>(x: T) -> T { x }
fn main() {
// this signals overflow when checking is on
let x = 1_i8 << id(17);
// ... but when checking is off, the fallback will truncate the
// input to its lower three bits (= 1). Note that this is *not*
// the behavior of the x86 processor for 8- and 16-bit types,
// but it is necessary to avoid undefined behavior from LLVM.
//
// We check that here, by ensuring the result has only been
// shifted by one place; if overflow checking is turned off, then
// this assertion will pass (and the compiletest driver will
// report that the test did not produce the error expected above).
assert_eq!(x, 2_i8);
}

View File

@ -0,0 +1,19 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// error-pattern:thread '<main>' panicked at 'shift operation overflowed'
// compile-flags: -C debug-assertions
// (Work around constant-evaluation)
fn id<T>(x: T) -> T { x }
fn main() {
let _x = -1_i32 >> id(32);
}

View File

@ -0,0 +1,19 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// error-pattern:thread '<main>' panicked at 'shift operation overflowed'
// compile-flags: -C debug-assertions
// (Work around constant-evaluation)
fn id<T>(x: T) -> T { x }
fn main() {
let _x = -1_i32 >> id(-1);
}

View File

@ -0,0 +1,19 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// error-pattern:thread '<main>' panicked at 'shift operation overflowed'
// compile-flags: -C debug-assertions
// (Work around constant-evaluation)
fn id<T>(x: T) -> T { x }
fn main() {
let _x = -1_i64 >> id(64);
}

View File

@ -0,0 +1,34 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// error-pattern:thread '<main>' panicked at 'shift operation overflowed'
// compile-flags: -C debug-assertions
// This function is checking that our (type-based) automatic
// truncation does not sidestep the overflow checking.
// (Work around constant-evaluation)
fn id<T>(x: T) -> T { x }
fn main() {
// this signals overflow when checking is on
let x = 2_i8 >> id(17);
// ... but when checking is off, the fallback will truncate the
// input to its lower three bits (= 1). Note that this is *not*
// the behavior of the x86 processor for 8- and 16-bit types,
// but it is necessary to avoid undefined behavior from LLVM.
//
// We check that here, by ensuring the result is not zero; if
// overflow checking is turned off, then this assertion will pass
// (and the compiletest driver will report that the test did not
// produce the error expected above).
assert_eq!(x, 1_i8);
}

View File

@ -11,7 +11,7 @@
// Regression test for issue #152.
pub fn main() {
let mut b: uint = 1_usize;
while b <= 32_usize {
while b < std::mem::size_of::<usize>() {
0_usize << b;
b <<= 1_usize;
println!("{}", b);

View File

@ -0,0 +1,100 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// compile-flags: -C debug-assertions
// Check that we do *not* overflow on a number of edge cases.
// (compare with test/run-fail/overflowing-{lsh,rsh}*.rs)
// (Work around constant-evaluation)
fn id<T>(x: T) -> T { x }
fn main() {
test_left_shift();
test_right_shift();
}
fn test_left_shift() {
// negative rhs can panic, but values in [0,N-1] are okay for iN
macro_rules! tests {
($iN:ty, $uN:ty, $max_rhs:expr, $expect_i:expr, $expect_u:expr) => { {
let x = (1 as $iN) << id(0);
assert_eq!(x, 1);
let x = (1 as $uN) << id(0);
assert_eq!(x, 1);
let x = (1 as $iN) << id($max_rhs);
assert_eq!(x, $expect_i);
let x = (1 as $uN) << id($max_rhs);
assert_eq!(x, $expect_u);
// high-order bits on LHS are silently discarded without panic.
let x = (3 as $iN) << id($max_rhs);
assert_eq!(x, $expect_i);
let x = (3 as $uN) << id($max_rhs);
assert_eq!(x, $expect_u);
} }
}
let x = 1_i8 << id(0);
assert_eq!(x, 1);
let x = 1_u8 << id(0);
assert_eq!(x, 1);
let x = 1_i8 << id(7);
assert_eq!(x, std::i8::MIN);
let x = 1_u8 << id(7);
assert_eq!(x, 0x80);
// high-order bits on LHS are silently discarded without panic.
let x = 3_i8 << id(7);
assert_eq!(x, std::i8::MIN);
let x = 3_u8 << id(7);
assert_eq!(x, 0x80);
// above is (approximately) expanded from:
tests!(i8, u8, 7, std::i8::MIN, 0x80_u8);
tests!(i16, u16, 15, std::i16::MIN, 0x8000_u16);
tests!(i32, u32, 31, std::i32::MIN, 0x8000_0000_u32);
tests!(i64, u64, 63, std::i64::MIN, 0x8000_0000_0000_0000_u64);
}
fn test_right_shift() {
// negative rhs can panic, but values in [0,N-1] are okay for iN
macro_rules! tests {
($iN:ty, $uN:ty, $max_rhs:expr,
$signbit_i:expr, $highbit_i:expr, $highbit_u:expr) =>
{ {
let x = (1 as $iN) >> id(0);
assert_eq!(x, 1);
let x = (1 as $uN) >> id(0);
assert_eq!(x, 1);
let x = ($highbit_i) >> id($max_rhs-1);
assert_eq!(x, 1);
let x = ($highbit_u) >> id($max_rhs);
assert_eq!(x, 1);
// sign-bit is carried by arithmetic right shift
let x = ($signbit_i) >> id($max_rhs);
assert_eq!(x, -1);
// low-order bits on LHS are silently discarded without panic.
let x = ($highbit_i + 1) >> id($max_rhs-1);
assert_eq!(x, 1);
let x = ($highbit_u + 1) >> id($max_rhs);
assert_eq!(x, 1);
let x = ($signbit_i + 1) >> id($max_rhs);
assert_eq!(x, -1);
} }
}
tests!(i8, u8, 7, std::i8::MIN, 0x40_i8, 0x80_u8);
tests!(i16, u16, 15, std::i16::MIN, 0x4000_u16, 0x8000_u16);
tests!(i32, u32, 31, std::i32::MIN, 0x4000_0000_u32, 0x8000_0000_u32);
tests!(i64, u64, 63, std::i64::MIN,
0x4000_0000_0000_0000_u64, 0x8000_0000_0000_0000_u64);
}