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.

Sign up for GitHub

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

Bumps solana_rbpf to v0.2.14 #18869

Merged
merged 4 commits into from
Aug 4, 2021
Merged

Bumps solana_rbpf to v0.2.14 #18869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
61712a8
Bumps solana_rbpf to v0.2.14
Lichtso Jul 23, 2021
0dfb773
Feature gate for verify_mul64_imm_nonzero as discussed in #17520.
Lichtso Jul 26, 2021
fabf8ae
Merge branch 'master' into rbpf/v0.2.14
Lichtso Jul 30, 2021
5ee2435
Merge branch 'master' into rbpf/v0.2.14
Lichtso Aug 3, 2021
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
4 changes: 2 additions & 2 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cli/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ solana-config-program = { path = "../programs/config", version = "=1.8.0" }
solana-faucet = { path = "../faucet", version = "=1.8.0" }
solana-logger = { path = "../logger", version = "=1.8.0" }
solana-net-utils = { path = "../net-utils", version = "=1.8.0" }
solana_rbpf = "=0.2.13"
solana_rbpf = "=0.2.14"
solana-remote-wallet = { path = "../remote-wallet", version = "=1.8.0" }
solana-sdk = { path = "../sdk", version = "=1.8.0" }
solana-transaction-status = { path = "../transaction-status", version = "=1.8.0" }
Expand Down
3 changes: 2 additions & 1 deletion cli/src/program.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1824,9 +1824,10 @@ fn read_and_verify_elf(program_location: &str) -> Result<Vec<u8>, Box<dyn std::e
// Verify the program
<dyn Executable<BpfError, ThisInstructionMeter>>::from_elf(
&program_data,
Some(|x| verifier::check(x)),
Some(verifier::check),
Config {
reject_unresolved_syscalls: true,
verify_mul64_imm_nonzero: true, // TODO: Remove me after feature gate
..Config::default()
},
register_syscalls(&mut invoke_context).unwrap(),
Expand Down
4 changes: 2 additions & 2 deletions programs/bpf/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion programs/bpf/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ solana-bpf-loader-program = { path = "../bpf_loader", version = "=1.8.0" }
solana-cli-output = { path = "../../cli-output", version = "=1.8.0" }
solana-logger = { path = "../../logger", version = "=1.8.0" }
solana-measure = { path = "../../measure", version = "=1.8.0" }
solana_rbpf = "=0.2.13"
solana_rbpf = "=0.2.14"
solana-runtime = { path = "../../runtime", version = "=1.8.0" }
solana-sdk = { path = "../../sdk", version = "=1.8.0" }
solana-transaction-status = { path = "../../transaction-status", version = "=1.8.0" }
Expand Down
2 changes: 1 addition & 1 deletion programs/bpf_loader/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ sha3 = "0.9.1"
solana-measure = { path = "../../measure", version = "=1.8.0" }
solana-runtime = { path = "../../runtime", version = "=1.8.0" }
solana-sdk = { path = "../../sdk", version = "=1.8.0" }
solana_rbpf = "=0.2.13"
solana_rbpf = "=0.2.14"
thiserror = "1.0"

[dev-dependencies]
Expand Down
23 changes: 11 additions & 12 deletions programs/bpf_loader/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@ use log::{log_enabled, trace, Level::Trace};
use solana_measure::measure::Measure;
use solana_rbpf::{
aligned_memory::AlignedMemory,
ebpf::{HOST_ALIGN, MM_HEAP_START},
ebpf::HOST_ALIGN,
error::{EbpfError, UserDefinedError},
memory_region::MemoryRegion,
static_analysis::Analysis,
verifier::{self, VerifierError},
vm::{Config, EbpfVm, Executable, InstructionMeter},
Expand All @@ -31,7 +30,7 @@ use solana_sdk::{
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
clock::Clock,
entrypoint::{HEAP_LENGTH, SUCCESS},
feature_set::add_missing_program_error_mappings,
feature_set::{add_missing_program_error_mappings, stop_verify_mul64_imm_nonzero},
ic_logger_msg, ic_msg,
instruction::InstructionError,
keyed_account::{from_keyed_account, keyed_account_at_index},
Expand Down Expand Up @@ -83,6 +82,8 @@ pub fn create_executor(
max_call_depth: compute_budget.max_call_depth,
stack_frame_size: compute_budget.stack_frame_size,
enable_instruction_tracing: log_enabled!(Trace),
verify_mul64_imm_nonzero: !invoke_context
.is_feature_active(&stop_verify_mul64_imm_nonzero::id()), // TODO: Feature gate and then remove me
..Config::default()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the stack frame gaps disabled by default now? If so they should be featurized

Copy link
Contributor

@jackcmay jackcmay Jul 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the Config's instruction_meter_checkpoint_distance default or the compute meter handling change in v0.2.14?? If so should be feature gated

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still enabled by default: enable_stack_frame_gaps: true
https://github.com/solana-labs/rbpf/blob/302de4d10888dfb76534693308f4ef8060b86fcd/src/vm.rs#L206
Should we disable it in the next release?

Copy link
Contributor Author

@Lichtso Lichtso Aug 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instruction_meter_checkpoint_distance should have no influence on what the program sees, the point where programs are aborted and what error is thrown has to be the same for interpreter and JIT anyway. The option only makes the detection of that condition faster (so it affects the performance only).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good as long as there isn't an edge case that's falls in the middle. Also, maybe check with Tao about cost model stuff to make sure this change doesn't result in a behavioral change there.

};
let mut executable = {
Expand All @@ -98,10 +99,8 @@ pub fn create_executor(
)
}
.map_err(|e| map_ebpf_error(invoke_context, e))?;
let (_, elf_bytes) = executable
.get_text_bytes()
.map_err(|e| map_ebpf_error(invoke_context, e))?;
verifier::check(elf_bytes)
let text_bytes = executable.get_text_bytes().1;
verifier::check(text_bytes, &config)
.map_err(|e| map_ebpf_error(invoke_context, EbpfError::UserError(e.into())))?;
if use_jit {
if let Err(err) = executable.jit_compile() {
Expand Down Expand Up @@ -150,10 +149,9 @@ pub fn create_vm<'a>(
invoke_context: &'a mut dyn InvokeContext,
) -> Result<EbpfVm<'a, BpfError, ThisInstructionMeter>, EbpfError<BpfError>> {
let compute_budget = invoke_context.get_compute_budget();
let heap =
let mut heap =
AlignedMemory::new_with_size(compute_budget.heap_size.unwrap_or(HEAP_LENGTH), HOST_ALIGN);
let heap_region = MemoryRegion::new_from_slice(heap.as_slice(), MM_HEAP_START, 0, true);
let mut vm = EbpfVm::new(program, parameter_bytes, &[heap_region])?;
let mut vm = EbpfVm::new(program, heap.as_slice_mut(), parameter_bytes)?;
syscalls::bind_syscall_context_objects(loader_id, &mut vm, invoke_context, heap)?;
Ok(vm)
}
Expand Down Expand Up @@ -909,7 +907,8 @@ mod tests {
)
.unwrap();
let mut vm =
EbpfVm::<BpfError, TestInstructionMeter>::new(program.as_ref(), input, &[]).unwrap();
EbpfVm::<BpfError, TestInstructionMeter>::new(program.as_ref(), &mut [], input)
.unwrap();
let mut instruction_meter = TestInstructionMeter { remaining: 10 };
vm.execute_program_interpreted(&mut instruction_meter)
.unwrap();
Expand All @@ -921,7 +920,7 @@ mod tests {
let prog = &[
0x18, 0x00, 0x00, 0x00, 0x88, 0x77, 0x66, 0x55, // first half of lddw
];
verifier::check(prog).unwrap();
verifier::check(prog, &Config::default()).unwrap();
}

#[test]
Expand Down
Loading