Fix - Upcoming arithmetic_side_effects lints (#33000)

* dereplicode address alignment check

* Uses `checked_div` and `checked_rem` in built-in loaders.

* Uses `checked_div` and `checked_rem`.

* sdk: replace sub() with saturating_sub()

* eliminate `String` "arithmetic"

* allow arithmetic side-effects in tests and benches and on types we don't control

---------

Co-authored-by: Trent Nelson <trent@solana.com>

bloom/src/bloom.rs

@@ -101,7 +101,9 @@ impl<T: BloomHashIndex> Bloom<T> {
         }
     }
     fn pos(&self, key: &T, k: u64) -> u64 {
-        key.hash_at_index(k).wrapping_rem(self.bits.len())
+        key.hash_at_index(k)
+            .checked_rem(self.bits.len())
+            .unwrap_or(0)
     }
     pub fn clear(&mut self) {
         self.bits = BitVec::new_fill(false, self.bits.len());
@@ -164,7 +166,10 @@ impl<T: BloomHashIndex> From<Bloom<T>> for AtomicBloom<T> {
 
 impl<T: BloomHashIndex> AtomicBloom<T> {
     fn pos(&self, key: &T, hash_index: u64) -> (usize, u64) {
-        let pos = key.hash_at_index(hash_index).wrapping_rem(self.num_bits);
+        let pos = key
+            .hash_at_index(hash_index)
+            .checked_rem(self.num_bits)
+            .unwrap_or(0);
         // Divide by 64 to figure out which of the
         // AtomicU64 bit chunks we need to modify.
         let index = pos.wrapping_shr(6);

perf/src/perf_libs.rs

@@ -140,9 +140,11 @@ fn find_cuda_home(perf_libs_path: &Path) -> Option<PathBuf> {
     None
 }
 
-pub fn append_to_ld_library_path(path: String) {
-    let ld_library_path =
-        path + ":" + &env::var("LD_LIBRARY_PATH").unwrap_or_else(|_| "".to_string());
+pub fn append_to_ld_library_path(mut ld_library_path: String) {
+    if let Ok(env_value) = env::var("LD_LIBRARY_PATH") {
+        ld_library_path.push(':');
+        ld_library_path.push_str(&env_value);
+    }
     info!("setting ld_library_path to: {:?}", ld_library_path);
     env::set_var("LD_LIBRARY_PATH", ld_library_path);
 }

perf/src/sigverify.rs

@@ -614,13 +614,15 @@ pub fn ed25519_verify(
     // power-of-two number around that accounting for the fact that the CPU
     // may be busy doing other things while being a real validator
     // TODO: dynamically adjust this crossover
-    if valid_packet_count < 64
-        || 100usize
-            .wrapping_mul(valid_packet_count)
-            .wrapping_div(total_packet_count)
-            < 90
-    {
-        return ed25519_verify_cpu(batches, reject_non_vote, valid_packet_count);
+    let maybe_valid_percentage = 100usize
+        .wrapping_mul(valid_packet_count)
+        .checked_div(total_packet_count);
+    let Some(valid_percentage) = maybe_valid_percentage else {
+        return;
+    };
+    if valid_percentage < 90 || valid_packet_count < 64 {
+        ed25519_verify_cpu(batches, reject_non_vote, valid_packet_count);
+        return;
     }
 
     let (signature_offsets, pubkey_offsets, msg_start_offsets, msg_sizes, sig_lens) =

program-runtime/src/prioritization_fee.rs

@@ -20,26 +20,23 @@ impl PrioritizationFeeDetails {
         match fee_type {
             // TODO: remove support of 'Deprecated' after feature remove_deprecated_request_unit_ix::id() is activated
             PrioritizationFeeType::Deprecated(fee) => {
-                let priority = if compute_unit_limit == 0 {
-                    0
-                } else {
-                    let micro_lamport_fee: MicroLamports =
-                        (fee as u128).saturating_mul(MICRO_LAMPORTS_PER_LAMPORT as u128);
-                    let priority = micro_lamport_fee.saturating_div(compute_unit_limit as u128);
-                    u64::try_from(priority).unwrap_or(u64::MAX)
-                };
+                let micro_lamport_fee: MicroLamports =
+                    (fee as u128).saturating_mul(MICRO_LAMPORTS_PER_LAMPORT as u128);
+                let priority = micro_lamport_fee
+                    .checked_div(compute_unit_limit as u128)
+                    .map(|priority| u64::try_from(priority).unwrap_or(u64::MAX))
+                    .unwrap_or(0);
 
                 Self { fee, priority }
             }
             PrioritizationFeeType::ComputeUnitPrice(cu_price) => {
-                let fee = {
-                    let micro_lamport_fee: MicroLamports =
-                        (cu_price as u128).saturating_mul(compute_unit_limit as u128);
-                    let fee = micro_lamport_fee
-                        .saturating_add(MICRO_LAMPORTS_PER_LAMPORT.saturating_sub(1) as u128)
-                        .saturating_div(MICRO_LAMPORTS_PER_LAMPORT as u128);
-                    u64::try_from(fee).unwrap_or(u64::MAX)
-                };
+                let micro_lamport_fee: MicroLamports =
+                    (cu_price as u128).saturating_mul(compute_unit_limit as u128);
+                let fee = micro_lamport_fee
+                    .saturating_add(MICRO_LAMPORTS_PER_LAMPORT.saturating_sub(1) as u128)
+                    .checked_div(MICRO_LAMPORTS_PER_LAMPORT as u128)
+                    .and_then(|fee| u64::try_from(fee).ok())
+                    .unwrap_or(u64::MAX);
 
                 Self {
                     fee,

programs/bpf_loader/src/lib.rs

@@ -183,7 +183,8 @@ pub fn calculate_heap_cost(heap_size: u64, heap_cost: u64, enable_rounding_fix:
             .saturating_add(PAGE_SIZE_KB.saturating_mul(KIBIBYTE).saturating_sub(1));
     }
     rounded_heap_size
-        .saturating_div(PAGE_SIZE_KB.saturating_mul(KIBIBYTE))
+        .checked_div(PAGE_SIZE_KB.saturating_mul(KIBIBYTE))
+        .expect("PAGE_SIZE_KB * KIBIBYTE > 0")
         .saturating_sub(1)
         .saturating_mul(heap_cost)
 }

programs/bpf_loader/src/syscalls/cpi.rs

@@ -82,7 +82,8 @@ impl<'a> CallerAccount<'a> {
             consume_compute_meter(
                 invoke_context,
                 (data.len() as u64)
-                    .saturating_div(invoke_context.get_compute_budget().cpi_bytes_per_unit),
+                    .checked_div(invoke_context.get_compute_budget().cpi_bytes_per_unit)
+                    .unwrap_or(u64::MAX),
             )?;
 
             let translated = translate(
@@ -181,7 +182,8 @@ impl<'a> CallerAccount<'a> {
             invoke_context,
             account_info
                 .data_len
-                .saturating_div(invoke_context.get_compute_budget().cpi_bytes_per_unit),
+                .checked_div(invoke_context.get_compute_budget().cpi_bytes_per_unit)
+                .unwrap_or(u64::MAX),
         )?;
 
         let bpf_account_data_direct_mapping = invoke_context
@@ -333,7 +335,8 @@ impl SyscallInvokeSigned for SyscallInvokeSignedRust {
             consume_compute_meter(
                 invoke_context,
                 (ix_data_len)
-                    .saturating_div(invoke_context.get_compute_budget().cpi_bytes_per_unit),
+                    .checked_div(invoke_context.get_compute_budget().cpi_bytes_per_unit)
+                    .unwrap_or(u64::MAX),
             )?;
         }
 
@@ -551,7 +554,8 @@ impl SyscallInvokeSigned for SyscallInvokeSignedC {
             consume_compute_meter(
                 invoke_context,
                 (ix_data_len)
-                    .saturating_div(invoke_context.get_compute_budget().cpi_bytes_per_unit),
+                    .checked_div(invoke_context.get_compute_budget().cpi_bytes_per_unit)
+                    .unwrap_or(u64::MAX),
             )?;
         }
 
@@ -756,7 +760,8 @@ where
             consume_compute_meter(
                 invoke_context,
                 (callee_account.get_data().len() as u64)
-                    .saturating_div(invoke_context.get_compute_budget().cpi_bytes_per_unit),
+                    .checked_div(invoke_context.get_compute_budget().cpi_bytes_per_unit)
+                    .unwrap_or(u64::MAX),
             )?;
 
             accounts.push((instruction_account.index_in_caller, None));

programs/bpf_loader/src/syscalls/mem_ops.rs

@@ -7,9 +7,10 @@ use {
 
 fn mem_op_consume(invoke_context: &mut InvokeContext, n: u64) -> Result<(), Error> {
     let compute_budget = invoke_context.get_compute_budget();
-    let cost = compute_budget
-        .mem_op_base_cost
-        .max(n.saturating_div(compute_budget.cpi_bytes_per_unit));
+    let cost = compute_budget.mem_op_base_cost.max(
+        n.checked_div(compute_budget.cpi_bytes_per_unit)
+            .unwrap_or(u64::MAX),
+    );
     consume_compute_meter(invoke_context, cost)
 }
 

programs/bpf_loader/src/syscalls/mod.rs

@@ -331,6 +331,13 @@ pub fn create_program_runtime_environment_v1<'a>(
     Ok(result)
 }
 
+fn address_is_aligned<T>(address: u64) -> bool {
+    (address as *mut T as usize)
+        .checked_rem(align_of::<T>())
+        .map(|rem| rem == 0)
+        .expect("T to be non-zero aligned")
+}
+
 fn translate(
     memory_mapping: &MemoryMapping,
     access_type: AccessType,
@@ -348,7 +355,7 @@ fn translate_type_inner<'a, T>(
 ) -> Result<&'a mut T, Error> {
     let host_addr = translate(memory_mapping, access_type, vm_addr, size_of::<T>() as u64)?;
 
-    if check_aligned && (host_addr as *mut T as usize).wrapping_rem(align_of::<T>()) != 0 {
+    if check_aligned && !address_is_aligned::<T>(host_addr) {
         return Err(SyscallError::UnalignedPointer.into());
     }
     Ok(unsafe { &mut *(host_addr as *mut T) })
@@ -388,7 +395,7 @@ fn translate_slice_inner<'a, T>(
 
     let host_addr = translate(memory_mapping, access_type, vm_addr, total_size)?;
 
-    if check_aligned && (host_addr as *mut T as usize).wrapping_rem(align_of::<T>()) != 0 {
+    if check_aligned && !address_is_aligned::<T>(host_addr) {
         return Err(SyscallError::UnalignedPointer.into());
     }
     Ok(unsafe { from_raw_parts_mut(host_addr as *mut T, len as usize) })
@@ -761,9 +768,11 @@ declare_syscall!(
                     invoke_context.get_check_size(),
                 )?;
                 let cost = compute_budget.mem_op_base_cost.max(
-                    compute_budget
-                        .sha256_byte_cost
-                        .saturating_mul((val.len() as u64).saturating_div(2)),
+                    compute_budget.sha256_byte_cost.saturating_mul(
+                        (val.len() as u64)
+                            .checked_div(2)
+                            .expect("div by non-zero literal"),
+                    ),
                 );
                 consume_compute_meter(invoke_context, cost)?;
                 hasher.hash(bytes);
@@ -824,9 +833,11 @@ declare_syscall!(
                     invoke_context.get_check_size(),
                 )?;
                 let cost = compute_budget.mem_op_base_cost.max(
-                    compute_budget
-                        .sha256_byte_cost
-                        .saturating_mul((val.len() as u64).saturating_div(2)),
+                    compute_budget.sha256_byte_cost.saturating_mul(
+                        (val.len() as u64)
+                            .checked_div(2)
+                            .expect("div by non-zero literal"),
+                    ),
                 );
                 consume_compute_meter(invoke_context, cost)?;
                 hasher.hash(bytes);
@@ -1321,9 +1332,11 @@ declare_syscall!(
                     invoke_context.get_check_size(),
                 )?;
                 let cost = compute_budget.mem_op_base_cost.max(
-                    compute_budget
-                        .sha256_byte_cost
-                        .saturating_mul((val.len() as u64).saturating_div(2)),
+                    compute_budget.sha256_byte_cost.saturating_mul(
+                        (val.len() as u64)
+                            .checked_div(2)
+                            .expect("div by non-zero literal"),
+                    ),
                 );
                 consume_compute_meter(invoke_context, cost)?;
                 hasher.hash(bytes);
@@ -1349,7 +1362,8 @@ declare_syscall!(
         let budget = invoke_context.get_compute_budget();
 
         let cost = len
-            .saturating_div(budget.cpi_bytes_per_unit)
+            .checked_div(budget.cpi_bytes_per_unit)
+            .unwrap_or(u64::MAX)
             .saturating_add(budget.syscall_base_cost);
         consume_compute_meter(invoke_context, cost)?;
 
@@ -1403,7 +1417,8 @@ declare_syscall!(
         if length != 0 {
             let cost = length
                 .saturating_add(size_of::<Pubkey>() as u64)
-                .saturating_div(budget.cpi_bytes_per_unit);
+                .checked_div(budget.cpi_bytes_per_unit)
+                .unwrap_or(u64::MAX);
             consume_compute_meter(invoke_context, cost)?;
 
             let return_data_result = translate_slice_mut::<u8>(
@@ -1636,7 +1651,9 @@ declare_syscall!(
                 ALT_BN128_MULTIPLICATION_OUTPUT_LEN,
             ),
             ALT_BN128_PAIRING => {
-                let ele_len = input_size.saturating_div(ALT_BN128_PAIRING_ELEMENT_LEN as u64);
+                let ele_len = input_size
+                    .checked_div(ALT_BN128_PAIRING_ELEMENT_LEN as u64)
+                    .expect("div by non-zero constant");
                 let cost = budget
                     .alt_bn128_pairing_one_pair_cost_first
                     .saturating_add(
@@ -1732,7 +1749,8 @@ declare_syscall!(
             budget.syscall_base_cost.saturating_add(
                 input_len
                     .saturating_mul(input_len)
-                    .saturating_div(budget.big_modular_exponentiation_cost),
+                    .checked_div(budget.big_modular_exponentiation_cost)
+                    .unwrap_or(u64::MAX),
             ),
         )?;
 
@@ -2405,10 +2423,7 @@ mod tests {
             );
             let address = result.unwrap();
             assert_ne!(address, 0);
-            assert_eq!(
-                (address as *const u8 as usize).wrapping_rem(align_of::<T>()),
-                0
-            );
+            assert!(address_is_aligned::<T>(address));
         }
         aligned::<u8>();
         aligned::<u16>();
@@ -4032,4 +4047,11 @@ mod tests {
         let size = val.len().wrapping_mul(mem::size_of::<T>());
         unsafe { slice::from_raw_parts_mut(val.as_mut_ptr().cast(), size) }
     }
+
+    #[test]
+    fn test_address_is_aligned() {
+        for address in 0..std::mem::size_of::<u64>() {
+            assert_eq!(address_is_aligned::<u64>(address as u64), address == 0);
+        }
+    }
 }

programs/loader-v4/src/lib.rs

@@ -107,7 +107,8 @@ fn calculate_heap_cost(heap_size: u64, heap_cost: u64) -> u64 {
     const PAGE_SIZE_KB: u64 = 32;
     heap_size
         .saturating_add(PAGE_SIZE_KB.saturating_mul(KIBIBYTE).saturating_sub(1))
-        .saturating_div(PAGE_SIZE_KB.saturating_mul(KIBIBYTE))
+        .checked_div(PAGE_SIZE_KB.saturating_mul(KIBIBYTE))
+        .expect("PAGE_SIZE_KB * KIBIBYTE > 0")
         .saturating_sub(1)
         .saturating_mul(heap_cost)
 }

programs/sbf/build.rs

@@ -42,10 +42,11 @@ fn main() {
         return;
     }
 
+    let build_profile = env::var("PROFILE").expect("`PROFILE` envvar to be set");
+    let install_dir = format!("target/{build_profile}/sbf");
     let sbf_c = env::var("CARGO_FEATURE_SBF_C").is_ok();
     if sbf_c {
-        let install_dir = "OUT_DIR=../target/".to_string() + &env::var("PROFILE").unwrap() + "/sbf";
-
+        let install_dir = format!("OUT_DIR=../{install_dir}");
         println!("cargo:warning=(not a warning) Building C-based on-chain programs");
         assert!(Command::new("make")
             .current_dir("c")
@@ -60,8 +61,6 @@ fn main() {
 
     let sbf_rust = env::var("CARGO_FEATURE_SBF_RUST").is_ok();
     if sbf_rust {
-        let install_dir = "target/".to_string() + &env::var("PROFILE").unwrap() + "/sbf";
-
         let rust_programs = [
             "128bit",
             "alloc",

programs/sbf/rust/secp256k1_recover/src/lib.rs

@@ -1,3 +1,4 @@
+#![allow(clippy::arithmetic_side_effects)]
 //! Secp256k1Recover Syscall test
 
 extern crate solana_program;

programs/zk-token-proof/benches/verify_proofs.rs

@@ -1,3 +1,4 @@
+#![allow(clippy::arithmetic_side_effects)]
 use {
     criterion::{criterion_group, criterion_main, Criterion},
     curve25519_dalek::scalar::Scalar,

sdk/cargo-build-sbf/Cargo.toml

@@ -13,6 +13,7 @@ edition = { workspace = true }
 bzip2 = { workspace = true }
 cargo_metadata = { workspace = true }
 clap = { version = "3.1.5", features = ["cargo", "env"] }
+itertools = { workspace = true }
 log = { workspace = true, features = ["std"] }
 regex = { workspace = true }
 reqwest = { workspace = true, features = ["blocking", "rustls-tls"] }