fix: close the tail items from the targeted re-audit
Resolves the 13 rows still partial in docs/rewrite/notes/review-rv4- codex.md (details in final-Z5.md): - Escaped config strings are decoded to raw bytes at every OS boundary: Flatpak cwd/argv to D-Bus, screen-file paths, the default config template, GHOSTTY_RESOURCES_DIR/TERMINFO/shell integration/ bindtextdomain consumers, custom shader loading, list-themes hyperlinks. - osbytes escapes literal U+FDD0 on non-Unix too (bijective everywhere); the GTK paste dialog only decodes text that was actually encoded. - shadertoy: whole-array uses of iChannelTime rebuild a plain float[4] from the std140-padded members (naga-validated, MSL offsets checked). - libghostty-vt: the OSC parser holds caller-allocated storage inline and command payloads borrow from it, so ghostty_osc_end performs no global allocations (verified from Rust and C). - crash: aarch64 Linux minidump context fixed; macOS fatal signals record siginfo, registers and a fault-safe stack walk. - tripwire: selectionString map allocation-failure cleanup test ported. - libghostty-vt emits the versioned soname libghostty-vt.so.0 at build time (no patchelf needed at install). Test Plan: - cargo test --workspace (3342 passed) - just check-macos; just check-macos x86_64-apple-darwin; just check-windows; just check-wasm - cargo check --target aarch64-unknown-linux-gnu -p ghostty-crash
This commit is contained in:
ddidderr committed
1 parent
bc297c823b
commit
184cdd5c62
49 files changed
+1785
-557
No files matched your search
@@ -1,14 +1,49 @@
|
||||
/// libghostty-vt version (Zig: `GhosttyLibVt` `version = .{ 0, 1, 0 }`).
|
||||
/// `scripts/install-lib.sh` reads this line to name the installed files, so
|
||||
/// keep the format.
|
||||
const VT_VERSION: &str = "0.1.0";
|
||||
|
||||
/// The soname / install name of the shared library: `libghostty-vt.so.<major>`
|
||||
/// on ELF platforms (Zig sets `.version` on the shared library, which makes
|
||||
/// the linker emit the versioned soname), `@rpath/libghostty-vt.<major>.dylib`
|
||||
/// on Apple platforms.
|
||||
fn lib_name(target_os: &str, version: &str) -> Option<String> {
|
||||
let major = version.split('.').next().unwrap_or(version);
|
||||
match target_os {
|
||||
"linux" | "android" | "freebsd" => Some(format!("libghostty-vt.so.{major}")),
|
||||
"macos" | "ios" => Some(format!("@rpath/libghostty-vt.{major}.dylib")),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {
|
||||
// Ship the shared library under Ghostty's name (`libghostty-vt.so`).
|
||||
let target_os = std::env::var("CARGO_CFG_TARGET_OS").unwrap_or_default();
|
||||
match target_os.as_str() {
|
||||
"linux" | "android" | "freebsd" => {
|
||||
println!("cargo:rustc-cdylib-link-arg=-Wl,-soname,libghostty-vt.so");
|
||||
match (lib_name(&target_os, VT_VERSION), target_os.as_str()) {
|
||||
(Some(name), "linux" | "android" | "freebsd") => {
|
||||
println!("cargo:rustc-cdylib-link-arg=-Wl,-soname,{name}");
|
||||
}
|
||||
"macos" | "ios" => {
|
||||
println!("cargo:rustc-cdylib-link-arg=-Wl,-install_name,@rpath/libghostty-vt.dylib");
|
||||
(Some(name), _) => {
|
||||
println!("cargo:rustc-cdylib-link-arg=-Wl,-install_name,{name}");
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
println!("cargo:rerun-if-changed=build.rs");
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::lib_name;
|
||||
|
||||
#[test]
|
||||
fn versioned_names() {
|
||||
assert_eq!(
|
||||
lib_name("linux", "0.1.0").as_deref(),
|
||||
Some("libghostty-vt.so.0")
|
||||
);
|
||||
assert_eq!(
|
||||
lib_name("macos", "1.2.3").as_deref(),
|
||||
Some("@rpath/libghostty-vt.1.dylib")
|
||||
);
|
||||
assert_eq!(lib_name("windows", "0.1.0"), None);
|
||||
}
|
||||
}
|
||||
@@ -10,9 +10,12 @@
|
||||
//! `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.
|
||||
//! Commands produced by `ghostty_osc_end` borrow their byte payloads from the
|
||||
//! parser's storage (as Zig's do), so ending an OSC does not allocate for the
|
||||
//! payload at all; the parser itself holds the storage inline (no `Box`). The
|
||||
//! only values still built on the global allocator are the small request
|
||||
//! lists of the color OSCs (4, 5, 10-19, 21, 104-119), which are not exposed
|
||||
//! through the C API and are dropped before `ghostty_osc_end` returns.
|
||||
#![allow(unsafe_code)]
|
||||
|
||||
use std::{alloc::Layout, ffi::c_void, ptr::NonNull};
|
||||
@@ -276,8 +279,4 @@ impl ByteStorage for CByteStorage {
|
||||
self.block = b;
|
||||
}
|
||||
}
|
||||
|
||||
fn clone_box(&self) -> Box<dyn ByteStorage> {
|
||||
Box::new(self.bytes().to_vec())
|
||||
}
|
||||
}
|
||||
@@ -16,7 +16,7 @@ use crate::{
|
||||
/// C: `GhosttyOscParser` (opaque).
|
||||
pub struct OscParser {
|
||||
alloc: Alloc,
|
||||
parser: Parser,
|
||||
parser: Parser<CByteStorage>,
|
||||
/// Backing store for the last command returned by `ghostty_osc_end`.
|
||||
command: OscCommand,
|
||||
}
|
||||
@@ -67,12 +67,12 @@ pub unsafe extern "C" fn ghostty_osc_new(
|
||||
// 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 {
|
||||
let Some(storage) = CByteStorage::new(alloc, ghostty_vt::osc::MAX_BUF) else {
|
||||
return GhosttyResult::OutOfMemory;
|
||||
};
|
||||
let Some(p) = alloc.create(OscParser {
|
||||
alloc,
|
||||
parser: Parser::with_storage(Box::new(storage)),
|
||||
parser: Parser::with_storage(storage),
|
||||
command: OscCommand {
|
||||
key: 0,
|
||||
title_z: None,
|
||||
@@ -131,8 +131,11 @@ pub unsafe extern "C" fn ghostty_osc_end(
|
||||
let Some(p) = (unsafe { parser.as_mut() }) else {
|
||||
return std::ptr::null_mut();
|
||||
};
|
||||
let cmd = p.parser.end(Some(terminator)).unwrap_or(Command::Invalid);
|
||||
// The command borrows its payload from the parser's own (caller
|
||||
// allocated) storage, so nothing is copied or allocated here except the
|
||||
// NUL-terminated title handed to C.
|
||||
p.command.title_z = None;
|
||||
let cmd = p.parser.end(Some(terminator)).unwrap_or(Command::Invalid);
|
||||
p.command.key = cmd.key() as c_int;
|
||||
if let Command::ChangeWindowTitle(t) = &cmd {
|
||||
match CBlock::from_bytes_z(p.alloc, t) {
|
||||
@@ -192,10 +195,157 @@ pub unsafe extern "C" fn ghostty_osc_command_data(
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::ffi::CStr;
|
||||
use std::{
|
||||
alloc::{GlobalAlloc, Layout, System},
|
||||
cell::Cell,
|
||||
ffi::CStr,
|
||||
};
|
||||
|
||||
use super::*;
|
||||
|
||||
/// Counts Rust global-allocator calls made by the current thread, so the
|
||||
/// tests can prove that OSC parsing and `ghostty_osc_end` use only the
|
||||
/// caller's allocator.
|
||||
struct CountingGlobal;
|
||||
thread_local! {
|
||||
static GLOBAL_ALLOCS: Cell<usize> = const { Cell::new(0) };
|
||||
}
|
||||
// SAFETY: forwards to the system allocator.
|
||||
unsafe impl GlobalAlloc for CountingGlobal {
|
||||
unsafe fn alloc(&self, l: Layout) -> *mut u8 {
|
||||
let _ = GLOBAL_ALLOCS.try_with(|c| c.set(c.get() + 1));
|
||||
// SAFETY: same contract.
|
||||
unsafe { System.alloc(l) }
|
||||
}
|
||||
unsafe fn dealloc(&self, p: *mut u8, l: Layout) {
|
||||
// SAFETY: same contract.
|
||||
unsafe { System.dealloc(p, l) }
|
||||
}
|
||||
unsafe fn realloc(&self, p: *mut u8, l: Layout, n: usize) -> *mut u8 {
|
||||
let _ = GLOBAL_ALLOCS.try_with(|c| c.set(c.get() + 1));
|
||||
// SAFETY: same contract.
|
||||
unsafe { System.realloc(p, l, n) }
|
||||
}
|
||||
}
|
||||
#[global_allocator]
|
||||
static GLOBAL: CountingGlobal = CountingGlobal;
|
||||
|
||||
fn global_allocs() -> usize {
|
||||
GLOBAL_ALLOCS.with(Cell::get)
|
||||
}
|
||||
|
||||
use std::{
|
||||
ffi::c_void,
|
||||
sync::atomic::{AtomicUsize, Ordering},
|
||||
};
|
||||
|
||||
use crate::alloc::{Allocator, AllocatorVtable};
|
||||
|
||||
/// A caller allocator backed by the system allocator that counts calls.
|
||||
unsafe extern "C" fn c_alloc(
|
||||
ctx: *mut c_void,
|
||||
len: usize,
|
||||
align_log2: u8,
|
||||
_: usize,
|
||||
) -> *mut c_void {
|
||||
// SAFETY: `ctx` is the `AtomicUsize` owned by the test.
|
||||
unsafe { &*ctx.cast::<AtomicUsize>() }.fetch_add(1, Ordering::SeqCst);
|
||||
let l = Layout::from_size_align(len, 1 << align_log2).unwrap();
|
||||
// SAFETY: non-zero size (the crate never calls with zero).
|
||||
unsafe { System.alloc(l) }.cast()
|
||||
}
|
||||
unsafe extern "C" fn c_resize(
|
||||
_: *mut c_void,
|
||||
_: *mut c_void,
|
||||
_: usize,
|
||||
_: u8,
|
||||
_: usize,
|
||||
_: usize,
|
||||
) -> bool {
|
||||
false
|
||||
}
|
||||
unsafe extern "C" fn c_remap(
|
||||
_: *mut c_void,
|
||||
_: *mut c_void,
|
||||
_: usize,
|
||||
_: u8,
|
||||
_: usize,
|
||||
_: usize,
|
||||
) -> *mut c_void {
|
||||
std::ptr::null_mut()
|
||||
}
|
||||
unsafe extern "C" fn c_free(
|
||||
_: *mut c_void,
|
||||
m: *mut c_void,
|
||||
len: usize,
|
||||
align_log2: u8,
|
||||
_: usize,
|
||||
) {
|
||||
let l = Layout::from_size_align(len, 1 << align_log2).unwrap();
|
||||
// SAFETY: allocated by `c_alloc` with the same layout.
|
||||
unsafe { System.dealloc(m.cast(), l) };
|
||||
}
|
||||
static VT: AllocatorVtable = AllocatorVtable {
|
||||
alloc: c_alloc,
|
||||
resize: c_resize,
|
||||
remap: c_remap,
|
||||
free: c_free,
|
||||
};
|
||||
|
||||
#[test]
|
||||
fn end_makes_no_global_allocations() {
|
||||
let caller_allocs = AtomicUsize::new(0);
|
||||
let a = Allocator {
|
||||
ctx: (&raw const caller_allocs).cast_mut().cast(),
|
||||
vtable: &raw const VT,
|
||||
};
|
||||
let mut p = std::ptr::null_mut();
|
||||
// SAFETY: valid allocator and out pointer.
|
||||
assert_eq!(
|
||||
unsafe { ghostty_osc_new(&raw const a, &mut p) },
|
||||
GhosttyResult::Success
|
||||
);
|
||||
// SAFETY: `p` is live for the whole block.
|
||||
unsafe {
|
||||
for (osc, title) in [
|
||||
("7;file:///tmp/x", false),
|
||||
("8;id=a;https://example.com", false),
|
||||
("8;;", false),
|
||||
("52;c;aGVsbG8=", false),
|
||||
("133;A", false),
|
||||
("777;notify;title;body", false),
|
||||
("9;4;1;50", false),
|
||||
("9;2;message", false),
|
||||
("22;pointer", false),
|
||||
("66;s=2;hi", false),
|
||||
("3008;start=abc;type=shell", false),
|
||||
("5522;type=read;data", false),
|
||||
("0;a title", true),
|
||||
] {
|
||||
ghostty_osc_reset(p);
|
||||
for b in osc.bytes() {
|
||||
ghostty_osc_next(p, b);
|
||||
}
|
||||
let global_before = global_allocs();
|
||||
let caller_before = caller_allocs.load(Ordering::SeqCst);
|
||||
let cmd = ghostty_osc_end(p, 0x07);
|
||||
assert_ne!(ghostty_osc_command_type(cmd), 0, "{osc}");
|
||||
assert_eq!(
|
||||
global_allocs(),
|
||||
global_before,
|
||||
"global allocation ending {osc}"
|
||||
);
|
||||
// Only the NUL-terminated title handed to C is allocated.
|
||||
assert_eq!(
|
||||
caller_allocs.load(Ordering::SeqCst) - caller_before,
|
||||
usize::from(title),
|
||||
"caller allocations ending {osc}"
|
||||
);
|
||||
}
|
||||
ghostty_osc_free(p);
|
||||
}
|
||||
}
|
||||
|
||||
fn new() -> *mut OscParser {
|
||||
let mut p = std::ptr::null_mut();
|
||||
// SAFETY: valid out pointer, default allocator.
|
||||
|
||||
@@ -159,6 +159,49 @@ int main(void) {
|
||||
assert(c.live == 0);
|
||||
}
|
||||
|
||||
// 6. Ending an OSC copies no payload: with the allocator frozen after
|
||||
// creation (any allocation would fail), commands whose payloads are
|
||||
// borrowed from the parser's storage still parse, and every command
|
||||
// makes exactly the allocations the caller can see (none, except the
|
||||
// NUL-terminated title copy).
|
||||
{
|
||||
static const struct { const char *osc; int type; } cases[] = {
|
||||
{"7;file:///tmp/x", GHOSTTY_OSC_COMMAND_REPORT_PWD},
|
||||
{"8;id=a;https://example.com", GHOSTTY_OSC_COMMAND_HYPERLINK_START},
|
||||
{"8;;", GHOSTTY_OSC_COMMAND_HYPERLINK_END},
|
||||
{"52;c;aGVsbG8=", GHOSTTY_OSC_COMMAND_CLIPBOARD_CONTENTS},
|
||||
{"133;A", GHOSTTY_OSC_COMMAND_SEMANTIC_PROMPT},
|
||||
{"777;notify;title;body", GHOSTTY_OSC_COMMAND_SHOW_DESKTOP_NOTIFICATION},
|
||||
{"9;4;1;50", GHOSTTY_OSC_COMMAND_CONEMU_PROGRESS_REPORT},
|
||||
{"22;pointer", GHOSTTY_OSC_COMMAND_MOUSE_SHAPE},
|
||||
{"66;s=2;hi", GHOSTTY_OSC_COMMAND_KITTY_TEXT_SIZING},
|
||||
};
|
||||
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; // freeze: no allocation may succeed
|
||||
for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) {
|
||||
size_t allocs = c.allocs;
|
||||
ghostty_osc_reset(p);
|
||||
feed(p, cases[i].osc, strlen(cases[i].osc));
|
||||
GhosttyOscCommand cmd = ghostty_osc_end(p, 0x07);
|
||||
assert(ghostty_osc_command_type(cmd) == cases[i].type);
|
||||
assert(c.allocs == allocs);
|
||||
}
|
||||
// The title is the one payload handed to C: exactly one allocation.
|
||||
c.fail_after = SIZE_MAX;
|
||||
size_t allocs = c.allocs;
|
||||
ghostty_osc_reset(p);
|
||||
feed(p, "2;title", 7);
|
||||
GhosttyOscCommand cmd = ghostty_osc_end(p, 0x07);
|
||||
assert(ghostty_osc_command_type(cmd) == GHOSTTY_OSC_COMMAND_CHANGE_WINDOW_TITLE);
|
||||
assert(c.allocs == allocs + 1);
|
||||
ghostty_osc_free(p);
|
||||
assert(c.live == 0);
|
||||
assert(c.allocs == c.frees);
|
||||
}
|
||||
|
||||
puts("ok");
|
||||
return 0;
|
||||
}
|
||||
@@ -29,6 +29,9 @@ fn cargo() -> Command {
|
||||
Command::new(std::env::var_os("CARGO").unwrap_or_else(|| "cargo".into()))
|
||||
}
|
||||
|
||||
/// Major version of libghostty-vt (the soname suffix set by build.rs).
|
||||
pub const VT_MAJOR: u32 = 0;
|
||||
|
||||
static INSTALLED: OnceLock<PathBuf> = OnceLock::new();
|
||||
|
||||
/// Build the cdylib/staticlib for the host and install them as
|
||||
@@ -61,6 +64,19 @@ fn install_native_lib_once() -> PathBuf {
|
||||
std::fs::rename(&tmp, &dst).expect("install library");
|
||||
}
|
||||
}
|
||||
// The cdylib's soname is versioned (`libghostty-vt.so.<major>`, see
|
||||
// build.rs), so executables linked against it need that name at run time
|
||||
// (otherwise the loader would fall back to an unrelated system copy).
|
||||
let so = dir.join("libghostty-vt.so");
|
||||
if so.exists() {
|
||||
let versioned = dir.join(format!("libghostty-vt.so.{VT_MAJOR}"));
|
||||
let tmp = dir.join(format!(
|
||||
"libghostty-vt.so.{VT_MAJOR}.{}.tmp",
|
||||
std::process::id()
|
||||
));
|
||||
std::fs::copy(&so, &tmp).expect("install versioned library");
|
||||
std::fs::rename(&tmp, &versioned).expect("install versioned library");
|
||||
}
|
||||
dir
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
//! The shared library must carry the versioned soname `libghostty-vt.so.<major>`
|
||||
//! straight from the build (Zig sets `.version` on the shared library), not
|
||||
//! via a post-install `patchelf` step.
|
||||
#![cfg(target_os = "linux")]
|
||||
#![allow(clippy::unwrap_used)]
|
||||
|
||||
mod common;
|
||||
|
||||
use std::process::Command;
|
||||
|
||||
#[test]
|
||||
fn shared_library_has_versioned_soname() {
|
||||
let lib = common::install_native_lib();
|
||||
let so = lib.join("libghostty-vt.so");
|
||||
let Ok(out) = Command::new("readelf").arg("-d").arg(&so).output() else {
|
||||
eprintln!("readelf not available; skipping");
|
||||
return;
|
||||
};
|
||||
let text = String::from_utf8_lossy(&out.stdout);
|
||||
let want = format!("[libghostty-vt.so.{}]", common::VT_MAJOR);
|
||||
assert!(
|
||||
text.lines()
|
||||
.any(|l| l.contains("SONAME") && l.contains(&want)),
|
||||
"expected SONAME {want}:\n{text}"
|
||||
);
|
||||
// build.rs is the single source of the version (install-lib.sh reads it).
|
||||
let build_rs = std::fs::read_to_string(common::manifest_dir().join("build.rs")).unwrap();
|
||||
assert!(build_rs.contains(&format!("const VT_VERSION: &str = \"{}.", common::VT_MAJOR)));
|
||||
}
|
||||
Reference in new issue
Block a user