diff --git a/crates/ghostty-bench/src/benchmark.rs b/crates/ghostty-bench/src/benchmark.rs index 9ade5a0..d4f897a 100644 --- a/crates/ghostty-bench/src/benchmark.rs +++ b/crates/ghostty-bench/src/benchmark.rs @@ -49,6 +49,16 @@ pub fn run(b: &mut dyn Benchmark, mode: RunMode) -> Result { fn run_steps(b: &mut dyn Benchmark, mode: RunMode) -> Result { let mut result = RunResult::default(); + + // On Darwin, emit an Instruments signpost interval around the run so the + // results are easy to find in Instruments (ends when dropped). + #[cfg(target_vendor = "apple")] + let _signpost = crate::signpost::Interval::begin( + c"com.mitchellh.ghostty", + c"ghostty", + (&raw const *b).cast::<()>() as usize, + ); + let start = Instant::now(); loop { b.step()?; diff --git a/crates/ghostty-bench/src/lib.rs b/crates/ghostty-bench/src/lib.rs index d6fe7f0..770308d 100644 --- a/crates/ghostty-bench/src/lib.rs +++ b/crates/ghostty-bench/src/lib.rs @@ -10,5 +10,7 @@ pub mod args; pub mod benches; pub mod benchmark; +#[cfg(target_vendor = "apple")] +pub mod signpost; mod simd_width; pub mod synthetic; diff --git a/crates/ghostty-bench/src/signpost.rs b/crates/ghostty-bench/src/signpost.rs new file mode 100644 index 0000000..87b2396 --- /dev/null +++ b/crates/ghostty-bench/src/signpost.rs @@ -0,0 +1,92 @@ +//! Darwin Instruments signposts (port of `pkg/macos/os/{log,signpost}.zig`, +//! the subset used by `benchmark/Benchmark.zig`). macOS/iOS only. +//! +//! The `os_signpost_*` C macros expand to calls of +//! `_os_signpost_emit_with_name_impl`, which is what is declared here. +#![allow(unsafe_code)] + +use std::ffi::{CStr, c_char, c_void}; + +/// `OS_SIGNPOST_INTERVAL_BEGIN` / `OS_SIGNPOST_INTERVAL_END`. +const INTERVAL_BEGIN: u8 = 1; +const INTERVAL_END: u8 = 2; + +/// `os_log_t` handle category that shows up as the "Points of Interest" track +/// in Instruments. +pub const CATEGORY_POINTS_OF_INTEREST: &CStr = c"PointsOfInterest"; + +unsafe extern "C" { + /// The Mach-O header of the image this code is linked into, provided by + /// the linker. Unlike Zig (ziglang/zig#24370) Rust can reference it. + static __dso_handle: c_void; + + fn os_log_create(subsystem: *const c_char, category: *const c_char) -> *mut c_void; + fn os_release(object: *mut c_void); + fn os_signpost_id_make_with_pointer(log: *mut c_void, ptr: *const c_void) -> u64; + fn _os_signpost_emit_with_name_impl( + dso: *const c_void, + log: *mut c_void, + typ: u8, + spid: u64, + name: *const c_char, + format: *const c_char, + buf: *mut u8, + size: u32, + ); +} + +/// An `os_log_t` plus the signpost id of one interval. The interval ends (and +/// the log is released) on drop. +pub struct Interval { + log: *mut c_void, + id: u64, + name: &'static CStr, +} + +impl Interval { + /// Begin an interval named `name` in `subsystem`/"PointsOfInterest", + /// correlated by the address `key` (Zig: `Id.forPointer(log, self.ptr)`). + #[must_use] + pub fn begin(subsystem: &CStr, name: &'static CStr, key: usize) -> Option { + // SAFETY: both strings are NUL-terminated and outlive the call. + let log = + unsafe { os_log_create(subsystem.as_ptr(), CATEGORY_POINTS_OF_INTEREST.as_ptr()) }; + if log.is_null() { + return None; + } + // SAFETY: `log` is a valid os_log_t; `key` is only used as an opaque key. + let id = unsafe { os_signpost_id_make_with_pointer(log, key as *const c_void) }; + let i = Interval { log, id, name }; + i.emit(INTERVAL_BEGIN); + Some(i) + } + + fn emit(&self, typ: u8) { + // Two zero bytes: summary flags and argument count (no arguments). + let mut buf = [0u8; 2]; + // SAFETY: `log` is valid for the lifetime of `self`, `name` is a + // NUL-terminated static string, `format` is "" and `buf` is the + // 2-byte empty argument buffer the C macro would build. + unsafe { + _os_signpost_emit_with_name_impl( + &raw const __dso_handle, + self.log, + typ, + self.id, + self.name.as_ptr(), + c"".as_ptr(), + buf.as_mut_ptr(), + 2, + ); + } + } +} + +impl Drop for Interval { + fn drop(&mut self) { + self.emit(INTERVAL_END); + // SAFETY: `log` came from `os_log_create` (+1 retained) and is not + // used afterwards. + unsafe { os_release(self.log) }; + } +} diff --git a/crates/ghostty-crash/Cargo.toml b/crates/ghostty-crash/Cargo.toml index 68b4706..96513ae 100644 --- a/crates/ghostty-crash/Cargo.toml +++ b/crates/ghostty-crash/Cargo.toml @@ -19,7 +19,11 @@ thiserror = "2.0.21" [target.'cfg(unix)'.dependencies] libc = "0.2" +[target.'cfg(target_os = "linux")'.dependencies] +minidump-writer = "0.13.0" + [dev-dependencies] +minidump = "0.27.0" tempfile = "3.27.0" # The fatal-signal handler needs sigaction/sigaltstack/raw syscalls. Unsafe is diff --git a/crates/ghostty-crash/src/lib.rs b/crates/ghostty-crash/src/lib.rs index c07214f..cc0683d 100644 --- a/crates/ghostty-crash/src/lib.rs +++ b/crates/ghostty-crash/src/lib.rs @@ -11,6 +11,8 @@ pub mod context; pub mod dir; pub mod envelope; +#[cfg(target_os = "linux")] +mod minidump; pub mod sentry; #[cfg(unix)] mod signal; diff --git a/crates/ghostty-crash/src/minidump.rs b/crates/ghostty-crash/src/minidump.rs new file mode 100644 index 0000000..231896f --- /dev/null +++ b/crates/ghostty-crash/src/minidump.rs @@ -0,0 +1,326 @@ +//! Minidump capture for fatal signals (Linux), the counterpart of what +//! sentry-native's Breakpad/crashpad backend does upstream. +//! +//! A process cannot ptrace itself, so (like Breakpad's `ExceptionHandler`) +//! the signal handler forks a short-lived child, allows it to ptrace the +//! crashing process (`PR_SET_PTRACER`), and waits. The child uses +//! `minidump-writer` to snapshot the parent -- every thread's registers and +//! stack, the module list, memory mappings and, for the crashing thread, the +//! `siginfo` + machine context captured by the handler -- and writes the final +//! envelope: the event item followed by the minidump as an +//! `event.minidump` attachment. If anything fails (ptrace forbidden by a +//! sandbox, the child dies or times out) the caller falls back to the plain +//! event-only envelope written from the signal-safe path. +//! +//! The fork uses the raw `clone` syscall without `CLONE_VM`, so no `atfork` +//! handlers (which could take locks the crashed thread holds) run. The child +//! allocates, which could in theory deadlock on a copied locked malloc arena; +//! the parent therefore enforces a timeout and kills the child. +#![allow(unsafe_code)] + +use std::{ + cell::UnsafeCell, + io::{Cursor, Write}, + mem::MaybeUninit, + os::fd::FromRawFd, + time::{Duration, Instant}, +}; + +use minidump_writer::{ + CrashContextExt, + crash_context::{CrashContext, ucontext_t}, + minidump_writer::MinidumpWriterConfig, +}; + +/// Stack size of the dumping child. +const STACK: usize = 4 << 20; + +/// How long the crashing process waits for the dumping child. +const TIMEOUT: Duration = Duration::from_secs(20); + +/// Everything the child needs; borrowed from the handler's scratch space. +pub(crate) struct Capture<'a> { + /// NUL-terminated destination path of the envelope. + pub path: &'a [u8], + /// Envelope header line + event item header line. + pub head: &'a [u8], + /// The event JSON payload. + pub event: &'a [u8], +} + +/// The crash context lives in static memory: the handler may run on a tiny +/// alternate stack (Rust's default is a few KiB) that cannot hold a ~2 KiB +/// struct plus the temporaries of building it. +struct CcCell(UnsafeCell>); +// SAFETY: access is serialized by the caller (`signal::BUSY`). +unsafe impl Sync for CcCell {} +static CC: CcCell = CcCell(UnsafeCell::new(MaybeUninit::zeroed())); + +/// Fill the static `crash-context` description of the fault from the +/// handler's arguments and return it. +/// +/// # Safety +/// `info` and `uc` must be the pointers the kernel passed to an `SA_SIGINFO` +/// handler (or null), and the caller must have exclusive access to the static +/// (the crash flag `signal::BUSY`). +pub(crate) unsafe fn crash_context( + info: *const libc::siginfo_t, + uc: *const libc::c_void, +) -> &'static CrashContext { + // SAFETY: exclusive access per the contract; CrashContext is plain old + // data for which the all-zero pattern is valid. + let cc: &'static mut CrashContext = unsafe { (*CC.0.get()).assume_init_mut() }; + // SAFETY: gettid/getpid have no preconditions. + cc.tid = unsafe { libc::syscall(libc::SYS_gettid) } as libc::pid_t; + // SAFETY: as above. + cc.pid = unsafe { libc::getpid() }; + + // SAFETY: per the function contract; the kernel's siginfo is valid. + if let Some(info) = unsafe { info.as_ref() } { + cc.siginfo.ssi_signo = info.si_signo as u32; + cc.siginfo.ssi_code = info.si_code; + cc.siginfo.ssi_errno = info.si_errno; + // SAFETY: si_addr is valid for fault signals; for others it is + // unspecified but reading it is harmless. + cc.siginfo.ssi_addr = unsafe { info.si_addr() } as u64; + } + if !uc.is_null() { + // SAFETY: the kernel passes a ucontext whose layout is arch-specific + // and identical to `crash_context::ucontext_t` (the premise of that + // crate); `uc` is valid for reads of that size and `cc.context` for + // writes; they cannot overlap (static vs. signal frame). + unsafe { + std::ptr::copy_nonoverlapping(uc.cast::(), &raw mut cc.context, 1); + } + #[cfg(not(target_arch = "arm"))] + { + let fp = cc.context.uc_mcontext.fpregs; + if !fp.is_null() { + // SAFETY: the kernel points fpregs at the saved FP state in + // the signal frame; the destination is our static. + unsafe { std::ptr::copy_nonoverlapping(fp, &raw mut cc.float_state, 1) }; + } + } + } + cc +} + +/// Fork the dumping child and wait for it. Returns true if the envelope with +/// the minidump has been written to `cap.path`. +pub(crate) fn capture(cap: &Capture<'_>, cc: &CrashContext) -> bool { + // Allow any process (our child) to ptrace us under Yama's ptrace_scope=1. + // SAFETY: prctl with these arguments only changes our own ptracer policy. + unsafe { + libc::prctl( + libc::PR_SET_PTRACER, + libc::PR_SET_PTRACER_ANY as libc::c_ulong, + 0, + 0, + 0, + ) + }; + + // The handler runs on a small alternate stack, so the child gets its own. + // SAFETY: anonymous private mapping; checked for failure below. + let stack = unsafe { + libc::mmap( + std::ptr::null_mut(), + STACK, + libc::PROT_READ | libc::PROT_WRITE, + libc::MAP_PRIVATE | libc::MAP_ANONYMOUS | libc::MAP_NORESERVE | libc::MAP_STACK, + -1, + 0, + ) + }; + if stack == libc::MAP_FAILED { + return false; + } + let mut arg = ChildArg { cap, cc }; + // SAFETY: fork-style clone (no CLONE_VM, so the child gets a private + // copy-on-write image and `arg` stays valid in it) running `child_entry` + // on the fresh stack; glibc's wrapper runs no atfork handlers. + let pid = unsafe { + libc::clone( + child_entry, + stack.cast::().add(STACK).cast(), + libc::SIGCHLD, + (&raw mut arg).cast(), + ) + }; + if pid < 0 { + // SAFETY: unmapping our own mapping. + unsafe { libc::munmap(stack, STACK) }; + return false; + } + let r = wait_for(pid); + // SAFETY: the child is gone; unmapping our own mapping. + unsafe { libc::munmap(stack, STACK) }; + r +} + +fn wait_for(pid: libc::pid_t) -> bool { + let start = Instant::now(); + loop { + let mut status = 0; + // SAFETY: waitpid with a valid out pointer. + let r = unsafe { libc::waitpid(pid, &raw mut status, libc::WNOHANG) }; + if r == pid { + return libc::WIFEXITED(status) && libc::WEXITSTATUS(status) == 0; + } + if r < 0 && std::io::Error::last_os_error().kind() != std::io::ErrorKind::Interrupted { + return false; + } + if start.elapsed() > TIMEOUT { + // SAFETY: kill/waitpid on our own child. + unsafe { + libc::kill(pid, libc::SIGKILL); + libc::waitpid(pid, &raw mut status, 0); + } + return false; + } + let ts = libc::timespec { + tv_sec: 0, + tv_nsec: 5_000_000, + }; + // SAFETY: nanosleep with valid pointers. + unsafe { libc::nanosleep(&raw const ts, std::ptr::null_mut()) }; + } +} + +struct ChildArg<'a, 'b> { + cap: &'a Capture<'b>, + cc: &'a CrashContext, +} + +extern "C" fn child_entry(arg: *mut libc::c_void) -> libc::c_int { + // SAFETY: `arg` points to the parent's `ChildArg`, copied into this + // process by the clone. + let arg = unsafe { &*arg.cast::>() }; + i32::from(!child_main(arg.cap, arg.cc)) +} + +fn child_main(cap: &Capture<'_>, cc: &CrashContext) -> bool { + #[allow(clippy::result_large_err)] + let attempt = |cc: &CrashContext| { + let mut writer = MinidumpWriterConfig::new(cc.pid, cc.tid); + writer.set_crash_context(CrashContextExt { inner: cc.clone() }); + let mut sink = Cursor::new(Vec::new()); + writer + .write(&mut sink) + .map(|d| if d.is_empty() { sink.into_inner() } else { d }) + }; + let dump = match attempt(cc) { + Ok(d) => d, + Err(e) => { + // The child may allocate; say why the first try failed. + let _ = writeln!( + std::io::stderr(), + "warning(sentry): minidump failed err={e}" + ); + // A stack overflow faults with the stack pointer in the guard + // page, which the writer cannot read a stack from. Retry with + // the pointer moved to the first readable address above it so + // the thread's stack is still captured. + let mut fixed = cc.clone(); + if !clamp_stack_pointer(&mut fixed) { + return false; + } + match attempt(&fixed) { + Ok(d) => d, + Err(e) => { + let _ = writeln!( + std::io::stderr(), + "warning(sentry): minidump retry failed err={e}" + ); + return false; + } + } + } + }; + write_envelope(cap, &dump) +} + +#[cfg(target_arch = "x86_64")] +fn sp_mut(cc: &mut CrashContext) -> &mut i64 { + &mut cc.context.uc_mcontext.gregs[libc::REG_RSP as usize] +} +#[cfg(target_arch = "aarch64")] +fn sp_mut(cc: &mut CrashContext) -> &mut u64 { + &mut cc.context.uc_mcontext.sp +} + +/// If the page holding the stack pointer cannot be read (a stack overflow +/// faults in/at the guard page), move the pointer up to the first readable page +/// above it (within the 1 MiB the kernel's stack guard gap can span). Returns +/// whether it was changed. +#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] +fn clamp_stack_pointer(cc: &mut CrashContext) -> bool { + const PAGE: u64 = 4096; + let pid = cc.pid; + let readable = |addr: u64| { + let mut b = [0u8; 8]; + let local = libc::iovec { + iov_base: b.as_mut_ptr().cast(), + iov_len: b.len(), + }; + let remote = libc::iovec { + iov_base: addr as *mut libc::c_void, + iov_len: b.len(), + }; + // SAFETY: both iovecs describe valid ranges (the remote one is only + // interpreted by the kernel in the other process). + unsafe { libc::process_vm_readv(pid, &raw const local, 1, &raw const remote, 1, 0) == 8 } + }; + let sp = *sp_mut(cc) as u64; + let base = sp & !(PAGE - 1); + if readable(base) { + return false; + } + let mut addr = base + PAGE; + while addr <= base + (1 << 20) { + if readable(addr) { + *sp_mut(cc) = addr as _; + return true; + } + addr += PAGE; + } + false +} + +#[cfg(not(any(target_arch = "x86_64", target_arch = "aarch64")))] +fn clamp_stack_pointer(_cc: &mut CrashContext) -> bool { + false +} + +fn write_envelope(cap: &Capture<'_>, dump: &[u8]) -> bool { + // SAFETY: `path` is NUL terminated by the caller. + let fd = unsafe { + libc::open( + cap.path.as_ptr().cast(), + libc::O_WRONLY | libc::O_CREAT | libc::O_EXCL | libc::O_CLOEXEC, + 0o644, + ) + }; + if fd < 0 { + return false; + } + // SAFETY: we own the fd just opened. + let mut f = unsafe { std::fs::File::from_raw_fd(fd) }; + let item_head = format!( + "\n{{\"type\":\"attachment\",\"length\":{},\"filename\":\"minidump.dmp\",\"attachment_type\":\"event.minidump\"}}\n", + dump.len() + ); + let ok = f + .write_all(cap.head) + .and_then(|()| f.write_all(cap.event)) + .and_then(|()| f.write_all(item_head.as_bytes())) + .and_then(|()| f.write_all(dump)) + .and_then(|()| f.write_all(b"\n")) + .is_ok(); + drop(f); + if !ok { + // SAFETY: remove the partial file so the caller can fall back. + unsafe { libc::unlink(cap.path.as_ptr().cast()) }; + } + ok +} diff --git a/crates/ghostty-crash/src/signal.rs b/crates/ghostty-crash/src/signal.rs index 800c96d..13e7310 100644 --- a/crates/ghostty-crash/src/signal.rs +++ b/crates/ghostty-crash/src/signal.rs @@ -133,7 +133,7 @@ fn mkdir_p(path: &mut [u8], len: usize) { /// Write a crash report for `sig`. Returns without doing anything if the /// crash handler isn't active or another crash is being written. -fn write_signal_report(name: &str, desc: &str) { +fn write_signal_report(name: &str, desc: &str, info: *mut libc::siginfo_t, uc: *mut libc::c_void) { let Some(st) = sentry::state() else { return }; if BUSY.swap(true, Ordering::Acquire) { return; @@ -191,6 +191,30 @@ fn write_signal_report(name: &str, desc: &str) { // The directory must exist for writing to work. mkdir_p(&mut sc.path, st.dir.len()); + + // Preferred: an envelope with a minidump of the whole process, written by + // a forked helper that ptraces us (Linux only; see `minidump`). + #[cfg(target_os = "linux")] + { + // SAFETY: `info`/`uc` are the pointers the kernel gave the handler. + let cc = unsafe { crate::minidump::crash_context(info, uc) }; + let cap = crate::minidump::Capture { + path: &sc.path[..=o], + head: &sc.head[..head_len], + event: &sc.event[..ev_len], + }; + if crate::minidump::capture(&cap, cc) { + write_all( + 2, + b"warning(sentry): crash report with minidump written to disk path=", + ); + write_all(2, &sc.path[..o]); + write_all(2, b"\n"); + return; + } + } + #[cfg(not(target_os = "linux"))] + let _ = (info, uc); // SAFETY: path is NUL terminated. let fd = unsafe { libc::open( @@ -212,9 +236,9 @@ fn write_signal_report(name: &str, desc: &str) { } } -extern "C" fn handler(sig: libc::c_int, _info: *mut libc::siginfo_t, _ctx: *mut libc::c_void) { +extern "C" fn handler(sig: libc::c_int, info: *mut libc::siginfo_t, ctx: *mut libc::c_void) { if let Some((_, name, desc)) = SIGNALS.iter().find(|(s, ..)| *s == sig) { - write_signal_report(name, desc); + write_signal_report(name, desc, info, ctx); } // Restore the default action and re-raise so the process dies with the // original signal (core dump, exit status) as if we were never here. diff --git a/crates/ghostty-crash/tests/minidump.rs b/crates/ghostty-crash/tests/minidump.rs new file mode 100644 index 0000000..c1c9f7e --- /dev/null +++ b/crates/ghostty-crash/tests/minidump.rs @@ -0,0 +1,165 @@ +//! The fatal-signal path writes an envelope whose second item is a minidump +//! (`event.minidump` attachment) of the crashed process. A child process +//! (this test executable re-run with `GHOSTTY_MD_CHILD`) crashes on a spawned +//! thread; the parent parses the minidump with the `minidump` crate and checks +//! that it contains the crashing thread, its stack and the exception. +#![cfg(target_os = "linux")] +#![allow(clippy::unwrap_used, unsafe_code)] + +use std::{os::unix::process::ExitStatusExt, path::Path, process::Command}; + +use ghostty_crash::{ + Options, + envelope::{Envelope, Item, ItemType}, +}; +use minidump::{ + Minidump, + MinidumpException, + MinidumpMemoryList, + MinidumpModuleList, + MinidumpSystemInfo, + MinidumpThreadList, + Module, +}; + +fn child_main(mode: &str) { + let dir = std::env::var("GHOSTTY_CRASH_DIR").unwrap(); + let tid_file = std::env::var("GHOSTTY_MD_TID").unwrap(); + ghostty_crash::install(&Options { + dir: Some(dir.into()), + ..Options::default() + }) + .unwrap(); + let mode = mode.to_owned(); + // Crash on a non-main thread so "the crashing thread" is distinguishable + // from the thread that is merely waiting. + let h = std::thread::Builder::new() + .name("crasher".into()) + .spawn(move || { + // SAFETY: gettid has no preconditions. + let tid = unsafe { libc::syscall(libc::SYS_gettid) }; + std::fs::write(tid_file, tid.to_string()).unwrap(); + match mode.as_str() { + "segv" => { + // SAFETY: intentionally invalid write to fault. + unsafe { std::ptr::write_volatile(8 as *mut u8, 1) }; + } + "abort" => std::process::abort(), + "overflow" => { + // Unbounded on purpose: overflowing the stack is the test. + #[allow(unconditional_recursion)] + fn recurse(n: u64) -> u64 { + let pad = [n as u8; 256]; + std::hint::black_box(&pad); + recurse(n + 1) + u64::from(pad[0]) + } + std::process::exit(i32::from(recurse(0) == 1)); + } + _ => unreachable!(), + } + }) + .unwrap(); + let _ = h.join(); + std::thread::sleep(std::time::Duration::from_secs(30)); +} + +#[test] +fn minidump_child() { + if let Ok(mode) = std::env::var("GHOSTTY_MD_CHILD") { + child_main(&mode); + } +} + +/// Run the crashing child; returns (exit status, crashing tid, temp dir). +fn crash(mode: &str) -> (std::process::ExitStatus, i32, tempfile::TempDir) { + let t = tempfile::tempdir().unwrap(); + let dir = t.path().join("crash"); + let tid_file = t.path().join("tid"); + let st = Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "minidump_child", + "--nocapture", + "--test-threads=1", + ]) + .env("GHOSTTY_MD_CHILD", mode) + .env("GHOSTTY_CRASH_DIR", &dir) + .env("GHOSTTY_MD_TID", &tid_file) + .env("RUST_BACKTRACE", "0") + .status() + .unwrap(); + let tid = std::fs::read_to_string(tid_file).unwrap().parse().unwrap(); + (st, tid, t) +} + +fn only_report(dir: &Path) -> Vec { + let files: Vec<_> = std::fs::read_dir(dir) + .unwrap() + .map(|e| e.unwrap().path()) + .collect(); + assert_eq!(1, files.len(), "{files:?}"); + std::fs::read(&files[0]).unwrap() +} + +fn check(mode: &str, signal: i32) { + let (st, tid, t) = crash(mode); + assert_eq!(Some(signal), st.signal(), "{st:?}"); + + // Envelope: event + minidump attachment. + let data = only_report(&t.path().join("crash")); + let mut env = Envelope::parse_slice(&data).unwrap(); + assert_eq!(2, env.items.len(), "no minidump attached"); + assert_eq!(ItemType::Event, env.items[0].item_type()); + env.items[1].decode().unwrap(); + let Item::Attachment(a) = &env.items[1] else { + panic!("second item is not an attachment") + }; + assert_eq!("minidump.dmp", a.filename); + assert_eq!(Some("event.minidump"), a.attachment_type.as_deref()); + + // The minidump parses. + let dump = Minidump::read(a.payload.clone()).expect("minidump parses"); + + // The crashing thread is in the thread list, with stack memory. + let threads = dump.get_stream::().unwrap(); + let ids: Vec = threads.threads.iter().map(|t| t.raw.thread_id).collect(); + let crasher = threads + .threads + .iter() + .find(|th| th.raw.thread_id == tid as u32) + .unwrap_or_else(|| panic!("tid {tid} not in {ids:?}")); + assert!(crasher.raw.stack.memory.data_size > 0, "no stack captured"); + assert!(ids.len() >= 2, "main thread missing: {ids:?}"); + + // The exception stream blames that thread and carries registers + signal. + let exc = dump.get_stream::().unwrap(); + assert_eq!(tid as u32, exc.get_crashing_thread_id()); + assert_eq!(signal as u32, exc.raw.exception_record.exception_code); + let sys = dump.get_stream::().unwrap(); + let ctx = exc + .context(&sys, None) + .expect("exception context (registers)"); + assert!(ctx.valid_registers().count() > 10); + + // Modules and memory were captured (needed to symbolicate/stack-walk). + let modules = dump.get_stream::().unwrap(); + assert!(modules.iter().count() > 1); + assert!(modules.iter().any(|m| m.code_file().contains("minidump"))); + let mem = dump.get_stream::().unwrap(); + assert!(mem.iter().count() > 0); +} + +#[test] +fn sigsegv_report_contains_minidump() { + check("segv", libc::SIGSEGV); +} + +#[test] +fn abort_report_contains_minidump() { + check("abort", libc::SIGABRT); +} + +#[test] +fn stack_overflow_report_contains_minidump() { + check("overflow", libc::SIGSEGV); +} diff --git a/crates/ghostty-renderer/src/shadertoy.rs b/crates/ghostty-renderer/src/shadertoy.rs index 897d879..1611e73 100644 --- a/crates/ghostty-renderer/src/shadertoy.rs +++ b/crates/ghostty-renderer/src/shadertoy.rs @@ -371,7 +371,118 @@ pub fn msl_from_compiled(compiled: &Compiled) -> Result { let (msl, _info) = msl_out::write_string(&module, &compiled.info, &options, &pipeline) .map_err(|e| fail(&e))?; - Ok(msl) + Ok(fix_uniform_array_strides(&module, msl)) +} + +/// A uniform-block member that is an array of scalars. std140 gives it a +/// 16-byte element stride (what we upload and what spirv-cross emits padded +/// types for), but naga's MSL writer ignores the array `stride` and emits a +/// tightly packed `float[N]`. +struct StridedArray { + member: String, + /// naga's MSL name of the array wrapper struct (`type_`). + type_name: String, + /// MSL scalar element type. + scalar: &'static str, + len: u32, + /// Lanes of the padded element (stride / scalar size). + lanes: u32, +} + +fn strided_uniform_arrays(module: &naga::Module) -> Vec { + let mut out = Vec::new(); + for (_, gv) in module.global_variables.iter() { + if gv.space != naga::AddressSpace::Uniform { + continue; + } + let naga::TypeInner::Struct { members, .. } = &module.types[gv.ty].inner else { + continue; + }; + for m in members { + let naga::TypeInner::Array { + base, + size: naga::ArraySize::Constant(n), + stride, + } = &module.types[m.ty].inner + else { + continue; + }; + let naga::TypeInner::Scalar(sc) = &module.types[*base].inner else { + continue; + }; + let scalar = match (sc.kind, sc.width) { + (naga::ScalarKind::Float, 4) => "float", + (naga::ScalarKind::Sint, 4) => "int", + (naga::ScalarKind::Uint, 4) => "uint", + _ => continue, + }; + let size = u32::from(sc.width); + if *stride <= size { + continue; + } + let (Some(name), true) = (m.name.clone(), stride % size == 0) else { + continue; + }; + out.push(StridedArray { + member: name, + type_name: format!("type_{}", m.ty.index()), + scalar, + len: n.get(), + lanes: stride / size, + }); + } + } + out +} + +/// Make the MSL uniform block match the std140 layout we upload: scalar +/// arrays become arrays of `vecN` (stride 16 for std140) and element reads +/// take `.x`. The writer ignores array strides, so this is done on the text. +fn fix_uniform_array_strides(module: &naga::Module, mut msl: String) -> String { + for a in strided_uniform_arrays(module) { + let wrapper = format!("{}_std140", a.type_name); + // 1. The block member uses the padded wrapper. + let decl = format!(" {} {};", a.type_name, a.member); + if !msl.contains(&decl) { + continue; + } + msl = msl.replace(&decl, &format!(" {wrapper} {};", a.member)); + // 2. Define the wrapper before the struct that uses it. + let def = format!( + "struct {wrapper} {{\n metal::vec<{}, {}> inner[{}];\n}};\n", + a.scalar, a.lanes, a.len + ); + if let Some(at) = msl.find(&format!("\n {wrapper} {};", a.member)) + && let Some(start) = msl[..at].rfind("\nstruct ") + { + msl.insert_str(start + 1, &def); + } + // 3. Element reads: `.member.inner[idx]` -> `... [idx].x`. + let needle = format!(".{}.inner[", a.member); + let mut from = 0; + while let Some(rel) = msl[from..].find(&needle) { + let open = from + rel + needle.len() - 1; + let mut depth = 0usize; + let mut close = None; + for (k, ch) in msl[open..].char_indices() { + match ch { + '[' => depth += 1, + ']' => { + depth -= 1; + if depth == 0 { + close = Some(open + k); + break; + } + } + _ => {} + } + } + let Some(close) = close else { break }; + msl.insert_str(close + 1, ".x"); + from = close + 3; + } + } + msl } /// Convert a compiled shader to GLSL 430 core (`glslFromSpv`). @@ -628,4 +739,124 @@ mod tests { assert_eq!(offset_of!(Uniforms, selection_foreground_color), 4480); assert_eq!(size_of::(), 4496); } + + /// (size, align) of an MSL type as emitted by naga for the shadertoy + /// uniform block, resolving the wrapper structs defined in `msl`. + fn msl_size_align(msl: &str, ty: &str) -> (usize, usize) { + match ty { + "float" | "int" | "uint" => (4, 4), + "metal::packed_float3" => (12, 4), + "metal::float2" => (8, 8), + "metal::float3" | "metal::float4" => (16, 16), + _ => { + if let Some(rest) = ty.strip_prefix("metal::vec<") { + let (scalar, n) = rest.trim_end_matches('>').split_once(", ").unwrap(); + let n: usize = n.parse().unwrap(); + assert!(matches!(scalar, "float" | "int" | "uint")); + let n = if n == 3 { 4 } else { n }; + return (4 * n, 4 * n); + } + // A wrapper struct `struct { inner[N]; };`. + let head = format!("struct {ty} {{\n "); + let at = msl.find(&head).unwrap_or_else(|| panic!("no struct {ty}")); + let body = &msl[at + head.len()..]; + let line = &body[..body.find(';').unwrap()]; + let (elem, count) = line.rsplit_once(" inner[").unwrap(); + let count: usize = count.trim_end_matches(']').parse().unwrap(); + let (es, ea) = msl_size_align(msl, elem); + (es * count, ea) + } + } + } + + /// The `(offset, name)` of each member of `struct Globals` in `msl`, + /// skipping `char _padN[k]` fillers, plus the struct size. + fn msl_globals_layout(msl: &str) -> (Vec<(usize, String)>, usize) { + let at = msl.find("struct Globals {\n").unwrap(); + let body = &msl[at + "struct Globals {\n".len()..]; + let body = &body[..body.find("\n};").unwrap()]; + let mut off = 0usize; + let mut max_align = 1; + let mut members = Vec::new(); + for line in body.lines() { + let line = line.trim().trim_end_matches(';'); + let (ty, name) = line.rsplit_once(' ').unwrap(); + if ty == "char" { + let n: usize = name + .split('[') + .nth(1) + .unwrap() + .trim_end_matches(']') + .parse() + .unwrap(); + off += n; + continue; + } + let (size, align) = msl_size_align(msl, ty); + off = off.next_multiple_of(align); + max_align = max_align.max(align); + members.push((off, name.to_owned())); + off += size; + } + (members, off.next_multiple_of(max_align)) + } + + #[test] + fn msl_uniform_block_matches_std140_upload_layout() { + use std::mem::offset_of as o; + // A shader that reads the scalar array so the accessor path is used. + let user = "void mainImage(out vec4 c, in vec2 p) { \ + int i = int(p.x) & 3; c = vec4(iChannelTime[i], iChannelTime[2], iTime, iSampleRate); }"; + let compiled = compile_shadertoy(user, None).unwrap(); + let msl = msl_from_compiled(&compiled).unwrap(); + let (members, size) = msl_globals_layout(&msl); + + let expect = [ + ("iResolution", o!(Uniforms, resolution)), + ("iTime", o!(Uniforms, time)), + ("iTimeDelta", o!(Uniforms, time_delta)), + ("iFrameRate", o!(Uniforms, frame_rate)), + ("iFrame", o!(Uniforms, frame)), + ("iChannelTime", o!(Uniforms, channel_time)), + ("iChannelResolution", o!(Uniforms, channel_resolution)), + ("iMouse", o!(Uniforms, mouse)), + ("iDate", o!(Uniforms, date)), + ("iSampleRate", o!(Uniforms, sample_rate)), + ("iCurrentCursor", o!(Uniforms, current_cursor)), + ("iPreviousCursor", o!(Uniforms, previous_cursor)), + ("iCurrentCursorColor", o!(Uniforms, current_cursor_color)), + ("iPreviousCursorColor", o!(Uniforms, previous_cursor_color)), + ("iCurrentCursorStyle", o!(Uniforms, current_cursor_style)), + ("iPreviousCursorStyle", o!(Uniforms, previous_cursor_style)), + ("iCursorVisible", o!(Uniforms, cursor_visible)), + ("iTimeCursorChange", o!(Uniforms, cursor_change_time)), + ("iTimeFocus", o!(Uniforms, time_focus)), + ("iFocus", o!(Uniforms, focus)), + ("iPalette", o!(Uniforms, palette)), + ("iBackgroundColor", o!(Uniforms, background_color)), + ("iForegroundColor", o!(Uniforms, foreground_color)), + ("iCursorColor", o!(Uniforms, cursor_color)), + ("iCursorText", o!(Uniforms, cursor_text)), + // Upstream quirk: the Zig struct (and so ours) has the selection + // colors swapped relative to the GLSL block; both are vec3 so the + // offsets agree. + ( + "iSelectionForegroundColor", + o!(Uniforms, selection_background_color), + ), + ( + "iSelectionBackgroundColor", + o!(Uniforms, selection_foreground_color), + ), + ]; + assert_eq!(members.len(), expect.len(), "{msl}"); + for ((off, name), (ename, eoff)) in members.iter().zip(expect) { + assert_eq!(name, ename); + assert_eq!(*off, eoff, "MSL offset of {name}\n{msl}"); + } + assert_eq!(size, std::mem::size_of::()); + // Reads of the padded array take the first lane. + assert!(msl.contains(".iChannelTime.inner["), "{msl}"); + assert!(msl.contains("].x"), "{msl}"); + } } diff --git a/crates/ghostty-renderer/src/shadertoy_sampler.rs b/crates/ghostty-renderer/src/shadertoy_sampler.rs index 3e0756a..727c45c 100644 --- a/crates/ghostty-renderer/src/shadertoy_sampler.rs +++ b/crates/ghostty-renderer/src/shadertoy_sampler.rs @@ -18,9 +18,12 @@ //! //! Limitations (documented, reported as compile errors by naga): a sampler //! argument must be a plain identifier (no ternaries or struct members), -//! samplers cannot be stored in local variables/arrays/struct fields, and -//! overloads of one function name must agree on which parameters are -//! samplers. +//! samplers cannot be stored in local variables/arrays/struct fields. +//! +//! Overloads are keyed by their full signature (the per-parameter sampler +//! layout), not by name. At a call site the overload is chosen by arity and by +//! which arguments are samplers (`iChannelN` or a sampler parameter of the +//! enclosing function), which is what GLSL overload resolution would see. use std::collections::HashMap; @@ -171,10 +174,11 @@ struct Def { samplers: Vec>, } -struct FnInfo { - /// Per parameter: the sampler name, if the parameter is a `sampler2D`. - samplers: Vec>, -} +/// Overload signature: per parameter, whether it is a `sampler2D`. +type SamplerLayout = Vec; + +/// The combined samplers the host provides. +const GLOBAL_SAMPLERS: [&str; 4] = ["iChannel0", "iChannel1", "iChannel2", "iChannel3"]; /// Rewrite the user's ShaderToy source. Returns the source unchanged if it /// needs no adaptation other than `texture2D(` -> `texture(`. @@ -188,7 +192,7 @@ pub fn rewrite(src: &str) -> String { .collect(); // Pass 1: find top-level functions with sampler2D parameters. - let mut funcs: HashMap<&str, FnInfo> = HashMap::new(); + let mut funcs: HashMap<&str, Vec> = HashMap::new(); let mut defs: Vec = Vec::new(); let mut k = 0; let mut brace = 0i32; @@ -211,7 +215,6 @@ pub fn rewrite(src: &str) -> String { if next == Some("{") || next == Some(";") { let params = split_args(&toks, &sig, k + 3, close); let mut samplers = Vec::new(); - let mut any = false; for &(a, b) in ¶ms { let has = (a..b).any(|q| toks[sig[q]].text == "sampler2D"); if has { @@ -220,7 +223,6 @@ pub fn rewrite(src: &str) -> String { .map(|q| &toks[sig[q]]) .find(|t| t.kind == Kind::Ident && t.text != "sampler2D") .map(|t| t.text.to_owned()); - any |= name.is_some(); samplers.push(name); } else { samplers.push(None); @@ -231,12 +233,12 @@ pub fn rewrite(src: &str) -> String { } else { None }; - if any { - funcs - .entry(toks[sig[name_k]].text) - .or_insert_with(|| FnInfo { - samplers: samplers.clone(), - }); + // Every overload is recorded (also the ones without + // samplers) so call sites can tell them apart. + let layout: SamplerLayout = samplers.iter().map(Option::is_some).collect(); + let list = funcs.entry(toks[sig[name_k]].text).or_default(); + if !list.contains(&layout) { + list.push(layout); } defs.push(Def { name_k, @@ -293,18 +295,32 @@ pub fn rewrite(src: &str) -> String { if def_names.contains(&k) { continue; } - let Some(info) = funcs.get(t.text) else { + let Some(layouts) = funcs.get(t.text) else { continue; }; let Some(close) = matching(&toks, &sig, k + 1) else { continue; }; let args = split_args(&toks, &sig, k + 2, close); - for (ai, &(a, b)) in args.iter().enumerate() { - if info.samplers.get(ai).is_some_and(Option::is_some) - && b == a + 1 + // Sampler parameters of the function this call is in. + let enclosing: Vec<&str> = defs + .iter() + .filter(|d| d.body.is_some_and(|(o, c)| o < k && k < c)) + .flat_map(|d| d.samplers.iter().flatten().map(String::as_str)) + .collect(); + let is_sampler_arg = |&(a, b): &(usize, usize)| { + b == a + 1 && toks[sig[a]].kind == Kind::Ident - { + && (GLOBAL_SAMPLERS.contains(&toks[sig[a]].text) + || enclosing.contains(&toks[sig[a]].text)) + }; + let actual: SamplerLayout = args.iter().map(is_sampler_arg).collect(); + // Pick the overload with this arity and sampler positions. + if !layouts.contains(&actual) { + continue; + } + for (ai, &(a, _)) in args.iter().enumerate() { + if actual[ai] { let n = toks[sig[a]].text; repl.insert(sig[a], format!("{n}_tex, {n}_smp")); } @@ -336,6 +352,35 @@ mod tests { assert!(out.contains("fetch(iChannel0_tex, iChannel0_smp, p)")); } + #[test] + fn overloads_are_keyed_by_signature() { + let out = rewrite( + "vec4 f(sampler2D s, vec2 uv) { return texture(s, uv); }\n\ + vec4 f(vec2 uv, sampler2D s) { return texture(s, uv); }\n\ + vec4 f(vec2 uv, float x) { return vec4(uv, x, 1.0); }\n\ + void mainImage(out vec4 c, in vec2 p) {\n\ + c = f(iChannel0, p) + f(p, iChannel1) + f(p, 1.0);\n\ + }", + ); + assert!(out.contains("vec4 f(texture2D s_tex, sampler s_smp, vec2 uv)")); + assert!(out.contains("vec4 f(vec2 uv, texture2D s_tex, sampler s_smp)")); + assert!(out.contains("vec4 f(vec2 uv, float x)")); + assert!(out.contains("f(iChannel0_tex, iChannel0_smp, p)")); + assert!(out.contains("f(p, iChannel1_tex, iChannel1_smp)")); + assert!(out.contains("f(p, 1.0)")); + } + + #[test] + fn overloads_through_sampler_parameters() { + let out = rewrite( + "float g(sampler2D t) { return texture(t, vec2(0.0)).x; }\n\ + float g(float t) { return t; }\n\ + float h(sampler2D u, float v) { return g(u) + g(v); }\n", + ); + assert!(out.contains("g(u_tex, u_smp)")); + assert!(out.contains("+ g(v)")); + } + #[test] fn leaves_plain_code_and_rewrites_legacy_texture2d() { let src = "void mainImage(out vec4 c, in vec2 p) { c = texture2D(iChannel0, p); }"; diff --git a/crates/ghostty-terminal/src/kitty/graphics_image.rs b/crates/ghostty-terminal/src/kitty/graphics_image.rs index db63df6..abaa66b 100644 --- a/crates/ghostty-terminal/src/kitty/graphics_image.rs +++ b/crates/ghostty-terminal/src/kitty/graphics_image.rs @@ -353,6 +353,42 @@ impl LoadingImage { Ok(()) } + /// The file name `debug_dump` writes (Zig `@tagName` spellings). + #[cfg(debug_assertions)] + #[must_use] + pub fn debug_dump_filename(&self) -> String { + let format = match self.image.format { + TransmissionFormat::Rgb => "rgb", + TransmissionFormat::Rgba => "rgba", + TransmissionFormat::Png => "png", + TransmissionFormat::GrayAlpha => "gray_alpha", + TransmissionFormat::Gray => "gray", + }; + let compression = match self.image.compression { + TransmissionCompression::None => "none", + TransmissionCompression::ZlibDeflate => "zlib_deflate", + }; + format!( + "image-{format}-{compression}-{}x{}-{}.data", + self.image.width, self.image.height, self.image.id + ) + } + + /// Debug function to write the data to a file in `dir`. This is useful + /// for capturing some test data for unit tests. Only exists in debug + /// builds (Zig: `@compileError("debugDump in non-debug")`). + #[cfg(debug_assertions)] + pub fn debug_dump_to(&self, dir: &std::path::Path) -> std::io::Result<()> { + std::fs::write(dir.join(self.debug_dump_filename()), &self.data) + } + + /// Same as [`Self::debug_dump_to`] using the current working directory, + /// like Zig's `debugDump`. + #[cfg(debug_assertions)] + pub fn debug_dump(&self) -> std::io::Result<()> { + self.debug_dump_to(std::path::Path::new(".")) + } + /// Adds a chunk of data to the image. Use this if the image /// is coming in chunks (the "m" parameter in the protocol). pub fn add_data(&mut self, data: &[u8]) -> Result<(), Error> { @@ -623,6 +659,37 @@ mod tests { use super::*; + #[cfg(debug_assertions)] + #[test] + fn loading_image_debug_dump_writes_data() { + let mut li = LoadingImage { + image: Image { + id: 7, + width: 3, + height: 2, + format: TransmissionFormat::GrayAlpha, + compression: TransmissionCompression::ZlibDeflate, + ..Image::default() + }, + data: vec![1, 2, 3, 4], + display: None, + quiet: Quiet::default(), + }; + assert_eq!( + "image-gray_alpha-zlib_deflate-3x2-7.data", + li.debug_dump_filename() + ); + let dir = std::env::temp_dir().join(format!("ghostty-dump-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + li.debug_dump_to(&dir).unwrap(); + let got = std::fs::read(dir.join(li.debug_dump_filename())).unwrap(); + assert_eq!(vec![1, 2, 3, 4], got); + li.image.format = TransmissionFormat::Png; + li.image.compression = TransmissionCompression::None; + assert_eq!("image-png-none-3x2-7.data", li.debug_dump_filename()); + std::fs::remove_dir_all(&dir).unwrap(); + } + const RGB_NONE_20X15: &[u8] = include_bytes!("testdata/image-rgb-none-20x15-2147483647-raw.data"); const RGB_ZLIB_128X96: &[u8] = diff --git a/crates/ghostty-terminal/src/lib.rs b/crates/ghostty-terminal/src/lib.rs index 0a9b031..a381b25 100644 --- a/crates/ghostty-terminal/src/lib.rs +++ b/crates/ghostty-terminal/src/lib.rs @@ -22,3 +22,4 @@ pub mod style; pub mod tabstops; pub mod terminal; pub mod tmux; +pub mod tripwire; diff --git a/crates/ghostty-terminal/src/page_list.rs b/crates/ghostty-terminal/src/page_list.rs index 90441ab..2591284 100644 --- a/crates/ghostty-terminal/src/page_list.rs +++ b/crates/ghostty-terminal/src/page_list.rs @@ -196,6 +196,29 @@ pub struct PageList { pin_serial: u64, pause_checks: usize, } +/// Failure points of [`PageList::init`] (Zig: `init_tw`). The Zig +/// `init_memory_pool` point has no counterpart: Rust has no up-front memory +/// pool, the slabs start empty and grow with the fallible reserves below. +/// `viewport_pin` and `viewport_pin_track` are two allocations in Zig (pool +/// create + set insert) that are one reserve in `track_pin` here; both points +/// are checked before it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum InitPoint { + InitPages, + ViewportPin, + ViewportPinTrack, +} +crate::tripwire_module!(init_tw, InitPoint, Error); + +/// Failure points of the page creation done by `init` (Zig: `initPages_tw`). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum InitPagesPoint { + Node, + BufStd, + BufNonStd, +} +crate::tripwire_module!(init_pages_tw, InitPagesPoint, Error); + fn std_size() -> usize { Page::layout(page::STD_CAPACITY).total_size } @@ -242,12 +265,15 @@ impl PageList { pin_serial: 0, pause_checks: 0, }; + init_tw::check(InitPoint::InitPages)?; while s.total_rows < usize::from(rows) { s.grow()?; } let pin = s .get_top_left(Tag::Screen) .ok_or(Error::InvalidCoordinate)?; + init_tw::check(InitPoint::ViewportPin)?; + init_tw::check(InitPoint::ViewportPinTrack)?; s.viewport_pin = Some(s.track_pin(pin)?); Ok(s) } @@ -269,6 +295,7 @@ impl PageList { if Page::layout(cap).total_size > u32::MAX as usize { return Err(Error::OutOfSpace); } + init_pages_tw::check(InitPagesPoint::Node)?; self.nodes.try_reserve(1).map_err(|_| Error::OutOfMemory)?; let mut data = if let Some(mut p) = self.clean_pool.pop().filter(|p| p.capacity == cap) { p.size = page::Size { @@ -281,6 +308,11 @@ impl PageList { p.reinit()?; p } else { + if Page::layout(cap).total_size <= std_size() { + init_pages_tw::check(InitPagesPoint::BufStd)?; + } else { + init_pages_tw::check(InitPagesPoint::BufNonStd)?; + } Page::init(cap)? }; data.size.rows = 0; @@ -2972,6 +3004,55 @@ mod tests { s.clear_dirty(); } #[test] + fn page_list_init_error() { + use crate::tripwire::ResetMode; + // Test every failure point in `init` and ensure we exit early + // without panicking or leaving the tripwire armed. + for tag in [ + InitPoint::InitPages, + InitPoint::ViewportPin, + InitPoint::ViewportPinTrack, + ] { + init_tw::error_always(tag, Error::OutOfMemory); + assert_eq!( + Err(Error::OutOfMemory), + PageList::init(80, 24, None).map(drop) + ); + init_tw::end(ResetMode::Reset).unwrap(); + } + + // init creates pages transitively, so check each failure there too. + let big = page::STD_CAPACITY.max_cols().unwrap() + 1; + for tag in [ + InitPagesPoint::Node, + InitPagesPoint::BufStd, + InitPagesPoint::BufNonStd, + ] { + init_pages_tw::error_always(tag, Error::OutOfMemory); + let cols = if tag == InitPagesPoint::BufStd { + 80 + } else { + big + }; + assert_eq!( + Err(Error::OutOfMemory), + PageList::init(cols, 24, None).map(drop) + ); + init_pages_tw::end(ResetMode::Reset).unwrap(); + } + + // Non-standard pages: fail on the second one. + init_pages_tw::error_after(InitPagesPoint::BufNonStd, Error::OutOfMemory, 1); + assert_eq!( + Err(Error::OutOfMemory), + PageList::init(big, page::STD_CAPACITY.rows + 1, None).map(drop) + ); + init_pages_tw::end(ResetMode::Reset).unwrap(); + + // And with nothing armed init still works. + PageList::init(80, 24, None).unwrap(); + } + #[test] fn page_list_init_rows_across_two_pages() { let mut cols = 50; while initial_capacity(cols).rows >= 100 { diff --git a/crates/ghostty-terminal/src/search/screen.rs b/crates/ghostty-terminal/src/search/screen.rs index 8a0f1cd..97a3a6d 100644 --- a/crates/ghostty-terminal/src/search/screen.rs +++ b/crates/ghostty-terminal/src/search/screen.rs @@ -30,6 +30,14 @@ use crate::{ size::CellCountInt, }; +/// Failure points of [`ScreenSearch::reload_active`] (Zig: `reloadActive_tw`). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ReloadActivePoint { + HistoryAppendNew, + HistoryAppendExisting, +} +crate::tripwire_module!(reload_active_tw, ReloadActivePoint, Error); + #[derive(Debug)] pub struct ScreenSearch { /// Identity of the screen being searched (its address). Only used to @@ -568,6 +576,7 @@ impl ScreenSearch { if hl.chunks[0].node == history_node { continue; } + reload_active_tw::check(ReloadActivePoint::HistoryAppendNew)?; results.push(hl); } @@ -584,6 +593,7 @@ impl ScreenSearch { // Matches! Reverse our list then append all the remaining // history items that didn't start on our original node. results.reverse(); + reload_active_tw::check(ReloadActivePoint::HistoryAppendExisting)?; results.append(&mut self.history_results); self.history_results = results; @@ -1086,6 +1096,75 @@ mod tests { search.deinit(t.screen_mut()); } + /// Shared setup of the two `reload_active partial history cleanup` tests: + /// several "Fizz" matches in history, then more that move into history on + /// the next reload. + fn reload_failure_setup() -> (Terminal, ScreenSearch) { + let mut t = terminal_scrollback(10, 2, usize::MAX); + feed(&mut t, b"Fizz\r\nFizz\r\n"); + while total_pages(&t) < 3 { + feed(&mut t, b"\r\n"); + } + for _ in 0..t.screen().pages.rows { + feed(&mut t, b"\r\n"); + } + feed(&mut t, b"Fizz."); + + // Complete initial search + let mut search = ScreenSearch::new(t.screen_mut(), b"Fizz").unwrap(); + search.search_all(t.screen_mut()).unwrap(); + + // Trigger reload by adding more content that changes the + // active/history boundary. + feed(&mut t, b"\r\nFizz\r\nFizz\r\n"); + while total_pages(&t) < 4 { + feed(&mut t, b"\r\n"); + } + for _ in 0..t.screen().pages.rows { + feed(&mut t, b"\r\n"); + } + (t, search) + } + + #[test] + fn reload_active_partial_history_cleanup_on_append_slice_error() { + use crate::tripwire::ResetMode; + let (mut t, mut search) = reload_failure_setup(); + let before = search.history_results.len(); + + // Fail at the final append (after the loop completes). Nothing may be + // half-applied to the existing history results. + reload_active_tw::error_always( + ReloadActivePoint::HistoryAppendExisting, + Error::OutOfMemory, + ); + // reload_active is called by select(), which triggers the error path. + assert_eq!( + Err(Error::OutOfMemory), + search.select(t.screen_mut(), Select::Next) + ); + reload_active_tw::end(ResetMode::Reset).unwrap(); + assert_eq!(before, search.history_results.len()); + search.deinit(t.screen_mut()); + } + + #[test] + fn reload_active_partial_history_cleanup_on_loop_append_error() { + use crate::tripwire::ResetMode; + let (mut t, mut search) = reload_failure_setup(); + let before = search.history_results.len(); + + // Fail after the first loop append succeeded. + reload_active_tw::error_after(ReloadActivePoint::HistoryAppendNew, Error::OutOfMemory, 1); + assert_eq!( + Err(Error::OutOfMemory), + search.select(t.screen_mut(), Select::Next) + ); + reload_active_tw::end(ResetMode::Reset).unwrap(); + assert_eq!(before, search.history_results.len()); + search.deinit(t.screen_mut()); + } + #[test] fn select_into_history() { let mut t = terminal_scrollback(10, 2, usize::MAX); diff --git a/crates/ghostty-terminal/src/tabstops.rs b/crates/ghostty-terminal/src/tabstops.rs index bf90bd7..1091c1f 100644 --- a/crates/ghostty-terminal/src/tabstops.rs +++ b/crates/ghostty-terminal/src/tabstops.rs @@ -59,6 +59,13 @@ pub struct Tabstops { dynamic_stops: Vec, } +/// Failure points of [`Tabstops::resize`] (Zig: `resize_tw`). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ResizePoint { + DynamicAlloc, +} +crate::tripwire_module!(resize_tw, ResizePoint, TryReserveError); + impl Default for Tabstops { fn default() -> Self { Self { @@ -141,6 +148,7 @@ impl Tabstops { } // Allocate first so that failure leaves our state untouched. + resize_tw::check(ResizePoint::DynamicAlloc)?; let mut new: Vec = Vec::new(); new.try_reserve_exact(size)?; new.resize(size, 0); @@ -175,6 +183,7 @@ impl Tabstops { #[cfg(test)] mod tests { use super::*; + use crate::tripwire::ResetMode; #[test] fn tabstops_basic() { @@ -238,7 +247,21 @@ mod tests { assert_eq!(9, count); } - // "Tabstops: resize alloc failure preserves state" is skipped: it uses - // Zig tripwire allocation failure injection. The Rust resize allocates - // before mutating, so a failure leaves `cols` unchanged by construction. + #[test] + fn tabstops_resize_alloc_failure_preserves_state() { + // If resize() fails during allocation, the original cols value is + // preserved (not corrupted). + let mut t = Tabstops::new(80, 8).unwrap(); + let original_cols = t.cols; + + // Trigger allocation failure when resizing beyond prealloc + let oom = Vec::::new().try_reserve(usize::MAX).unwrap_err(); + resize_tw::error_always(ResizePoint::DynamicAlloc, oom.clone()); + let result = t.resize(PREALLOC_COLUMNS * 2); + assert_eq!(Err(oom), result); + resize_tw::end(ResetMode::Reset).unwrap(); + + // cols should be unchanged after failed resize + assert_eq!(original_cols, t.cols); + } } diff --git a/crates/ghostty-terminal/src/tripwire.rs b/crates/ghostty-terminal/src/tripwire.rs new file mode 100644 index 0000000..1a014a9 --- /dev/null +++ b/crates/ghostty-terminal/src/tripwire.rs @@ -0,0 +1,264 @@ +//! Failure injection for testing error handling paths (port of +//! `tripwire.zig`). +//! +//! Improper cleanup on error paths is one of the highest sources of bugs. +//! Many of those points are hard to exercise in unit tests and rare to +//! encounter in production. This module injects errors at named points +//! during unit tests so that every error path can be exercised. +//! +//! # Usage +//! +//! Declare a module per fallible function with [`tripwire_module!`]. The +//! point enum is hand-curated; the error type is the function's own error. +//! Sprinkle `tw::check(Point::X)?` before every operation that you want to be +//! able to fail, then in a unit test configure expected failures with +//! [`error_always`](TripwireState::error_always) / +//! [`error_after`](TripwireState::error_after) and finish with +//! `tw::end(ResetMode::Reset)` to verify the expectations were met. +//! +//! ```ignore +//! tripwire_module!(resize_tw, ResizePoint, TryReserveError); +//! +//! fn resize() -> Result<(), E> { +//! resize_tw::check(ResizePoint::DynamicAlloc)?; +//! // ... +//! } +//! +//! #[test] +//! fn resize_fails() { +//! resize_tw::error_always(ResizePoint::DynamicAlloc, e); +//! assert!(resize().is_err()); +//! resize_tw::end(ResetMode::Reset).unwrap(); +//! } +//! ``` +//! +//! Outside of unit tests (`cfg(test)` of the crate that expands the macro) +//! `check` is a no-op that the optimizer removes. +//! +//! State is per module *and per thread*. Rust runs tests on separate threads +//! in parallel, so this gives each test the same isolation Zig gets from +//! running tests sequentially. + +use std::fmt::Debug; + +/// Whether to reset expectations in [`TripwireState::end`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ResetMode { + Reset, + Retain, +} + +/// Returned from `end` if a configured point was never tripped. +#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] +#[error("untripped tripwire")] +pub struct UntrippedError; + +#[derive(Debug, Clone)] +struct Tripwire { + /// Error to return when tripped. + err: E, + /// The number of times this tripwire has been reached. This is NOT the + /// number of times it has tripped, since we may have `min`. + reached: usize, + /// The minimum number of times this must be reached before tripping. + /// After this point it trips every time. This is a "before" check, so if + /// this is 1 it trips the second time it is reached. + min: usize, + /// True if this has been tripped at least once. + tripped: bool, +} + +/// The configured tripwires of one module. +#[derive(Debug, Clone)] +pub struct TripwireState { + wires: Vec<(P, Tripwire)>, +} + +impl Default for TripwireState { + fn default() -> Self { + Self { wires: Vec::new() } + } +} + +impl TripwireState { + /// Check for a failure at the given failure point. Place directly before + /// the operation that may fail. + pub fn check(&mut self, point: P) -> Result<(), E> { + let Some((_, w)) = self.wires.iter_mut().find(|(p, _)| *p == point) else { + return Ok(()); + }; + w.reached += 1; + if w.reached <= w.min { + return Ok(()); + } + w.tripped = true; + Err(w.err.clone()) + } + + /// Mark a failure point to always trip with the given error. + pub fn error_always(&mut self, point: P, err: E) { + self.error_after(point, err, 0); + } + + /// Mark a failure point to trip with the given error after it has been + /// reached at least `min` times. Zero is equivalent to `error_always`. + pub fn error_after(&mut self, point: P, err: E, min: usize) { + let wire = Tripwire { + err, + reached: 0, + min, + tripped: false, + }; + if let Some(slot) = self.wires.iter_mut().find(|(p, _)| *p == point) { + slot.1 = wire; + } else { + self.wires.push((point, wire)); + } + } + + /// Ends the tripwire session. Errors if there were untripped + /// expectations. Expectations are reset (per `mode`) even on error. + pub fn end(&mut self, mode: ResetMode) -> Result<(), UntrippedError> { + let mut untripped = false; + for (p, w) in &self.wires { + if !w.tripped { + log::warn!("untripped point={p:?}"); + untripped = true; + } + } + if mode == ResetMode::Reset { + self.reset(); + } + if untripped { + Err(UntrippedError) + } else { + Ok(()) + } + } + + /// Unset all the tripwires. You should usually call `end` instead. + pub fn reset(&mut self) { + self.wires.clear(); + } +} + +/// Declare a tripwire module `$name` with failure points `$point` (a `Copy + +/// PartialEq + Debug` enum) and error type `$err` (`Clone`). Generates +/// `$name::{check, error_always, error_after, end, reset}`. +#[macro_export] +macro_rules! tripwire_module { + ($vis:vis $name:ident, $point:ty, $err:ty) => { + #[allow(dead_code)] + $vis mod $name { + use super::*; + use $crate::tripwire::{ResetMode, TripwireState, UntrippedError}; + + ::std::thread_local! { + static STATE: ::std::cell::RefCell> = + ::std::cell::RefCell::new(TripwireState::default()); + } + + /// Check for a failure at `point`. A no-op outside of tests. + #[inline(always)] + pub fn check(point: $point) -> ::std::result::Result<(), $err> { + #[cfg(test)] + { + STATE.with(|s| s.borrow_mut().check(point)) + } + #[cfg(not(test))] + { + let _ = point; + Ok(()) + } + } + + pub fn error_always(point: $point, err: $err) { + STATE.with(|s| s.borrow_mut().error_always(point, err)); + } + + pub fn error_after(point: $point, err: $err, min: usize) { + STATE.with(|s| s.borrow_mut().error_after(point, err, min)); + } + + pub fn end(mode: ResetMode) -> ::std::result::Result<(), UntrippedError> { + STATE.with(|s| s.borrow_mut().end(mode)) + } + + pub fn reset() { + STATE.with(|s| s.borrow_mut().reset()); + } + } + }; +} + +#[cfg(test)] +mod tests { + use super::*; + + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + enum Io { + Read, + Write, + } + #[derive(Debug, Clone, PartialEq, Eq)] + enum E { + Oom, + } + crate::tripwire_module!(io, Io, E); + + #[test] + fn tripwire_basic() { + // Reset should work + io::end(ResetMode::Reset).unwrap(); + // By default, it's pass-through + io::check(Io::Read).unwrap(); + // Always trip + io::error_always(Io::Read, E::Oom); + assert_eq!(Err(E::Oom), io::check(Io::Read)); + // Happens again + assert_eq!(Err(E::Oom), io::check(Io::Read)); + // Other points are unaffected + io::check(Io::Write).unwrap(); + io::end(ResetMode::Reset).unwrap(); + } + + #[test] + fn tripwire_error_after() { + // Trip after 2 calls (on the 3rd call) + io::error_after(Io::Read, E::Oom, 2); + io::check(Io::Read).unwrap(); + io::check(Io::Read).unwrap(); + assert_eq!(Err(E::Oom), io::check(Io::Read)); + assert_eq!(Err(E::Oom), io::check(Io::Read)); + io::end(ResetMode::Reset).unwrap(); + } + + #[test] + fn tripwire_error_after_untripped_if_min_not_reached() { + io::error_after(Io::Read, E::Oom, 2); + io::check(Io::Read).unwrap(); + assert_eq!(Err(UntrippedError), io::end(ResetMode::Reset)); + // Expectations are reset even when end() fails. + io::check(Io::Read).unwrap(); + io::end(ResetMode::Reset).unwrap(); + } + + #[test] + fn tripwire_untripped_never_reached_and_retain() { + io::error_always(Io::Write, E::Oom); + assert_eq!(Err(UntrippedError), io::end(ResetMode::Retain)); + // Retained: still armed. + assert_eq!(Err(E::Oom), io::check(Io::Write)); + io::end(ResetMode::Reset).unwrap(); + } + + #[test] + fn tripwire_state_is_per_thread() { + io::error_always(Io::Read, E::Oom); + std::thread::spawn(|| io::check(Io::Read).unwrap()) + .join() + .unwrap(); + assert_eq!(Err(E::Oom), io::check(Io::Read)); + io::end(ResetMode::Reset).unwrap(); + } +} diff --git a/crates/ghostty-vt-capi/src/alloc.rs b/crates/ghostty-vt-capi/src/alloc.rs index 8cb88bb..075239a 100644 --- a/crates/ghostty-vt-capi/src/alloc.rs +++ b/crates/ghostty-vt-capi/src/alloc.rs @@ -3,12 +3,22 @@ //! Wrapper objects handed out through the C API (parsers, events, encoders) //! and the SGR parameter copy are allocated with the caller's allocator when //! one is given and with the Rust global allocator otherwise (Zig: libc -//! `c_allocator` / `wasm_allocator`). Interior Rust collections (e.g. the OSC -//! parser's buffer) always use the global allocator. +//! `c_allocator` / `wasm_allocator`). Parser-owned byte storage (the OSC +//! payload buffer, [`CByteStorage`]) and the copies handed back to C (the OSC +//! window title, [`CBlock`]) are allocated through the same allocator, so the +//! caller's limits and failures govern them: a failed allocation surfaces as +//! `GHOSTTY_OUT_OF_MEMORY` where Zig can report it (object creation) and +//! invalidates the OSC being parsed where Zig's writer fails. +//! +//! Transient values built while a command is parsed (the payload copies the +//! per-command parsers make before `ghostty_osc_end` returns) use the global +//! allocator; nothing of them outlives that call. #![allow(unsafe_code)] use std::{alloc::Layout, ffi::c_void, ptr::NonNull}; +use ghostty_vt::osc::{ByteStorage, StorageFull}; + /// C: `GhosttyAllocatorVtable` #[repr(C)] pub struct AllocatorVtable { @@ -133,3 +143,141 @@ impl Alloc { } } } + +/// A block of bytes allocated through an [`Alloc`] (Zig: an allocator-owned +/// slice). Freed with the same allocator on drop. +pub struct CBlock { + alloc: Alloc, + ptr: NonNull, + cap: usize, +} + +impl CBlock { + /// Allocate `cap` bytes (alignment 1); `None` when the allocator fails. + pub fn new(alloc: Alloc, cap: usize) -> Option { + let layout = Layout::from_size_align(cap, 1).ok()?; + let ptr = NonNull::new(alloc.alloc_raw(layout))?; + if cap > 0 { + // SAFETY: freshly allocated `cap` bytes; zeroing makes the whole + // block initialized so slices over it are sound. + unsafe { ptr.as_ptr().write_bytes(0, cap) }; + } + Some(CBlock { alloc, ptr, cap }) + } + + /// A copy of `bytes` followed by a NUL terminator. + pub fn from_bytes_z(alloc: Alloc, bytes: &[u8]) -> Option { + let mut b = CBlock::new(alloc, bytes.len() + 1)?; + b.as_mut_slice()[..bytes.len()].copy_from_slice(bytes); + b.as_mut_slice()[bytes.len()] = 0; + Some(b) + } + + pub fn as_ptr(&self) -> *const u8 { + self.ptr.as_ptr() + } + + pub fn capacity(&self) -> usize { + self.cap + } + + pub fn as_slice(&self) -> &[u8] { + // SAFETY: `ptr` is a live, zero-initialized allocation of `cap` bytes. + unsafe { std::slice::from_raw_parts(self.ptr.as_ptr(), self.cap) } + } + + pub fn as_mut_slice(&mut self) -> &mut [u8] { + // SAFETY: as above, with unique access through `&mut self`. + unsafe { std::slice::from_raw_parts_mut(self.ptr.as_ptr(), self.cap) } + } +} + +impl Drop for CBlock { + fn drop(&mut self) { + // SAFETY: allocated by `self.alloc` with this layout in `new`. + unsafe { + self.alloc.free_raw( + self.ptr.as_ptr(), + Layout::from_size_align_unchecked(self.cap, 1), + ); + } + } +} + +/// OSC payload storage backed by the C allocator. Starts with `base` bytes +/// (the parser's fixed buffer, allocated together with the parser so creation +/// can fail with out-of-memory) and grows by allocate-copy-free. +pub struct CByteStorage { + block: CBlock, + len: usize, + base: usize, +} + +// SAFETY: the storage is uniquely owned by its parser. Moving a parser to +// another thread is allowed under the same contract as Zig's: the caller's +// allocator must be usable from the thread that uses the parser. +unsafe impl Send for CByteStorage {} + +impl std::fmt::Debug for CByteStorage { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("CByteStorage") + .field("len", &self.len) + .field("cap", &self.block.capacity()) + .field("base", &self.base) + .finish() + } +} + +impl CByteStorage { + /// `None` if the initial `base` bytes cannot be allocated. + pub fn new(alloc: Alloc, base: usize) -> Option { + Some(CByteStorage { + block: CBlock::new(alloc, base)?, + len: 0, + base, + }) + } + + fn grow_to(&mut self, needed: usize) -> Result<(), StorageFull> { + if needed <= self.block.capacity() { + return Ok(()); + } + let cap = needed.max(self.block.capacity().saturating_mul(2)); + let mut new = CBlock::new(self.block.alloc, cap).ok_or(StorageFull)?; + new.as_mut_slice()[..self.len].copy_from_slice(&self.block.as_slice()[..self.len]); + self.block = new; + Ok(()) + } +} + +impl ByteStorage for CByteStorage { + fn bytes(&self) -> &[u8] { + &self.block.as_slice()[..self.len] + } + + fn try_reserve(&mut self, additional: usize) -> Result<(), StorageFull> { + self.grow_to(self.len.checked_add(additional).ok_or(StorageFull)?) + } + + fn try_extend(&mut self, bytes: &[u8]) -> Result<(), StorageFull> { + self.grow_to(self.len.checked_add(bytes.len()).ok_or(StorageFull)?)?; + self.block.as_mut_slice()[self.len..self.len + bytes.len()].copy_from_slice(bytes); + self.len += bytes.len(); + Ok(()) + } + + fn reset(&mut self) { + self.len = 0; + // Give back memory acquired by growth (Zig deinits the allocating + // writer). If the small block cannot be allocated, keep the big one. + if self.block.capacity() > self.base + && let Some(b) = CBlock::new(self.block.alloc, self.base) + { + self.block = b; + } + } + + fn clone_box(&self) -> Box { + Box::new(self.bytes().to_vec()) + } +} diff --git a/crates/ghostty-vt-capi/src/osc.rs b/crates/ghostty-vt-capi/src/osc.rs index 384bcec..fdd7ddd 100644 --- a/crates/ghostty-vt-capi/src/osc.rs +++ b/crates/ghostty-vt-capi/src/osc.rs @@ -9,7 +9,7 @@ use std::{ use ghostty_vt::osc::{Command, Parser}; use crate::{ - alloc::{Alloc, Allocator}, + alloc::{Alloc, Allocator, CBlock, CByteStorage}, result::GhosttyResult, }; @@ -28,9 +28,11 @@ pub struct OscParser { /// string payloads, as Zig's sentinel slices are) and stays valid until the /// next `ghostty_osc_end`/`ghostty_osc_reset`/`ghostty_osc_free`. pub struct OscCommand { - command: Command, - /// NUL-terminated window title (only for `ChangeWindowTitle`). - title_z: Vec, + /// `GhosttyOscCommandType` of the last command. + key: c_int, + /// NUL-terminated window title (only for `ChangeWindowTitle`), allocated + /// with the caller's allocator. + title_z: Option, } /// C: `GhosttyOscCommandData` @@ -62,12 +64,18 @@ pub unsafe extern "C" fn ghostty_osc_new( let alloc = unsafe { Alloc::from_c(allocator) }; // C-built parsers have an allocator (Zig: `.init(alloc)`), so commands // that need unbounded buffers (OSC 52, ...) are accepted. + // The parser's payload buffer (Zig's inline fixed buffer / allocating + // writer) lives in memory from the same allocator; if either allocation + // fails the caller gets OUT_OF_MEMORY. + let Some(storage) = CByteStorage::new(alloc, Parser::MAX_BUF) else { + return GhosttyResult::OutOfMemory; + }; let Some(p) = alloc.create(OscParser { alloc, - parser: Parser::init(true), + parser: Parser::with_storage(Box::new(storage)), command: OscCommand { - command: Command::Invalid, - title_z: Vec::new(), + key: 0, + title_z: None, }, }) else { return GhosttyResult::OutOfMemory; @@ -124,12 +132,16 @@ pub unsafe extern "C" fn ghostty_osc_end( return std::ptr::null_mut(); }; let cmd = p.parser.end(Some(terminator)).unwrap_or(Command::Invalid); - p.command.title_z.clear(); + p.command.title_z = None; + p.command.key = cmd.key() as c_int; if let Command::ChangeWindowTitle(t) = &cmd { - p.command.title_z.extend_from_slice(t); - p.command.title_z.push(0); + match CBlock::from_bytes_z(p.alloc, t) { + Some(b) => p.command.title_z = Some(b), + // The title cannot be materialized with the caller's allocator: + // report an invalid command rather than a dangling title. + None => p.command.key = Command::Invalid.key() as c_int, + } } - p.command.command = cmd; &raw mut p.command } @@ -141,7 +153,7 @@ pub unsafe extern "C" fn ghostty_osc_end( pub unsafe extern "C" fn ghostty_osc_command_type(command: *mut OscCommand) -> c_int { // SAFETY: caller guarantees validity (or null). match unsafe { command.as_ref() } { - Some(c) => c.command.key() as c_int, + Some(c) => c.key, None => 0, // invalid } } @@ -165,12 +177,11 @@ pub unsafe extern "C" fn ghostty_osc_command_data( }; match data { OscCommandData::Invalid => false, - OscCommandData::ChangeWindowTitleStr => match &command.command { - Command::ChangeWindowTitle(_) if !out.is_null() => { + OscCommandData::ChangeWindowTitleStr => match &command.title_z { + Some(title) if !out.is_null() => { // SAFETY: caller guarantees `out` is a `const char **`. unsafe { - out.cast::<*const u8>() - .write_unaligned(command.title_z.as_ptr()); + out.cast::<*const u8>().write_unaligned(title.as_ptr()); } true } diff --git a/crates/ghostty-vt-capi/tests/c/alloc_test.c b/crates/ghostty-vt-capi/tests/c/alloc_test.c new file mode 100644 index 0000000..a99c321 --- /dev/null +++ b/crates/ghostty-vt-capi/tests/c/alloc_test.c @@ -0,0 +1,164 @@ +// Counting / failing / limiting GhosttyAllocator exercised through the OSC +// parser: every byte the parser owns must come from (and return to) the +// caller's allocator, and its failures must surface like Zig's. +#include +#include +#include +#include +#include +#include + +typedef struct { + size_t allocs, frees; + size_t live; // bytes currently allocated + size_t limit; // max live bytes (0 = unlimited) + size_t fail_after; // fail every allocation after this many succeed (SIZE_MAX = never) +} Counter; + +static void *c_alloc(void *ctx, size_t len, uint8_t alignment, uintptr_t ret) { + (void)alignment; (void)ret; + Counter *c = ctx; + if (c->allocs >= c->fail_after) return NULL; + if (c->limit && c->live + len > c->limit) return NULL; + void *p = malloc(len ? len : 1); + if (!p) return NULL; + c->allocs++; + c->live += len; + return p; +} +static bool c_resize(void *ctx, void *m, size_t ml, uint8_t a, size_t nl, uintptr_t r) { + (void)ctx; (void)m; (void)ml; (void)a; (void)nl; (void)r; + return false; +} +static void *c_remap(void *ctx, void *m, size_t ml, uint8_t a, size_t nl, uintptr_t r) { + (void)ctx; (void)m; (void)ml; (void)a; (void)nl; (void)r; + return NULL; +} +static void c_free(void *ctx, void *m, size_t len, uint8_t a, uintptr_t r) { + (void)a; (void)r; + Counter *c = ctx; + c->frees++; + c->live -= len; + free(m); +} +static const GhosttyAllocatorVtable VT = {c_alloc, c_resize, c_remap, c_free}; + +static Counter counter(size_t limit, size_t fail_after) { + Counter c = {0, 0, 0, limit, fail_after}; + return c; +} + +static void feed(GhosttyOscParser p, const char *s, size_t n) { + for (size_t i = 0; i < n; i++) ghostty_osc_next(p, (uint8_t)s[i]); +} + +static int parse_clipboard(Counter *c, size_t payload_len, int *type_out) { + GhosttyAllocator a = {.ctx = c, .vtable = &VT}; + GhosttyOscParser p; + if (ghostty_osc_new(&a, &p) != GHOSTTY_SUCCESS) return -1; + feed(p, "52;c;", 5); + for (size_t i = 0; i < payload_len; i++) ghostty_osc_next(p, 'A'); + *type_out = ghostty_osc_command_type(ghostty_osc_end(p, 0x07)); + ghostty_osc_free(p); + return 0; +} + +int main(void) { + // 1. Everything the parser allocates goes through the allocator and is + // returned on free. + { + Counter c = counter(0, SIZE_MAX); + GhosttyAllocator a = {.ctx = &c, .vtable = &VT}; + GhosttyOscParser p; + assert(ghostty_osc_new(&a, &p) == GHOSTTY_SUCCESS); + assert(c.allocs >= 2); // the parser object and its buffer + assert(c.live >= 2048); // the fixed payload buffer is ours + feed(p, "0;hello", 7); + GhosttyOscCommand cmd = ghostty_osc_end(p, 0x07); + assert(ghostty_osc_command_type(cmd) == GHOSTTY_OSC_COMMAND_CHANGE_WINDOW_TITLE); + const char *title = NULL; + assert(ghostty_osc_command_data(cmd, GHOSTTY_OSC_DATA_CHANGE_WINDOW_TITLE_STR, &title)); + assert(strcmp(title, "hello") == 0); + size_t before_free = c.live; + assert(before_free > 2048); // + the title copy + ghostty_osc_free(p); + assert(c.live == 0); + assert(c.allocs == c.frees); + } + + // 2. Creation failure at each allocation point is OUT_OF_MEMORY and leaks + // nothing. + for (size_t fail_after = 0; fail_after < 2; fail_after++) { + Counter c = counter(0, fail_after); + GhosttyAllocator a = {.ctx = &c, .vtable = &VT}; + GhosttyOscParser p = NULL; + assert(ghostty_osc_new(&a, &p) == GHOSTTY_OUT_OF_MEMORY); + assert(c.live == 0); + assert(c.allocs == c.frees); + } + + // 3. A live-bytes limit governs payload growth: a 1 KiB clipboard payload + // fits; a 64 KiB one does not (the OSC becomes invalid, like Zig's + // WriteFailed) and nothing leaks. + { + Counter c = counter(0, SIZE_MAX); + int unlimited = -1; + assert(parse_clipboard(&c, 65536, &unlimited) == 0); + assert(unlimited == GHOSTTY_OSC_COMMAND_CLIPBOARD_CONTENTS); + assert(c.live == 0); + size_t peak_needed = 65536; + + Counter small = counter(1024 * 1024, SIZE_MAX); + int ok = -1; + assert(parse_clipboard(&small, 1024, &ok) == 0); + assert(ok == GHOSTTY_OSC_COMMAND_CLIPBOARD_CONTENTS); + assert(small.live == 0); + + Counter tight = counter(16 * 1024, SIZE_MAX); + int limited = -1; + assert(parse_clipboard(&tight, peak_needed, &limited) == 0); + assert(limited == GHOSTTY_OSC_COMMAND_INVALID); + assert(tight.live == 0); + assert(tight.allocs == tight.frees); + } + + // 4. If the allocator fails after creation, the title copy cannot be made + // and the command is reported invalid. + { + Counter c = counter(0, SIZE_MAX); + GhosttyAllocator a = {.ctx = &c, .vtable = &VT}; + GhosttyOscParser p; + assert(ghostty_osc_new(&a, &p) == GHOSTTY_SUCCESS); + c.fail_after = c.allocs; // no further allocations succeed + feed(p, "0;hello", 7); + GhosttyOscCommand cmd = ghostty_osc_end(p, 0x07); + assert(ghostty_osc_command_type(cmd) == GHOSTTY_OSC_COMMAND_INVALID); + const char *title = NULL; + assert(!ghostty_osc_command_data(cmd, GHOSTTY_OSC_DATA_CHANGE_WINDOW_TITLE_STR, &title)); + ghostty_osc_free(p); + assert(c.live == 0); + } + + // 5. A failing growth in the middle of a payload invalidates only that OSC; + // the parser is reusable afterwards. + { + Counter c = counter(0, SIZE_MAX); + GhosttyAllocator a = {.ctx = &c, .vtable = &VT}; + GhosttyOscParser p; + assert(ghostty_osc_new(&a, &p) == GHOSTTY_SUCCESS); + c.fail_after = c.allocs; + feed(p, "52;c;", 5); + for (int i = 0; i < 5000; i++) ghostty_osc_next(p, 'A'); + assert(ghostty_osc_command_type(ghostty_osc_end(p, 0x07)) == GHOSTTY_OSC_COMMAND_INVALID); + c.fail_after = SIZE_MAX; + ghostty_osc_reset(p); + feed(p, "0;ok", 4); + GhosttyOscCommand cmd = ghostty_osc_end(p, 0x07); + assert(ghostty_osc_command_type(cmd) == GHOSTTY_OSC_COMMAND_CHANGE_WINDOW_TITLE); + ghostty_osc_free(p); + assert(c.live == 0); + } + + puts("ok"); + return 0; +} diff --git a/crates/ghostty-vt-capi/tests/c_allocator.rs b/crates/ghostty-vt-capi/tests/c_allocator.rs new file mode 100644 index 0000000..0378be0 --- /dev/null +++ b/crates/ghostty-vt-capi/tests/c_allocator.rs @@ -0,0 +1,27 @@ +//! Compiles `tests/c/alloc_test.c` (a counting / failing / limiting C +//! allocator driving the OSC parser) against the Rust `libghostty-vt`. +#![cfg(unix)] +#![allow(clippy::unwrap_used)] + +mod common; + +use std::process::Command; + +#[test] +fn c_allocator_governs_osc_parser_storage() { + let lib = common::install_native_lib(); + let scratch = common::scratch_dir("c_allocator"); + let src = common::manifest_dir().join("tests/c/alloc_test.c"); + let exe = scratch.join("alloc_test"); + // No -DNDEBUG: the test relies on assert(). + common::compile_c(&lib, &[&src], &exe, &["-Wall", "-O0"]); + let out = Command::new(&exe).output().expect("run alloc_test"); + assert!( + out.status.success(), + "alloc_test failed: {:?}\n{}{}", + out.status, + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!(String::from_utf8(out.stdout).unwrap(), "ok\n"); +} diff --git a/crates/ghostty-vt/src/osc/mod.rs b/crates/ghostty-vt/src/osc/mod.rs index 8df076f..a44e3ca 100644 --- a/crates/ghostty-vt/src/osc/mod.rs +++ b/crates/ghostty-vt/src/osc/mod.rs @@ -351,6 +351,114 @@ enum Writer { Allocating, } +/// The payload storage ran out of memory (the allocator refused to grow). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct StorageFull; + +/// Allocator abstraction for the parser's payload buffer (Zig: the +/// `Allocator` the parser was initialized with). The default storage is a +/// `Vec` on the global allocator; the C API supplies storage that is +/// allocated through the caller's `GhosttyAllocator`, so allocation limits and +/// failures of that allocator govern the parser's memory like they do in Zig. +/// +/// A failed `try_extend`/`try_reserve` invalidates the OSC being parsed +/// (Zig: the writer's `error.WriteFailed` sets the state to invalid; a failed +/// initial allocating-writer allocation falls back to the fixed buffer). +pub trait ByteStorage: std::fmt::Debug + Send { + /// The bytes accumulated so far. + fn bytes(&self) -> &[u8]; + /// Make room for `additional` more bytes. + fn try_reserve(&mut self, additional: usize) -> Result<(), StorageFull>; + /// Append bytes; on error nothing is appended. + fn try_extend(&mut self, bytes: &[u8]) -> Result<(), StorageFull>; + /// Drop the contents and release any memory acquired by growth (Zig + /// `reset` deinits the allocating writer). + fn reset(&mut self); + /// For `Parser: Clone`. + fn clone_box(&self) -> Box; +} + +impl ByteStorage for Vec { + fn bytes(&self) -> &[u8] { + self + } + fn try_reserve(&mut self, additional: usize) -> Result<(), StorageFull> { + Vec::try_reserve(self, additional).map_err(|_| StorageFull) + } + fn try_extend(&mut self, bytes: &[u8]) -> Result<(), StorageFull> { + Vec::try_reserve(self, bytes.len()).map_err(|_| StorageFull)?; + self.extend_from_slice(bytes); + Ok(()) + } + fn reset(&mut self) { + self.clear(); + } + fn clone_box(&self) -> Box { + Box::new(self.clone()) + } +} + +impl Clone for Box { + fn clone(&self) -> Self { + self.clone_box() + } +} + +/// Payload buffer: a plain `Vec` (fast path) or custom allocator-backed +/// storage. +#[derive(Debug, Clone)] +enum Buffer { + Vec(Vec), + Custom(Box), +} + +impl Buffer { + #[inline] + fn bytes(&self) -> &[u8] { + match self { + Buffer::Vec(v) => v, + Buffer::Custom(c) => c.bytes(), + } + } + + #[inline] + fn len(&self) -> usize { + self.bytes().len() + } + + #[inline] + fn try_extend(&mut self, b: &[u8]) -> Result<(), StorageFull> { + match self { + Buffer::Vec(v) => { + v.extend_from_slice(b); + Ok(()) + } + Buffer::Custom(c) => c.try_extend(b), + } + } + + fn try_reserve(&mut self, n: usize) -> Result<(), StorageFull> { + match self { + Buffer::Vec(v) => { + v.reserve(n); + Ok(()) + } + Buffer::Custom(c) => c.try_reserve(n), + } + } + + fn reset(&mut self) { + match self { + Buffer::Vec(v) => v.clear(), + Buffer::Custom(c) => c.reset(), + } + } + + fn to_vec(&self) -> Vec { + self.bytes().to_vec() + } +} + #[derive(Debug, Clone)] pub struct Parser { /// Whether an allocator is available, i.e. whether data longer than @@ -363,7 +471,7 @@ pub struct Parser { pub state: State, /// Buffer for temporary storage of OSC data - buffer: Vec, + buffer: Buffer, writer: Writer, } @@ -381,7 +489,18 @@ impl Parser { Parser { alloc, state: State::Start, - buffer: Vec::new(), + buffer: Buffer::Vec(Vec::new()), + writer: Writer::None, + } + } + + /// Like [`Parser::init`] with an allocator-backed payload buffer + /// (`alloc` is true: an allocator is available). + pub fn with_storage(storage: Box) -> Parser { + Parser { + alloc: true, + state: State::Start, + buffer: Buffer::Custom(storage), writer: Writer::None, } } @@ -389,7 +508,7 @@ impl Parser { /// Reset the parser state. pub fn reset(&mut self) { self.state = State::Start; - self.buffer.clear(); + self.buffer.reset(); self.writer = Writer::None; } @@ -420,6 +539,12 @@ impl Parser { self.write_to_fixed(); return; } + // Zig: `Writer.Allocating.initCapacity(alloc, 2048) catch` falls back + // to the fixed buffer. + if self.buffer.try_reserve(Self::MAX_BUF).is_err() { + self.write_to_fixed(); + return; + } self.writer = Writer::Allocating; } @@ -438,7 +563,7 @@ impl Parser { self.state = State::Invalid; return None; } - Some(self.buffer.clone()) + Some(self.buffer.to_vec()) } /// Zig `writer.writeByte(0)` capacity check: when the fixed buffer is @@ -460,7 +585,7 @@ impl Parser { self.state = State::Invalid; return None; } - Some(self.buffer.clone()) + Some(self.buffer.to_vec()) } /// Whether any payload writer has been set up (Zig `parser.writer != null`). @@ -468,6 +593,15 @@ impl Parser { self.writer != Writer::None } + /// Append to the buffer; an allocation failure invalidates the parser + /// (Zig: `error.WriteFailed` -> `.invalid`). + #[inline] + fn push_or_invalid(&mut self, bytes: &[u8]) { + if self.buffer.try_extend(bytes).is_err() { + self.state = State::Invalid; + } + } + #[inline] fn write_byte(&mut self, c: u8) { match self.writer { @@ -477,10 +611,10 @@ impl Parser { // Overflow: discard any further input. self.state = State::Invalid; } else { - self.buffer.push(c); + self.push_or_invalid(&[c]); } } - Writer::Allocating => self.buffer.push(c), + Writer::Allocating => self.push_or_invalid(&[c]), } } @@ -519,16 +653,16 @@ impl Parser { bytes = rest; } Writer::Allocating => { - self.buffer.extend_from_slice(bytes); + self.push_or_invalid(bytes); return; } Writer::Fixed => { let room = Self::MAX_BUF.saturating_sub(self.buffer.len()); if bytes.len() <= room { - self.buffer.extend_from_slice(bytes); + self.push_or_invalid(bytes); } else { // Overflow: keep what fits, discard the rest. - self.buffer.extend_from_slice(&bytes[..room]); + self.push_or_invalid(&bytes[..room]); self.state = State::Invalid; } return; @@ -795,6 +929,56 @@ impl Parser { mod next_slice_tests { use super::*; + /// Storage that holds at most `cap` bytes (an allocator with a limit). + #[derive(Debug, Clone)] + struct Limited { + data: Vec, + cap: usize, + } + impl ByteStorage for Limited { + fn bytes(&self) -> &[u8] { + &self.data + } + fn try_reserve(&mut self, additional: usize) -> Result<(), StorageFull> { + if self.data.len() + additional > self.cap { + return Err(StorageFull); + } + Ok(()) + } + fn try_extend(&mut self, bytes: &[u8]) -> Result<(), StorageFull> { + self.try_reserve(bytes.len())?; + self.data.extend_from_slice(bytes); + Ok(()) + } + fn reset(&mut self) { + self.data.clear(); + } + fn clone_box(&self) -> Box { + Box::new(self.clone()) + } + } + + #[test] + fn storage_failure_invalidates_osc() { + let mut p = Parser::with_storage(Box::new(Limited { + data: Vec::new(), + cap: 8, + })); + for b in b"0;abcdefghijkl" { + p.next(*b); + } + assert_eq!(p.state, State::Invalid); + assert!(p.end(Some(0x07)).is_none()); + + // Within the limit it parses as usual. + p.reset(); + for b in b"0;abc" { + p.next(*b); + } + let cmd = p.end(Some(0x07)).expect("command"); + assert!(matches!(cmd, Command::ChangeWindowTitle(t) if t == b"abc")); + } + #[test] fn next_slice_matches_next() { for alloc in [false, true] { @@ -811,7 +995,7 @@ mod next_slice_tests { b.next_slice(&input[..mid]); b.next_slice(&input[mid..]); assert_eq!(a.state, b.state); - assert_eq!(a.buffer, b.buffer); + assert_eq!(a.buffer.bytes(), b.buffer.bytes()); assert_eq!(a.writer, b.writer); } } diff --git a/docs/rewrite/notes/final-Z3.md b/docs/rewrite/notes/final-Z3.md new file mode 100644 index 0000000..985a35a --- /dev/null +++ b/docs/rewrite/notes/final-Z3.md @@ -0,0 +1,72 @@ +# Z3 final fixes (vt-capi, shadertoy, tripwire, crash, kitty debug dump, bench signpost) + +Supersedes the stale statements in libghostty-vt.md ("interior collections use the global allocator"), +crash-CR.md ("no minidump") and macos-M1-renderer.md ("iChannelTime stride limitation"). + +## 1. C allocator governs parser storage (lib/allocator.zig, terminal/c/osc.zig) +* `ghostty-vt::osc` gets a small allocator abstraction: trait `ByteStorage` (fallible `try_reserve`/`try_extend`, + `reset` releases growth) behind `Parser::with_storage`; default stays a `Vec` (also implements the trait). + A failed grow sets the parser state invalid, exactly like Zig's `WriteFailed`; a failed 2048-byte reserve when + switching to the allocating writer falls back to the fixed buffer (Zig `initCapacity catch`). +* `ghostty-vt-capi/alloc.rs`: `CByteStorage` (payload buffer, 2048 bytes allocated at `ghostty_osc_new`, growth by + alloc-copy-free through the vtable) and `CBlock` (NUL-terminated title copy). `ghostty_osc_new` returns + `GHOSTTY_OUT_OF_MEMORY` when the parser object or its buffer cannot be allocated; if the title copy fails, + `ghostty_osc_end` reports an invalid command. `OscCommand` now holds the type key + C-allocated title only. +* Remaining global-allocator use: transient `Command` payload copies made inside `ghostty_osc_end` (Zig borrows + the parser buffer); they never outlive the call. +* Tests: `tests/c/alloc_test.c` + `tests/c_allocator.rs` (counting/failing/limiting C allocator: leak-free + accounting, OOM at each creation allocation, live-bytes limit invalidating a large OSC 52, title-copy failure, + mid-payload failure then reuse); unit test `storage_failure_invalidates_osc` in ghostty-vt. + +## 2. Shadertoy +* `shadertoy_sampler.rs`: functions keyed by full signature (per-parameter sampler layout, all overloads kept); + a call picks the overload by arity and which arguments are samplers (`iChannelN` or a sampler param of the enclosing + function). Tests `overloads_are_keyed_by_signature`, `overloads_through_sampler_parameters`. +* `shadertoy.rs`: `msl_from_compiled` post-processes naga's MSL: uniform-block scalar arrays with std140 stride + (`iChannelTime[4]`, found by reflection over the naga module, not by name) become `metal::vec inner[K]` wrappers + and element reads get `.x`. Limitation: copying the whole array into a local variable is not rewritten. + Test `msl_uniform_block_matches_std140_upload_layout` parses the emitted `struct Globals`, computes MSL + offsets/size and compares every member to `offset_of!(Uniforms, ..)` and `size_of::()` (4496). + +## 3. Tripwire (tripwire.zig) +* `ghostty-terminal/src/tripwire.rs`: `tripwire_module!(name, PointEnum, Error)` -> `name::{check, error_always, + error_after, end(ResetMode), reset}`; `end` returns `UntrippedError` (warn-logs untripped points); state is + thread-local so parallel tests are isolated; `check` is a no-op outside `cfg(test)`. 5 tests ported/added + (the 3 Zig tests + retain/reset + thread isolation). +* Ported OOM tests: `Tabstops: resize alloc failure preserves state` (tabstops.rs, `resize_tw`); + `PageList init error` (page_list.rs, `init_tw` + `init_pages_tw`; Zig `init_memory_pool` has no Rust analogue + (no pool), `viewport_pin`/`viewport_pin_track` are both checked before the single `track_pin` reserve); + both `reloadActive partial history cleanup` tests (search/screen.rs, `reload_active_tw`; assert OOM is returned + and `history_results` is unchanged). +* Not portable (infallible Rust code, documented): `selectionString map allocation failure cleanup` + (`selection_string*` return `String`/`Vec`, no fallible copy; nothing to unwind). Font: Atlas `init/reserve/grow` + errors (Atlas storage is infallible `Vec`) and SharedGrid `init error` (no allocation points; the + render-rollback test already exists as `render_glyph_error_after_cache_insert_rolls_back_cache_entry`). + ghostty-font can use `ghostty_terminal::tripwire` if fallible paths are added later. +* Fixed the `enum_variant_names` clippy lint reported on `InitPagesPoint` (variants renamed `Node/BufStd/BufNonStd`). + +## 4. Crash minidump (crash/sentry.zig) +* Linux fatal-signal path: handler keeps building head+event in static memory, then captures `siginfo` + + machine context (+FP state) into a static `crash_context::CrashContext` and calls `minidump::capture`: raw + `clone` (fork semantics, own 4 MiB mmap'd stack because the handler runs on a tiny altstack, no atfork handlers), + `PR_SET_PTRACER_ANY`, parent polls `waitpid` with a 20 s timeout. The child uses `minidump-writer 0.13` + (`MinidumpWriterConfig` + `set_crash_context`, the crate's documented clone-and-ptrace pattern for the local + process) and writes the full envelope: event + `minidump.dmp` attachment (`attachment_type: event.minidump`). + Any failure falls back to the previous event-only envelope. Stack overflow: if the SP page is unreadable + (guard page) the writer is retried with SP moved to the first readable page above it (reported RSP is then + approximate, stack contents captured). +* Tests (`crates/ghostty-crash/tests/minidump.rs`): SIGSEGV, SIGABRT and stack overflow on a spawned thread; + parse with the `minidump` crate; assert thread list contains the crashing tid with stack memory and the main thread, + exception stream blames that tid with the signal number and registers, modules/memory lists present. + Existing root `tests/crash.rs` still pass. +* Not done: macOS/Windows native minidumps (sentry-native uses its own backend there); no symbol upload. + +## 5. kitty debugDump, bench signpost +* `LoadingImage::{debug_dump_filename, debug_dump_to, debug_dump}` (debug builds only), same file name format as Zig; test. +* `ghostty-bench/src/signpost.rs` (apple only): `os_log_create` + `os_signpost_id_make_with_pointer` + + `_os_signpost_emit_with_name_impl` interval begin/end around the benchmark loop, subsystem `com.mitchellh.ghostty`, + category PointsOfInterest, name `ghostty`. Compile/clippy-checked for aarch64-apple-darwin only (not run). + +## Gate +workspace `cargo clippy --all-targets -D warnings`, `cargo +nightly fmt --check`, `cargo test --workspace` pass; +aarch64-apple-darwin clippy passes for bench, renderer, crash, terminal, vt-capi.