Warn on clippy::cast_possible_wrap

JBYoshi · JonathanWoollett-Light · commit 824639598258 · 2023-07-11T11:46:22.000+01:00
Fixes #3198 Signed-off-by: Jonathan Browne <12983479+JBYoshi@users.noreply.github.com>
diff --git a/.cargo/config b/.cargo/config
@@ -5,6 +5,7 @@ rustflags = [
   "-Dclippy::ptr_as_ptr",
   "-Dclippy::undocumented_unsafe_blocks",
   "-Dclippy::cast_lossless",
+  "-Dclippy::cast_possible_wrap",
   "-Dclippy::cast_sign_loss",
   "-Dmissing_debug_implementations"
 ]
diff --git a/src/jailer/src/resource_limits.rs b/src/jailer/src/resource_limits.rs
@@ -27,12 +27,14 @@ impl From<Resource> for u32 {
     fn from(resource: Resource) -> u32 {
         match resource {
             #[allow(clippy::unnecessary_cast)]
+            #[allow(clippy::cast_possible_wrap)]
             // Definition of libc::RLIMIT_FSIZE depends on the target_env:
             //      * when equals to "musl" -> libc::RLIMIT_FSIZE is a c_int (which is an i32)
             //      * when equals to "gnu" -> libc::RLIMIT_FSIZE is __rlimit_resource_t which is a
             //        c_uint (which is an u32)
             Resource::RlimitFsize => libc::RLIMIT_FSIZE as u32,
             #[allow(clippy::unnecessary_cast)]
+            #[allow(clippy::cast_possible_wrap)]
             // Definition of libc::RLIMIT_NOFILE depends on the target_env:
             //      * when equals to "musl" -> libc::RLIMIT_NOFILE is a c_int (which is an i32)
             //      * when equals to "gnu" -> libc::RLIMIT_NOFILE is __rlimit_resource_t which is a
@@ -42,6 +44,27 @@ impl From<Resource> for u32 {
     }
 }
 
+impl From<Resource> for i32 {
+    fn from(resource: Resource) -> i32 {
+        match resource {
+            #[allow(clippy::unnecessary_cast)]
+            #[allow(clippy::cast_possible_wrap)]
+            // Definition of libc::RLIMIT_FSIZE depends on the target_env:
+            //      * when equals to "musl" -> libc::RLIMIT_FSIZE is a c_int (which is an i32)
+            //      * when equals to "gnu" -> libc::RLIMIT_FSIZE is __rlimit_resource_t which is a
+            //        c_uint (which is an u32)
+            Resource::RlimitFsize => libc::RLIMIT_FSIZE as i32,
+            #[allow(clippy::unnecessary_cast)]
+            #[allow(clippy::cast_possible_wrap)]
+            // Definition of libc::RLIMIT_NOFILE depends on the target_env:
+            //      * when equals to "musl" -> libc::RLIMIT_NOFILE is a c_int (which is an i32)
+            //      * when equals to "gnu" -> libc::RLIMIT_NOFILE is __rlimit_resource_t which is a
+            //        c_uint (which is an u32)
+            Resource::RlimitNoFile => libc::RLIMIT_NOFILE as i32,
+        }
+    }
+}
+
 impl Display for Resource {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         match self {
@@ -86,7 +109,7 @@ impl ResourceLimits {
 
         // SAFETY: Safe because `resource` is a known-valid constant, and `&rlim`
         // is non-dangling.
-        SyscallReturnCode(unsafe { libc::setrlimit(u32::from(resource) as _, &rlim) })
+        SyscallReturnCode(unsafe { libc::setrlimit(resource.into(), &rlim) })
             .into_empty_result()
             .map_err(|_| Error::Setrlimit(resource.to_string()))
     }
@@ -148,7 +171,7 @@ mod tests {
             rlim_cur: 0,
             rlim_max: 0,
         };
-        unsafe { libc::getrlimit(u32::from(resource) as _, &mut rlim) };
+        unsafe { libc::getrlimit(resource.into(), &mut rlim) };
         assert_ne!(rlim.rlim_cur, new_limit);
         assert_ne!(rlim.rlim_max, new_limit);
 
@@ -160,7 +183,7 @@ mod tests {
             rlim_cur: 0,
             rlim_max: 0,
         };
-        unsafe { libc::getrlimit(u32::from(resource) as _, &mut rlim) };
+        unsafe { libc::getrlimit(resource.into(), &mut rlim) };
         assert_eq!(rlim.rlim_cur, new_limit);
         assert_eq!(rlim.rlim_max, new_limit);
     }
diff --git a/src/logger/src/metrics.rs b/src/logger/src/metrics.rs
@@ -1148,7 +1148,8 @@ impl SerializeToUtcTimestampMs {
 impl Serialize for SerializeToUtcTimestampMs {
     fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
         serializer.serialize_i64(
-            utils::time::get_time_ns(utils::time::ClockType::Real) as i64 / 1_000_000,
+            i64::try_from(utils::time::get_time_ns(utils::time::ClockType::Real) / 1_000_000)
+                .unwrap(),
         )
     }
 }
diff --git a/src/utils/src/byte_order.rs b/src/utils/src/byte_order.rs
@@ -38,6 +38,7 @@ generate_read_fn!(read_be_u32, u32, u8, 4, from_be_bytes);
 
 generate_write_fn!(write_le_u16, u16, u8, to_le_bytes);
 generate_write_fn!(write_le_u32, u32, u8, to_le_bytes);
+generate_write_fn!(write_le_u32_to_i8, u32, i8, to_le_bytes);
 generate_write_fn!(write_le_u64, u64, u8, to_le_bytes);
 generate_write_fn!(write_le_i32, i32, i8, to_le_bytes);
 
diff --git a/src/utils/src/time.rs b/src/utils/src/time.rs
@@ -181,7 +181,7 @@ pub fn get_time_ms(clock_type: ClockType) -> u64 {
 ///
 /// * `value` - Timestamp in seconds.
 pub fn seconds_to_nanoseconds(value: i64) -> Option<i64> {
-    value.checked_mul(NANOS_PER_SECOND as i64)
+    value.checked_mul(i64::try_from(NANOS_PER_SECOND).unwrap())
 }
 
 #[cfg(test)]
diff --git a/src/utils/src/vm_memory.rs b/src/utils/src/vm_memory.rs
@@ -86,7 +86,7 @@ fn build_guarded_region(
             prot,
             flags | libc::MAP_FIXED,
             fd,
-            offset as libc::off_t,
+            libc::off_t::try_from(offset).unwrap(),
         )
     };
 
diff --git a/src/vmm/src/arch/x86_64/interrupts.rs b/src/vmm/src/arch/x86_64/interrupts.rs
@@ -35,7 +35,7 @@ fn get_klapic_reg(klapic: &kvm_lapic_state, reg_offset: usize) -> u32 {
 fn set_klapic_reg(klapic: &mut kvm_lapic_state, reg_offset: usize, value: u32) {
     let range = reg_offset..reg_offset + 4;
     let reg = klapic.regs.get_mut(range).expect("set_klapic_reg range");
-    byte_order::write_le_i32(reg, value as i32)
+    byte_order::write_le_u32_to_i8(reg, value)
 }
 
 fn set_apic_delivery_mode(reg: u32, mode: u32) -> u32 {
diff --git a/src/vmm/src/arch/x86_64/mptable.rs b/src/vmm/src/arch/x86_64/mptable.rs
@@ -285,7 +285,9 @@ pub fn setup_mptable(mem: &GuestMemoryMmap, num_cpus: u8) -> Result<(), MptableE
         mpc_table.0.productid = MPC_PRODUCT_ID;
         mpc_table.0.lapic = APIC_DEFAULT_PHYS_BASE;
         checksum = checksum.wrapping_add(compute_checksum(&mpc_table.0));
-        mpc_table.0.checksum = (!checksum).wrapping_add(1) as i8;
+        #[allow(clippy::cast_possible_wrap)]
+        let checksum_final = (!checksum).wrapping_add(1) as i8;
+        mpc_table.0.checksum = checksum_final;
         mem.write_obj(mpc_table, table_base)
             .map_err(|_| MptableError::WriteMpcTable)?;
     }
diff --git a/src/vmm/src/devices/virtio/net/device.rs b/src/vmm/src/devices/virtio/net/device.rs
@@ -196,7 +196,7 @@ impl Net {
         )
         .map_err(NetError::TapSetOffload)?;
 
-        let vnet_hdr_size = vnet_hdr_len() as i32;
+        let vnet_hdr_size = i32::try_from(vnet_hdr_len()).unwrap();
         tap.set_vnet_hdr_size(vnet_hdr_size)
             .map_err(NetError::TapSetVnetHdrSize)?;
 
diff --git a/src/vmm/src/devices/virtio/net/tap.rs b/src/vmm/src/devices/virtio/net/tap.rs
@@ -193,7 +193,7 @@ impl Tap {
 
     /// Write an `IoVecBuffer` to tap
     pub(crate) fn write_iovec(&mut self, buffer: &IoVecBuffer) -> Result<usize, IoError> {
-        let iovcnt = buffer.iovec_count() as i32;
+        let iovcnt = i32::try_from(buffer.iovec_count()).unwrap();
         let iov = buffer.as_iovec_ptr();
 
         // SAFETY: `writev` is safe. Called with a valid tap fd, the iovec pointer and length
diff --git a/src/vmm/src/io_uring/mod.rs b/src/vmm/src/io_uring/mod.rs
@@ -17,7 +17,7 @@ use std::io::Error as IOError;
 use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
 
 use bindings::io_uring_params;
-use operation::{Cqe, OpCode, Operation};
+use operation::{Cqe, FixedFd, OpCode, Operation};
 use probe::{ProbeWrapper, PROBE_LEN};
 pub use queue::completion::CQueueError;
 use queue::completion::CompletionQueue;
@@ -43,7 +43,7 @@ pub enum IoUringError {
     /// The number of ops in the ring is >= CQ::count
     FullCQueue,
     /// Fd was not registered.
-    InvalidFixedFd(i32),
+    InvalidFixedFd(FixedFd),
     /// There are no registered fds.
     NoRegisteredFds,
     /// Error probing the io_uring subsystem.
@@ -165,12 +165,10 @@ impl IoUring {
     /// It's up to the caller to make sure that this value is ever freed (not leaked).
     pub unsafe fn push<T: Debug>(&mut self, op: Operation<T>) -> Result<(), (IoUringError, T)> {
         // validate that we actually did register fds
-        let fd = op.fd() as i32;
+        let fd = op.fd();
         match self.registered_fds_count {
             0 => Err((IoUringError::NoRegisteredFds, op.user_data())),
-            len if fd < 0 || (len as i32 - 1) < fd => {
-                Err((IoUringError::InvalidFixedFd(fd), op.user_data()))
-            }
+            len if fd >= len => Err((IoUringError::InvalidFixedFd(fd), op.user_data())),
             _ => {
                 if self.num_ops >= self.cqueue.count() {
                     return Err((IoUringError::FullCQueue, op.user_data()));
@@ -527,7 +525,7 @@ mod tests {
                                         write_mem_region.as_ptr().add(operation.addr.unwrap())
                                             as *const libc::c_void,
                                         operation.len.unwrap() as usize,
-                                        operation.offset.unwrap() as i64,
+                                        i64::try_from(operation.offset.unwrap()).unwrap(),
                                     ) as libc::c_int
                                 })
                                 .into_result()
@@ -543,7 +541,7 @@ mod tests {
                                             .add(operation.addr.unwrap())
                                             .cast::<libc::c_void>(),
                                         operation.len.unwrap() as usize,
-                                        operation.offset.unwrap() as i64,
+                                        i64::try_from(operation.offset.unwrap()).unwrap(),
                                     ) as libc::c_int
                                 })
                                 .into_result()
diff --git a/src/vmm/src/io_uring/operation/mod.rs b/src/vmm/src/io_uring/operation/mod.rs
@@ -136,7 +136,7 @@ impl<T: Debug> Operation<T> {
         let mut inner: io_uring_sqe = std::mem::zeroed();
 
         inner.opcode = self.opcode as u8;
-        inner.fd = self.fd as i32;
+        inner.fd = i32::try_from(self.fd).unwrap();
         // Simplifying assumption that we only used pre-registered FDs.
         inner.flags = self.flags | (1 << IOSQE_FIXED_FILE_BIT);
 
diff --git a/src/vmm/src/vstate/system.rs b/src/vmm/src/vstate/system.rs
@@ -47,6 +47,7 @@ impl KvmContext {
         let kvm = Kvm::new()?;
 
         // Check that KVM has the correct version.
+        #[allow(clippy::cast_possible_wrap)] // This is a constant
         if kvm.get_api_version() != KVM_API_VERSION as i32 {
             return Err(SystemError::ApiVersion(kvm.get_api_version()));
         }

Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,7 @@ rustflags = [`
`5`	`5`	`"-Dclippy::ptr_as_ptr",`
`6`	`6`	`"-Dclippy::undocumented_unsafe_blocks",`
`7`	`7`	`"-Dclippy::cast_lossless",`
	`8`	`+ "-Dclippy::cast_possible_wrap",`
`8`	`9`	`"-Dclippy::cast_sign_loss",`
`9`	`10`	`"-Dmissing_debug_implementations"`
`10`	`11`	`]`
Original file line number	Diff line number	Diff line change
`@@ -1148,7 +1148,8 @@ impl SerializeToUtcTimestampMs {`
`1148`	`1148`	`impl Serialize for SerializeToUtcTimestampMs {`
`1149`	`1149`	`fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {`
`1150`	`1150`	`serializer.serialize_i64(`
`1151`		`- utils::time::get_time_ns(utils::time::ClockType::Real) as i64 / 1_000_000,`
	`1151`	`+ i64::try_from(utils::time::get_time_ns(utils::time::ClockType::Real) / 1_000_000)`
	`1152`	`+ .unwrap(),`
`1152`	`1153`	`)`
`1153`	`1154`	`}`
`1154`	`1155`	`}`
Original file line number	Diff line number	Diff line change
`@@ -181,7 +181,7 @@ pub fn get_time_ms(clock_type: ClockType) -> u64 {`
`181`	`181`	`///`
`182`	`182`	/// * `value` - Timestamp in seconds.
`183`	`183`	`pub fn seconds_to_nanoseconds(value: i64) -> Option<i64> {`
`184`		`- value.checked_mul(NANOS_PER_SECOND as i64)`
	`184`	`+ value.checked_mul(i64::try_from(NANOS_PER_SECOND).unwrap())`
`185`	`185`	`}`
`186`	`186`
`187`	`187`	`#[cfg(test)]`
Original file line number	Diff line number	Diff line change
`@@ -86,7 +86,7 @@ fn build_guarded_region(`
`86`	`86`	`prot,`
`87`	`87`	`flags \| libc::MAP_FIXED,`
`88`	`88`	`fd,`
`89`		`- offset as libc::off_t,`
	`89`	`+ libc::off_t::try_from(offset).unwrap(),`
`90`	`90`	`)`
`91`	`91`	`};`
`92`	`92`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ fn get_klapic_reg(klapic: &kvm_lapic_state, reg_offset: usize) -> u32 {`
`35`	`35`	`fn set_klapic_reg(klapic: &mut kvm_lapic_state, reg_offset: usize, value: u32) {`
`36`	`36`	`let range = reg_offset..reg_offset + 4;`
`37`	`37`	`let reg = klapic.regs.get_mut(range).expect("set_klapic_reg range");`
`38`		`- byte_order::write_le_i32(reg, value as i32)`
	`38`	`+ byte_order::write_le_u32_to_i8(reg, value)`
`39`	`39`	`}`
`40`	`40`
`41`	`41`	`fn set_apic_delivery_mode(reg: u32, mode: u32) -> u32 {`
Original file line number	Diff line number	Diff line change
`@@ -196,7 +196,7 @@ impl Net {`
`196`	`196`	`)`
`197`	`197`	`.map_err(NetError::TapSetOffload)?;`
`198`	`198`
`199`		`- let vnet_hdr_size = vnet_hdr_len() as i32;`
	`199`	`+ let vnet_hdr_size = i32::try_from(vnet_hdr_len()).unwrap();`
`200`	`200`	`tap.set_vnet_hdr_size(vnet_hdr_size)`
`201`	`201`	`.map_err(NetError::TapSetVnetHdrSize)?;`
`202`	`202`