Skip to content
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

Warn on clippy::cast_possible_truncation #3557

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
75d8bf1
Bump versionize-derive and vmm-sys-util
JBYoshi Aug 23, 2023
31def53
Shrink the sizes of some constants
JBYoshi Aug 30, 2023
784eae4
[Test code] Shrink the sizes of some constants
JBYoshi Aug 30, 2023
9db57aa
[Test code] Shrink the sizes of some values
JBYoshi Aug 23, 2023
d53caef
Add try_from to usages of generated constants
JBYoshi Aug 23, 2023
d0323d5
Remove casts from SyscallReturnCode
JBYoshi Aug 23, 2023
d6b6cc4
Switch metrics from usize to u64
JBYoshi Aug 30, 2023
7836349
[Test code] Switch metrics from usize to u64
JBYoshi Aug 30, 2023
ec9f2be
Add a u64_to_usize function for safe conversions
JBYoshi Aug 23, 2023
b5e3985
[Test code] Use u64_to_usize in tests
JBYoshi Aug 23, 2023
e6e3c74
Apply bit masks to explicitly allow some wrapping
JBYoshi Aug 23, 2023
3394c24
[Test code] Apply bit masks to allow some wrapping
JBYoshi Sep 6, 2023
22fd438
Update types for (in)equality checks
JBYoshi Aug 23, 2023
0032796
[Test code] Update types for (in)equality checks
JBYoshi Sep 6, 2023
cf37025
Change packet lengths to sized types
JBYoshi Sep 6, 2023
84a6bfa
[Test code] Change packet lengths to sized types
JBYoshi Sep 6, 2023
5a44fb7
Miscellaneous fixes that don't fall anywhere else
JBYoshi Aug 23, 2023
8b95c04
Change u64 to usize in vmm/devices/virtio/queue
JBYoshi Aug 23, 2023
6730d35
Add try_from()/unwrap() in cases where it's safe
JBYoshi Aug 25, 2023
9521787
[Test code] Add try_from()/unwrap() for test code
JBYoshi Aug 23, 2023
fd28c78
Unwraps bounded by device spec limitations
JBYoshi Aug 30, 2023
4a3f660
Add try_from/unwrap for u128 time values
JBYoshi Aug 30, 2023
0acfed8
Add an explicit allow for code covered by proofs
JBYoshi Sep 24, 2023
6db3e92
ARM64: Decrease the sizes of some values
JBYoshi Sep 1, 2023
a3c6985
ARM64: Update types for (in)equality checks
JBYoshi Sep 24, 2023
ec7eea4
[Test code] ARM64: Decrease sizes of some values
JBYoshi Sep 1, 2023
de41752
ARM64: Add bit masks to allow some wrapping
JBYoshi Sep 11, 2023
afa606e
ARM64: Add try_from()/unwrap() in cases where it's safe
JBYoshi Sep 11, 2023
bea150b
[Test code] ARM64: Add try_from()/unwrap() in test
JBYoshi Sep 11, 2023
ca27459
ARM64: Add bounds checks on device reads/writes
JBYoshi Sep 11, 2023
e5304cd
Change interrupt_status to u32
JBYoshi Aug 23, 2023
c207b8f
Enable warnings for clippy::cast_possible_truncation
JBYoshi Aug 30, 2023
d0deb48
[Test code] Fix THC tolerance tests
JBYoshi Oct 8, 2023
225388c
Merge branch 'main' into clippy-cast-possible-truncation
roypat Oct 9, 2023
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 .cargo/config
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ rustflags = [
"-Wclippy::ptr_as_ptr",
"-Wclippy::undocumented_unsafe_blocks",
"-Wclippy::cast_lossless",
"-Wclippy::cast_possible_truncation",
"-Wclippy::cast_possible_wrap",
"-Wclippy::cast_sign_loss",
"-Wmissing_debug_implementations",
Expand Down
6 changes: 3 additions & 3 deletions src/firecracker/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,17 @@ pub mod tests {
let metrics = Arc::new(Mutex::new(PeriodicMetrics::new()));
event_manager.add_subscriber(metrics.clone());

let flush_period_ms = 50;
let flush_period_ms = 50u16;
metrics
.lock()
.expect("Unlock failed.")
.start(flush_period_ms);
.start(u64::from(flush_period_ms));
// .start() does an initial flush.
assert_eq!(metrics.lock().expect("Unlock failed.").flush_counter, 1);

// Wait for at most 1.5x period.
event_manager
.run_with_timeout((flush_period_ms + flush_period_ms / 2) as i32)
.run_with_timeout(i32::from(flush_period_ms) + i32::from(flush_period_ms) / 2)
.expect("Metrics event timeout or error.");
// Verify there was another flush.
assert_eq!(metrics.lock().expect("Unlock failed.").flush_counter, 2);
Expand Down
2 changes: 1 addition & 1 deletion src/jailer/src/chroot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
// SAFETY: Safe because we provide valid parameters.
SyscallReturnCode(unsafe {
libc::syscall(libc::SYS_pivot_root, cwd.as_ptr(), old_root_dir.as_ptr())
} as libc::c_int)
})

