Skip to content

Commit

Permalink
Refactor internal profiling APIs to be safer
Browse files Browse the repository at this point in the history
Pass around `&[u8]` instead of `*const u8` and `usize` to avoid the need
for raw unsafe abstractions.

Closes bytecodealliance#8905
  • Loading branch information
alexcrichton committed Jul 8, 2024
1 parent e4fe818 commit 14bd6a8
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 37 deletions.
16 changes: 6 additions & 10 deletions crates/jit-debug/src/perf_jitdump.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,7 @@ impl JitDumpFile {
pub fn dump_code_load_record(
&mut self,
method_name: &str,
addr: *const u8,
len: usize,
code: &[u8],
timestamp: u64,
pid: u32,
tid: u32,
Expand All @@ -263,24 +262,21 @@ impl JitDumpFile {

let rh = RecordHeader {
id: RecordId::JitCodeLoad as u32,
record_size: size_limit as u32 + name_len as u32 + len as u32,
record_size: size_limit as u32 + name_len as u32 + code.len() as u32,
timestamp,
};

let clr = CodeLoadRecord {
header: rh,
pid,
tid,
virtual_address: addr as u64,
address: addr as u64,
size: len as u64,
virtual_address: code.as_ptr() as u64,
address: code.as_ptr() as u64,
size: code.len() as u64,
index: self.next_code_index(),
};

unsafe {
let code_buffer: &[u8] = std::slice::from_raw_parts(addr, len);
self.write_code_load_record(method_name, clr, code_buffer)
}
self.write_code_load_record(method_name, clr, code)
}
}

Expand Down
12 changes: 6 additions & 6 deletions crates/wasmtime/src/profiling_agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ cfg_if::cfg_if! {

/// Common interface for profiling tools.
pub trait ProfilingAgent: Send + Sync + 'static {
fn register_function(&self, name: &str, addr: *const u8, size: usize);
fn register_function(&self, name: &str, code: &[u8]);

fn register_module(&self, code: &[u8], custom_name: &dyn Fn(usize) -> Option<String>) {
use object::{File, Object as _, ObjectSection, ObjectSymbol, SectionKind, SymbolKind};
Expand All @@ -55,9 +55,9 @@ pub trait ProfilingAgent: Send + Sync + 'static {
Err(_) => return,
};

let text_base = match image.sections().find(|s| s.kind() == SectionKind::Text) {
let text = match image.sections().find(|s| s.kind() == SectionKind::Text) {
Some(section) => match section.data() {
Ok(data) => data.as_ptr() as usize,
Ok(data) => data,
Err(_) => return,
},
None => return,
Expand All @@ -76,7 +76,6 @@ pub trait ProfilingAgent: Send + Sync + 'static {
continue;
}
if let Ok(name) = sym.name() {
let addr = text_base + address as usize;
let owned;
let name = match custom_name(address as usize) {
Some(name) => {
Expand All @@ -85,7 +84,8 @@ pub trait ProfilingAgent: Send + Sync + 'static {
}
None => name,
};
self.register_function(name, addr as *const u8, size as usize);
let code = &text[address as usize..][..size as usize];
self.register_function(name, code)
}
}
}
Expand All @@ -99,6 +99,6 @@ pub fn new_null() -> Box<dyn ProfilingAgent> {
struct NullProfilerAgent;

impl ProfilingAgent for NullProfilerAgent {
fn register_function(&self, _name: &str, _addr: *const u8, _size: usize) {}
fn register_function(&self, _name: &str, _code: &[u8]) {}
fn register_module(&self, _code: &[u8], _custom_name: &dyn Fn(usize) -> Option<String>) {}
}
5 changes: 2 additions & 3 deletions crates/wasmtime/src/profiling_agent/jitdump.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,13 @@ pub fn new() -> Result<Box<dyn ProfilingAgent>> {
}

impl ProfilingAgent for JitDumpAgent {
fn register_function(&self, name: &str, addr: *const u8, size: usize) {
fn register_function(&self, name: &str, code: &[u8]) {
let mut jitdump_file = JITDUMP_FILE.lock().unwrap();
let jitdump_file = jitdump_file.as_mut().unwrap();
let timestamp = jitdump_file.get_time_stamp();
#[allow(trivial_numeric_casts)]
let tid = rustix::thread::gettid().as_raw_nonzero().get() as u32;
if let Err(err) =
jitdump_file.dump_code_load_record(&name, addr, size, timestamp, self.pid, tid)
if let Err(err) = jitdump_file.dump_code_load_record(&name, code, timestamp, self.pid, tid)
{
println!("Jitdump: write_code_load_failed_record failed: {:?}\n", err);
}
Expand Down
19 changes: 10 additions & 9 deletions crates/wasmtime/src/profiling_agent/perfmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,26 +21,27 @@ pub fn new() -> Result<Box<dyn ProfilingAgent>> {
}

impl PerfMapAgent {
fn make_line(
writer: &mut dyn Write,
name: &str,
addr: *const u8,
len: usize,
) -> io::Result<()> {
fn make_line(writer: &mut dyn Write, name: &str, code: &[u8]) -> io::Result<()> {
// Format is documented here: https://github.com/torvalds/linux/blob/master/tools/perf/Documentation/jit-interface.txt
// Try our best to sanitize the name, since wasm allows for any utf8 string in there.
let sanitized_name = name.replace('\n', "_").replace('\r', "_");
write!(writer, "{:x} {:x} {}\n", addr as usize, len, sanitized_name)?;
write!(
writer,
"{:p} {:x} {}\n",
code.as_ptr(),
code.len(),
sanitized_name
)?;
writer.flush()?;
Ok(())
}
}

impl ProfilingAgent for PerfMapAgent {
fn register_function(&self, name: &str, addr: *const u8, size: usize) {
fn register_function(&self, name: &str, code: &[u8]) {
let mut file = PERFMAP_FILE.lock().unwrap();
let file = file.as_mut().unwrap();
if let Err(err) = Self::make_line(file, name, addr, size) {
if let Err(err) = Self::make_line(file, name, code) {
eprintln!("Error when writing import trampoline info to the perf map file: {err}");
}
}
Expand Down
15 changes: 6 additions & 9 deletions crates/wasmtime/src/profiling_agent/vtune.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ impl Drop for VTuneAgent {

impl State {
/// Notify vtune about a newly tracked code region.
fn notify_code(&mut self, module_name: &str, method_name: &str, addr: *const u8, len: usize) {
fn notify_code(&mut self, module_name: &str, method_name: &str, code: &[u8]) {
self.vtune
.load_method(
MethodLoadBuilder::new(method_name.to_owned(), addr, len)
MethodLoadBuilder::new(method_name.to_owned(), code.as_ptr(), code.len())
.class_file_name(module_name.to_owned())
.source_file_name("<unknown wasm filename>".to_owned()),
)
Expand All @@ -65,16 +65,13 @@ impl State {
}

impl ProfilingAgent for VTuneAgent {
fn register_function(&self, name: &str, addr: *const u8, size: usize) {
self.state
.lock()
.unwrap()
.register_function(name, addr, size);
fn register_function(&self, name: &str, code: &[u8]) {
self.state.lock().unwrap().register_function(name, code);
}
}

impl State {
fn register_function(&mut self, name: &str, addr: *const u8, size: usize) {
self.notify_code("wasmtime", name, addr, size);
fn register_function(&mut self, name: &str, code: &[u8]) {
self.notify_code("wasmtime", name, code);
}
}

0 comments on commit 14bd6a8

Please sign in to comment.