From 3f0fdf290fe523fdbb6be21eafe4915638373fe8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 23 Jul 2022 08:24:33 -0400 Subject: [PATCH 1/3] pass clippy::cast_lossless --- src/lib.rs | 3 ++- src/shims/env.rs | 2 +- src/shims/tls.rs | 2 +- src/stacked_borrows/item.rs | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 35351664caf..1a07ce9aa12 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -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, @@ -24,6 +24,7 @@ clippy::derive_hash_xor_eq, clippy::too_many_arguments )] +#![warn(rust_2018_idioms, clippy::cast_lossless)] extern crate rustc_apfloat; extern crate rustc_ast; diff --git a/src/shims/env.rs b/src/shims/env.rs index f0818e71b66..c1d39dc0928 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -202,7 +202,7 @@ fn FreeEnvironmentStringsW( 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( diff --git a/src/shims/tls.rs b/src/shims/tls.rs index 13af447f76c..9d19160b84d 100644 --- a/src/shims/tls.rs +++ b/src/shims/tls.rs @@ -73,7 +73,7 @@ pub fn create_tls_key( 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) diff --git a/src/stacked_borrows/item.rs b/src/stacked_borrows/item.rs index ad1b9b075b4..709b27d191b 100644 --- a/src/stacked_borrows/item.rs +++ b/src/stacked_borrows/item.rs @@ -22,7 +22,7 @@ pub fn new(tag: SbTag, perm: Permission, protected: bool) -> Self { 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); From b67a6ff09911736f331933074939dd4fb7b38200 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 23 Jul 2022 08:35:45 -0400 Subject: [PATCH 2/3] pass clippy::cast_sign_loss and clippy::cast_possible_wrap --- src/intptrcast.rs | 1 + src/lib.rs | 7 ++++++- src/shims/env.rs | 2 +- src/shims/foreign_items.rs | 12 ++++++++++-- src/shims/unix/fs.rs | 8 ++++++-- src/shims/windows/dlsym.rs | 2 +- 6 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/intptrcast.rs b/src/intptrcast.rs index aa7111cb81f..99fc086a229 100644 --- a/src/intptrcast.rs +++ b/src/intptrcast.rs @@ -230,6 +230,7 @@ pub fn abs_ptr_to_rel( // 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, diff --git a/src/lib.rs b/src/lib.rs index 1a07ce9aa12..c1b0c4afca6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,7 +24,12 @@ clippy::derive_hash_xor_eq, clippy::too_many_arguments )] -#![warn(rust_2018_idioms, clippy::cast_lossless)] +#![warn( + rust_2018_idioms, + clippy::cast_possible_wrap, // unsigned -> signed + clippy::cast_sign_loss, // signed -> unsigned + clippy::cast_lossless, +)] extern crate rustc_apfloat; extern crate rustc_ast; diff --git a/src/shims/env.rs b/src/shims/env.rs index c1d39dc0928..db1ddf6291f 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -459,7 +459,7 @@ fn getpid(&mut self) -> InterpResult<'tcx, i32> { // 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) } diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 99f06171621..317eab082c0 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -526,8 +526,12 @@ fn emulate_foreign_item_by_name( "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)] + let val = val as u8; + if let Some(idx) = this .read_bytes_ptr(ptr, Size::from_bytes(num))? .iter() @@ -543,8 +547,12 @@ fn emulate_foreign_item_by_name( "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)] + let val = val as u8; + let idx = this .read_bytes_ptr(ptr, Size::from_bytes(num))? .iter() diff --git a/src/shims/unix/fs.rs b/src/shims/unix/fs.rs index 50a2053078f..3b82cb3c4eb 100644 --- a/src/shims/unix/fs.rs +++ b/src/shims/unix/fs.rs @@ -776,7 +776,9 @@ fn read( // 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) { @@ -827,7 +829,9 @@ fn write( // 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) { diff --git a/src/shims/windows/dlsym.rs b/src/shims/windows/dlsym.rs index ee4f3922777..51b0e3b83d4 100644 --- a/src/shims/windows/dlsym.rs +++ b/src/shims/windows/dlsym.rs @@ -102,7 +102,7 @@ fn call_dlsym( // 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, )?; } From 7f6034862d0a22c7337eb428c67fcdb23b9168ae Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 23 Jul 2022 08:44:16 -0400 Subject: [PATCH 3/3] pass clippy::cast_possible_truncation --- src/lib.rs | 1 + src/shims/backtrace.rs | 6 ++++-- src/shims/foreign_items.rs | 12 ++++++++---- src/shims/intrinsics/mod.rs | 5 +---- src/shims/unix/fs.rs | 4 ++-- src/shims/windows/dlsym.rs | 3 ++- src/shims/windows/foreign_items.rs | 2 +- 7 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c1b0c4afca6..caae17b2022 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -29,6 +29,7 @@ clippy::cast_possible_wrap, // unsigned -> signed clippy::cast_sign_loss, // signed -> unsigned clippy::cast_lossless, + clippy::cast_possible_truncation, )] extern crate rustc_apfloat; diff --git a/src/shims/backtrace.rs b/src/shims/backtrace.rs index 54ab8665ce3..9182b2a7217 100644 --- a/src/shims/backtrace.rs +++ b/src/shims/backtrace.rs @@ -171,9 +171,11 @@ fn handle_miri_resolve_frame( ); } - 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() { diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 317eab082c0..208e7ea788f 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -82,8 +82,12 @@ fn malloc( 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()) } @@ -529,7 +533,7 @@ fn emulate_foreign_item_by_name( 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)] + #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)] let val = val as u8; if let Some(idx) = this @@ -550,7 +554,7 @@ fn emulate_foreign_item_by_name( 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)] + #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)] let val = val as u8; let idx = this diff --git a/src/shims/intrinsics/mod.rs b/src/shims/intrinsics/mod.rs index c07ed8b2948..4c2d08ffcea 100644 --- a/src/shims/intrinsics/mod.rs +++ b/src/shims/intrinsics/mod.rs @@ -117,10 +117,7 @@ fn emulate_intrinsic_by_name( 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 diff --git a/src/shims/unix/fs.rs b/src/shims/unix/fs.rs index 3b82cb3c4eb..dc31237a319 100644 --- a/src/shims/unix/fs.rs +++ b/src/shims/unix/fs.rs @@ -785,8 +785,8 @@ fn read( 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 = diff --git a/src/shims/windows/dlsym.rs b/src/shims/windows/dlsym.rs index 51b0e3b83d4..eab5f99c878 100644 --- a/src/shims/windows/dlsym.rs +++ b/src/shims/windows/dlsym.rs @@ -84,7 +84,8 @@ fn call_dlsym( } 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" diff --git a/src/shims/windows/foreign_items.rs b/src/shims/windows/foreign_items.rs index 3f4b8b14002..29afe52cafd 100644 --- a/src/shims/windows/foreign_items.rs +++ b/src/shims/windows/foreign_items.rs @@ -116,7 +116,7 @@ fn emulate_foreign_item_by_name( // 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;