diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index ca380ee3a93749..91b3667d25e861 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -5708,6 +5708,7 @@ version = "2.1.0" dependencies = [ "agave-validator", "bincode", + "borsh 1.5.1", "byteorder 1.5.0", "elf", "itertools 0.10.5", @@ -5790,6 +5791,14 @@ dependencies = [ "solana-program", ] +[[package]] +name = "solana-sbf-rust-call-args" +version = "2.1.0" +dependencies = [ + "borsh 1.5.1", + "solana-program", +] + [[package]] name = "solana-sbf-rust-call-depth" version = "2.1.0" diff --git a/programs/sbf/Cargo.toml b/programs/sbf/Cargo.toml index 44ca0165600fd1..4c21253716b46a 100644 --- a/programs/sbf/Cargo.toml +++ b/programs/sbf/Cargo.toml @@ -11,6 +11,7 @@ edition = "2021" array-bytes = "=1.4.1" bincode = { version = "1.1.4", default-features = false } blake3 = "1.0.0" +borsh = "1.5.1" byteorder = "1.3.2" elf = "0.0.10" getrandom = "0.2.10" @@ -90,6 +91,7 @@ frozen-abi = [] [dev-dependencies] agave-validator = { workspace = true } bincode = { workspace = true } +borsh = { workspace = true } byteorder = { workspace = true } elf = { workspace = true } itertools = { workspace = true } @@ -131,6 +133,7 @@ members = [ "rust/alt_bn128", "rust/alt_bn128_compression", "rust/big_mod_exp", + "rust/call_args", "rust/call_depth", "rust/caller_access", "rust/curve25519", diff --git a/programs/sbf/rust/call_args/Cargo.toml b/programs/sbf/rust/call_args/Cargo.toml new file mode 100644 index 00000000000000..82f81f04a457e1 --- /dev/null +++ b/programs/sbf/rust/call_args/Cargo.toml @@ -0,0 +1,16 @@ +[package] +name = "solana-sbf-rust-call-args" +version = { workspace = true } +description = { workspace = true } +authors = { workspace = true } +repository = { workspace = true } +homepage = { workspace = true } +license = { workspace = true } +edition = { workspace = true } + +[dependencies] +borsh = { workspace = true } +solana-program = { workspace = true } + +[lib] +crate-type = ["cdylib"] diff --git a/programs/sbf/rust/call_args/src/lib.rs b/programs/sbf/rust/call_args/src/lib.rs new file mode 100644 index 00000000000000..1925eff36125bb --- /dev/null +++ b/programs/sbf/rust/call_args/src/lib.rs @@ -0,0 +1,177 @@ +use { + borsh::{from_slice, to_vec, BorshDeserialize, BorshSerialize}, + solana_program::{ + account_info::AccountInfo, entrypoint::ProgramResult, program::set_return_data, + pubkey::Pubkey, + }, +}; + +#[derive(BorshSerialize, BorshDeserialize, Clone, Copy)] +struct Test128 { + a: u128, + b: u128, +} + +#[derive(BorshDeserialize)] +struct InputData { + test_128: Test128, + arg1: i64, + arg2: i64, + arg3: i64, + arg4: i64, + arg5: i64, + arg6: i64, + arg7: i64, + arg8: i64, +} + +#[derive(BorshSerialize)] +struct OutputData { + res_128: u128, + res_256: Test128, + many_args_1: i64, + many_args_2: i64, +} + +solana_program::entrypoint!(entry); + +pub fn entry(_program_id: &Pubkey, _accounts: &[AccountInfo], data: &[u8]) -> ProgramResult { + // This code is supposed to occupy stack space. The purpose of this test is to make sure + // we operate on the limits of the stack frame safely. + let buffer: [u8; 3800] = [1; 3800]; + + let mut x: [u8; 16] = [0; 16]; + x.copy_from_slice(&buffer[3784..3800]); + x[10] = 0x39; + x[11] = 0x37; + + // Assert the function call hasn't overwritten these values + check_arr(x); + assert_eq!(x[10], 0x39); + assert_eq!(x[11], 0x37); + + // The function call must not overwrite the values and the return must be correct. + let y = check_arr_and_return(x); + assert_eq!(x[10], 0x39); + assert_eq!(x[11], 0x37); + assert_eq!(y[10], 0x39); + assert_eq!(y[11], 0x37); + assert_eq!(y[15], 17); + + let decoded: InputData = from_slice::(data).unwrap(); + + let output = OutputData { + res_128: test_128_arg(decoded.test_128.a, decoded.test_128.b), + res_256: test_256_arg(decoded.test_128), + many_args_1: many_args( + decoded.arg1, + decoded.arg2, + decoded.arg3, + decoded.arg4, + decoded.arg5, + decoded.arg6, + decoded.arg7, + decoded.arg8, + ), + many_args_2: many_args_stack_space( + decoded.arg1, + decoded.arg2, + decoded.arg3, + decoded.arg4, + decoded.arg5, + decoded.arg6, + decoded.arg7, + decoded.arg8, + ), + }; + + let encoded = to_vec(&output).unwrap(); + + set_return_data(encoded.as_slice()); + + Ok(()) +} + +// In this function the argument is promoted to a pointer, so it does not overwrite the stack. +#[allow(improper_ctypes_definitions)] +#[inline(never)] +extern "C" fn check_arr(x: [u8; 16]) { + for (idx, item) in x.iter().enumerate() { + if idx != 10 && idx != 11 { + assert!(*item == 1u8); + } + } + assert_eq!(x[11], 0x37); + assert_eq!(x[10], 0x39); +} + +// Both the argument and return value are promoted to pointers. +#[allow(improper_ctypes_definitions)] +#[inline(never)] +extern "C" fn check_arr_and_return(mut x: [u8; 16]) -> [u8; 16] { + for (idx, item) in x.iter().enumerate() { + if idx != 10 && idx != 11 { + assert!(*item == 1u8); + } + } + assert_eq!(x[11], 0x37); + assert_eq!(x[10], 0x39); + x[15] = 17; + x +} + +// Test a 128 bit argument +#[allow(clippy::arithmetic_side_effects)] +#[inline(never)] +fn test_128_arg(x: u128, y: u128) -> u128 { + x % y +} + +// Test a 256-bit argument +#[allow(clippy::arithmetic_side_effects)] +#[inline(never)] +fn test_256_arg(x: Test128) -> Test128 { + Test128 { + a: x.a + x.b, + b: x.a - x.b, + } +} + +// Test a function that needs to save arguments in the stack +#[allow(clippy::arithmetic_side_effects)] +#[inline(never)] +extern "C" fn many_args(a: i64, b: i64, c: i64, d: i64, e: i64, f: i64, g: i64, h: i64) -> i64 { + let i = a + b; + let j = i - c; + let k = j + d; + let l = k - e; + let m = l % f; + let n = m - g; + n + h +} + +// Test a function that utilizes stack space and needs to retrieve arguments from the caller stack +#[allow(clippy::arithmetic_side_effects)] +#[inline(never)] +extern "C" fn many_args_stack_space( + a: i64, + b: i64, + c: i64, + d: i64, + e: i64, + f: i64, + g: i64, + h: i64, +) -> i64 { + let s: [i64; 3] = [1, 2, 3]; + let i = a + b; + let j = i - c; + let k = j + d; + let l = k - e; + let m = l % f; + let n = m - g; + let o = n + h; + let p = o + s[0]; + let q = p + s[1]; + q - s[2] +} diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index c9c5f99b05cd2b..f9afcf03013566 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -9,6 +9,7 @@ #[cfg(feature = "sbf_rust")] use { + borsh::{from_slice, to_vec, BorshDeserialize, BorshSerialize}, itertools::izip, solana_account_decoder::parse_bpf_loader::{ parse_bpf_upgradeable_loader, BpfUpgradeableLoaderAccountType, @@ -70,6 +71,7 @@ use { map_inner_instructions, ConfirmedTransactionWithStatusMeta, TransactionStatusMeta, TransactionWithStatusMeta, VersionedTransactionWithStatusMeta, }, + solana_type_overrides::rand, std::{ assert_eq, cell::RefCell, @@ -5126,3 +5128,147 @@ fn test_stack_heap_zeroed() { ); } } + +#[test] +fn test_function_call_args() { + // This function tests edge compiler edge cases when calling functions with more than five + // arguments and passing by value arguments with more than 16 bytes. + solana_logger::setup(); + + let GenesisConfigInfo { + genesis_config, + mint_keypair, + .. + } = create_genesis_config(100_123_456_789); + + let (bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config); + let mut bank_client = BankClient::new_shared(bank); + let authority_keypair = Keypair::new(); + + let (bank, program_id) = load_upgradeable_program_and_advance_slot( + &mut bank_client, + bank_forks.as_ref(), + &mint_keypair, + &authority_keypair, + "solana_sbf_rust_call_args", + ); + + #[derive(BorshSerialize, BorshDeserialize, PartialEq, Eq, Debug)] + struct Test128 { + a: u128, + b: u128, + } + + #[derive(BorshSerialize)] + struct InputData { + test_128: Test128, + arg1: i64, + arg2: i64, + arg3: i64, + arg4: i64, + arg5: i64, + arg6: i64, + arg7: i64, + arg8: i64, + } + + #[derive(BorshDeserialize)] + struct OutputData { + res_128: u128, + res_256: Test128, + many_args_1: i64, + many_args_2: i64, + } + + let input_data = InputData { + test_128: Test128 { + a: rand::random::(), + b: rand::random::(), + }, + arg1: rand::random::(), + arg2: rand::random::(), + arg3: rand::random::(), + arg4: rand::random::(), + arg5: rand::random::(), + arg6: rand::random::(), + arg7: rand::random::(), + arg8: rand::random::(), + }; + + let instruction_data = to_vec(&input_data).unwrap(); + let account_metas = vec![ + AccountMeta::new(mint_keypair.pubkey(), true), + AccountMeta::new(Keypair::new().pubkey(), false), + ]; + + let instruction = Instruction::new_with_bytes(program_id, &instruction_data, account_metas); + let message = Message::new(&[instruction], Some(&mint_keypair.pubkey())); + + let tx = Transaction::new(&[&mint_keypair], message.clone(), bank.last_blockhash()); + + let txs = vec![tx]; + let tx_batch = bank.prepare_batch_for_tests(txs); + let result = bank + .load_execute_and_commit_transactions( + &tx_batch, + MAX_PROCESSING_AGE, + false, + ExecutionRecordingConfig { + enable_cpi_recording: false, + enable_log_recording: false, + enable_return_data_recording: true, + }, + &mut ExecuteTimings::default(), + None, + ) + .0; + + fn verify_many_args(input: &InputData) -> i64 { + let a = input + .arg1 + .overflowing_add(input.arg2) + .0 + .overflowing_sub(input.arg3) + .0 + .overflowing_add(input.arg4) + .0 + .overflowing_sub(input.arg5) + .0; + (a % input.arg6) + .overflowing_sub(input.arg7) + .0 + .overflowing_add(input.arg8) + .0 + } + + let return_data = &result[0] + .as_ref() + .unwrap() + .execution_details + .return_data + .as_ref() + .unwrap() + .data; + let decoded: OutputData = from_slice::(return_data).unwrap(); + assert_eq!( + decoded.res_128, + input_data.test_128.a % input_data.test_128.b + ); + assert_eq!( + decoded.res_256, + Test128 { + a: input_data + .test_128 + .a + .overflowing_add(input_data.test_128.b) + .0, + b: input_data + .test_128 + .a + .overflowing_sub(input_data.test_128.b) + .0 + } + ); + assert_eq!(decoded.many_args_1, verify_many_args(&input_data)); + assert_eq!(decoded.many_args_2, verify_many_args(&input_data)); +}