Check warning on line 86 in src/jailer/src/chroot.rs

View check run for this annotation

Codecov / codecov/patch

src/jailer/src/chroot.rs#L86

Added line #L86 was not covered by tests
.into_empty_result()
.map_err(JailerError::PivotRoot)?;

Expand Down
24 changes: 16 additions & 8 deletions src/jailer/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,17 +82,25 @@
fn clone(child_stack: *mut libc::c_void, flags: libc::c_int) -> Result<libc::c_int, JailerError> {
// Clone parameters order is different between x86_64 and aarch64.
#[cfg(target_arch = "x86_64")]
// SAFETY: This is safe because we are using a library function with valid parameters.
return SyscallReturnCode(unsafe {
libc::syscall(libc::SYS_clone, flags, child_stack, 0, 0, 0) as libc::c_int
})
return SyscallReturnCode(
// SAFETY: This is safe because we are using a library function with valid parameters.
libc::c_int::try_from(unsafe {
libc::syscall(libc::SYS_clone, flags, child_stack, 0, 0, 0)
})
// Unwrap is needed because PIDs are 32-bit.
.unwrap(),
)

Check warning on line 92 in src/jailer/src/env.rs

View check run for this annotation

Codecov / codecov/patch

src/jailer/src/env.rs#L85-L92

Added lines #L85 - L92 were not covered by tests
.into_result()
.map_err(JailerError::Clone);
#[cfg(target_arch = "aarch64")]
// SAFETY: This is safe because we are using a library function with valid parameters.
return SyscallReturnCode(unsafe {
libc::syscall(libc::SYS_clone, flags, child_stack, 0, 0, 0) as libc::c_int
})
return SyscallReturnCode(
// SAFETY: This is safe because we are using a library function with valid parameters.
libc::c_int::try_from(unsafe {
libc::syscall(libc::SYS_clone, flags, child_stack, 0, 0, 0)
})
// Unwrap is needed because PIDs are 32-bit.
.unwrap(),
)

Check warning on line 103 in src/jailer/src/env.rs

View check run for this annotation

Codecov / codecov/patch

src/jailer/src/env.rs#L96-L103

Added lines #L96 - L103 were not covered by tests
.into_result()
.map_err(JailerError::Clone);
}
Expand Down
2 changes: 1 addition & 1 deletion src/jailer/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ fn close_fds_by_close_range() -> Result<(), JailerError> {
libc::c_uint::MAX,
libc::CLOSE_RANGE_UNSHARE,
)
} as libc::c_int)
})
.into_empty_result()
.map_err(JailerError::CloseRange)
}
Expand Down
3 changes: 2 additions & 1 deletion src/rebase-snap/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::os::unix::io::AsRawFd;

use utils::arg_parser::{ArgParser, Argument, Arguments, Error as ArgError};
use utils::seek_hole::SeekHole;
use utils::u64_to_usize;

