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

Cleanup - Move test helpers to test_utils crate #20

Merged
merged 4 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 benches/elf_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ extern crate test_utils;
use solana_sbpf::{
elf::Executable,
program::{BuiltinFunction, BuiltinProgram, FunctionRegistry},
syscalls,
vm::{Config, TestContextObject},
vm::Config,
};
use std::{fs::File, io::Read, sync::Arc};
use test::Bencher;
use test_utils::{syscalls, TestContextObject};

fn loader() -> Arc<BuiltinProgram<TestContextObject>> {
let mut function_registry = FunctionRegistry::<BuiltinFunction<TestContextObject>>::default();
Expand Down
6 changes: 2 additions & 4 deletions benches/jit_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,10 @@
extern crate solana_sbpf;
extern crate test;

use solana_sbpf::{
elf::Executable, program::BuiltinProgram, verifier::RequisiteVerifier, vm::TestContextObject,
};
use solana_sbpf::{elf::Executable, program::BuiltinProgram, verifier::RequisiteVerifier};
use std::{fs::File, io::Read, sync::Arc};
use test::Bencher;
use test_utils::create_vm;
use test_utils::{create_vm, TestContextObject};

#[bench]
fn bench_init_vm(bencher: &mut Bencher) {
Expand Down
6 changes: 2 additions & 4 deletions benches/vm_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,10 @@ use solana_sbpf::{
program::{FunctionRegistry, SBPFVersion},
vm::Config,
};
use solana_sbpf::{
elf::Executable, program::BuiltinProgram, verifier::RequisiteVerifier, vm::TestContextObject,
};
use solana_sbpf::{elf::Executable, program::BuiltinProgram, verifier::RequisiteVerifier};
use std::{fs::File, io::Read, sync::Arc};
use test::Bencher;
use test_utils::create_vm;
use test_utils::{create_vm, TestContextObject};

#[bench]
fn bench_init_interpreter_start(bencher: &mut Bencher) {
Expand Down
21 changes: 7 additions & 14 deletions cli/Cargo.lock

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

3 changes: 2 additions & 1 deletion cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ use solana_sbpf::{
program::{BuiltinProgram, FunctionRegistry},
static_analysis::Analysis,
verifier::RequisiteVerifier,
vm::{Config, DynamicAnalysis, EbpfVm, TestContextObject},
vm::{Config, DynamicAnalysis, EbpfVm},
};
use std::{fs::File, io::Read, path::Path, sync::Arc};
use test_utils::TestContextObject;

fn main() {
let matches = App::new("Solana BPF CLI")
Expand Down
2 changes: 1 addition & 1 deletion examples/disassemble.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ use solana_sbpf::{
elf::Executable,
program::{BuiltinProgram, FunctionRegistry, SBPFVersion},
static_analysis::Analysis,
vm::TestContextObject,
};
use std::sync::Arc;
use test_utils::TestContextObject;

// Simply disassemble a program into human-readable instructions.
fn main() {
Expand Down
2 changes: 1 addition & 1 deletion examples/to_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ use solana_sbpf::{
elf::Executable,
program::{BuiltinProgram, FunctionRegistry, SBPFVersion},
static_analysis::Analysis,
vm::TestContextObject,
};
use std::sync::Arc;
use test_utils::TestContextObject;
// Turn a program into a JSON string.
//
// Relies on `json` crate.
Expand Down
3 changes: 1 addition & 2 deletions fuzz/fuzz_targets/dumb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@ use solana_sbpf::{
memory_region::MemoryRegion,
program::{BuiltinFunction, BuiltinProgram, FunctionRegistry, SBPFVersion},
verifier::{RequisiteVerifier, Verifier},
vm::TestContextObject,
};
use test_utils::create_vm;
use test_utils::{create_vm, TestContextObject};

use crate::common::ConfigTemplate;

Expand Down
3 changes: 1 addition & 2 deletions fuzz/fuzz_targets/smart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ use solana_sbpf::{
memory_region::MemoryRegion,
program::{BuiltinFunction, BuiltinProgram, FunctionRegistry, SBPFVersion},
verifier::{RequisiteVerifier, Verifier},
vm::TestContextObject,
};
use test_utils::create_vm;
use test_utils::{create_vm, TestContextObject};

use crate::common::ConfigTemplate;

Expand Down
3 changes: 1 addition & 2 deletions fuzz/fuzz_targets/smart_jit_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@ use solana_sbpf::{
memory_region::MemoryRegion,
program::{BuiltinFunction, BuiltinProgram, FunctionRegistry, SBPFVersion},
verifier::{RequisiteVerifier, Verifier},
vm::TestContextObject,
};
use test_utils::create_vm;
use test_utils::{create_vm, TestContextObject};

use crate::common::ConfigTemplate;

Expand Down
3 changes: 1 addition & 2 deletions fuzz/fuzz_targets/smarter_jit_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@ use solana_sbpf::{
memory_region::MemoryRegion,
program::{BuiltinFunction, BuiltinProgram, FunctionRegistry, SBPFVersion},
verifier::{RequisiteVerifier, Verifier},
vm::TestContextObject,
};
use test_utils::create_vm;
use test_utils::{create_vm, TestContextObject};

use crate::common::ConfigTemplate;

Expand Down
2 changes: 1 addition & 1 deletion fuzz/fuzz_targets/verify_semantic_aware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use solana_sbpf::{
insn_builder::IntoBytes,
program::{BuiltinFunction, FunctionRegistry, SBPFVersion},
verifier::{RequisiteVerifier, Verifier},
vm::TestContextObject,
};
use test_utils::TestContextObject;

use crate::common::ConfigTemplate;

Expand Down
3 changes: 2 additions & 1 deletion src/assembler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,8 @@ fn resolve_label(
/// # Examples
///
/// ```
/// use solana_sbpf::{assembler::assemble, program::BuiltinProgram, vm::{Config, TestContextObject}};
/// use solana_sbpf::{assembler::assemble, program::BuiltinProgram, vm::Config};
/// use test_utils::TestContextObject;
/// let executable = assemble::<TestContextObject>(
/// "add64 r1, 0x605
/// mov64 r2, 0x32
Expand Down
27 changes: 0 additions & 27 deletions src/fuzz.rs

This file was deleted.

3 changes: 1 addition & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

//! Virtual machine for SBPF programs.
#![warn(missing_docs)]
#![allow(clippy::literal_string_with_formatting_args)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we ignoring this case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It reports all kinds of false positives (even in comments) and it can't even track the origin correctly, so it will just output random stuff with no way to tell what it was complaining about. There are already multiple bug reports upstream. No idea why they did not just revert it.

#![deny(clippy::arithmetic_side_effects)]
#![deny(clippy::ptr_as_ptr)]

Expand All @@ -31,7 +32,6 @@ pub mod ebpf;
pub mod elf;
pub mod elf_parser;
pub mod error;
pub mod fuzz;
pub mod insn_builder;
pub mod interpreter;
#[cfg(all(feature = "jit", not(target_os = "windows"), target_arch = "x86_64"))]
Expand All @@ -41,7 +41,6 @@ mod memory_management;
pub mod memory_region;
pub mod program;
pub mod static_analysis;
pub mod syscalls;
pub mod verifier;
pub mod vm;
#[cfg(all(feature = "jit", not(target_os = "windows"), target_arch = "x86_64"))]
Expand Down
18 changes: 15 additions & 3 deletions src/static_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
elf::Executable,
error::EbpfError,
program::SBPFVersion,
vm::{ContextObject, DynamicAnalysis, TestContextObject},
vm::{ContextObject, DynamicAnalysis},
};
use rustc_demangle::demangle;
use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
Expand Down Expand Up @@ -124,10 +124,22 @@ impl Default for CfgNode {
}
}

struct DummyContextObject {}

impl ContextObject for DummyContextObject {
fn trace(&mut self, _state: [u64; 12]) {}

fn consume(&mut self, _amount: u64) {}

fn get_remaining(&self) -> u64 {
0
}
}

/// Result of the executable analysis
pub struct Analysis<'a> {
/// The program which is analyzed
executable: &'a Executable<TestContextObject>,
executable: &'a Executable<DummyContextObject>,
/// Plain list of instructions as they occur in the executable
pub instructions: Vec<ebpf::Insn>,
/// Functions in the executable
Expand Down Expand Up @@ -182,7 +194,7 @@ impl<'a> Analysis<'a> {
let mut result = Self {
// Removes the generic ContextObject which is safe because we are not going to execute the program
executable: unsafe {
std::mem::transmute::<&Executable<C>, &Executable<TestContextObject>>(executable)
std::mem::transmute::<&Executable<C>, &Executable<DummyContextObject>>(executable)
},
instructions,
functions,
Expand Down
50 changes: 3 additions & 47 deletions src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::{
interpreter::Interpreter,
memory_region::MemoryMapping,
program::{BuiltinFunction, BuiltinProgram, FunctionRegistry, SBPFVersion},
static_analysis::{Analysis, TraceLogEntry},
static_analysis::Analysis,
};
use std::{collections::BTreeMap, fmt::Debug};

Expand Down Expand Up @@ -137,51 +137,6 @@ pub trait ContextObject {
fn get_remaining(&self) -> u64;
}

/// Simple instruction meter for testing
#[derive(Debug, Clone, Default)]
pub struct TestContextObject {
/// Contains the register state at every instruction in order of execution
pub trace_log: Vec<TraceLogEntry>,
/// Maximal amount of instructions which still can be executed
pub remaining: u64,
}

impl ContextObject for TestContextObject {
fn trace(&mut self, state: [u64; 12]) {
self.trace_log.push(state);
}

fn consume(&mut self, amount: u64) {
self.remaining = self.remaining.saturating_sub(amount);
}

fn get_remaining(&self) -> u64 {
self.remaining
}
}

impl TestContextObject {
/// Initialize with instruction meter
pub fn new(remaining: u64) -> Self {
Self {
trace_log: Vec::new(),
remaining,
}
}

/// Compares an interpreter trace and a JIT trace.
///
/// The log of the JIT can be longer because it only validates the instruction meter at branches.
pub fn compare_trace_log(interpreter: &Self, jit: &Self) -> bool {
let interpreter = interpreter.trace_log.as_slice();
let mut jit = jit.trace_log.as_slice();
if jit.len() > interpreter.len() {
jit = &jit[0..interpreter.len()];
}
interpreter == jit
}
}

/// Statistic of taken branches (from a recorded trace)
pub struct DynamicAnalysis {
/// Maximal edge counter value
Expand Down Expand Up @@ -263,8 +218,9 @@ pub enum RuntimeEnvironmentSlot {
/// memory_region::{MemoryMapping, MemoryRegion},
/// program::{BuiltinProgram, FunctionRegistry, SBPFVersion},
/// verifier::RequisiteVerifier,
/// vm::{Config, EbpfVm, TestContextObject},
/// vm::{Config, EbpfVm},
/// };
/// use test_utils::TestContextObject;
///
/// let prog = &[
/// 0x9d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 // exit
Expand Down
Loading
Loading