Skip to content

enable clippy lints against integer casts #2422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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