const REBASE_SNAP_VERSION: &str = env!("CARGO_PKG_VERSION");
const BASE_FILE: &str = "base-file";
Expand Down Expand Up @@ -101,7 +102,7 @@ fn rebase(base_file: &mut File, diff_file: &mut File) -> Result<(), FileError> {
base_file.as_raw_fd(),
diff_file.as_raw_fd(),
(&mut cursor as *mut u64).cast::<i64>(),
block_end.saturating_sub(cursor) as usize,
u64_to_usize(block_end.saturating_sub(cursor)),
)
};
if num_transferred_bytes < 0 {
Expand Down
15 changes: 9 additions & 6 deletions src/seccompiler/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ impl SeccompCondition {
/// [`SeccompCondition`]: struct.SeccompCondition.html
fn value_segments(&self) -> (u32, u32, u8, u8) {
// Splits the specified value into its most significant and least significant halves.
let (msb, lsb) = ((self.value >> 32) as u32, self.value as u32);
let (msb, lsb) = ((self.value >> 32) as u32, (self.value & 0xFFFFFFFF) as u32);

// Offset to the argument specified by `arg_number`.
// Cannot overflow because the value will be at most 16 + 6 * 8 = 64.
Expand Down Expand Up @@ -431,8 +431,11 @@ impl SeccompCondition {
fn into_masked_eq_bpf(self, offset: u8, mask: u64) -> Vec<sock_filter> {
let (_, _, msb_offset, lsb_offset) = self.value_segments();
let masked_value = self.value & mask;
let (msb, lsb) = ((masked_value >> 32) as u32, masked_value as u32);
let (mask_msb, mask_lsb) = ((mask >> 32) as u32, mask as u32);
let (msb, lsb) = (
(masked_value >> 32) as u32,
(masked_value & 0xFFFFFFFF) as u32,
);
let (mask_msb, mask_lsb) = ((mask >> 32) as u32, (mask & 0xFFFFFFFF) as u32);

let mut bpf = match self.arg_len {
SeccompCmpArgLen::Dword => vec![],
Expand Down Expand Up @@ -725,7 +728,7 @@ impl SeccompFilter {
accumulator.push(built_syscall);

// BPF programs are limited to 4096 statements.
if *filter_len >= BPF_MAX_LEN {
if *filter_len >= usize::from(BPF_MAX_LEN) {
return Err(FilterError::FilterTooLarge);
}

Expand Down Expand Up @@ -781,7 +784,7 @@ impl TryInto<BpfProgram> for SeccompFilter {
.into_iter()
.for_each(|mut instructions| result.append(&mut instructions));

if result.len() >= BPF_MAX_LEN {
if result.len() >= usize::from(BPF_MAX_LEN) {
return Err(FilterError::FilterTooLarge);
}

Expand Down Expand Up @@ -914,7 +917,7 @@ mod tests {
assert_eq!(rc, 0);
}
let bpf_prog = sock_fprog {
len: bpf_filter.len() as u16,
len: u16::try_from(bpf_filter.len()).unwrap(),
filter: bpf_filter.as_ptr(),
};
let bpf_prog_ptr = &bpf_prog as *const sock_fprog;
Expand Down
2 changes: 1 addition & 1 deletion src/seccompiler/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use serde::{Deserialize, Serialize};

/// The maximum seccomp-BPF program length allowed by the linux kernel.
pub const BPF_MAX_LEN: usize = 4096;
pub const BPF_MAX_LEN: u16 = 4096;

/// BPF instruction structure definition.
/// See /usr/include/linux/filter.h .
Expand Down
6 changes: 4 additions & 2 deletions src/seccompiler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ pub fn apply_filter(bpf_filter: BpfProgramRef) -> std::result::Result<(), Instal

// If the program length is greater than the limit allowed by the kernel,
// fail quickly. Otherwise, `prctl` will give a more cryptic error code.
if bpf_filter.len() > BPF_MAX_LEN {
let bpf_filter_len =
u16::try_from(bpf_filter.len()).map_err(|_| InstallationError::FilterTooLarge)?;
if bpf_filter_len > BPF_MAX_LEN {
return Err(InstallationError::FilterTooLarge);
}

Expand All @@ -103,7 +105,7 @@ pub fn apply_filter(bpf_filter: BpfProgramRef) -> std::result::Result<(), Instal
}

let bpf_prog = sock_fprog {
len: bpf_filter.len() as u16,
len: bpf_filter_len,
filter: bpf_filter.as_ptr(),
};
let bpf_prog_ptr = &bpf_prog as *const sock_fprog;
Expand Down
7 changes: 4 additions & 3 deletions src/snapshot-editor/src/edit_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::path::PathBuf;

use clap::Subcommand;
use fc_utils::seek_hole::SeekHole;
use fc_utils::u64_to_usize;

#[derive(Debug, thiserror::Error, displaydoc::Display)]
pub enum EditMemoryError {
Expand Down Expand Up @@ -89,7 +90,7 @@ fn rebase(memory_path: PathBuf, diff_path: PathBuf) -> Result<(), EditMemoryErro
base_file.as_raw_fd(),
diff_file.as_raw_fd(),
(&mut cursor as *mut u64).cast::<i64>(),
block_end.saturating_sub(cursor) as usize,
u64_to_usize(block_end.saturating_sub(cursor)),
)
};
if num_transferred_bytes < 0 {
Expand All @@ -113,8 +114,8 @@ mod tests {

fn check_file_content(file: &File, expected_content: &[u8]) {
assert_eq!(
file.metadata().unwrap().len() as usize,
expected_content.len()
file.metadata().unwrap().len(),
expected_content.len() as u64
);
let mut buf = vec![0u8; expected_content.len()];
file.read_exact_at(buf.as_mut_slice(), 0).unwrap();
Expand Down
3 changes: 2 additions & 1 deletion src/snapshot-editor/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
use std::fs::{File, OpenOptions};
use std::path::PathBuf;

use fc_utils::u64_to_usize;
use snapshot::Snapshot;
use vmm::persist::MicrovmState;
use vmm::version_map::VERSION_MAP;
Expand All @@ -29,7 +30,7 @@
let version_map = VERSION_MAP.clone();
let mut snapshot_reader = File::open(snapshot_path).map_err(UtilsError::VmStateFileOpen)?;
let metadata = std::fs::metadata(snapshot_path).map_err(UtilsError::VmStateFileMeta)?;
let snapshot_len = metadata.len() as usize;
let snapshot_len = u64_to_usize(metadata.len());

Check warning on line 33 in src/snapshot-editor/src/utils.rs

View check run for this annotation

Codecov / codecov/patch

src/snapshot-editor/src/utils.rs#L33

Added line #L33 was not covered by tests
Snapshot::load(&mut snapshot_reader, snapshot_len, version_map).map_err(UtilsError::VmStateLoad)
}

Expand Down
2 changes: 1 addition & 1 deletion src/snapshot/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ bench = false
[dependencies]
libc = "0.2.117"
versionize = "0.1.10"
versionize_derive = "0.1.5"
versionize_derive = "0.1.6"
thiserror = "1.0.32"
displaydoc = "0.2.4"

Expand Down
4 changes: 2 additions & 2 deletions src/utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ serde = { version = "1.0.165", features = ["derive"] }
thiserror = "1.0.32"
displaydoc = "0.2.4"
versionize = "0.1.10"
versionize_derive = "0.1.5"
vmm-sys-util = "0.11.0"
versionize_derive = "0.1.6"
vmm-sys-util = "0.11.2"
vm-memory = { version = "0.12.0", features = ["backend-mmap", "backend-bitmap"] }

net_gen = { path = "../net_gen" }
Expand Down
16 changes: 16 additions & 0 deletions src/utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub mod time;
pub mod validators;
pub mod vm_memory;

use std::num::Wrapping;
use std::result::Result;

/// Return the default page size of the platform, in bytes.
Expand All @@ -31,3 +32,18 @@ pub fn get_page_size() -> Result<usize, errno::Error> {
ps => Ok(usize::try_from(ps).unwrap()),
}
}

/// Safely converts a u64 value to a usize value.
/// This bypasses the Clippy lint check because we only support 64-bit platforms.
#[cfg(target_pointer_width = "64")]
#[inline]
#[allow(clippy::cast_possible_truncation)]
pub const fn u64_to_usize(num: u64) -> usize {
num as usize
}

/// Converts a usize into a wrapping u32.
#[inline]
pub const fn wrap_usize_to_u32(num: usize) -> Wrapping<u32> {
Wrapping(((num as u64) & 0xFFFFFFFF) as u32)
}
12 changes: 6 additions & 6 deletions src/utils/src/net/mac.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ use versionize::{VersionMap, Versionize, VersionizeResult};
use versionize_derive::Versionize;

/// The number of tuples (the ones separated by ":") contained in a MAC address.
pub const MAC_ADDR_LEN: usize = 6;
pub const MAC_ADDR_LEN: u8 = 6;

/// Represents a MAC address
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Versionize)]
/// Representation of a MAC address.
pub struct MacAddr {
bytes: [u8; MAC_ADDR_LEN],
bytes: [u8; MAC_ADDR_LEN as usize],
}

impl fmt::Display for MacAddr {
Expand Down Expand Up @@ -69,13 +69,13 @@ impl FromStr for MacAddr {
/// ```
fn from_str(s: &str) -> Result<Self, Self::Err> {
let v: Vec<&str> = s.split(':').collect();
let mut bytes = [0u8; MAC_ADDR_LEN];
let mut bytes = [0u8; MAC_ADDR_LEN as usize];

if v.len() != MAC_ADDR_LEN {
if v.len() != MAC_ADDR_LEN as usize {
return Err(String::from(s));
}

for i in 0..MAC_ADDR_LEN {
for i in 0..MAC_ADDR_LEN as usize {
if v[i].len() != 2 {
return Err(String::from(s));
}
Expand Down Expand Up @@ -103,7 +103,7 @@ impl MacAddr {
pub fn from_bytes_unchecked(src: &[u8]) -> MacAddr {
// TODO: using something like std::mem::uninitialized could avoid the extra initialization,
// if this ever becomes a performance bottleneck.
let mut bytes = [0u8; MAC_ADDR_LEN];
let mut bytes = [0u8; MAC_ADDR_LEN as usize];
bytes[..].copy_from_slice(src);

MacAddr { bytes }
Expand Down
6 changes: 4 additions & 2 deletions src/utils/src/vm_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ pub use vm_memory::{
VolatileMemory, VolatileMemoryError, VolatileSlice,
};

use crate::u64_to_usize;

pub type GuestMemoryMmap = vm_memory::GuestMemoryMmap<Option<AtomicBitmap>>;
pub type GuestRegionMmap = vm_memory::GuestRegionMmap<Option<AtomicBitmap>>;
pub type GuestMmapRegion = vm_memory::MmapRegion<Option<AtomicBitmap>>;
Expand Down Expand Up @@ -136,7 +138,7 @@ pub fn create_guest_memory(
pub fn mark_dirty_mem(mem: &GuestMemoryMmap, addr: GuestAddress, len: usize) {
let _ = mem.try_access(len, addr, |_total, count, caddr, region| {
if let Some(bitmap) = region.bitmap() {
bitmap.mark_dirty(caddr.0 as usize, count);
bitmap.mark_dirty(u64_to_usize(caddr.0), count);
}
Ok(count)
});
Expand Down Expand Up @@ -692,7 +694,7 @@ mod tests {
*len,
GuestAddress(*addr as u64),
|_total, count, caddr, region| {
let offset = caddr.0 as usize;
let offset = usize::try_from(caddr.0).unwrap();
let bitmap = region.bitmap().as_ref().unwrap();
for i in offset..offset + count {
assert_eq!(bitmap.dirty_at(i), *dirty);
Expand Down
2 changes: 1 addition & 1 deletion src/vmm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ thiserror = "1.0.32"
displaydoc = "0.2.4"
userfaultfd = "0.7.0"
versionize = "0.1.10"
versionize_derive = "0.1.5"
versionize_derive = "0.1.6"
vm-allocator = "0.1.0"
vm-fdt = "0.2.0"
vm-superio = "0.7.0"
Expand Down
21 changes: 12 additions & 9 deletions src/vmm/src/arch/aarch64/cache_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub(crate) struct CacheEntry {
pub level: u8,
// Type of cache: Unified, Data, Instruction.
pub type_: CacheType,
pub size_: Option<usize>,
pub size_: Option<u32>,
pub number_of_sets: Option<u16>,
pub line_size: Option<u16>,
// How many CPUS share this cache.
Expand Down Expand Up @@ -211,12 +211,12 @@ fn readln_special<T: AsRef<Path>>(file_path: &T) -> Result<String, CacheInfoErro
Ok(line.trim_end().to_string())
}

fn to_bytes(cache_size_pretty: &mut String) -> Result<usize, CacheInfoError> {
fn to_bytes(cache_size_pretty: &mut String) -> Result<u32, CacheInfoError> {
match cache_size_pretty.pop() {
Some('K') => Ok(cache_size_pretty.parse::<usize>().map_err(|err| {
Some('K') => Ok(cache_size_pretty.parse::<u32>().map_err(|err| {
CacheInfoError::InvalidCacheAttr("size".to_string(), err.to_string())
})? * 1024),
Some('M') => Ok(cache_size_pretty.parse::<usize>().map_err(|err| {
Some('M') => Ok(cache_size_pretty.parse::<u32>().map_err(|err| {
CacheInfoError::InvalidCacheAttr("size".to_string(), err.to_string())
})? * 1024
* 1024),
Expand Down Expand Up @@ -248,11 +248,14 @@ fn mask_str2bit_count(mask_str: &str) -> Result<u16, CacheInfoError> {
if s_zero_free.is_empty() {
s_zero_free = "0";
}
bit_count += u32::from_str_radix(s_zero_free, 16)
.map_err(|err| {
CacheInfoError::InvalidCacheAttr("shared_cpu_map".to_string(), err.to_string())
})?
.count_ones() as u16;
bit_count += u16::try_from(
u32::from_str_radix(s_zero_free, 16)
.map_err(|err| {
CacheInfoError::InvalidCacheAttr("shared_cpu_map".to_string(), err.to_string())
})?
.count_ones(),
roypat marked this conversation as resolved.
Show resolved Hide resolved
)
.unwrap(); // Safe because this is at most 32
}
if bit_count == 0 {
return Err(CacheInfoError::InvalidCacheAttr(
Expand Down
Loading