Skip to content

Commit

Permalink
YJIT: Remove CString allocation when using src_loc!()
Browse files Browse the repository at this point in the history
Since we often take the VM lock as the first thing we do when entering
YJIT, and that needs a `src_loc!()`, this removes a allocation from
that. The main trick here is `concat!(file!(), '\0')` to get a C string
statically baked into the binary.
  • Loading branch information
XrXr committed Apr 29, 2024
1 parent adae813 commit 7189dd2
Showing 1 changed file with 14 additions and 15 deletions.
29 changes: 14 additions & 15 deletions yjit/src/cruby.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
#![allow(non_upper_case_globals)]

use std::convert::From;
use std::ffi::CString;
use std::ffi::{CString, CStr};
use std::os::raw::{c_char, c_int, c_uint};
use std::panic::{catch_unwind, UnwindSafe};

Expand Down Expand Up @@ -208,8 +208,6 @@ pub use rb_RCLASS_ORIGIN as RCLASS_ORIGIN;

/// Helper so we can get a Rust string for insn_name()
pub fn insn_name(opcode: usize) -> String {
use std::ffi::CStr;

unsafe {
// Look up Ruby's NULL-terminated insn name string
let op_name = raw_insn_name(VALUE(opcode));
Expand Down Expand Up @@ -608,7 +606,6 @@ pub fn rust_str_to_sym(str: &str) -> VALUE {
pub fn cstr_to_rust_string(c_char_ptr: *const c_char) -> Option<String> {
assert!(c_char_ptr != std::ptr::null());

use std::ffi::CStr;
let c_str: &CStr = unsafe { CStr::from_ptr(c_char_ptr) };

match c_str.to_str() {
Expand All @@ -620,17 +617,20 @@ pub fn cstr_to_rust_string(c_char_ptr: *const c_char) -> Option<String> {
/// A location in Rust code for integrating with debugging facilities defined in C.
/// Use the [src_loc!] macro to crate an instance.
pub struct SourceLocation {
pub file: CString,
pub file: &'static CStr,
pub line: c_int,
}

/// Make a [SourceLocation] at the current spot.
macro_rules! src_loc {
() => {
// NOTE(alan): `CString::new` allocates so we might want to limit this to debug builds.
$crate::cruby::SourceLocation {
file: std::ffi::CString::new(file!()).unwrap(), // ASCII source file paths
line: line!().try_into().unwrap(), // not that many lines
{
// Nul-terminated string with static lifetime, make a CStr out of it safely.
let file: &'static str = concat!(file!(), '\0');
$crate::cruby::SourceLocation {
file: unsafe { std::ffi::CStr::from_ptr(file.as_ptr().cast()) },
line: line!().try_into().unwrap(),
}
}
};
}
Expand All @@ -657,32 +657,31 @@ pub fn with_vm_lock<F, R>(loc: SourceLocation, func: F) -> R
where
F: FnOnce() -> R + UnwindSafe,
{
let file = loc.file.as_ptr();
let file = loc.file;
let line = loc.line;
let mut recursive_lock_level: c_uint = 0;

unsafe { rb_yjit_vm_lock_then_barrier(&mut recursive_lock_level, file, line) };
unsafe { rb_yjit_vm_lock_then_barrier(&mut recursive_lock_level, file.as_ptr(), line) };

let ret = match catch_unwind(func) {
Ok(result) => result,
Err(_) => {
// Theoretically we can recover from some of these panics,
// but it's too late if the unwind reaches here.
use std::{process, str};

let _ = catch_unwind(|| {
// IO functions can panic too.
eprintln!(
"YJIT panicked while holding VM lock acquired at {}:{}. Aborting...",
str::from_utf8(loc.file.as_bytes()).unwrap_or("<not utf8>"),
loc.file.to_string_lossy(),
line,
);
});
process::abort();
std::process::abort();
}
};

unsafe { rb_yjit_vm_unlock(&mut recursive_lock_level, file, line) };
unsafe { rb_yjit_vm_unlock(&mut recursive_lock_level, file.as_ptr(), line) };

ret
}
Expand Down

0 comments on commit 7189dd2

Please sign in to comment.