Skip to content

Commit

Permalink
Auto merge of #2422 - RalfJung:integers, r=oli-obk
Browse files Browse the repository at this point in the history
enable clippy lints against integer casts

Cc #1236
  • Loading branch information
bors committed Jul 25, 2022
2 parents 3d237be + 7f60348 commit 6227e1e
Show file tree
Hide file tree
Showing 11 changed files with 46 additions and 22 deletions.
1 change: 1 addition & 0 deletions src/intptrcast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ impl<'mir, 'tcx> GlobalStateInner {

// Wrapping "addr - base_addr"
let dl = ecx.data_layout();
#[allow(clippy::cast_possible_wrap)] // we want to wrap here
let neg_base_addr = (base_addr as i64).wrapping_neg();
Some((
alloc_id,
Expand Down
9 changes: 8 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#![feature(is_some_with)]
#![feature(nonzero_ops)]
#![feature(local_key_cell_methods)]
#![warn(rust_2018_idioms)]
// Configure clippy and other lints
#![allow(
clippy::collapsible_else_if,
clippy::collapsible_if,
Expand All @@ -24,6 +24,13 @@
clippy::derive_hash_xor_eq,
clippy::too_many_arguments
)]
#![warn(
rust_2018_idioms,
clippy::cast_possible_wrap, // unsigned -> signed
clippy::cast_sign_loss, // signed -> unsigned
clippy::cast_lossless,
clippy::cast_possible_truncation,
)]

extern crate rustc_apfloat;
extern crate rustc_ast;
Expand Down
6 changes: 4 additions & 2 deletions src/shims/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
);
}

let lineno: u32 = lo.line as u32;
// `u32` is not enough to fit line/colno, which can be `usize`. It seems unlikely that a
// file would have more than 2^32 lines or columns, but whatever, just default to 0.
let lineno: u32 = u32::try_from(lo.line).unwrap_or(0);
// `lo.col` is 0-based - add 1 to make it 1-based for the caller.
let colno: u32 = lo.col.0 as u32 + 1;
let colno: u32 = u32::try_from(lo.col.0 + 1).unwrap_or(0);

let dest = this.force_allocation(dest)?;
if let ty::Adt(adt, _) = dest.layout.ty.kind() {
Expand Down
4 changes: 2 additions & 2 deletions src/shims/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let env_block_ptr = this.read_pointer(env_block_op)?;
let result = this.deallocate_ptr(env_block_ptr, None, MiriMemoryKind::Runtime.into());
// If the function succeeds, the return value is nonzero.
Ok(result.is_ok() as i32)
Ok(i32::from(result.is_ok()))
}

fn setenv(
Expand Down Expand Up @@ -459,7 +459,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// The reason we need to do this wacky of a conversion is because
// `libc::getpid` returns an i32, however, `std::process::id()` return an u32.
// So we un-do the conversion that stdlib does and turn it back into an i32.

#[allow(clippy::cast_possible_wrap)]
Ok(std::process::id() as i32)
}

Expand Down
20 changes: 16 additions & 4 deletions src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let align = this.min_align(size, kind);
let ptr = this.allocate_ptr(Size::from_bytes(size), align, kind.into())?;
if zero_init {
// We just allocated this, the access is definitely in-bounds.
this.write_bytes_ptr(ptr.into(), iter::repeat(0u8).take(size as usize)).unwrap();
// We just allocated this, the access is definitely in-bounds and fits into our address space.
this.write_bytes_ptr(
ptr.into(),
iter::repeat(0u8).take(usize::try_from(size).unwrap()),
)
.unwrap();
}
Ok(ptr.into())
}
Expand Down Expand Up @@ -526,8 +530,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
"memrchr" => {
let [ptr, val, num] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let ptr = this.read_pointer(ptr)?;
let val = this.read_scalar(val)?.to_i32()? as u8;
let val = this.read_scalar(val)?.to_i32()?;
let num = this.read_scalar(num)?.to_machine_usize(this)?;
// The docs say val is "interpreted as unsigned char".
#[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
let val = val as u8;

if let Some(idx) = this
.read_bytes_ptr(ptr, Size::from_bytes(num))?
.iter()
Expand All @@ -543,8 +551,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
"memchr" => {
let [ptr, val, num] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let ptr = this.read_pointer(ptr)?;
let val = this.read_scalar(val)?.to_i32()? as u8;
let val = this.read_scalar(val)?.to_i32()?;
let num = this.read_scalar(num)?.to_machine_usize(this)?;
// The docs say val is "interpreted as unsigned char".
#[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
let val = val as u8;

let idx = this
.read_bytes_ptr(ptr, Size::from_bytes(num))?
.iter()
Expand Down
5 changes: 1 addition & 4 deletions src/shims/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let byte_count = ty_layout.size.checked_mul(count, this).ok_or_else(|| {
err_ub_format!("overflow computing total size of `{intrinsic_name}`")
})?;
this.write_bytes_ptr(
ptr,
iter::repeat(val_byte).take(byte_count.bytes() as usize),
)?;
this.write_bytes_ptr(ptr, iter::repeat(val_byte).take(byte_count.bytes_usize()))?;
}

// Floating-point operations
Expand Down
2 changes: 1 addition & 1 deletion src/shims/tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ impl<'tcx> TlsData<'tcx> {
self.keys.try_insert(new_key, TlsEntry { data: Default::default(), dtor }).unwrap();
trace!("New TLS key allocated: {} with dtor {:?}", new_key, dtor);

if max_size.bits() < 128 && new_key >= (1u128 << max_size.bits() as u128) {
if max_size.bits() < 128 && new_key >= (1u128 << max_size.bits()) {
throw_unsup_format!("we ran out of TLS key space");
}
Ok(new_key)
Expand Down
12 changes: 8 additions & 4 deletions src/shims/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,15 +776,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx

// We cap the number of read bytes to the largest value that we are able to fit in both the
// host's and target's `isize`. This saves us from having to handle overflows later.
let count = count.min(this.machine_isize_max() as u64).min(isize::MAX as u64);
let count = count
.min(u64::try_from(this.machine_isize_max()).unwrap())
.min(u64::try_from(isize::MAX).unwrap());
let communicate = this.machine.communicate();

if let Some(file_descriptor) = this.machine.file_handler.handles.get_mut(&fd) {
trace!("read: FD mapped to {:?}", file_descriptor);
// We want to read at most `count` bytes. We are sure that `count` is not negative
// because it was a target's `usize`. Also we are sure that its smaller than
// `usize::MAX` because it is a host's `isize`.
let mut bytes = vec![0; count as usize];
// `usize::MAX` because it is bounded by the host's `isize`.
let mut bytes = vec![0; usize::try_from(count).unwrap()];
// `File::read` never returns a value larger than `count`,
// so this cannot fail.
let result =
Expand Down Expand Up @@ -827,7 +829,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx

// We cap the number of written bytes to the largest value that we are able to fit in both the
// host's and target's `isize`. This saves us from having to handle overflows later.
let count = count.min(this.machine_isize_max() as u64).min(isize::MAX as u64);
let count = count
.min(u64::try_from(this.machine_isize_max()).unwrap())
.min(u64::try_from(isize::MAX).unwrap());
let communicate = this.machine.communicate();

if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) {
Expand Down
5 changes: 3 additions & 2 deletions src/shims/windows/dlsym.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
} else {
io::stderr().write(buf_cont)
};
res.ok().map(|n| n as u32)
// We write at most `n` bytes, which is a `u32`, so we cannot have written more than that.
res.ok().map(|n| u32::try_from(n).unwrap())
} else {
throw_unsup_format!(
"on Windows, writing to anything except stdout/stderr is not supported"
Expand All @@ -102,7 +103,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// Return whether this was a success. >= 0 is success.
// For the error code we arbitrarily pick 0xC0000185, STATUS_IO_DEVICE_ERROR.
this.write_scalar(
Scalar::from_i32(if written.is_some() { 0 } else { 0xC0000185u32 as i32 }),
Scalar::from_u32(if written.is_some() { 0 } else { 0xC0000185u32 }),
dest,
)?;
}
Expand Down
2 changes: 1 addition & 1 deletion src/shims/windows/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// Initialize with `0`.
this.write_bytes_ptr(
system_info.ptr,
iter::repeat(0u8).take(system_info.layout.size.bytes() as usize),
iter::repeat(0u8).take(system_info.layout.size.bytes_usize()),
)?;
// Set selected fields.
let word_layout = this.machine.layouts.u16;
Expand Down
2 changes: 1 addition & 1 deletion src/stacked_borrows/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl Item {
assert!(tag.0.get() <= TAG_MASK);
let packed_tag = tag.0.get();
let packed_perm = perm.to_bits() << PERM_SHIFT;
let packed_protected = (protected as u64) << PROTECTED_SHIFT;
let packed_protected = u64::from(protected) << PROTECTED_SHIFT;

let new = Self(packed_tag | packed_perm | packed_protected);

Expand Down

0 comments on commit 6227e1e

Please sign in to comment.