From ae97c67d32dbb039b05345281f12dcfd80bb3540 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 05:26:10 +0200 Subject: [PATCH 1/7] A killed program is recorded as file, offset and build-id, and userland names its frames The kernel stops reading programs' symbol tables. A user frame in a crash report is now ` + id=`: the file the kernel opened, the offset in that file's own addresses, and the GNU build-id read at the file's PT_NOTE (a page of it at most). The per-spawn backtrace table (`read_backtrace_table`, up to 16 MiB of untrusted .symtab/.strtab per spawn, at least one 2 MiB allocation per running program) and the kernel's demangling of user names are deleted; every task record carries an `Arc` instead of an `Arc`. - toyos-elf: `note::segments` and `note::build_id`, the gABI note layout, no alloc, no panic, fuzzed and checked against `object`. - toyos-symbols: the frame record's one encoder and one decoder (`frame`), `frame_offset`, and `name`, the namer's lookup over the same `locate`/`SymTab::resolve`/`rustc-demangle` the kernel names itself with. The kernel reaches rustc-demangle only through it now. - userland/symbolize: names the frames of a log, on ToyOS or any host (`--root DIR`). Shipped, and in the test boot. - rust fork: the ToyOS targets link with bare `--build-id`. - disk_backtrace asserts the record and the naming, not only the exit. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- Cargo.lock | 6 +- ...ang-links-for-toyos-carries-no-build-id.md | 19 ++ issues/diagnostics/toyos-explains-itself.md | 4 +- ...ernel-still-parses-what-userland-writes.md | 45 ++-- kernel/Cargo.lock | 2 +- kernel/Cargo.toml | 1 - kernel/src/arch/aarch64/trap.rs | 6 +- kernel/src/arch/x86_64/idt/exceptions.rs | 10 +- kernel/src/hardlockup/mod.rs | 2 +- kernel/src/loader/mod.rs | 34 +-- kernel/src/loader/symbols.rs | 74 ------ kernel/src/process.rs | 104 ++++---- kernel/src/sched/driver.rs | 14 +- kernel/src/sched/kthread.rs | 9 +- kernel/src/sched/payload.rs | 8 +- kernel/src/scheduler.rs | 4 +- kernel/src/symbols.rs | 132 ++-------- rust | 2 +- system.toml | 4 + tests/testcases/system.toml | 3 + .../src/bin/disk_backtrace.rs | 65 ++++- .../src/bin/disk_backtrace_child.rs | 4 - toyos-elf/Cargo.toml | 3 + toyos-elf/src/header.rs | 1 + toyos-elf/src/lib.rs | 1 + toyos-elf/src/note.rs | 68 ++++++ toyos-elf/tests/fixtures/lld-headers.bin | Bin 0 -> 4096 bytes toyos-elf/tests/fuzz.rs | 41 ++++ toyos-elf/tests/note.rs | 153 ++++++++++++ toyos-elf/tests/real.rs | 51 ++++ toyos-symbols/Cargo.toml | 23 +- toyos-symbols/src/frame.rs | 226 ++++++++++++++++++ toyos-symbols/src/lib.rs | 61 ++++- toyos-symbols/tests/frame.rs | 189 +++++++++++++++ toyos-symbols/tests/name.rs | 118 +++++++++ userland/Cargo.lock | 33 ++- userland/Cargo.toml | 1 + userland/symbolize/Cargo.toml | 9 + userland/symbolize/src/main.rs | 112 +++++++++ 39 files changed, 1302 insertions(+), 340 deletions(-) create mode 100644 issues/build/a-c-program-clang-links-for-toyos-carries-no-build-id.md create mode 100644 toyos-elf/src/note.rs create mode 100644 toyos-elf/tests/fixtures/lld-headers.bin create mode 100644 toyos-elf/tests/note.rs create mode 100644 toyos-symbols/src/frame.rs create mode 100644 toyos-symbols/tests/frame.rs create mode 100644 toyos-symbols/tests/name.rs create mode 100644 userland/symbolize/Cargo.toml create mode 100644 userland/symbolize/src/main.rs diff --git a/Cargo.lock b/Cargo.lock index b4e0cfe43fe..c06b1405433 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -555,7 +555,6 @@ dependencies = [ "bcachefs", "dlmalloc", "hashbrown 0.16.1", - "rustc-demangle", "toyos-abi", "toyos-acpi", "toyos-blackbox", @@ -1242,6 +1241,9 @@ version = "0.1.0" [[package]] name = "toyos-elf" version = "0.1.0" +dependencies = [ + "object", +] [[package]] name = "toyos-elide" @@ -1407,6 +1409,8 @@ dependencies = [ name = "toyos-symbols" version = "0.1.0" dependencies = [ + "object", + "rustc-demangle", "toyos-abi", "toyos-elf", "toyos-elide", diff --git a/issues/build/a-c-program-clang-links-for-toyos-carries-no-build-id.md b/issues/build/a-c-program-clang-links-for-toyos-carries-no-build-id.md new file mode 100644 index 00000000000..6874c66fd0f --- /dev/null +++ b/issues/build/a-c-program-clang-links-for-toyos-carries-no-build-id.md @@ -0,0 +1,19 @@ +--- +status: open +kind: defect +opened: 2026-10-04 +--- + +# A C program clang links for ToyOS carries no build-id + +A killed program's frame is recorded with its file's build-id, and +`/system/bin/symbolize` names it only from the file carrying the same one +(`toyos-symbols/src/frame.rs`). The Rust target links with `--build-id`; the +clang ToyOS driver (`clang/lib/Driver/ToolChains/ToyOS.cpp` in the LLVM fork) +passes no `--build-id` to `ld.lld`, so a C program has no `PT_NOTE` and its +frames are recorded `id=-`. The namer then names one from whatever file of that +name it finds, with nothing to tell another build of it apart. Fuchsia's +driver passes the flag (`Fuchsia.cpp`). + +**Exit**: a C program the ToyOS clang links carries a GNU build-id note, and a +C case killed by a fault is recorded with it. diff --git a/issues/diagnostics/toyos-explains-itself.md b/issues/diagnostics/toyos-explains-itself.md index a1f9badb6f1..7f33c38c134 100644 --- a/issues/diagnostics/toyos-explains-itself.md +++ b/issues/diagnostics/toyos-explains-itself.md @@ -10,8 +10,8 @@ ToyOS answers what it is doing, what it did and what it is made of from inside itself, with programs it ships. Today the answers are scattered: the log carries numbers in prose (`irq:`, `tlb:`, `PMM:`, `sched:`, `syscalls:`), the trace ring has LLDB as its only reader, nothing reads APERF, MPERF, -`MSR_SMI_COUNT`, RAPL or C-state residency, a process's memory is a byte sum -that reads 0 under contention, and a user symbol is resolved by the kernel. +`MSR_SMI_COUNT`, RAPL or C-state residency, and a process's memory is a byte +sum that reads 0 under contention. Each pillar's own track carries its steps and its exits; this track holds the counters and sizes pillars, which no other track does. diff --git a/issues/kernel/the-kernel-still-parses-what-userland-writes.md b/issues/kernel/the-kernel-still-parses-what-userland-writes.md index 6a46f578b80..ad81951f2da 100644 --- a/issues/kernel/the-kernel-still-parses-what-userland-writes.md +++ b/issues/kernel/the-kernel-still-parses-what-userland-writes.md @@ -42,27 +42,30 @@ first: the kernel mounts ROOT itself, but what mounts `/boot` and `/home`, and with what authority, is the open question. **Move 3 — the panic-time symbol resolver**, small and independent. Ruled -(owner, 2026-10-03, "adopt", `issues/diagnostics/toyos-explains-itself.md`): the -kernel names only itself; a killed program is reported as file and offset, and -one userland naming service shares the kernel's lookup and demangling code. -The kernel keeps its own ELF reader, `toyos-elf`: the orchestrator's reading -of the owner's crate ruling of the same day, "if it makes sense we wrote our -own elf parser or aml interpreter due to security or other important reasons -we do it". ELF parsing is already a pure crate that forbids unsafe and is -tested against a crafted corpus, and the symbol machinery moved onto it in the -2026-08-15 consolidation. What is left: the kernel stops naming a program's -addresses (`resolve_user_symbol` in `kernel/src/process.rs`); the per-spawn -table `read_backtrace_table` builds in `kernel/src/loader/symbols.rs`, and the -copy each task carries, are deleted; a userland service names a reported file -and offset; and `rustc-demangle`'s standing is decided. It is an unforked -crates.io dependency, ~2k lines of third-party string parsing in Ring 0 on the -panic path: fork it into the estate like every other third-party source, or -record the exemption deliberately. **Exit**: a guest test whose program dies -of a fault reads the kernel's report as the program's file and the faulting -offset, with no user symbol in it, and the userland service names the -faulting function from that file and offset; no kernel source builds a backtrace -table from a program's file; and `rustc-demangle` is forked or its exemption -recorded. +(owner, 2026-10-03, `issues/diagnostics/toyos-explains-itself.md`): he chose +"Adopt it: Built after the folder work, as part of the observability plan. +Frees at least 2 MiB per running program, the kernel stops parsing program +files' name tables, library frames get names." and wrote "adopt, make sure the +code can never panic or fail is that possible? i dont want to allow a kernel +crash because of that and it seems brittle by nature do you agree? can we +share a lot of code there? should we?". The kernel keeps its own ELF reader, +`toyos-elf`: the orchestrator's reading of the owner's crate ruling of the same +day, "if it makes sense we wrote our own elf parser or aml interpreter due to +security or other important reasons we do it". The kernel names only itself: a +killed program's frame is recorded as the file it ran, the offset in that file +and the file's build-id (`toyos-symbols/src/frame.rs`), and +`/system/bin/symbolize` names it with the kernel's own lookup and demangling +code. What is left: a library's frames print bare, because the kernel records +only the executable's image for a process; and `rustc-demangle`'s standing. It +is an unforked crates.io dependency, ~2k lines of third-party string parsing in +Ring 0 on the panic path, now for the kernel's own names alone: fork it into +the estate like every other third-party source, or record the exemption +deliberately. **Exit**: `disk_backtrace`, in the guest suite and on the T14, +reads its killed child's report as the child's file, offset and build-id with +no user symbol in it, and `symbolize` names the faulting function from them; +no kernel source builds a backtrace table from a program's file; a library's +frame is recorded as its file and offset; and `rustc-demangle` is forked or its +exemption recorded. **Small trim to evaluate:** main-thread exit killing the process is kernel policy the review called unnecessary. A process could end when its last thread does, or diff --git a/kernel/Cargo.lock b/kernel/Cargo.lock index d7c8738dd07..8be11ba0208 100644 --- a/kernel/Cargo.lock +++ b/kernel/Cargo.lock @@ -36,7 +36,6 @@ dependencies = [ "bcachefs", "dlmalloc", "hashbrown", - "rustc-demangle", "toyos-abi", "toyos-acpi", "toyos-blackbox", @@ -154,6 +153,7 @@ version = "0.1.0" name = "toyos-symbols" version = "0.1.0" dependencies = [ + "rustc-demangle", "toyos-abi", "toyos-elf", "toyos-elide", diff --git a/kernel/Cargo.toml b/kernel/Cargo.toml index 7dea204185d..74bc7cc0267 100644 --- a/kernel/Cargo.toml +++ b/kernel/Cargo.toml @@ -511,6 +511,5 @@ toyos-untrusted = { path = "../toyos-untrusted" } toyos-userbound = { path = "../toyos-userbound" } toyos-wallclock = { path = "../toyos-wallclock" } toyos-xhci = { path = "../toyos-xhci" } -rustc-demangle = "0.1" hashbrown = { version = "0.16", default-features = false } dlmalloc = { version = "0.2", default-features = false } diff --git a/kernel/src/arch/aarch64/trap.rs b/kernel/src/arch/aarch64/trap.rs index 298d52a54ed..c244c782801 100644 --- a/kernel/src/arch/aarch64/trap.rs +++ b/kernel/src/arch/aarch64/trap.rs @@ -332,7 +332,7 @@ fn user_report(frame: &Frame) { } log!(" pc:"); if percpu::current_pid().is_some() { - crate::process::resolve_user_symbol(frame.elr).log_bare(frame.elr); + crate::process::record_user_frame(frame.elr).log_bare(frame.elr); } else { log!(" {:#x}", frame.elr); } @@ -360,7 +360,7 @@ fn user_backtrace(start_fp: u64, max_frames: usize) { if ret == 0 { break; } - crate::process::resolve_user_symbol_return(ret).log_bare(ret); + crate::process::record_user_frame_return(ret).log_bare(ret); fp = saved; } } @@ -482,7 +482,7 @@ pub(crate) fn report_panic(message: &core::panic::PanicInfo, frame: u64) { let (pc, fp, sp) = percpu::syscall_context(); log!(" Syscall: num={} user_pc={pc:#x} user_sp={sp:#x}", percpu::syscall_num()); log!(" User backtrace:"); - crate::process::resolve_user_symbol(pc).log_bare(pc); + crate::process::record_user_frame(pc).log_bare(pc); user_backtrace(fp, 20); } } diff --git a/kernel/src/arch/x86_64/idt/exceptions.rs b/kernel/src/arch/x86_64/idt/exceptions.rs index f5cf3986d0d..6f0134d95ea 100644 --- a/kernel/src/arch/x86_64/idt/exceptions.rs +++ b/kernel/src/arch/x86_64/idt/exceptions.rs @@ -24,7 +24,7 @@ fn user_backtrace(start_rbp: u64, max_frames: usize) { }; let Ok(return_addr) = read_user_u64(rbp + 8) else { break }; if return_addr == 0 { break; } - process::resolve_user_symbol_return(return_addr).log_bare(return_addr); + process::record_user_frame_return(return_addr).log_bare(return_addr); rbp = saved_rbp; } } @@ -211,7 +211,7 @@ fn crash_report_exception(ctx: &ExceptionContext) { log!(" rip:"); if ring3 { if pid.is_some() { - process::resolve_user_symbol(ctx.frame.rip).log_bare(ctx.frame.rip); + process::record_user_frame(ctx.frame.rip).log_bare(ctx.frame.rip); } else { log!(" {:#x}", ctx.frame.rip); } @@ -267,7 +267,7 @@ fn crash_report_exception(ctx: &ExceptionContext) { log!(" Syscall: num={} user_rip={:#x} user_rsp={:#x}", percpu::syscall_num(), user_rip, percpu::user_rsp()); log!(" User backtrace:"); - process::resolve_user_symbol(user_rip).log_bare(user_rip); + process::record_user_frame(user_rip).log_bare(user_rip); user_backtrace(percpu::syscall_rbp(), 20); } } @@ -325,7 +325,7 @@ fn crash_report_panic(info: &core::panic::PanicInfo, rbp: u64) { log!(" Syscall: num={} user_rip={:#x} user_rsp={:#x}", percpu::syscall_num(), user_rip, percpu::user_rsp()); log!(" User backtrace:"); - process::resolve_user_symbol(user_rip).log_bare(user_rip); + process::record_user_frame(user_rip).log_bare(user_rip); user_backtrace(percpu::syscall_rbp(), 20); } } @@ -391,7 +391,7 @@ pub(super) fn double_fault_handler(frame: &TrapFrame) -> ! { log!(" User backtrace:"); if pid.is_some() { - process::resolve_user_symbol(maybe_rip).log_bare(maybe_rip); + process::record_user_frame(maybe_rip).log_bare(maybe_rip); user_backtrace(user_rbp, 20); } else { log!(" {:#x}", maybe_rip); diff --git a/kernel/src/hardlockup/mod.rs b/kernel/src/hardlockup/mod.rs index 3b2e518d8f5..3e5739f7eda 100644 --- a/kernel/src/hardlockup/mod.rs +++ b/kernel/src/hardlockup/mod.rs @@ -380,7 +380,7 @@ impl fmt::Display for At { Some((name, offset)) => write!( f, " {}+{offset:#x}", - toyos_symbols::symbol_text(rustc_demangle::demangle(name)), + toyos_symbols::demangled(name), ), } } diff --git a/kernel/src/loader/mod.rs b/kernel/src/loader/mod.rs index 2526058146b..557969dbe29 100644 --- a/kernel/src/loader/mod.rs +++ b/kernel/src/loader/mod.rs @@ -29,8 +29,8 @@ use crate::mm::policy::{CachePolicy, Prot}; use crate::mm::{PAGE_2M, PAGE_BYTES}; use crate::process::{ Admission, ElfInfo, Endowments, OwnedAlloc, PageAlloc, PageFaultTrace, PageTables, Parent, - Pid, ProcessAccounting, ProcessData, ProcessEntry, ThreadData, ThreadEntry, UserStack, - PROCESS_TABLE, + Pid, ProcessAccounting, ProcessData, ProcessEntry, ThreadData, ThreadEntry, UserImage, + UserStack, PROCESS_TABLE, }; use crate::sync::Lock; use crate::{scheduler, vfs, UserAddr}; @@ -40,6 +40,7 @@ use toyos_elf::section::SectionTable; use toyos_elf::sym::{self, SymTab}; use toyos_elf::rela::{FillLattice, Rules, FILL_GRANULE}; use toyos_elf::{Layout, RelocError, TlsSegment}; +use toyos_symbols::frame::BuildId; const USER_STACK_SIZE: usize = 4 * PAGE_2M as usize; // 8 MB @@ -572,12 +573,17 @@ pub fn spawn( let sp = user_stack.write_argv(argv); let t_tls = crate::clock::nanos_since_boot(); - let syms = symbols::read_backtrace_table( - backing.as_ref(), &layout, path, base, - image_start.raw(), image_end, - user_stack.base().raw(), user_stack.top(), - ); - let sym_bytes = syms.resident_bytes(); + // What a crash record names a frame by. The build-id is read at each + // `PT_NOTE`'s own offset, a page of it at most. + let image = Arc::new(UserImage { + name: String::from(path), + build_id: BuildId::find(&header_data, |note| { + Some(read_file_range(backing.as_ref(), note.offset, note.filesz.min(PAGE_BYTES as u64) as usize)) + }), + start: image_start.raw(), + end: image_end, + bias: base, + }); let (ks_alloc, ks_sp) = match alloc_kernel_stack(process_start, entry, sp, 0) { Some(ks) => ks, @@ -637,9 +643,6 @@ pub fn spawn( syscall_total_ns: 0, })); - // One table, two holders: cloned so a crash report on this thread reads names without the process table. - let syms = Arc::new(syms); - #[cfg(feature = "test-actuators")] crate::process::debug_kill_marked_place(parent); @@ -649,7 +652,8 @@ pub fn spawn( Arc::clone(&object), start::make_name(name), proc_data, - Arc::clone(&syms), + // Two holders: a crash report on this thread reads it without the process table. + Some(Arc::clone(&image)), ThreadEntry::new(thread_data), node, )); @@ -662,7 +666,7 @@ pub fn spawn( ks_sp, child_pt.clone(), thread_pointer, - syms, + Some(image), ); table.get_mut(pid).unwrap().threads_mut().get_mut(tid).unwrap().set_sched(sched); (tid, dst) @@ -678,8 +682,8 @@ pub fn spawn( crate::process::debug_hold_marked_spawn(parent, &object); let t3 = crate::clock::nanos_since_boot(); - log!("spawn: {} pid={} tid={} dst={} base={:#x} entry={:#x} root={:#x} symbols={}KiB (layout={}ms relocs={}ms deps={}ms tls={}ms total={}ms)", - path, pid, tid, dst.0, base, entry, child_pt.lock().root().phys(), sym_bytes / 1024, + log!("spawn: {} pid={} tid={} dst={} base={:#x} entry={:#x} root={:#x} (layout={}ms relocs={}ms deps={}ms tls={}ms total={}ms)", + path, pid, tid, dst.0, base, entry, child_pt.lock().root().phys(), (t1 - t0) / 1_000_000, (t2 - t1) / 1_000_000, (t_deps - t2) / 1_000_000, (t_tls - t_deps) / 1_000_000, (t3 - t0) / 1_000_000); diff --git a/kernel/src/loader/symbols.rs b/kernel/src/loader/symbols.rs index b3e90add22a..9d1411fd927 100644 --- a/kernel/src/loader/symbols.rs +++ b/kernel/src/loader/symbols.rs @@ -9,9 +9,6 @@ use alloc::collections::BTreeMap; use super::read_elf_table; use crate::file_backing::FileBacking; -use crate::mm::pmm::Category; -use crate::process::PageAlloc; -use crate::symbols::SymbolTable; use crate::UserAddr; use toyos_elf::section::{SectionTable, SHT_SYMTAB}; use toyos_elf::sym::{SymTab, STT_TLS}; @@ -76,74 +73,3 @@ pub fn read_symtab(backing: &dyn FileBacking, layout: &Layout) -> Option<(Vec SymbolTable { - let empty = || SymbolTable::empty_with_bounds(prog_base, prog_end, stack_base, stack_end); - - let Some(table) = layout.section_headers() else { return empty() }; - let shdrs = super::read_file_range(backing, table.file_offset, table.byte_len()); - let Some((syms, strs)) = SectionTable::new(&shdrs).symbols(SHT_SYMTAB) else { - return empty(); - }; - - // Bounded against the file: an unchecked section past EOF would read as - // zeros, a silent null table. - let file_size = backing.file_size(); - let fits = |sh: &toyos_elf::section::SectionHeader| { - sh.offset.checked_add(sh.size).is_some_and(|end| end <= file_size) - }; - if syms.size == 0 || !fits(&syms) || !fits(&strs) { - return empty(); - } - - let total = (syms.size + strs.size) as usize; - if total > MAX_SYMBOL_BYTES { - log!( - "ELF: {}: {} bytes of symbol table past the {} byte bound — its backtraces will be \ - bare addresses", - path, total, MAX_SYMBOL_BYTES - ); - return empty(); - } - - let Some(pages) = PageAlloc::new(total, Category::Elf) else { - log!("ELF: {}: no {} bytes for its symbol table", path, total); - return empty(); - }; - // `subslice` bounds each read against what `pages` returned, not a - // recomputed sum. - let dst = pages.window(); - if crate::elf::read_backing_into(backing, syms.offset, dst.subslice(0, syms.size as usize)) - .is_err() - || crate::elf::read_backing_into( - backing, - strs.offset, - dst.subslice(syms.size as usize, strs.size as usize), - ) - .is_err() - { - log!("ELF: {}: its symbol table could not be read off the device", path); - return empty(); - } - - SymbolTable::from_pages( - pages, - syms.size as usize, - strs.size as usize, - base, - prog_base, prog_end, stack_base, stack_end, - ) -} diff --git a/kernel/src/process.rs b/kernel/src/process.rs index 8788e7bfd86..363b038ca30 100644 --- a/kernel/src/process.rs +++ b/kernel/src/process.rs @@ -1,8 +1,8 @@ //! The process table, and what a process is made of. //! //! [`dump_crash_diagnostics`] and [`try_for_each_thread`] never wait on -//! [`PROCESS_TABLE`]: the faulting thread may hold it. A crash report resolves -//! symbols off the task's own record ([`resolve_user_symbol`]), never off +//! [`PROCESS_TABLE`]: the faulting thread may hold it. A crash report records a +//! user frame off the task's own record ([`record_user_frame`]), never off //! this table. //! //! `kernel::proclife` decides lifecycle transitions; this file only performs them. @@ -17,7 +17,6 @@ use crate::mm::policy::{CachePolicy, Prot, WindowProt}; use crate::mm::{PAGE_2M, PAGE_BYTES}; use crate::object::{ops, HandleTable}; use crate::sync::Lock; -use crate::symbols::SymbolTable; use crate::sched::payload::ThreadSched; use crate::time::{Deadline, Duration}; use crate::{elf, pipe, scheduler}; @@ -327,8 +326,8 @@ pub struct ProcessEntry { object: Arc, name: [u8; THREAD_NAME_LEN], process_data: Arc>, - /// No `Lock`: written once by the loader, then read-only, so teardown releases it by dropping this `Arc` — a crash report then reaches names through the task it is reporting on, never through this table. - symbols: Arc, + /// No `Lock`: written once by the loader, then read-only — a crash report reaches it through the task it is reporting on, never through this table. `None` for a kernel thread's. + image: Option>, main_tid: Tid, threads: crate::id_map::IdMap, /// Set once, with the exit's code, by the exit or kill that claims teardown; checked by `spawn_thread` so no thread appears after the retire set. @@ -345,7 +344,7 @@ impl ProcessEntry { object: Arc, name: [u8; THREAD_NAME_LEN], process_data: Arc>, - symbols: Arc, + image: Option>, main_thread: ThreadEntry, node: Node, ) -> Self { @@ -356,7 +355,7 @@ impl ProcessEntry { object, name, process_data, - symbols, + image, main_tid, threads, teardown_code: None, @@ -983,8 +982,8 @@ pub fn spawn_thread(entry: u64, stack_ptr: u64, arg: u64, stack_base: u64) -> Op })); let proc = table.get_mut(parent_process) .expect("spawn_thread: the entry the insert admission just answered for"); - // Every thread names the same symbols, so a crash report never asks this table. - let symbols = Arc::clone(&proc.symbols); + // Every thread carries the same image, so a crash report never asks this table. + let image = proc.image.clone(); let tid = proc.threads.insert(ThreadEntry::new(thread_data)); // Before the thread's first instruction, which is the enqueue below: the // thread reads its own id here without a syscall (`toyos_abi::TCB_TID`). @@ -1005,7 +1004,7 @@ pub fn spawn_thread(entry: u64, stack_ptr: u64, arg: u64, stack_base: u64) -> Op ks_sp, parent_addr_space, thread_pointer, - symbols, + image, ); proc.threads.get_mut(tid).unwrap().set_sched(sched); drop(guard); @@ -1071,12 +1070,12 @@ fn teardown_resources( (syscall_total, syscall_total_ns) } -/// Table-side teardown bookkeeping: drop the symbol table and total the CPU time of every thread still in the table. +/// Table-side teardown bookkeeping: drop the image record and total the CPU time of every thread still in the table. /// Caller must hold `PROCESS_TABLE`, be the last thread out and have freed the resources. fn teardown_bookkeeping(table: &mut ProcessTable, process_pid: Pid, code: i32) -> u64 { let proc = table.get_mut(process_pid) .expect("teardown_bookkeeping: process not found"); - proc.symbols = Arc::new(SymbolTable::empty()); + proc.image = None; let cpu_ns: u64 = proc.threads.iter().map(|(_, t)| t.sched().map_or(0, scheduler::task_cpu_ns)).sum(); let name = proc.name_str(); log!("exit: {name} pid={process_pid} code={code} cpu={}ms", cpu_ns / 1_000_000); @@ -1638,63 +1637,82 @@ pub fn dump_crash_diagnostics(fault_addr: u64, rip: u64) { } } -/// What a crash report learned when it asked for a user address's symbol. Not a `bool`: "no symbol was logged" would conflate an address the tables genuinely don't cover with one nothing looked up because a lock was held. -#[must_use = "an address with no symbol line still has to be printed"] +/// The executable a process runs, as its crash report records a frame in it +/// (`toyos_symbols::frame`): the file the kernel opened, the build-id that +/// file carries, and where it is mapped. +pub struct UserImage { + pub name: String, + pub build_id: Option, + /// `[start, end)`: the image's span in the process. + pub start: u64, + pub end: u64, + /// What a file address is moved by: an address `a` in the file is at `a + bias`. + pub bias: u64, +} + +/// What a crash report did with a user address. Not a `bool`: "no frame was recorded" would conflate an address the image does not cover with one nothing looked up because a lock was held. +#[must_use = "an address with no frame line still has to be printed"] #[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub enum SymbolLookup { - /// Resolved: the line naming it has already been logged. - Named, - /// The symbol table was read and covers no such address. - Unnamed, - /// Nothing was read: a scheduler pass already held this CPU's task record when the report began, so the running task's symbols were unreachable. +pub enum FrameRecord { + /// Recorded as file and offset: the line is already logged. + Recorded, + /// The running task's image does not hold the address. + Outside, + /// Nothing was read: a scheduler pass already held this CPU's task record when the report began, so the running task's image was unreachable. InPass, /// Nothing was read: no task is running on this CPU (idle, or before the first task). NoTask, } -impl SymbolLookup { - /// Log the bare address for a lookup with no symbol line, saying why; [`Named`](Self::Named) logs nothing — its line is already out. +impl FrameRecord { + /// Log the bare address for an address with no frame line, saying why; [`Recorded`](Self::Recorded) logs nothing — its line is already out. pub fn log_bare(self, addr: u64) { match self { - Self::Named => {} - Self::Unnamed => log!(" {:#x}", addr), + Self::Recorded => {} + Self::Outside => log!(" {:#x}", addr), Self::InPass => { - log!(" {:#x} ", addr) + log!(" {:#x} ", addr) } Self::NoTask => { - log!(" {:#x} ", addr) + log!(" {:#x} ", addr) } } } } -/// Resolve and log a user-mode address against the running process's symbol table; see [`with_current_symbols`]. -pub fn resolve_user_symbol(addr: u64) -> SymbolLookup { - with_current_symbols(|syms| crate::symbols::resolve_user(syms, addr)) +/// Record the user address `pc` as a frame of the running process's image; see [`with_current_image`]. +pub fn record_user_frame(pc: u64) -> FrameRecord { + with_current_image(|image| record(image, pc, Some(pc))) +} + +/// [`record_user_frame`] for a backtrace frame's return address: its offset is the call's last byte, so a call in tail position names its caller. +pub fn record_user_frame_return(return_addr: u64) -> FrameRecord { + with_current_image(|image| record(image, return_addr, return_addr.checked_sub(1))) } -/// [`resolve_user_symbol`] for a backtrace frame's return address — see [`crate::symbols::SymbolTable::resolve_return`]. -pub fn resolve_user_symbol_return(return_addr: u64) -> SymbolLookup { - with_current_symbols(|syms| crate::symbols::resolve_user_return(syms, return_addr)) +/// Log `pc`'s frame, its offset that of `at`, when `image` holds `at`. +fn record(image: &UserImage, pc: u64, at: Option) -> bool { + let offset = at.and_then(|at| toyos_symbols::frame::frame_offset(at, image.start, image.end, image.bias)); + let Some(offset) = offset else { return false }; + log!("{}", toyos_symbols::frame::UserFrame { pc, name: &image.name, offset, build_id: image.build_id }); + true } -/// Run `f` against the symbol table of the task this CPU is running, and say what happened. -/// Three-way, not two: `f` decides between [`Named`](SymbolLookup::Named) and [`Unnamed`](SymbolLookup::Unnamed); every other answer means the address was never looked up, and the caller must print it via [`SymbolLookup::log_bare`]. -/// No lock: this runs from fault/panic reports, where the faulting thread may hold `PROCESS_TABLE`, so a task carries its own process's symbols on its own record ([`sched::driver::current_symbols`]) instead. +/// Run `f` against the image of the task this CPU is running, and say what happened. +/// No lock: this runs from fault/panic reports, where the faulting thread may hold `PROCESS_TABLE`, so a task carries its own process's image on its own record ([`sched::driver::current_image`]) instead. /// `pid` is not a parameter: a report is always about the process whose CPU is producing it. -fn with_current_symbols(f: impl FnOnce(&crate::symbols::SymbolTable) -> bool) -> SymbolLookup { - let Some(syms) = crate::sched::driver::current_symbols() else { +fn with_current_image(f: impl FnOnce(&UserImage) -> bool) -> FrameRecord { + let Some(image) = crate::sched::driver::current_image() else { // This CPU cannot switch between the two causes mid-report: a report is not reschedulable while `PerCpu::fault_state` is non-zero. return if crate::sched::driver::in_pass() { - SymbolLookup::InPass + FrameRecord::InPass } else { - SymbolLookup::NoTask + FrameRecord::NoTask }; }; - if f(&syms) { - SymbolLookup::Named - } else { - SymbolLookup::Unnamed + match image { + Some(image) if f(&image) => FrameRecord::Recorded, + _ => FrameRecord::Outside, } } diff --git a/kernel/src/sched/driver.rs b/kernel/src/sched/driver.rs index 66b50714e4b..01ac10bf608 100644 --- a/kernel/src/sched/driver.rs +++ b/kernel/src/sched/driver.rs @@ -404,8 +404,8 @@ pub struct NewTask { pub address_space: PageTables, pub thread_pointer: u64, pub share: Arc, - /// The process's symbol table; a kernel thread names an empty one. - pub symbols: Arc, + /// The process's image; `None` for a kernel thread. + pub image: Option>, } /// Place a new task by message — never by reaching into the destination's queue. @@ -436,7 +436,7 @@ pub fn spawn(new: NewTask) -> (ThreadSched, CpuId) { kernel_stack: new.kernel_stack, address_space: new.address_space, handle: handle.clone(), - symbols: new.symbols, + image: new.image, }, rt: RtState::default(), } @@ -731,14 +731,14 @@ pub fn with_current_handle(f: impl FnOnce(&crate::sched::payload::TaskHandle) try_with_cpu(|cpu| cpu.running().map(|t| f(t.ext().handle.as_ref())))? } -/// The symbol table of the task this CPU is running. +/// The image of the task this CPU is running, `None` inside for a kernel thread. /// -/// Takes no lock: the table is immutable and the `Arc` outlives teardown. `None`: a pass already holds this +/// Takes no lock: the image is immutable and the `Arc` outlives teardown. `None`: a pass already holds this /// record, or none is running. /// /// No pass can start underneath the read: a fault handler runs with preemption declined. -pub fn current_symbols() -> Option> { - try_with_cpu(|cpu| cpu.running().map(|t| t.ext().symbols.clone())).flatten() +pub fn current_image() -> Option>> { + try_with_cpu(|cpu| cpu.running().map(|t| t.ext().image.clone())).flatten() } /// What the running task's marks say it does instead of returning to Ring 3 — one load, no clone, since an `Arc` refcount here is too costly on this path. diff --git a/kernel/src/sched/kthread.rs b/kernel/src/sched/kthread.rs index ac3cf7618d6..e63843293c5 100644 --- a/kernel/src/sched/kthread.rs +++ b/kernel/src/sched/kthread.rs @@ -13,7 +13,6 @@ use crate::process::{ ThreadEntry, PROCESS_TABLE, THREAD_NAME_LEN, }; use crate::scheduler::{self, TaskId}; -use crate::symbols::SymbolTable; use crate::sync::Lock; use kernel::proclife::Processes; @@ -92,9 +91,6 @@ pub fn spawn(name: &str, body: extern "C" fn(u64) -> !, arg: u64) -> ThreadSched let len = name.len().min(THREAD_NAME_LEN - 1); short[..len].copy_from_slice(&name.as_bytes()[..len]); - // No user half: the empty table, and frames resolve through `symbols::resolve_kernel`. - let syms = Arc::new(SymbolTable::empty()); - // One hold across insert and place: a visible pid already has its thread scheduled. let mut guard = PROCESS_TABLE.lock(); let table = guard.as_mut().expect("kthread: spawned before process::init"); @@ -102,7 +98,8 @@ pub fn spawn(name: &str, body: extern "C" fn(u64) -> !, arg: u64) -> ThreadSched crate::object::process::ProcessObject::new(pid), short, Arc::new(Lock::new(kernel_process_data(name))), - Arc::clone(&syms), + // No user image: its frames resolve through `symbols::resolve_kernel`. + None, ThreadEntry::new(Arc::new(Lock::new(kernel_thread_data()))), kernel::proclife::Node::root(), )); @@ -115,7 +112,7 @@ pub fn spawn(name: &str, body: extern "C" fn(u64) -> !, arg: u64) -> ThreadSched entry_sp, crate::mm::paging::kernel().clone(), 0, - syms, + None, ); table .get_mut(pid) diff --git a/kernel/src/sched/payload.rs b/kernel/src/sched/payload.rs index c4be530acd2..d36b28c560d 100644 --- a/kernel/src/sched/payload.rs +++ b/kernel/src/sched/payload.rs @@ -15,8 +15,7 @@ use kernel::sched::park::WaitTicket; use crate::watch::Watch; use crate::mm::paging::Root; use crate::scheduler::OperationSlot; -use crate::process::{OwnedAlloc, PageTables, ProcessAccounting, TaskId}; -use crate::symbols::SymbolTable; +use crate::process::{OwnedAlloc, PageTables, ProcessAccounting, TaskId, UserImage}; use crate::sync::Lock; /// The environment's leaf lock; holding it raises the preempt count, making a wake path a legal mailbox producer. @@ -62,9 +61,8 @@ pub struct KernelPayload { pub address_space: PageTables, /// The cross-CPU-readable face of this task; a `CpuSched` is `!Sync` and cannot be walked remotely. pub handle: Arc, - /// This task's process's symbol table; kept here, not looked up via the process table, so a crash report never takes that lock. - /// On the payload, not the handle: the handle outlives the task until reaped, and this is megabytes of process pages. - pub symbols: Arc, + /// This task's process's image, as a crash report records a frame in it; kept here, not looked up via the process table, so a crash report never takes that lock. `None` for a kernel thread. + pub image: Option>, } impl SchedPayload for KernelPayload { diff --git a/kernel/src/scheduler.rs b/kernel/src/scheduler.rs index 1d6f1a3d192..0ed55ae49a6 100644 --- a/kernel/src/scheduler.rs +++ b/kernel/src/scheduler.rs @@ -296,7 +296,7 @@ pub fn enqueue_new( entry_sp: u64, address_space: crate::process::PageTables, thread_pointer: u64, - symbols: alloc::sync::Arc, + image: Option>, ) -> (ThreadSched, CpuId) { driver::spawn(NewTask { id, @@ -305,7 +305,7 @@ pub fn enqueue_new( address_space, thread_pointer, share: share_for(id.0), - symbols, + image, }) } diff --git a/kernel/src/symbols.rs b/kernel/src/symbols.rs index f6756cf16d6..1eea73858e3 100644 --- a/kernel/src/symbols.rs +++ b/kernel/src/symbols.rs @@ -1,112 +1,50 @@ -//! Raw-pointer half of a kernel backtrace frame: [`SymbolTable`] is read from -//! the fault handler, panic handler and double fault, so it never allocates, -//! locks or does I/O. `toyos-symbols` holds the pure byte-level decisions; -//! this module holds only what must run in that context, plus the boot-time -//! globals and `log!` integration. +//! The kernel's own symbols: [`SymbolTable`] is read from the fault handler, +//! panic handler and double fault, so it never allocates, locks or does I/O. +//! `toyos-symbols` holds the pure byte-level decisions; this module holds only +//! what must run in that context, plus the boot-time global and `log!` +//! integration. The kernel names no program's frame: a user frame is recorded +//! as file and offset (`process::record_user_frame`). use core::sync::atomic::{AtomicPtr, AtomicU64, Ordering}; use alloc::boxed::Box; use toyos_elf::sym::SymTab; -use toyos_symbols::symbol_text; +use toyos_symbols::demangled; -use crate::process::PageAlloc; - -/// Zero-allocation symbol table: raw pointers into ELF sections, safe to call from any context including panic/double-fault. +/// Zero-allocation symbol table: raw pointers into the kernel image's ELF sections, safe to call from any context including panic/double-fault. pub struct SymbolTable { - /// `None` for kernel tables borrowed from the direct map; `Some` for owned process pages. - pages: Option, symtab: *const u8, symtab_len: usize, strtab: *const u8, strtab_len: usize, base: u64, - prog_base: u64, - prog_end: u64, - stack_base: u64, - stack_end: u64, } -// SAFETY: the pointers name only the static kernel image or pages this table -// owns and frees, so moving it carries the only reference to that memory; -// `SymbolTable` is `!Send` only because `*const u8` is. +// SAFETY: the pointers name only the kernel image in the direct map, mapped +// for the life of the machine; `SymbolTable` is `!Send` only because +// `*const u8` is. unsafe impl Send for SymbolTable {} // SAFETY: no method writes through either pointer, so concurrent reads from // multiple CPUs alias only immutable bytes. unsafe impl Sync for SymbolTable {} impl SymbolTable { - pub fn empty() -> Self { - Self::empty_with_bounds(0, 0, 0, 0) - } - - pub fn empty_with_bounds( - prog_base: u64, prog_end: u64, - stack_base: u64, stack_end: u64, - ) -> Self { - Self { - pages: None, - symtab: core::ptr::null(), - symtab_len: 0, - strtab: core::ptr::null(), - strtab_len: 0, - base: 0, - prog_base, prog_end, stack_base, stack_end, - } - } - - /// A process's tables in pages it owns: `.symtab` (`symtab_len` bytes) followed immediately by `.strtab`. - // Eight arguments: four are address-range bounds from the one caller that already holds them. - #[allow(clippy::too_many_arguments)] - pub fn from_pages( - pages: PageAlloc, - symtab_len: usize, - strtab_len: usize, - base: u64, - prog_base: u64, prog_end: u64, - stack_base: u64, stack_end: u64, - ) -> Self { - let start = pages.ptr(); - Self { - symtab: start, - symtab_len, - // SAFETY: the caller sized the allocation to `symtab_len + - // strtab_len` and wrote `.strtab` right after `.symtab`, so - // `symtab_len` is a valid offset inside `pages`. - strtab: unsafe { start.add(symtab_len) }, - strtab_len, - pages: Some(pages), - base, - prog_base, prog_end, stack_base, stack_end, - } - } - // A failed `locate` returns an empty table rather than an error: a kernel that cannot find its symbols still boots. fn from_elf(data: &[u8], base: u64) -> Self { - let Some((symtab, strtab)) = toyos_symbols::locate(data) else { return Self::empty() }; - + let (symtab, strtab) = toyos_symbols::locate(data).unwrap_or((&[], &[])); Self { - pages: None, symtab: symtab.as_ptr(), symtab_len: symtab.len(), strtab: strtab.as_ptr(), strtab_len: strtab.len(), base, - prog_base: 0, - prog_end: 0, - stack_base: 0, - stack_end: 0, } } fn tables(&self) -> SymTab<'_> { - if self.symtab.is_null() || self.strtab.is_null() { - return SymTab::empty(); - } - // SAFETY: both pointers are non-null by the guard above, both ranges - // are either the kernel image in the direct map or pages this table - // owns and frees in `Drop`, and both lengths were bounded at - // construction. + // SAFETY: both ranges are slices `locate` borrowed out of the kernel + // image in the direct map, mapped for the life of the machine, or the + // dangling-but-aligned pointer of an empty slice with length 0. unsafe { SymTab::new( core::slice::from_raw_parts(self.symtab, self.symtab_len), @@ -115,16 +53,6 @@ impl SymbolTable { } } - /// How much memory this table's bytes occupy, for the spawn log. - pub fn resident_bytes(&self) -> usize { - self.pages.as_ref().map_or(0, PageAlloc::size) - } - - pub fn is_valid_user_addr(&self, addr: u64) -> bool { - (addr >= self.prog_base && addr < self.prog_end) - || (addr >= self.stack_base && addr < self.stack_end) - } - /// Resolve an address to (mangled name, offset); no allocation, lock or panic. pub fn resolve(&self, addr: u64) -> Option<(&str, u64)> { self.tables().resolve(addr.checked_sub(self.base)?) @@ -136,10 +64,6 @@ impl SymbolTable { let (name, offset) = self.resolve(return_addr.saturating_sub(1))?; Some((name, offset + 1)) } - - pub fn prog_base(&self) -> u64 { - self.prog_base - } } // Kernel symbols — set once at boot, lock-free reads forever after. @@ -200,7 +124,7 @@ fn log_kernel(addr: u64, lookup: impl FnOnce(&SymbolTable) -> Option<(&str, u64) // `SymbolTable` that stays valid for the rest of boot. let table = unsafe { &*ptr }; if let Some((raw, offset)) = lookup(table) { - log!(" {:#x} {}+{:#x}", addr, symbol_text(rustc_demangle::demangle(raw)), offset); + log!(" {:#x} {}+{:#x}", addr, demangled(raw), offset); Some(offset) } else { let kb = KERNEL_BASE.load(Ordering::Relaxed); @@ -213,30 +137,6 @@ fn log_kernel(addr: u64, lookup: impl FnOnce(&SymbolTable) -> Option<(&str, u64) } } -/// Resolve and log a user address against a process's symbol table; returns -/// whether it was identified. -pub fn resolve_user(syms: &SymbolTable, addr: u64) -> bool { - log_user(syms, addr, syms.resolve(addr)) -} - -/// [`resolve_user`] for a backtrace frame's return address — see [`SymbolTable::resolve_return`]. -pub fn resolve_user_return(syms: &SymbolTable, return_addr: u64) -> bool { - log_user(syms, return_addr, syms.resolve_return(return_addr)) -} - -fn log_user(syms: &SymbolTable, addr: u64, resolved: Option<(&str, u64)>) -> bool { - if let Some((name, offset)) = resolved { - log!(" {:#x} {}+{:#x}", addr, symbol_text(rustc_demangle::demangle(name)), offset); - true - } else if syms.is_valid_user_addr(addr) { - let base_offset = addr.saturating_sub(syms.prog_base()); - log!(" {:#x} [exe+{:#x}]", addr, base_offset); - true - } else { - false - } -} - /// Walk the frame-pointer chain from `start_fp`, resolving each return /// address. Every architecture this kernel builds for lays a frame record out /// the same way under `-Cforce-frame-pointers=yes`: the caller's frame pointer, diff --git a/rust b/rust index 724238524f4..a5b419f27ad 160000 --- a/rust +++ b/rust @@ -1 +1 @@ -Subproject commit 724238524f4dcddc202ffa570cf034c512fe3e1d +Subproject commit a5b419f27ad0b9d00f5d8c0402cc01f08d4e4748 diff --git a/system.toml b/system.toml index 7ad1913158f..63251590908 100644 --- a/system.toml +++ b/system.toml @@ -105,6 +105,10 @@ receives = ["netstack"] receives = ["netstack", "soundserver", "log", "compositor"] syscap = ["inventory"] +# Names a killed program's frames from the file and offset the kernel records: +# it reads the files a record names and holds nothing else. +[programs.symbolize] + [programs.filepicker] service = true serves = ["filepicker"] diff --git a/tests/testcases/system.toml b/tests/testcases/system.toml index c0b5e30efff..8bceee59e79 100644 --- a/tests/testcases/system.toml +++ b/tests/testcases/system.toml @@ -43,6 +43,9 @@ syscap = ["device", "dup", "logread", "power", "roster"] [programs.toybox] receives = ["soundserver"] +# `disk_backtrace` names its child's crash record with it. +[programs.symbolize] + [symlinks] "bin/cat" = "/system/bin/toybox" "bin/cp" = "/system/bin/toybox" diff --git a/tests/toyos-rust-tests/src/bin/disk_backtrace.rs b/tests/toyos-rust-tests/src/bin/disk_backtrace.rs index d644400ef78..b6dcea90fe1 100644 --- a/tests/toyos-rust-tests/src/bin/disk_backtrace.rs +++ b/tests/toyos-rust-tests/src/bin/disk_backtrace.rs @@ -1,14 +1,27 @@ -//! A program running from a disk gets a backtrace with names in it. +//! A program that dies running from a disk is recorded as its file and the +//! faulting offset, and `symbolize` names the function from them. //! //! `/home` rather than `/tmp`: tmpfs has no device under it, so it would prove //! the mechanism without proving the claim. The claim is about a device. +//! +//! The kernel names no program's frame, so the function's name can only come +//! from the file: a record holding it, or one without the file's path, is the +//! kernel reading a program's symbol table again. use std::fs; use std::process::Command; +use toyos::endow::{Endowments, SYSCAP_LABEL}; +use toyos::log::{LogTail, Record}; +use toyos::syscap::SysCap; + const DIR: &str = "/home/disk_backtrace"; const IN_ROOT: &str = "/system/bin/test_rs_disk_backtrace_child"; const ON_DISK: &str = "/home/disk_backtrace/child"; +const FRAME: &str = "/home/disk_backtrace/frame"; +const NAMER: &str = "/system/bin/symbolize"; +/// The child's `#[inline(never)]` function the null read is in. +const FAULTS_IN: &str = "null_deref_run_from_disk"; fn main() { let _ = fs::create_dir(DIR); @@ -18,16 +31,50 @@ fn main() { fs::write(ON_DISK, &image).unwrap_or_else(|e| panic!("write {ON_DISK}: {e}")); println!(" copied {} bytes to {ON_DISK}", image.len()); - // The verdict is on the serial, not here: `check_disk_backtrace` reads the - // kernel's SEGFAULT report out of this test's own capture window. What this - // side proves is that the child ran from the disk at all and died the way - // it was supposed to — without which the serial check would be asserting on - // a report that never happened. - let status = Command::new(ON_DISK) - .status() + let mut child = Command::new(ON_DISK) + .spawn() .unwrap_or_else(|e| panic!("spawn {ON_DISK}: {e}")); + let pid = child.id(); + let status = child.wait().unwrap_or_else(|e| panic!("wait for {ON_DISK}: {e}")); assert!(!status.success(), "a child that dereferences null should be killed"); - println!(" PASS: {ON_DISK} faulted (exit={})", status.code().unwrap_or(-1)); + // Its report is recorded on its own thread before its end is, so it is + // in the log by the time the wait answers. + let pc = format!("{ON_DISK}+0x"); + let frame = records_of(pid) + .into_iter() + .find(|line| line.contains(&pc)) + .unwrap_or_else(|| panic!("no record of pid {pid} names a frame as {pc}…")); + println!(" recorded: {frame}"); + assert!(frame.contains(" id=") && !frame.ends_with(" id=-"), "the frame carries no build-id: {frame}"); + assert!(!frame.contains(FAULTS_IN), "the kernel named a program's frame: {frame}"); + + fs::write(FRAME, format!("{frame}\n")).unwrap_or_else(|e| panic!("write {FRAME}: {e}")); + let named = Command::new(NAMER) + .arg(FRAME) + .output() + .unwrap_or_else(|e| panic!("run {NAMER}: {e}")); + assert!(named.status.success(), "{NAMER} {FRAME}: {:?}", named.status); + let named = String::from_utf8_lossy(&named.stdout).into_owned(); + println!(" named: {}", named.trim_end()); + assert!(named.contains(FAULTS_IN), "{NAMER} did not name {FAULTS_IN} from {frame}: {named}"); + println!("disk backtrace test passed"); } + +/// Every kernel record of `pid` the log still holds, oldest first. +fn records_of(pid: u32) -> Vec { + let cap: SysCap = Endowments::get() + .take(SYSCAP_LABEL) + .expect("test-runner endows every binary it spawns a system capability"); + let mut tail = LogTail::new(); + let mut records = vec![Record::EMPTY; 64]; + let mut lines = Vec::new(); + loop { + let read = tail.read(&cap, &mut records).expect("read the kernel's records"); + if read.is_empty() { + return lines; + } + lines.extend(read.iter().filter(|r| r.pid == pid).map(|r| r.message().to_owned())); + } +} diff --git a/tests/toyos-rust-tests/src/bin/disk_backtrace_child.rs b/tests/toyos-rust-tests/src/bin/disk_backtrace_child.rs index f5f2152f83f..8d8710ac337 100644 --- a/tests/toyos-rust-tests/src/bin/disk_backtrace_child.rs +++ b/tests/toyos-rust-tests/src/bin/disk_backtrace_child.rs @@ -1,8 +1,4 @@ //! Dereferences null so the kernel prints a SEGFAULT report for it. -//! -//! `disk_backtrace` copies this binary onto a disk and runs it from there, so -//! the report has to name a symbol no *other* boot could have put in the same -//! capture window. #[inline(never)] fn null_deref_run_from_disk() -> u64 { diff --git a/toyos-elf/Cargo.toml b/toyos-elf/Cargo.toml index 7542cfac479..a73b991f87c 100644 --- a/toyos-elf/Cargo.toml +++ b/toyos-elf/Cargo.toml @@ -10,3 +10,6 @@ description = "ELF64 decoding of untrusted program images (no_std, no alloc, for version = "0.1.0" edition = "2021" license = "MIT OR Apache-2.0" + +[dev-dependencies] +object = { version = "0.38", default-features = false, features = ["read_core", "elf", "std"] } diff --git a/toyos-elf/src/header.rs b/toyos-elf/src/header.rs index e190f82b94e..b15f8b3e222 100644 --- a/toyos-elf/src/header.rs +++ b/toyos-elf/src/header.rs @@ -158,6 +158,7 @@ pub struct ProgramHeader { pub const PT_LOAD: u32 = 1; pub const PT_DYNAMIC: u32 = 2; pub const PT_INTERP: u32 = 3; +pub const PT_NOTE: u32 = 4; pub const PT_TLS: u32 = 7; pub const PT_GNU_EH_FRAME: u32 = 0x6474_e550; diff --git a/toyos-elf/src/lib.rs b/toyos-elf/src/lib.rs index 1d9685d7c65..9de3e282b3c 100644 --- a/toyos-elf/src/lib.rs +++ b/toyos-elf/src/lib.rs @@ -37,6 +37,7 @@ pub mod dynamic; pub mod gnu_hash; pub mod header; pub mod layout; +pub mod note; pub mod rela; pub mod section; pub mod sym; diff --git a/toyos-elf/src/note.rs b/toyos-elf/src/note.rs new file mode 100644 index 00000000000..9ca1ba587a9 --- /dev/null +++ b/toyos-elf/src/note.rs @@ -0,0 +1,68 @@ +//! `PT_NOTE`, for the one note ToyOS reads: the build-id a linker stamps into an +//! image, which is what lets a reader of a crash record say the file it opened +//! is the file that crashed. +//! +//! The layout is the gABI's note section: a 12-byte header (`namesz`, `descsz`, +//! `type`), the name, the descriptor, each padded to the segment's alignment — +//! 4, or 8 for a segment that declares it. Nothing here refuses a file: a note +//! that cannot be read names no build-id. + +use crate::header::{FileHeader, ProgramHeader, PROGRAM_HEADER_SIZE, PT_NOTE}; +use crate::read; + +/// `n_type` of a GNU build-id note. +pub const NT_GNU_BUILD_ID: u32 = 3; + +/// The longest build-id this crate answers. lld's and `toyos-ld`'s are 20 +/// bytes, a `uuid` or `md5` one 16; past this a descriptor is no build-id any +/// linker writes. +pub const MAX_BUILD_ID: usize = 32; + +/// One `PT_NOTE`: where its bytes are in the file, and the alignment its notes +/// are padded to. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct NoteSegment { + pub offset: u64, + pub filesz: u64, + pub align: u64, +} + +/// Every `PT_NOTE` in the program header table `data` holds, in table order; +/// none when `data` holds no readable table. +pub fn segments(data: &[u8]) -> impl Iterator + '_ { + let table = FileHeader::parse(data).and_then(|h| h.program_headers(data)).unwrap_or(&[]); + (0..table.len() / PROGRAM_HEADER_SIZE) + .filter_map(move |i| ProgramHeader::parse(table, i)) + .filter(|p| p.kind == PT_NOTE) + .map(|p| NoteSegment { offset: p.offset, filesz: p.filesz, align: p.align }) +} + +/// The descriptor of the first `NT_GNU_BUILD_ID` note named `"GNU"` in `notes`, +/// a `PT_NOTE`'s bytes padded to `align`, when it is 1 to [`MAX_BUILD_ID`] +/// bytes long. +pub fn build_id(notes: &[u8], align: u64) -> Option<&[u8]> { + let align = match align { + 0..=4 => 4, + 8 => 8, + _ => return None, + }; + let mut rest = notes; + loop { + let namesz = read::u32_at(rest, 0)? as usize; + let descsz = read::u32_at(rest, 4)? as usize; + let kind = read::u32_at(rest, 8)?; + let name_end = 12usize.checked_add(namesz)?; + let name = rest.get(12..name_end)?; + let desc_start = align_up(name_end, align)?; + let desc_end = desc_start.checked_add(descsz)?; + let desc = rest.get(desc_start..desc_end)?; + if kind == NT_GNU_BUILD_ID && name == b"GNU\0" { + return (1..=MAX_BUILD_ID).contains(&desc.len()).then_some(desc); + } + rest = rest.get(align_up(desc_end, align)?..)?; + } +} + +fn align_up(at: usize, align: usize) -> Option { + Some(at.checked_add(align - 1)? & !(align - 1)) +} diff --git a/toyos-elf/tests/fixtures/lld-headers.bin b/toyos-elf/tests/fixtures/lld-headers.bin new file mode 100644 index 0000000000000000000000000000000000000000..367ceb644b25389ec959fa0fe8c039d3ad16c791 GIT binary patch literal 4096 zcmb8xJ!n%=6bJCz*49XcW+)nqLk0_3gkYD5P#%JsK}xC!#VJFFj6qsRL1`2%5v#`f z(X_QTYKK6VY{o@0IAqM=5JV&hN^}b75X8ZEd+ve0Km7|ncs=Rwo%`|5z3(N9=PsV_ z@9P^d?ietWrqBO;y?kI-Jm>MX5%<6rDt5!y4Z5rc}BmA z*LZk;uDMCMcFUJ#p)ejFmvJ=enoaxd>^%SXur#K_E7R8;M#wjcoJO4O@nJcIq*}rW!xh8=WX!{xb|Ns-Xh)we}6~%4?PLi zLF*g^*ZCyCckW9+Dexaf@eH`mKTo_wyh^+QetK2LZG-E$J>ub~!8+tOq@Ni0$fmeQ zd`4QrA;z{CZ;yG~LS4D8$hZXSj zqI}=f!J~8H4e+q@CV19)7d*ct{e)M8^)d6}5paEu;^5nE|0Id0!KYuzdgX{0!FAjU z_}MM#rw%?_5^sU)JiFk_?<7A|3f5tKNjwU!&s_ps_kW6bhIpQMiFlQG16=o28(jBc z4_u#%@LI4A`hJUn>$-U0hh9qEW{78r7l@aM*N8WXcZi$y;Qi|SBgEsxlf=`+bHt0p zE5z%>Tg1D>LmT_oAxbgraW#TpBP2wHmW^@1hjezTWJ5D@FJWV_YuIo|(Z@!lAqbhjWd5w4-{H^<4(E=ZR zB=4&(@z9H4eKbExJV87KuD^#f#Ph^U;GUa*m3V`An|P0Scq>>39XAH9--90bUw-0b Ay#N3J literal 0 HcmV?d00001 diff --git a/toyos-elf/tests/fuzz.rs b/toyos-elf/tests/fuzz.rs index 2314fb05f15..dd608e25fc7 100644 --- a/toyos-elf/tests/fuzz.rs +++ b/toyos-elf/tests/fuzz.rs @@ -499,3 +499,44 @@ fn every_derived_address_lies_inside_the_image_or_the_image_is_refused() { assert!(n >= 1000, "only {n} accepted {what}: {reached:?}"); } } + +/// Seeded note segments, honest most of the time and hostile the rest: no +/// size, name, alignment or cut makes `note::build_id` panic, and every id it +/// answers is a `GNU` build-id note's whole descriptor out of the bytes given. +#[test] +fn no_note_segment_panics_the_build_id_read() { + use toyos_elf::note::{self, MAX_BUILD_ID, NT_GNU_BUILD_ID}; + let mut rng = Rng { state: 0xA076_1D64_78BD_642F, hostile: 20 }; + let mut found = 0u64; + const ITERATIONS: u64 = 200_000; + for i in 0..ITERATIONS { + let align = rng.pick(&[4u64, 4, 8, 0, 1, 2, 16, u64::MAX]); + let pad = if align == 8 { 8 } else { 4 }; + let mut bytes = Vec::new(); + for _ in 0..=rng.below(3) { + let name: &[u8] = rng.pick(&[&b"GNU\0"[..], b"GNU", b"abcde\0", b""]); + let desc_len = rng.below(MAX_BUILD_ID as u64 + 4) as usize; + let namesz = rng.value(name.len() as u64, &[u32::MAX as u64, 0x7fff_ffff]) as u32; + let descsz = rng.value(desc_len as u64, &[u32::MAX as u64, 0x7fff_ffff]) as u32; + let kind = rng.value(NT_GNU_BUILD_ID as u64, &[1, 2, 4]) as u32; + bytes.extend(namesz.to_le_bytes()); + bytes.extend(descsz.to_le_bytes()); + bytes.extend(kind.to_le_bytes()); + bytes.extend(name); + bytes.resize(bytes.len().next_multiple_of(pad), 0); + bytes.extend((0..desc_len).map(|_| rng.next() as u8)); + bytes.resize(bytes.len().next_multiple_of(pad), 0); + } + if rng.chance(10) { + bytes.truncate(rng.below(bytes.len() as u64 + 1) as usize); + } + let read = std::panic::catch_unwind(|| note::build_id(&bytes, align).map(<[u8]>::to_vec)); + let Ok(read) = read else { panic!("iteration {i}: a {}-byte segment at align {align} panicked", bytes.len()) }; + if let Some(id) = read { + assert!((1..=MAX_BUILD_ID).contains(&id.len()), "iteration {i}: {} bytes", id.len()); + assert!(bytes.windows(id.len()).any(|w| w == id), "iteration {i}: an id not in the segment"); + found += 1; + } + } + assert!(found * 20 >= ITERATIONS, "only {found} build-ids read: the generator reaches nothing"); +} diff --git a/toyos-elf/tests/note.rs b/toyos-elf/tests/note.rs new file mode 100644 index 00000000000..7991840057e --- /dev/null +++ b/toyos-elf/tests/note.rs @@ -0,0 +1,153 @@ +//! `PT_NOTE` and the build-id in it, against crafted notes and against +//! `object`'s note reader, which is not ToyOS code. + +#[allow(dead_code)] +mod common; + +use common::*; +use object::elf::{FileHeader64, NT_GNU_BUILD_ID as OBJECT_NT_GNU_BUILD_ID}; +use object::read::elf::NoteIterator; +use object::LittleEndian; +use toyos_elf::note::{self, NoteSegment, MAX_BUILD_ID, NT_GNU_BUILD_ID}; + +const PT_NOTE: u32 = 4; +const OTHER: u32 = 1; + +/// One note: `name` with its NUL, `desc`, each padded to `align`. +fn note(name: &[u8], kind: u32, desc: &[u8], align: usize) -> Vec { + let pad = |v: &mut Vec| v.resize(v.len().next_multiple_of(align), 0); + let mut out = Vec::new(); + out.extend((name.len() as u32).to_le_bytes()); + out.extend((desc.len() as u32).to_le_bytes()); + out.extend(kind.to_le_bytes()); + out.extend(name); + pad(&mut out); + out.extend(desc); + pad(&mut out); + out +} + +fn gnu_id(desc: &[u8], align: usize) -> Vec { + note(b"GNU\0", NT_GNU_BUILD_ID, desc, align) +} + +/// `object`'s answer for the same bytes: the first note it reads named `GNU` +/// of type `NT_GNU_BUILD_ID`. +fn objects(notes: &[u8], align: u64) -> Option> { + let mut iter = NoteIterator::>::new(LittleEndian, align, notes).ok()?; + while let Ok(Some(n)) = iter.next() { + if n.name() == b"GNU" && n.n_type(LittleEndian) == OBJECT_NT_GNU_BUILD_ID { + return Some(n.desc().to_vec()); + } + } + None +} + +/// Ours, and `object`'s on the same bytes, which must agree. +fn agreed(notes: &[u8], align: u64) -> Option> { + let ours = note::build_id(notes, align).map(<[u8]>::to_vec); + assert_eq!(ours, objects(notes, align), "toyos-elf and object disagree on {notes:02x?}"); + ours +} + +const ID: [u8; 20] = [0xab; 20]; + +#[test] +fn the_constant_is_the_gabis() { + assert_eq!(NT_GNU_BUILD_ID, OBJECT_NT_GNU_BUILD_ID); +} + +#[test] +fn a_lone_build_id_is_read() { + assert_eq!(agreed(&gnu_id(&ID, 4), 4), Some(ID.to_vec())); +} + +#[test] +fn no_build_id_note_names_none() { + assert_eq!(agreed(¬e(b"GNU\0", OTHER, &[1, 2, 3, 4], 4), 4), None); + assert_eq!(agreed(&[], 4), None); +} + +#[test] +fn a_build_id_under_another_name_is_none() { + assert_eq!(agreed(¬e(b"GNV\0", NT_GNU_BUILD_ID, &ID, 4), 4), None); +} + +#[test] +fn a_build_id_of_the_wrong_type_is_none() { + assert_eq!(agreed(¬e(b"GNU\0", OTHER, &ID, 4), 4), None); +} + +#[test] +fn a_descriptor_of_0_or_past_32_bytes_is_no_build_id() { + assert_eq!(note::build_id(&gnu_id(&[], 4), 4), None); + assert_eq!(note::build_id(&gnu_id(&[7; MAX_BUILD_ID + 1], 4), 4), None); + assert_eq!(agreed(&gnu_id(&[7; MAX_BUILD_ID], 4), 4), Some(vec![7; MAX_BUILD_ID])); + assert_eq!(agreed(&gnu_id(&[7], 4), 4), Some(vec![7])); +} + +/// A first note whose name is not a multiple of four: its padding is what puts +/// the second note where it is. +#[test] +fn the_build_id_after_an_unpadded_name_is_found() { + let mut notes = note(b"abcde\0", OTHER, &[9; 5], 4); + notes.extend(gnu_id(&ID, 4)); + assert_eq!(agreed(¬es, 4), Some(ID.to_vec())); +} + +#[test] +fn an_eight_aligned_segment_pads_to_eight() { + let mut notes = note(b"abcdef\0", OTHER, &[9; 5], 8); + notes.extend(gnu_id(&ID, 8)); + assert_eq!(agreed(¬es, 8), Some(ID.to_vec())); + // The same bytes read at four find no build-id where the note is. + assert_ne!(note::build_id(¬es, 4), Some(&ID[..])); +} + +#[test] +fn an_alignment_no_note_section_has_names_none() { + assert_eq!(note::build_id(&gnu_id(&ID, 4), 16), None); +} + +#[test] +fn a_note_running_past_its_segment_names_none() { + let notes = gnu_id(&ID, 4); + for cut in 0..notes.len() { + assert_eq!(note::build_id(¬es[..cut], 4), None, "cut at {cut}"); + } + let mut lying = gnu_id(&ID, 4); + lying[4..8].copy_from_slice(&u32::MAX.to_le_bytes()); + assert_eq!(note::build_id(&lying, 4), None); + lying[0..4].copy_from_slice(&u32::MAX.to_le_bytes()); + assert_eq!(note::build_id(&lying, 4), None); +} + +#[test] +fn the_first_of_two_build_ids_is_the_answer() { + let mut notes = gnu_id(&ID, 4); + notes.extend(gnu_id(&[1; 20], 4)); + assert_eq!(agreed(¬es, 4), Some(ID.to_vec())); +} + +#[test] +fn every_pt_note_is_listed_wherever_its_bytes_are() { + let bytes = Elf::honest(0x3000) + .ph(Phdr { kind: PT_NOTE, flags: PF_R, offset: 0x2000, vaddr: 0x2000, filesz: 0x24, memsz: 0x24, align: 4 }) + .ph(Phdr { kind: PT_NOTE, flags: PF_R, offset: 0x300, vaddr: 0x300, filesz: 0x10, memsz: 0x10, align: 8 }) + .build(); + let listed: Vec = note::segments(&bytes).collect(); + assert_eq!( + listed, + [ + NoteSegment { offset: 0x2000, filesz: 0x24, align: 4 }, + NoteSegment { offset: 0x300, filesz: 0x10, align: 8 }, + ] + ); +} + +#[test] +fn a_header_with_no_readable_table_lists_no_note() { + assert_eq!(note::segments(&[]).count(), 0); + let bytes = Elf::honest(0x1000).phoff(0x2000).build(); + assert_eq!(note::segments(&bytes).count(), 0); +} diff --git a/toyos-elf/tests/real.rs b/toyos-elf/tests/real.rs index 3fe9636746b..51740d70238 100644 --- a/toyos-elf/tests/real.rs +++ b/toyos-elf/tests/real.rs @@ -51,3 +51,54 @@ fn a_toyos_ld_binary_parses_to_what_readelf_says() { assert_eq!(layout.vaddr_to_file_offset(0x166490), Some(0x166490)); assert_eq!(layout.vaddr_to_file_offset(0x155000), Some(0x155000)); } + +/// The first 4 KiB of `/system/bin/shell` as rust-lld linked it with +/// `--build-id`: `dd if=userland/target/x86_64-unknown-toyos/toyos/shell +/// of=toyos-elf/tests/fixtures/lld-headers.bin bs=4096 count=1`. +const LLD_HEADERS: &[u8] = include_bytes!("fixtures/lld-headers.bin"); + +/// The build-id `object` reads out of `file`'s program headers. +fn objects_build_id(file: &[u8]) -> Option> { + use object::read::elf::{FileHeader, ProgramHeader}; + let header = object::elf::FileHeader64::::parse(file).ok()?; + let endian = header.endian().ok()?; + for phdr in header.program_headers(endian, file).ok()? { + let Ok(Some(mut notes)) = phdr.notes(endian, file) else { continue }; + while let Ok(Some(note)) = notes.next() { + if note.name() == b"GNU" && note.n_type(endian) == object::elf::NT_GNU_BUILD_ID { + return Some(note.desc().to_vec()); + } + } + } + None +} + +/// Ours, read as the kernel reads it: each `PT_NOTE` at its own offset. +fn our_build_id(file: &[u8]) -> Option> { + toyos_elf::note::segments(file).find_map(|s| { + let notes = file.get(s.offset as usize..(s.offset + s.filesz) as usize)?; + toyos_elf::note::build_id(notes, s.align).map(<[u8]>::to_vec) + }) +} + +#[test] +fn an_lld_binarys_build_id_is_what_object_and_readelf_read() { + Layout::parse(LLD_HEADERS, Machine::X86_64).expect("rust-lld's own output"); + let segments: Vec<_> = toyos_elf::note::segments(LLD_HEADERS).collect(); + // `readelf -l`: `NOTE 0x0002a8 0x00000000000002a8 0x00000000000002a8 0x000024 0x000024 R 0x4`. + assert_eq!(segments, [toyos_elf::note::NoteSegment { offset: 0x2a8, filesz: 0x24, align: 4 }]); + let ours = our_build_id(LLD_HEADERS); + assert_eq!(ours, objects_build_id(LLD_HEADERS)); + // `readelf -n`: `Build ID: 97082d0f339a81a4dadc05e2f47960b079ab178c`. + let readelf = [ + 0x97, 0x08, 0x2d, 0x0f, 0x33, 0x9a, 0x81, 0xa4, 0xda, 0xdc, 0x05, 0xe2, 0xf4, 0x79, 0x60, 0xb0, 0x79, + 0xab, 0x17, 0x8c, + ]; + assert_eq!(ours.as_deref(), Some(&readelf[..])); +} + +#[test] +fn a_toyos_ld_binary_without_the_flag_carries_no_build_id() { + assert_eq!(our_build_id(HEADERS), None); + assert_eq!(objects_build_id(HEADERS), None); +} diff --git a/toyos-symbols/Cargo.toml b/toyos-symbols/Cargo.toml index 9fc7edb5d3c..8d37e84e57f 100644 --- a/toyos-symbols/Cargo.toml +++ b/toyos-symbols/Cargo.toml @@ -1,21 +1,26 @@ # A member of the host workspace (root `Cargo.toml`), like toyos-elf and -# toyos-gpt: the kernel depends on it by path and its tests run on the host, -# with no target-triple juggling. What lives here is every pure decision -# `kernel/src/symbols.rs` makes over bytes: where in an ELF file its symbol -# tables live, how a return address maps back onto the call it followed, and -# how much of the backtrace record's budget a demangled name gets. The kernel -# keeps the raw-pointer table itself — it has to be readable from a panic, a -# double fault and an NMI, which is a `PageAlloc` this crate cannot own and an -# `unsafe` this crate does not need. +# toyos-gpt: the kernel and `userland/symbolize` depend on it by path and its +# tests run on the host, with no target-triple juggling. What lives here is +# every pure decision a backtrace frame needs over bytes: where in an ELF file +# its symbol tables live, how a return address maps back onto the call it +# followed, how much of the record's budget a demangled name gets, the record +# a killed program's frame is written as, and the function a recorded offset +# names. The kernel keeps the raw-pointer table of its own symbols — it has to +# be readable from a panic, a double fault and an NMI, which is a static this +# crate cannot own and an `unsafe` this crate does not need. [package] name = "toyos-symbols" -description = "Backtrace symbol lookup: locating an ELF's symbol tables and budgeting the demangled name (no_std, no alloc, forbid(unsafe_code))." +description = "Backtrace frames: locating an ELF's symbol tables, budgeting the demangled name, and a killed program's file+offset record and its naming (no_std, no alloc, forbid(unsafe_code))." version = "0.1.0" edition = "2021" license = "MIT OR Apache-2.0" [dependencies] +rustc-demangle = "0.1" toyos-abi = { path = "../toyos-abi" } toyos-elf = { path = "../toyos-elf" } toyos-elide = { path = "../toyos-elide" } + +[dev-dependencies] +object = { version = "0.38", default-features = false, features = ["read_core", "elf", "std"] } diff --git a/toyos-symbols/src/frame.rs b/toyos-symbols/src/frame.rs new file mode 100644 index 00000000000..7b1a8ba3e97 --- /dev/null +++ b/toyos-symbols/src/frame.rs @@ -0,0 +1,226 @@ +//! A killed program's frame, as the kernel records it and userland reads it +//! back: ` + id=`. +//! +//! The kernel names no user frame: it records the file it opened, the offset +//! of the instruction in flight in that file's own addresses, and the build-id +//! the file carries, and [`crate::name`] names the function from them. One +//! encoder ([`UserFrame`]'s `Display`) and one decoder ([`decode`]), so the +//! grammar has no second spelling. +//! +//! - **The offset names a byte of the instruction in flight**: the faulting +//! one, or for a return address the call's last byte (`pc - 1`), so a call +//! in tail position names its caller and not the function after it. +//! - **The name is escaped**: every byte below `!`, DEL, `\`, `+` and `[` is +//! `\xHH`, so the name is one token whatever path the kernel opened, and a +//! raw `[` in it is only ever the elision marker. +//! - **Only the name is elided** ([`NAME_HEAD`], [`NAME_TAIL`]): offset and id +//! are always whole, and an elided name is refused by name, never guessed at. +//! - **`id=-`** is a file that carries no build-id. + +use core::fmt::{self, Display, Write}; + +use toyos_abi::log::MAX_RECORD_MESSAGE; +use toyos_elf::note::{self, NoteSegment, MAX_BUILD_ID}; +use toyos_elide::{widest, Elided, MARKER_MAX}; + +/// A user frame's own text, every number at its widest: indent, `0x` and +/// sixteen digits, two spaces, `+`, the offset, ` id=` and the id in hex. +pub const USER_FRAME_TEXT: usize = 4 + 18 + 2 + 1 + 18 + 4 + 2 * MAX_BUILD_ID; +const NAME_KEPT: usize = MAX_RECORD_MESSAGE - USER_FRAME_TEXT - MARKER_MAX; +pub const NAME_HEAD: usize = NAME_KEPT / 2; +pub const NAME_TAIL: usize = NAME_KEPT - NAME_HEAD; +const _: () = assert!(widest(NAME_HEAD, NAME_TAIL) + USER_FRAME_TEXT <= MAX_RECORD_MESSAGE); + +/// A build-id: 1 to [`MAX_BUILD_ID`] bytes, checked where it is made. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct BuildId { + bytes: [u8; MAX_BUILD_ID], + len: u8, +} + +impl BuildId { + pub fn new(id: &[u8]) -> Option { + let mut bytes = [0; MAX_BUILD_ID]; + bytes.get_mut(..id.len()).filter(|_| !id.is_empty())?.copy_from_slice(id); + Some(BuildId { bytes, len: id.len() as u8 }) + } + + pub fn as_bytes(&self) -> &[u8] { + self.bytes.get(..usize::from(self.len)).unwrap_or(&[]) + } + + /// The build-id of the image whose program header table `headers` holds: + /// the first `PT_NOTE` that `read` hands back bytes for and that carries + /// one. `read` answers a segment's bytes, as many of them as it will read. + pub fn find>(headers: &[u8], mut read: impl FnMut(NoteSegment) -> Option) -> Option { + note::segments(headers).find_map(|segment| { + let bytes = read(segment)?; + note::build_id(bytes.as_ref(), segment.align).and_then(BuildId::new) + }) + } +} + +impl Display for BuildId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.as_bytes().iter().try_for_each(|b| write!(f, "{b:02x}")) + } +} + +/// `pc`'s offset in the file of an image mapped at `[start, end)`, `bias` above +/// the addresses the file names; `None` when `pc` is not in it. +pub fn frame_offset(pc: u64, start: u64, end: u64, bias: u64) -> Option { + if (start..end).contains(&pc) { pc.checked_sub(bias) } else { None } +} + +/// One frame of a killed program; its `Display` is the record line. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct UserFrame<'a> { + pub pc: u64, + /// The file the kernel opened. + pub name: &'a str, + pub offset: u64, + pub build_id: Option, +} + +impl Display for UserFrame<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let name = Elided::<_, NAME_HEAD, NAME_TAIL>(Escaped(self.name)); + write!(f, " {:#x} {}+{:#x} id=", self.pc, name, self.offset)?; + match &self.build_id { + Some(id) => id.fmt(f), + None => f.write_char('-'), + } + } +} + +struct Escaped<'a>(&'a str); + +fn escaped(c: char) -> bool { + c < '!' || matches!(c, '\x7f' | '\\' | '+' | '[') +} + +impl Display for Escaped<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + for c in self.0.chars() { + if escaped(c) { + write!(f, "\\x{:02x}", u32::from(c))?; + } else { + f.write_char(c)?; + } + } + Ok(()) + } +} + +/// Why a line shaped like a frame names nothing. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Refused { + /// The record elided the middle of the name, so it names no file. + Elided, + /// A `\` in the name that is not `\x` and two hex digits of an ASCII byte. + BadEscape, + /// The id is neither `-` nor 1 to 32 bytes of lowercase hex. + BadId, +} + +impl Refused { + pub const fn as_str(self) -> &'static str { + match self { + Refused::Elided => "the record elided the file's name", + Refused::BadEscape => "the file's name holds an escape no encoder writes", + Refused::BadId => "the build-id is not 1 to 32 bytes of lowercase hex", + } + } +} + +/// A frame read back out of a line. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Decoded<'a> { + pub pc: u64, + escaped_name: &'a str, + pub offset: u64, + pub build_id: Option, +} + +impl<'a> Decoded<'a> { + /// The file's name, unescaped. + pub fn name(&self) -> impl Iterator + 'a { + let mut chars = self.escaped_name.chars(); + core::iter::from_fn(move || match chars.next()? { + '\\' => { + let hex = [chars.next()?, chars.next()?, chars.next()?]; + let byte = hex_byte(hex[1], hex[2])?; + Some(char::from(byte)) + } + c => Some(c), + }) + } +} + +/// The frame `line` records, wherever in the line it starts: `None` when the +/// line is not shaped like one, and the refusal when it is and names nothing. +pub fn decode(line: &str) -> Option, Refused>> { + let line = line.trim_end(); + let (head, id) = line.rsplit_once(" id=")?; + let (head, offset) = head.rsplit_once("+0x")?; + let offset = hex_u64(offset)?; + let (head, escaped_name) = head.rsplit_once(" ")?; + let pc = head.rsplit(' ').next().and_then(|pc| pc.strip_prefix("0x")).and_then(hex_u64)?; + Some(decoded(pc, escaped_name, offset, id)) +} + +fn decoded<'a>(pc: u64, escaped_name: &'a str, offset: u64, id: &str) -> Result, Refused> { + if escaped_name.contains('[') { + return Err(Refused::Elided); + } + let mut rest = escaped_name; + while let Some((_, after)) = rest.split_once('\\') { + let mut c = after.chars(); + match (c.next(), c.next(), c.next()) { + (Some('x'), Some(hi), Some(lo)) if hex_byte(hi, lo).is_some() => rest = c.as_str(), + _ => return Err(Refused::BadEscape), + } + } + let build_id = match id { + "-" => None, + id => Some(build_id(id).ok_or(Refused::BadId)?), + }; + Ok(Decoded { pc, escaped_name, offset, build_id }) +} + +fn build_id(hex: &str) -> Option { + let mut bytes = [0u8; MAX_BUILD_ID]; + let mut len = 0usize; + let mut digits = hex.chars(); + while let Some(hi) = digits.next() { + *bytes.get_mut(len)? = hex_byte_any(hi, digits.next()?)?; + len += 1; + } + BuildId::new(bytes.get(..len)?) +} + +/// A lowercase hex digit's value. +fn digit(c: char) -> Option { + match c { + '0'..='9' | 'a'..='f' => c.to_digit(16).map(|d| d as u8), + _ => None, + } +} + +fn hex_byte_any(hi: char, lo: char) -> Option { + Some(digit(hi)? << 4 | digit(lo)?) +} + +/// An escaped byte: ASCII, since a name is UTF-8 and the encoder escapes only +/// ASCII characters. +fn hex_byte(hi: char, lo: char) -> Option { + hex_byte_any(hi, lo).filter(u8::is_ascii) +} + +/// 1 to 16 lowercase hex digits. +fn hex_u64(s: &str) -> Option { + if s.is_empty() || s.len() > 16 { + return None; + } + s.chars().try_fold(0u64, |n, c| Some(n << 4 | u64::from(digit(c)?))) +} diff --git a/toyos-symbols/src/lib.rs b/toyos-symbols/src/lib.rs index 260f60dab75..6c3d0472ef2 100644 --- a/toyos-symbols/src/lib.rs +++ b/toyos-symbols/src/lib.rs @@ -1,18 +1,19 @@ -//! The pure half of a kernel backtrace frame: where an ELF file's symbol -//! tables live, and how much of the record's budget a demangled name gets. +//! The pure half of a backtrace frame, shared by the kernel and by userland's +//! namer: where an ELF file's symbol tables live, how much of a record's budget +//! a demangled name gets, a killed program's frame record ([`frame`]), and the +//! function a recorded offset falls in ([`name`]). //! -//! `kernel/src/symbols.rs` keeps the rest — a `SymbolTable` holding raw -//! pointers into either the kernel image or pages it owns, because the +//! `kernel/src/symbols.rs` keeps the rest of the kernel's own frames — a +//! `SymbolTable` holding raw pointers into the kernel image, because the //! resolve path is reached from the fault handler and the panic handler and //! may not allocate, take a lock or do I/O. Nothing here needs any of that: -//! [`locate`] is a function of the bytes a caller already has, and the budget -//! constants are a function of two other crates' constants. Both halves are -//! tested here, on the host, against a real binary's own symbol table — -//! `tests/real.rs` names how it was produced and how to reproduce it. +//! every function is of the bytes a caller already has. //! -//! `no_std`, no allocation, no `unsafe`. [`locate`] borrows straight out of -//! its caller's bytes rather than copying them, which is what lets the -//! kernel's table point directly at the ELF instead of a copy of it. +//! `no_std`, no allocation, no `unsafe`, and no panic on any input: the kernel +//! runs it on its crash path and the namer runs it on whatever file a record +//! names. [`locate`] borrows straight out of its caller's bytes rather than +//! copying them, which is what lets the kernel's table point directly at the +//! ELF instead of a copy of it. #![no_std] #![forbid(unsafe_code)] @@ -20,10 +21,16 @@ #[cfg(test)] extern crate std; +pub mod frame; + use toyos_abi::log::MAX_RECORD_MESSAGE; use toyos_elf::section::{SectionTable, SHT_SYMTAB}; +use toyos_elf::sym::SymTab; +use toyos_elf::FileHeader; use toyos_elide::{widest, Elided, MARKER_MAX}; +use frame::BuildId; + /// A backtrace frame's own text: ` ` + `{addr:#x}` + ` ` + `+` + /// `{offset:#x}`, with both numbers at their widest — `0x` and sixteen hex /// digits is every `u64` there is. @@ -68,6 +75,11 @@ pub fn symbol_text(name: D) -> Elided { Elided(name) } +/// A raw symbol as a frame prints it: demangled, then [`symbol_text`]. +pub fn demangled(raw: &str) -> Elided, SYMBOL_HEAD, SYMBOL_TAIL> { + symbol_text(rustc_demangle::demangle(raw)) +} + /// `[offset, offset + len)` of `data`, or `None` when that is not wholly inside /// it. Both numbers came out of the file, so the addition is checked. pub fn file_range(data: &[u8], offset: u64, len: u64) -> Option<&[u8]> { @@ -101,6 +113,33 @@ pub fn locate(data: &[u8]) -> Option<(&[u8], &[u8])> { Some((symtab, strtab)) } +/// Why a recorded frame names no function. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Unnamed { + /// The file is no ELF image this crate reads. + NotElf(toyos_elf::Error), + /// The file's build-id is not the one the record carries: it is another + /// build of that name, or one of the two has none. + OtherBuild { file: Option }, + /// The file has no symbol table. + NoSymbols, + /// No function in the file's symbol table holds the offset. + NoSymbol, +} + +/// The function `offset` falls in, and how far into it, out of `file`'s own +/// symbol table — when `file` is the build `id` names (`None`: a file with no +/// build-id). The same lookup the kernel names its own frames with. +pub fn name<'a>(file: &'a [u8], offset: u64, id: Option<&BuildId>) -> Result<(&'a str, u64), Unnamed> { + FileHeader::parse(file).map_err(Unnamed::NotElf)?; + let carried = BuildId::find(file, |s| file_range(file, s.offset, s.filesz)); + if carried.as_ref() != id { + return Err(Unnamed::OtherBuild { file: carried }); + } + let (symtab, strtab) = locate(file).ok_or(Unnamed::NoSymbols)?; + SymTab::new(symtab, strtab).resolve(offset).ok_or(Unnamed::NoSymbol) +} + #[cfg(test)] mod tests { use super::*; diff --git a/toyos-symbols/tests/frame.rs b/toyos-symbols/tests/frame.rs new file mode 100644 index 00000000000..e8efaef8174 --- /dev/null +++ b/toyos-symbols/tests/frame.rs @@ -0,0 +1,189 @@ +//! The frame record: what the kernel writes, read back, and every way a line +//! can fail to be one. + +use toyos_abi::log::MAX_RECORD_MESSAGE; +use toyos_symbols::frame::{decode, frame_offset, BuildId, Decoded, Refused, UserFrame}; + +fn id(len: usize) -> BuildId { + BuildId::new(&(0..len as u8).map(|b| b.wrapping_mul(37)).collect::>()).unwrap() +} + +fn decoded(line: &str) -> Decoded<'_> { + decode(line).unwrap_or_else(|| panic!("not a frame: {line:?}")).unwrap_or_else(|r| panic!("{r:?}: {line:?}")) +} + +fn round_trip(frame: UserFrame<'_>) { + let line = frame.to_string(); + assert!(line.len() <= MAX_RECORD_MESSAGE, "{} bytes", line.len()); + for line in [line.clone(), format!("[kernel 12.345 cpu3 tid=7] {line}"), format!("{line}\r\n")] { + let back = decoded(&line); + assert_eq!((back.pc, back.offset, back.build_id), (frame.pc, frame.offset, frame.build_id), "{line:?}"); + assert_eq!(back.name().collect::(), frame.name, "{line:?}"); + } +} + +#[test] +fn a_frame_reads_back_as_written() { + for name in ["/home/disk_backtrace/child", "/system/bin/x", "", "/a b/c", "/ünï/çødé"] { + for build_id in [None, Some(id(1)), Some(id(20)), Some(id(32))] { + for (pc, offset) in [(0, 0), (0x100_0000_1234, 0x1234), (u64::MAX, u64::MAX)] { + round_trip(UserFrame { pc, name, offset, build_id }); + } + } + } +} + +/// Every byte the grammar spends: none of them is ever raw in a name. +#[test] +fn a_name_holding_the_grammars_bytes_reads_back_whole() { + let name = "/x\n y+0x1 id=ab\\[z]\t\u{7f}\u{1} +"; + let line = UserFrame { pc: 1, name, offset: 2, build_id: Some(id(4)) }.to_string(); + assert!(!line.contains('\n') && !line.contains('\t') && !line.contains('\u{7f}'), "{line:?}"); + assert_eq!(line.matches(" id=").count(), 1, "{line:?}"); + assert_eq!(line.matches('+').count(), 1, "{line:?}"); + round_trip(UserFrame { pc: 1, name, offset: 2, build_id: Some(id(4)) }); +} + +#[test] +fn every_byte_below_space_reads_back() { + let name: String = (0u8..0x80).map(char::from).collect(); + round_trip(UserFrame { pc: 1, name: &name, offset: 2, build_id: None }); +} + +#[test] +fn the_kernels_exact_line() { + let frame = UserFrame { pc: 0x100_0000_4321, name: "/bin/x", offset: 0x4321, build_id: Some(id(2)) }; + assert_eq!(frame.to_string(), " 0x10000004321 /bin/x+0x4321 id=0025"); + let none = UserFrame { build_id: None, ..frame }; + assert_eq!(none.to_string(), " 0x10000004321 /bin/x+0x4321 id=-"); +} + +/// A name past what a record holds loses its middle, never the offset or the +/// id, and the reader refuses it rather than open a file it does not name. +#[test] +fn an_elided_name_keeps_offset_and_id_and_is_refused() { + let long = format!("/{}", "d/".repeat(1500)); + let frame = UserFrame { pc: 0x10, name: &long, offset: 0xfedc, build_id: Some(id(32)) }; + let line = frame.to_string(); + assert!(line.len() <= MAX_RECORD_MESSAGE, "{} bytes", line.len()); + assert!(line.ends_with(&format!("+0xfedc id={}", id(32))), "{line}"); + assert_eq!(decode(&line), Some(Err(Refused::Elided))); + // Escapes count against the budget at their escaped width. + let controls = "\n".repeat(400); + let line = UserFrame { name: &controls, ..frame }.to_string(); + assert!(line.len() <= MAX_RECORD_MESSAGE, "{} bytes", line.len()); + assert_eq!(decode(&line), Some(Err(Refused::Elided))); +} + +#[test] +fn an_escape_no_encoder_writes_is_refused() { + for name in ["\\q", "\\x8f", "\\x4", "\\x", "\\", "a\\xZZ", "\\X41"] { + let line = format!(" 0x1 {name}+0x2 id=-"); + assert_eq!(decode(&line), Some(Err(Refused::BadEscape)), "{line:?}"); + } +} + +#[test] +fn an_id_that_is_not_lowercase_hex_bytes_is_refused() { + let too_long = "ab".repeat(33); + for bad in ["", "abc", "ABCD", "zz", "0x12", too_long.as_str(), "--"] { + let line = format!(" 0x1 /f+0x2 id={bad}"); + assert_eq!(decode(&line), Some(Err(Refused::BadId)), "{line:?}"); + } +} + +/// The kernel's own frames, and every other line, are not user frames. +#[test] +fn a_line_that_is_no_frame_is_none() { + for line in [ + "", + " 0xffffffff80001234 kernel::main+0x10", + " 0x1000", + "inbox: id=5", + " 0x1 /f+0x2", + " 0x1 /f+0x2 id=-", + " 1 /f+0x2 id=-", + " 0x1 /f+0xG id=-", + " 0x1 /f+2 id=-", + " 0x /f+0x2 id=-", + " 0x10000000000000000 /f+0x2 id=-", + " 0xABC /f+0x2 id=-", + ] { + assert_eq!(decode(line), None, "{line:?}"); + } +} + +/// A record cut anywhere is never read as the frame it was cut from. +#[test] +fn a_frame_cut_at_any_byte_is_not_that_frame() { + let frame = UserFrame { pc: 0x100_0000_1234, name: "/home/x\ny", offset: 0x1234, build_id: Some(id(20)) }; + let line = frame.to_string(); + let whole = decoded(&line); + for cut in 0..line.len() { + let Some(prefix) = line.get(..cut) else { continue }; + assert_ne!(decode(prefix), Some(Ok(whole)), "cut at {cut}: {prefix:?}"); + } +} + +/// Frames with random edits in the grammar's own alphabet: none panics the +/// reader, and the edits reach both a frame and a refusal. +#[test] +fn no_line_panics_the_reader() { + const ALPHABET: &[char] = &[' ', '0', 'x', '7', 'a', 'f', 'A', 'X', '+', '\\', '[', '-', 'i', 'd', '=', '/', '\n', 'é']; + let mut state = 0x2545_F491_4F6C_DD1Du64; + let mut next = move |n: usize| { + state ^= state << 13; + state ^= state >> 7; + state ^= state << 17; + (state % n as u64) as usize + }; + let (mut frames, mut refused) = (0, 0); + for _ in 0..200_000 { + let name: String = (0..next(12)).map(|_| ALPHABET[next(ALPHABET.len())]).collect(); + let id = (next(3) > 0).then(|| id(1 + next(32))); + let mut line: Vec = UserFrame { pc: 0x1234, name: &name, offset: 0x56, build_id: id }.to_string().chars().collect(); + for _ in 0..next(4) { + let at = next(line.len() + 1); + match next(3) { + 0 if at < line.len() => line[at] = ALPHABET[next(ALPHABET.len())], + 1 if at < line.len() => drop(line.remove(at)), + _ => line.insert(at, ALPHABET[next(ALPHABET.len())]), + } + } + let line: String = line.into_iter().collect(); + match decode(&line) { + Some(Ok(frame)) => { + frame.name().for_each(drop); + frames += 1; + } + Some(Err(_)) => refused += 1, + None => {} + } + } + assert!(frames > 1000 && refused > 1000, "{frames} frames, {refused} refusals: the edits reach too little"); +} + +#[test] +fn a_build_id_is_one_to_32_bytes() { + assert_eq!(BuildId::new(&[]), None); + assert_eq!(BuildId::new(&[0; 33]), None); + assert_eq!(BuildId::new(&[5]).map(|i| i.as_bytes().to_vec()), Some(vec![5])); + assert_eq!(BuildId::new(&[5; 32]).map(|i| i.as_bytes().len()), Some(32)); +} + +/// An image mapped at `[0x1000_0000, 0x1000_8000)` from a file whose lowest +/// address is `0x400000`: a pc maps back to the file's own addresses. +#[test] +fn an_offset_is_in_the_files_addresses() { + let (start, end, vaddr_min) = (0x1000_0000u64, 0x1000_8000u64, 0x40_0000u64); + let bias = start - vaddr_min; + assert_eq!(frame_offset(start, start, end, bias), Some(vaddr_min)); + assert_eq!(frame_offset(start + 0x123, start, end, bias), Some(vaddr_min + 0x123)); + assert_eq!(frame_offset(end - 1, start, end, bias), Some(vaddr_min + 0x7fff)); + assert_eq!(frame_offset(end, start, end, bias), None); + assert_eq!(frame_offset(start - 1, start, end, bias), None); + assert_eq!(frame_offset(0, start, end, bias), None); + assert_eq!(frame_offset(u64::MAX, start, end, bias), None); + // A bias the image does not hold answers nothing rather than wrapping. + assert_eq!(frame_offset(5, 0, 10, 6), None); +} diff --git a/toyos-symbols/tests/name.rs b/toyos-symbols/tests/name.rs new file mode 100644 index 00000000000..5fb212e62f3 --- /dev/null +++ b/toyos-symbols/tests/name.rs @@ -0,0 +1,118 @@ +//! Naming a recorded offset out of the file the record names: against a real +//! binary, against `object`'s symbol table reader (not ToyOS code), and +//! against every build-id disagreement. + +use object::{Object, ObjectSymbol, SymbolKind}; +use toyos_symbols::frame::BuildId; +use toyos_symbols::{name, Unnamed}; + +const BINARY: &[u8] = include_bytes!("fixtures/input-test.bin"); + +/// `input-test` with its `PT_GNU_EH_FRAME` turned into a `PT_NOTE` over a +/// build-id note appended past its end, far past its header page. +fn with_build_id(id: &[u8]) -> Vec { + let mut file = BINARY.to_vec(); + let at = file.len().next_multiple_of(4); + file.resize(at, 0); + file.extend(4u32.to_le_bytes()); + file.extend((id.len() as u32).to_le_bytes()); + file.extend(3u32.to_le_bytes()); + file.extend(b"GNU\0"); + file.extend(id); + // `readelf -l`: the fourth program header is GNU_EH_FRAME. + let ph = 64 + 3 * 56; + assert_eq!(file[ph..ph + 4], 0x6474_e550u32.to_le_bytes()); + file[ph..ph + 4].copy_from_slice(&4u32.to_le_bytes()); + file[ph + 8..ph + 16].copy_from_slice(&(at as u64).to_le_bytes()); + file[ph + 32..ph + 40].copy_from_slice(&((16 + id.len()) as u64).to_le_bytes()); + file[ph + 48..ph + 56].copy_from_slice(&4u64.to_le_bytes()); + file +} + +#[test] +fn a_real_binarys_offset_names_its_function() { + // `readelf --syms`: `3902: 00000000000013a0 55 FUNC GLOBAL DEFAULT 1 main` + assert_eq!(name(BINARY, 0x13a0 + 10, None), Ok(("main", 10))); +} + +/// Every function `object` reads out of the same file names one of the +/// functions `object` puts at its address, at its middle byte. +#[test] +fn every_function_names_what_object_reads() { + let file = object::File::parse(BINARY).expect("object reads input-test"); + let functions: Vec<_> = file + .symbols() + .filter(|s| s.kind() == SymbolKind::Text && s.size() > 0 && s.address() > 0) + .collect(); + for sym in &functions { + let at = sym.address() + sym.size() / 2; + let ours = name(BINARY, at, None); + let at_address: Vec<&str> = + functions.iter().filter(|o| o.address() == sym.address()).filter_map(|o| o.name().ok()).collect(); + assert!( + matches!(ours, Ok((n, within)) if at_address.contains(&n) && within == at - sym.address()), + "{at:#x}: ours {ours:?}, object's {at_address:?}" + ); + } + assert!(functions.len() > 1000, "only {} functions", functions.len()); +} + +#[test] +fn the_build_the_record_names_is_the_one_named() { + let id = [0x5a; 20]; + let file = with_build_id(&id); + let id = BuildId::new(&id).unwrap(); + assert_eq!(BuildId::find(&file, |s| file.get(s.offset as usize..(s.offset + s.filesz) as usize)), Some(id)); + assert_eq!(name(&file, 0x13a0, Some(&id)), Ok(("main", 0))); +} + +#[test] +fn another_build_is_refused_by_name() { + let file = with_build_id(&[0x5a; 20]); + let carried = BuildId::new(&[0x5a; 20]); + let other = BuildId::new(&[0x5b; 20]).unwrap(); + assert_eq!(name(&file, 0x13a0, Some(&other)), Err(Unnamed::OtherBuild { file: carried })); + assert_eq!(name(&file, 0x13a0, None), Err(Unnamed::OtherBuild { file: carried })); + assert_eq!(name(BINARY, 0x13a0, Some(&other)), Err(Unnamed::OtherBuild { file: None })); +} + +#[test] +fn a_file_that_cannot_name_says_why() { + assert!(matches!(name(&BINARY[..32], 0, None), Err(Unnamed::NotElf(_)))); + assert!(matches!(name(&[], 0, None), Err(Unnamed::NotElf(_)))); + // Past its section header table. + assert_eq!(name(&BINARY[..0x1000], 0x13a0, None), Err(Unnamed::NoSymbols)); + assert_eq!(name(BINARY, 0, None), Err(Unnamed::NoSymbol)); +} + +/// Seeded corruptions of the real file's headers and tables: none panics the +/// namer. +#[test] +fn no_corrupted_file_panics_the_namer() { + let mut state = 0x9E37_79B9_7F4A_7C15u64; + let mut next = move || { + state ^= state << 13; + state ^= state >> 7; + state ^= state << 17; + state + }; + let (_, strtab) = toyos_symbols::locate(BINARY).unwrap(); + let shoff = u64::from_le_bytes(BINARY[40..48].try_into().unwrap()) as usize; + let strtab_at = BINARY.windows(strtab.len().min(64)).position(|w| w == &strtab[..w.len()]).unwrap(); + // The file header, the program headers, the section headers and the first + // symbols: every structure the namer reads before a name. + let regions = [0..0x190, shoff..BINARY.len(), strtab_at - 0x400..strtab_at + 0x400]; + let mut file = with_build_id(&[1; 20]); + let id = BuildId::new(&[1; 20]).unwrap(); + let pristine = file.clone(); + for _ in 0..3000 { + file.copy_from_slice(&pristine); + for _ in 0..1 + next() % 8 { + let region = ®ions[(next() % regions.len() as u64) as usize]; + let at = region.start + (next() as usize) % (region.end - region.start); + file[at] = next() as u8; + } + let cut = if next() % 4 == 0 { (next() as usize) % file.len() } else { file.len() }; + let _ = name(&file[..cut], next() % 0x2_0000, Some(&id)); + } +} diff --git a/userland/Cargo.lock b/userland/Cargo.lock index 856df3b4c2f..21bfa4639d1 100644 --- a/userland/Cargo.lock +++ b/userland/Cargo.lock @@ -1161,7 +1161,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -3310,6 +3310,12 @@ dependencies = [ "wasm-bindgen-futures", ] +[[package]] +name = "rustc-demangle" +version = "0.1.27" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b50b8869d9fc858ce7266cce0194bd74df58b9d0e3f6df3a9fc8eb470d95c09d" + [[package]] name = "rustc_version" version = "0.4.1" @@ -3377,7 +3383,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -3969,6 +3975,13 @@ dependencies = [ "toyos-swap", ] +[[package]] +name = "symbolize" +version = "0.1.0" +dependencies = [ + "toyos-symbols", +] + [[package]] name = "syn" version = "2.0.117" @@ -4186,6 +4199,10 @@ dependencies = [ name = "toyos-dns" version = "0.1.0" +[[package]] +name = "toyos-elf" +version = "0.1.0" + [[package]] name = "toyos-elide" version = "0.1.0" @@ -4261,6 +4278,16 @@ dependencies = [ "toyos-manifest", ] +[[package]] +name = "toyos-symbols" +version = "0.1.0" +dependencies = [ + "rustc-demangle", + "toyos-abi", + "toyos-elf", + "toyos-elide", +] + [[package]] name = "toyos-tco" version = "0.1.0" @@ -4728,7 +4755,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/userland/Cargo.toml b/userland/Cargo.toml index 260fa329b7f..4897c7ecb57 100644 --- a/userland/Cargo.toml +++ b/userland/Cargo.toml @@ -29,6 +29,7 @@ members = [ "sprite", "sshserver", "supervisor", + "symbolize", "swap", "terminal", "test-runner", diff --git a/userland/symbolize/Cargo.toml b/userland/symbolize/Cargo.toml new file mode 100644 index 00000000000..000742d290e --- /dev/null +++ b/userland/symbolize/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "symbolize" +version = "0.1.0" +edition = "2021" +license = "MIT OR Apache-2.0" +description = "Names the frames of a killed program's crash record: the file and offset the kernel recorded, looked up in that file's own symbol table." + +[dependencies] +toyos-symbols = { path = "../../toyos-symbols" } diff --git a/userland/symbolize/src/main.rs b/userland/symbolize/src/main.rs new file mode 100644 index 00000000000..6fab596ebf7 --- /dev/null +++ b/userland/symbolize/src/main.rs @@ -0,0 +1,112 @@ +//! `symbolize [--root DIR] [FILE]`: names the frames of a killed program. +//! +//! The kernel records a user frame as the file it ran, the offset in that file +//! and the file's build-id (`toyos_symbols::frame`). This reads lines — FILE, +//! or stdin — writes each back unchanged, and after each frame line writes the +//! function that offset falls in, out of the named file's own symbol table, or +//! why it names none: a file that is another build, one that cannot be read, +//! one with no symbol table. A log is named on any machine by giving `--root` +//! the directory that holds the image's files. +//! +//! **Nothing a line or a file holds makes it fail**: the lookup is +//! `toyos_symbols::name`, the kernel's own and panic-free on any bytes, and +//! every other outcome is a sentence on the line it is about. + +use std::collections::HashMap; +use std::fs; +use std::io::{self, BufRead, BufReader, Read, Write}; +use std::path::PathBuf; +use std::process::ExitCode; + +use toyos_symbols::frame::{decode, Decoded}; +use toyos_symbols::{demangled, name, Unnamed}; + +const USAGE: &str = "usage: symbolize [--root DIR] [FILE]"; + +fn main() -> ExitCode { + let mut root = None; + let mut input = None; + let mut args = std::env::args_os().skip(1); + while let Some(arg) = args.next() { + match (arg.to_str(), &input) { + (Some("--root"), _) => match args.next() { + Some(dir) => root = Some(PathBuf::from(dir)), + None => return usage(), + }, + (Some(flag), _) if flag.starts_with('-') => return usage(), + (_, None) => input = Some(PathBuf::from(arg)), + (_, Some(_)) => return usage(), + } + } + let reader: Box = match &input { + Some(path) => match fs::File::open(path) { + Ok(file) => Box::new(file), + Err(e) => { + let _ = writeln!(io::stderr(), "symbolize: {}: {e}", path.display()); + return ExitCode::FAILURE; + } + }, + None => Box::new(io::stdin()), + }; + let mut namer = Namer { root, files: HashMap::new() }; + match namer.run(BufReader::new(reader), io::stdout().lock()) { + Ok(()) => ExitCode::SUCCESS, + Err(e) => { + let _ = writeln!(io::stderr(), "symbolize: {e}"); + ExitCode::FAILURE + } + } +} + +fn usage() -> ExitCode { + let _ = writeln!(io::stderr(), "{USAGE}"); + ExitCode::from(2) +} + +struct Namer { + root: Option, + /// Each file a record names, read once: a backtrace names one file many times. + files: HashMap, String>>, +} + +impl Namer { + fn run(&mut self, mut input: impl BufRead, mut out: impl Write) -> io::Result<()> { + let mut line = Vec::new(); + loop { + line.clear(); + if input.read_until(b'\n', &mut line)? == 0 { + return out.flush(); + } + let text = String::from_utf8_lossy(&line); + let text = text.trim_end_matches(['\n', '\r']); + match decode(text) { + None => writeln!(out, "{text}")?, + Some(Err(refused)) => writeln!(out, "{text} = ? {}", refused.as_str())?, + Some(Ok(frame)) => writeln!(out, "{text} = {}", self.name(&frame))?, + } + } + } + + fn name(&mut self, frame: &Decoded<'_>) -> String { + let file: String = frame.name().collect(); + let path = match &self.root { + Some(root) => root.join(file.trim_start_matches('/')), + None => PathBuf::from(&file), + }; + let bytes = self.files.entry(file.clone()).or_insert_with(|| { + fs::read(&path).map_err(|e| format!("? {}: {e}", path.display())) + }); + let bytes = match bytes { + Ok(bytes) => bytes, + Err(why) => return why.clone(), + }; + match name(bytes, frame.offset, frame.build_id.as_ref()) { + Ok((symbol, within)) => format!("{}+{within:#x}", demangled(symbol)), + Err(Unnamed::NotElf(e)) => format!("? {file}: {e}"), + Err(Unnamed::OtherBuild { file: Some(id) }) => format!("? {file} is another build: its id is {id}"), + Err(Unnamed::OtherBuild { file: None }) => format!("? {file} carries no build-id"), + Err(Unnamed::NoSymbols) => format!("? {file} has no symbol table"), + Err(Unnamed::NoSymbol) => format!("? no function in {file} holds {:#x}", frame.offset), + } + } +} From 44d48bcee6a1952b72414668c3c80f1c1942957f Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 05:47:14 +0200 Subject: [PATCH 2/7] The rust-lld header fixture is declared with the other committed binaries Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- src/licence.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/licence.rs b/src/licence.rs index cc0b165562c..7bfc59617a6 100644 --- a/src/licence.rs +++ b/src/licence.rs @@ -458,6 +458,12 @@ pub const COMMITTED_FILES: &[(&str, &str, &str, Terms)] = &[ "NOTICE", Terms::Font("OFL-1.1"), ), + ( + "toyos-elf/tests/fixtures/lld-headers.bin", + "ba65b2a85776a973ad4fd49a224598391fde212e06b10aa51e2c8bcacfae0c79", + "ours: the first 4096 bytes of a rust-lld output (toyos-elf/tests/real.rs)", + Terms::Spdx("MIT OR Apache-2.0"), + ), ( "toyos-elf/tests/fixtures/toyos-ld-headers.bin", "6243d543a15941133514c1a8a24c79d118060caeae7e985870a67d9fc3021354", From ae16017ce77889ae5b1a00b03f544765cc8e0392 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 06:06:29 +0200 Subject: [PATCH 3/7] File the screen_panic_muted race a loaded suite run showed Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...re-the-report-has-reached-its-armed-line.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 issues/panic-path/screen-panic-muted-reads-the-panel-before-the-report-has-reached-its-armed-line.md diff --git a/issues/panic-path/screen-panic-muted-reads-the-panel-before-the-report-has-reached-its-armed-line.md b/issues/panic-path/screen-panic-muted-reads-the-panel-before-the-report-has-reached-its-armed-line.md new file mode 100644 index 00000000000..3a3d7de135a --- /dev/null +++ b/issues/panic-path/screen-panic-muted-reads-the-panel-before-the-report-has-reached-its-armed-line.md @@ -0,0 +1,18 @@ +--- +status: open +kind: defect +opened: 2026-10-04 +--- + +# `screen_panic_muted` reads the panel before the report has reached its armed line + +`screen_panic_muted` (`tests/toyos.rs`) takes one screendump as soon as the +panel shows `PANIC:` and then asserts the line `halt_all_cpus` paints last, +`panic: rebooting in 60 s, timed by …`. Nothing orders the dump after that +line: a guest slow between the two is read mid-report. A whole-suite run with +the host's load average at 48 (14 cores) red it with the dump ending inside +the kernel backtrace, before the armed line; the same test alone was green in +10 s. + +**Exit**: the test waits for the armed line itself, bounded, and a guest +slowed between `PANIC:` and that line still passes. From 1863325cef3c677c192a76c59870d2f4be9214fc Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 06:34:11 +0200 Subject: [PATCH 4/7] Answer review round 1: an unchecked name says so, the namer stays under its root - `toyos_symbols::name` answers `Named { function, within, check }`. When neither the record nor the file carries a build-id, `check` is `Unchecked` and the name prints ` (unchecked: no build-id)` after it. Before, it printed exactly as a name the build-id vouched for, although nothing told this build from another of that path. That is every C program clang links today. - `symbolize --root` refuses a record whose name has a `..` component or a Windows prefix, on that record's line. Before, `/../../x` joined out of the root, so a crafted log line opened any host file. - `symbol_text` folds into `demangled`, its one production caller. - `toyos-symbols/Cargo.toml` loses the clause about return addresses: that decision is the kernel's. - `userland/Cargo.lock`: the three windows-sys 0.52.0 -> 0.61.2 edges (errno, rustix, winapi-util) are reverted. `cargo metadata --locked` accepts the lock without them. - The T14 wedge issue strikes `read_backtrace_table` as a candidate: this branch deletes the function. The build-id note read that replaces it in the same window is named in its place. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...-after-a-jobs-exit-and-nothing-said-why.md | 13 ++-- toyos-symbols/Cargo.toml | 7 +-- toyos-symbols/src/lib.rs | 60 ++++++++++++++----- toyos-symbols/tests/name.rs | 19 ++++-- userland/Cargo.lock | 6 +- userland/symbolize/src/main.rs | 56 +++++++++++++++-- 6 files changed, 123 insertions(+), 38 deletions(-) diff --git a/issues/hardware/a-t14-boot-wedges-after-a-jobs-exit-and-nothing-said-why.md b/issues/hardware/a-t14-boot-wedges-after-a-jobs-exit-and-nothing-said-why.md index 420da8563d0..5b1acf6724d 100644 --- a/issues/hardware/a-t14-boot-wedges-after-a-jobs-exit-and-nothing-said-why.md +++ b/issues/hardware/a-t14-boot-wedges-after-a-jobs-exit-and-nothing-said-why.md @@ -40,11 +40,14 @@ never on the stick. The window is **[the reboot binary's spawn → the reset register write]**, and the candidates in it are all consistent with what the file shows: -- `symbols::read_backtrace_table` reads 2 MiB of symbol tables off the boot - stick — 512 SCSI READ(10) commands, each holding the block `Handle` and - `XHCI` with preemption off. It is the only device call between the two - records and the only thing in the window that takes the 46–100 ms the healthy - gap is. +- The spawn's device reads after `spawn: TLS`. These boots ran + `symbols::read_backtrace_table`, 2 MiB of symbol tables off the boot stick in + 512 SCSI READ(10) commands, each holding the block `Handle` and `XHCI` with + preemption off; #711 deletes it, because the kernel no longer reads a + program's symbol tables. The device read left between the two records is the + build-id note: at most a page per `PT_NOTE`, one note in an lld-linked + program, through the same block path. A wedge in that path stays a candidate; + the 2 MiB read that matched the healthy 46–100 ms gap does not. - `quiesce`'s `flush_disks` and `hand_back`, which landed at `88841f56` a few minutes before the first hang; `hand_back` takes `XHCI` while `logd`'s own write to the same stick may still be in flight. diff --git a/toyos-symbols/Cargo.toml b/toyos-symbols/Cargo.toml index 8d37e84e57f..fd4e89f45ca 100644 --- a/toyos-symbols/Cargo.toml +++ b/toyos-symbols/Cargo.toml @@ -2,10 +2,9 @@ # toyos-gpt: the kernel and `userland/symbolize` depend on it by path and its # tests run on the host, with no target-triple juggling. What lives here is # every pure decision a backtrace frame needs over bytes: where in an ELF file -# its symbol tables live, how a return address maps back onto the call it -# followed, how much of the record's budget a demangled name gets, the record -# a killed program's frame is written as, and the function a recorded offset -# names. The kernel keeps the raw-pointer table of its own symbols — it has to +# its symbol tables live, how much of the record's budget a demangled name +# gets, the record a killed program's frame is written as, and the function a +# recorded offset names. The kernel keeps the raw-pointer table of its own symbols — it has to # be readable from a panic, a double fault and an NMI, which is a static this # crate cannot own and an `unsafe` this crate does not need. diff --git a/toyos-symbols/src/lib.rs b/toyos-symbols/src/lib.rs index 6c3d0472ef2..a8b047f2c61 100644 --- a/toyos-symbols/src/lib.rs +++ b/toyos-symbols/src/lib.rs @@ -1,7 +1,8 @@ //! The pure half of a backtrace frame, shared by the kernel and by userland's //! namer: where an ELF file's symbol tables live, how much of a record's budget //! a demangled name gets, a killed program's frame record ([`frame`]), and the -//! function a recorded offset falls in ([`name`]). +//! function a recorded offset falls in ([`name`]), marked when no build-id +//! vouches for it. //! //! `kernel/src/symbols.rs` keeps the rest of the kernel's own frames — a //! `SymbolTable` holding raw pointers into the kernel image, because the @@ -66,18 +67,12 @@ const _: () = assert!(widest(SYMBOL_HEAD, SYMBOL_TAIL) + FRAME_TEXT <= MAX_RECOR /// drift from them again — which it did the first time `FRAME_OVERHEAD` moved. const _: () = assert!(SYMBOL_BUDGET == 944 && SYMBOL_HEAD == 451 && SYMBOL_TAIL == 452); -/// A demangled symbol, rendered head-and-tail when it is wider than a record -/// can carry. `toyos-elide` is the mechanism and the argument. -/// -/// `toyos-elide`'s own tests are where the seams are checked, on the host, -/// against characters that straddle both of them. -pub fn symbol_text(name: D) -> Elided { - Elided(name) -} - -/// A raw symbol as a frame prints it: demangled, then [`symbol_text`]. +/// A raw symbol as a frame prints it: demangled, and rendered head-and-tail +/// when it is wider than a record can carry. `toyos-elide` is the mechanism +/// and the argument; its own tests check the seams, on the host, against +/// characters that straddle both of them. pub fn demangled(raw: &str) -> Elided, SYMBOL_HEAD, SYMBOL_TAIL> { - symbol_text(rustc_demangle::demangle(raw)) + Elided(rustc_demangle::demangle(raw)) } /// `[offset, offset + len)` of `data`, or `None` when that is not wholly inside @@ -127,17 +122,50 @@ pub enum Unnamed { NoSymbol, } +/// Whether a name is known to be of the build that ran. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Check { + /// The file carries the build-id the record does. + BuildId, + /// Neither carries one, so nothing tells this build from another build of + /// that path: the name is printed, and printed as unchecked. + Unchecked, +} + +/// A recorded offset named: the function it falls in, how far into it, and +/// what vouches that the file is the build that ran. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Named<'a> { + pub function: &'a str, + pub within: u64, + pub check: Check, +} + +/// `+`, and ` (unchecked: no build-id)` after a name +/// [`Check::Unchecked`]. +impl core::fmt::Display for Named<'_> { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "{}+{:#x}", demangled(self.function), self.within)?; + match self.check { + Check::BuildId => Ok(()), + Check::Unchecked => f.write_str(" (unchecked: no build-id)"), + } + } +} + /// The function `offset` falls in, and how far into it, out of `file`'s own /// symbol table — when `file` is the build `id` names (`None`: a file with no /// build-id). The same lookup the kernel names its own frames with. -pub fn name<'a>(file: &'a [u8], offset: u64, id: Option<&BuildId>) -> Result<(&'a str, u64), Unnamed> { +pub fn name<'a>(file: &'a [u8], offset: u64, id: Option<&BuildId>) -> Result, Unnamed> { FileHeader::parse(file).map_err(Unnamed::NotElf)?; let carried = BuildId::find(file, |s| file_range(file, s.offset, s.filesz)); if carried.as_ref() != id { return Err(Unnamed::OtherBuild { file: carried }); } + let check = if carried.is_some() { Check::BuildId } else { Check::Unchecked }; let (symtab, strtab) = locate(file).ok_or(Unnamed::NoSymbols)?; - SymTab::new(symtab, strtab).resolve(offset).ok_or(Unnamed::NoSymbol) + let (function, within) = SymTab::new(symtab, strtab).resolve(offset).ok_or(Unnamed::NoSymbol)?; + Ok(Named { function, within, check }) } #[cfg(test)] @@ -147,7 +175,7 @@ mod tests { #[test] fn a_short_name_is_untouched() { - assert_eq!(symbol_text("core::panic::PanicInfo").to_string(), "core::panic::PanicInfo"); + assert_eq!(demangled("core::panic::PanicInfo").to_string(), "core::panic::PanicInfo"); } /// `SYMBOL_HEAD + SYMBOL_TAIL` is 903 bytes, so a name past that is elided @@ -156,7 +184,7 @@ mod tests { #[test] fn a_name_past_head_plus_tail_is_elided() { let long = "x".repeat(SYMBOL_HEAD + SYMBOL_TAIL + 1); - let rendered = symbol_text(long.as_str()).to_string(); + let rendered = demangled(&long).to_string(); assert!(rendered.contains("bytes elided")); assert!(rendered.len() <= SYMBOL_BUDGET); } diff --git a/toyos-symbols/tests/name.rs b/toyos-symbols/tests/name.rs index 5fb212e62f3..1cead501cbb 100644 --- a/toyos-symbols/tests/name.rs +++ b/toyos-symbols/tests/name.rs @@ -4,7 +4,7 @@ use object::{Object, ObjectSymbol, SymbolKind}; use toyos_symbols::frame::BuildId; -use toyos_symbols::{name, Unnamed}; +use toyos_symbols::{name, Check, Named, Unnamed}; const BINARY: &[u8] = include_bytes!("fixtures/input-test.bin"); @@ -32,7 +32,18 @@ fn with_build_id(id: &[u8]) -> Vec { #[test] fn a_real_binarys_offset_names_its_function() { // `readelf --syms`: `3902: 00000000000013a0 55 FUNC GLOBAL DEFAULT 1 main` - assert_eq!(name(BINARY, 0x13a0 + 10, None), Ok(("main", 10))); + assert_eq!(name(BINARY, 0x13a0 + 10, None), Ok(Named { function: "main", within: 10, check: Check::Unchecked })); +} + +/// A name no build-id vouches for says so on its line; one a build-id vouches +/// for carries no mark. +#[test] +fn a_name_no_build_id_vouches_for_is_marked_unchecked() { + assert_eq!(name(BINARY, 0x13a0 + 10, None).unwrap().to_string(), "main+0xa (unchecked: no build-id)"); + let id = [0x5a; 20]; + let file = with_build_id(&id); + let id = BuildId::new(&id).unwrap(); + assert_eq!(name(&file, 0x13a0 + 10, Some(&id)).unwrap().to_string(), "main+0xa"); } /// Every function `object` reads out of the same file names one of the @@ -50,7 +61,7 @@ fn every_function_names_what_object_reads() { let at_address: Vec<&str> = functions.iter().filter(|o| o.address() == sym.address()).filter_map(|o| o.name().ok()).collect(); assert!( - matches!(ours, Ok((n, within)) if at_address.contains(&n) && within == at - sym.address()), + matches!(ours, Ok(Named { function, within, .. }) if at_address.contains(&function) && within == at - sym.address()), "{at:#x}: ours {ours:?}, object's {at_address:?}" ); } @@ -63,7 +74,7 @@ fn the_build_the_record_names_is_the_one_named() { let file = with_build_id(&id); let id = BuildId::new(&id).unwrap(); assert_eq!(BuildId::find(&file, |s| file.get(s.offset as usize..(s.offset + s.filesz) as usize)), Some(id)); - assert_eq!(name(&file, 0x13a0, Some(&id)), Ok(("main", 0))); + assert_eq!(name(&file, 0x13a0, Some(&id)), Ok(Named { function: "main", within: 0, check: Check::BuildId })); } #[test] diff --git a/userland/Cargo.lock b/userland/Cargo.lock index 21bfa4639d1..d89ae5acb8e 100644 --- a/userland/Cargo.lock +++ b/userland/Cargo.lock @@ -1161,7 +1161,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -3383,7 +3383,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -4755,7 +4755,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] diff --git a/userland/symbolize/src/main.rs b/userland/symbolize/src/main.rs index 6fab596ebf7..3117a112c50 100644 --- a/userland/symbolize/src/main.rs +++ b/userland/symbolize/src/main.rs @@ -5,8 +5,10 @@ //! or stdin — writes each back unchanged, and after each frame line writes the //! function that offset falls in, out of the named file's own symbol table, or //! why it names none: a file that is another build, one that cannot be read, -//! one with no symbol table. A log is named on any machine by giving `--root` -//! the directory that holds the image's files. +//! one with no symbol table. A name no build-id vouches for is printed marked +//! `(unchecked: no build-id)`. A log is named on any machine by giving `--root` +//! the directory that holds the image's files; a record whose name would leave +//! that directory is refused. //! //! **Nothing a line or a file holds makes it fail**: the lookup is //! `toyos_symbols::name`, the kernel's own and panic-free on any bytes, and @@ -15,11 +17,11 @@ use std::collections::HashMap; use std::fs; use std::io::{self, BufRead, BufReader, Read, Write}; -use std::path::PathBuf; +use std::path::{Component, Path, PathBuf}; use std::process::ExitCode; use toyos_symbols::frame::{decode, Decoded}; -use toyos_symbols::{demangled, name, Unnamed}; +use toyos_symbols::{name, Unnamed}; const USAGE: &str = "usage: symbolize [--root DIR] [FILE]"; @@ -90,7 +92,10 @@ impl Namer { fn name(&mut self, frame: &Decoded<'_>) -> String { let file: String = frame.name().collect(); let path = match &self.root { - Some(root) => root.join(file.trim_start_matches('/')), + Some(root) => match under(root, &file) { + Some(path) => path, + None => return format!("? {file} leaves the root"), + }, None => PathBuf::from(&file), }; let bytes = self.files.entry(file.clone()).or_insert_with(|| { @@ -101,7 +106,7 @@ impl Namer { Err(why) => return why.clone(), }; match name(bytes, frame.offset, frame.build_id.as_ref()) { - Ok((symbol, within)) => format!("{}+{within:#x}", demangled(symbol)), + Ok(named) => named.to_string(), Err(Unnamed::NotElf(e)) => format!("? {file}: {e}"), Err(Unnamed::OtherBuild { file: Some(id) }) => format!("? {file} is another build: its id is {id}"), Err(Unnamed::OtherBuild { file: None }) => format!("? {file} carries no build-id"), @@ -110,3 +115,42 @@ impl Namer { } } } + +/// `file`, a path on the machine that ran it, as a path under `root`; `None` +/// when a component would take it out of `root` — `..`, or a Windows prefix. +fn under(root: &Path, file: &str) -> Option { + let mut path = root.to_path_buf(); + for component in Path::new(file).components() { + match component { + Component::Normal(part) => path.push(part), + Component::RootDir | Component::CurDir => {} + Component::ParentDir | Component::Prefix(_) => return None, + } + } + Some(path) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_name_is_read_under_the_root() { + assert_eq!(under(Path::new("img"), "/home/a/child"), Some(PathBuf::from("img/home/a/child"))); + } + + #[test] + fn a_name_that_leaves_the_root_is_refused() { + assert_eq!(under(Path::new("img"), "/../../etc/passwd"), None); + assert_eq!(under(Path::new("img"), "/home/../../x"), None); + } + + #[test] + fn a_record_that_leaves_the_root_is_refused_on_its_line() { + let mut namer = Namer { root: Some(PathBuf::from("img")), files: HashMap::new() }; + let mut out = Vec::new(); + let line = " 0x1000 /../../x+0x10 id=-\n"; + namer.run(line.as_bytes(), &mut out).unwrap(); + assert_eq!(String::from_utf8(out).unwrap(), " 0x1000 /../../x+0x10 id=- = ? /../../x leaves the root\n"); + } +} From 4bc7174914943a4be46293bf99614d15f2f8dc3a Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 13:19:59 +0200 Subject: [PATCH 5/7] symbolize refuses a part that does not read back as itself, and the Move 3 exit names the T14 row Review round 2, BLOCKER: on Windows a record named `/home/C:x` parses mid-path as `Component::Normal("C:x")`, but `PathBuf::push` of `C:x` sees a drive prefix and replaces the whole buffer, so `--root` was escaped. `under` now pushes a `Normal` part only when `Path::new(part)` parses back as exactly that one component; any other is refused like `..`. The new test asserts on every host that a drive-shaped name either is refused or stays under the root as plain components. It is red without the fix only on a Windows host; on macOS the mutation that drops the check stays green (m10), because no Unix host reads `C:x` as anything but a name. The tests type-check for x86_64-pc-windows-msvc; no Windows host ran them. NOTE: the track's Move 3 exit named `disk_backtrace` "in the guest suite", which runs no shared-boot member. It names the T14's shared-boot row alone. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...ernel-still-parses-what-userland-writes.md | 2 +- userland/symbolize/src/main.rs | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/issues/kernel/the-kernel-still-parses-what-userland-writes.md b/issues/kernel/the-kernel-still-parses-what-userland-writes.md index ad81951f2da..c6e5a7c053f 100644 --- a/issues/kernel/the-kernel-still-parses-what-userland-writes.md +++ b/issues/kernel/the-kernel-still-parses-what-userland-writes.md @@ -60,7 +60,7 @@ only the executable's image for a process; and `rustc-demangle`'s standing. It is an unforked crates.io dependency, ~2k lines of third-party string parsing in Ring 0 on the panic path, now for the kernel's own names alone: fork it into the estate like every other third-party source, or record the exemption -deliberately. **Exit**: `disk_backtrace`, in the guest suite and on the T14, +deliberately. **Exit**: `disk_backtrace`, the T14's shared-boot row, reads its killed child's report as the child's file, offset and build-id with no user symbol in it, and `symbolize` names the faulting function from them; no kernel source builds a backtrace table from a program's file; a library's diff --git a/userland/symbolize/src/main.rs b/userland/symbolize/src/main.rs index 3117a112c50..f389d6bfa89 100644 --- a/userland/symbolize/src/main.rs +++ b/userland/symbolize/src/main.rs @@ -117,14 +117,16 @@ impl Namer { } /// `file`, a path on the machine that ran it, as a path under `root`; `None` -/// when a component would take it out of `root` — `..`, or a Windows prefix. +/// when a component would take it out of `root` — `..`, a Windows prefix, or a +/// part this host reads as more than itself once pushed alone, as Windows reads +/// `C:x`, which `PathBuf::push` lets replace the whole path. fn under(root: &Path, file: &str) -> Option { let mut path = root.to_path_buf(); for component in Path::new(file).components() { match component { - Component::Normal(part) => path.push(part), + Component::Normal(part) if Path::new(part).components().eq([component]) => path.push(part), Component::RootDir | Component::CurDir => {} - Component::ParentDir | Component::Prefix(_) => return None, + Component::Normal(_) | Component::ParentDir | Component::Prefix(_) => return None, } } Some(path) @@ -145,6 +147,17 @@ mod tests { assert_eq!(under(Path::new("img"), "/home/../../x"), None); } + #[test] + fn a_name_some_host_reads_as_a_drive_stays_under_the_root() { + let root = Path::new("img"); + for file in ["/home/C:x", "/home/C:/x", "/home/C:"] { + if let Some(path) = under(root, file) { + let rest = path.strip_prefix(root).unwrap_or_else(|_| panic!("{file} left the root as {path:?}")); + assert!(rest.components().all(|c| matches!(c, Component::Normal(_))), "{file} as {path:?}"); + } + } + } + #[test] fn a_record_that_leaves_the_root_is_refused_on_its_line() { let mut namer = Namer { root: Some(PathBuf::from("img")), files: HashMap::new() }; From 673233021840204f9e00195140db527de88af69d Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 13:35:36 +0200 Subject: [PATCH 6/7] symbolize decides the root guard on the record's string, the same on every host The round-3 guard re-parsed each part with this host's Path, so its refusal of a part Windows reads as a drive (`C:x`) could only be seen on Windows, and its test could not go red on any host the gates run. `under` now splits the record's name on `/` itself and refuses, on every host alike: `..`; a part holding `:` or `\`, the only bytes that let a part with no `/` read as a Windows prefix or separator (rust/library/std/src/sys/path/windows_prefix.rs); and a name Win32 reserves for a device (CON, PRN, AUX, NUL, COM0-9, LPT0-9 and the superscript 1-3 forms, with any extension), per Microsoft's "Naming Files, Paths, and Namespaces". A ToyOS file whose name holds `:` or `\` or is such a name is no longer named under `--root` on any host; on Windows it never could be. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- userland/symbolize/src/main.rs | 55 +++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/userland/symbolize/src/main.rs b/userland/symbolize/src/main.rs index f389d6bfa89..6b8d7537bdc 100644 --- a/userland/symbolize/src/main.rs +++ b/userland/symbolize/src/main.rs @@ -17,7 +17,7 @@ use std::collections::HashMap; use std::fs; use std::io::{self, BufRead, BufReader, Read, Write}; -use std::path::{Component, Path, PathBuf}; +use std::path::{Path, PathBuf}; use std::process::ExitCode; use toyos_symbols::frame::{decode, Decoded}; @@ -116,22 +116,37 @@ impl Namer { } } -/// `file`, a path on the machine that ran it, as a path under `root`; `None` -/// when a component would take it out of `root` — `..`, a Windows prefix, or a -/// part this host reads as more than itself once pushed alone, as Windows reads -/// `C:x`, which `PathBuf::push` lets replace the whole path. +/// `file`, a `/`-separated path on the machine that ran it, as a path under +/// `root`; `None` for a part that could take it out of `root` on some host. +/// The rule reads the string, never this host's `Path`, so every host answers +/// alike: `..`; a `:` or `\\`, the only bytes that let a part with no `/` read +/// as a Windows prefix or separator; and a name Windows reserves for a device. fn under(root: &Path, file: &str) -> Option { let mut path = root.to_path_buf(); - for component in Path::new(file).components() { - match component { - Component::Normal(part) if Path::new(part).components().eq([component]) => path.push(part), - Component::RootDir | Component::CurDir => {} - Component::Normal(_) | Component::ParentDir | Component::Prefix(_) => return None, + for part in file.split('/') { + match part { + "" | "." => {} + ".." => return None, + _ if part.contains([':', '\\']) || names_a_device(part) => return None, + _ => path.push(part), } } Some(path) } +/// Whether Win32 may open `part` as a device rather than a file: the reserved +/// names of learn.microsoft.com/windows/win32/fileio/naming-a-file, "Naming +/// Conventions", in any case and followed by any extension. +fn names_a_device(part: &str) -> bool { + const RESERVED: [&str; 4] = ["CON", "PRN", "AUX", "NUL"]; + let stem = part.split_once('.').map_or(part, |(stem, _)| stem).trim_end_matches(' '); + let numbered = |prefix: &str| { + stem.get(..3).is_some_and(|head| head.eq_ignore_ascii_case(prefix)) + && matches!(&stem[3..], "0" | "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9" | "\u{b9}" | "\u{b2}" | "\u{b3}") + }; + RESERVED.iter().any(|name| stem.eq_ignore_ascii_case(name)) || numbered("COM") || numbered("LPT") +} + #[cfg(test)] mod tests { use super::*; @@ -148,13 +163,19 @@ mod tests { } #[test] - fn a_name_some_host_reads_as_a_drive_stays_under_the_root() { - let root = Path::new("img"); - for file in ["/home/C:x", "/home/C:/x", "/home/C:"] { - if let Some(path) = under(root, file) { - let rest = path.strip_prefix(root).unwrap_or_else(|_| panic!("{file} left the root as {path:?}")); - assert!(rest.components().all(|c| matches!(c, Component::Normal(_))), "{file} as {path:?}"); - } + fn a_name_some_host_reads_as_a_drive_or_separator_is_refused() { + assert_eq!(under(Path::new("img"), "/home/C:x"), None); + assert_eq!(under(Path::new("img"), "/home/C:"), None); + assert_eq!(under(Path::new("img"), "/home/a\\b"), None); + } + + #[test] + fn a_name_windows_reserves_for_a_device_is_refused() { + for file in ["/home/CON", "/home/nul.txt", "/home/Aux .tar.gz", "/home/com1", "/home/LPT\u{b9}"] { + assert_eq!(under(Path::new("img"), file), None, "{file}"); + } + for file in ["/home/CONSOLE", "/home/com10", "/home/nulls.txt", "/home/LPT"] { + assert!(under(Path::new("img"), file).is_some(), "{file}"); } } From eabf60f11351d24172a1b77f9b6f7c1c7d6504fd Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 13:57:20 +0200 Subject: [PATCH 7/7] symbolize refuses CONIN$ and CONOUT$ as device names Win32 opens the console's input and output under these names; whether it does so for a qualified path such as img\home\CONIN$ is not documented, and a read of the console would hang the namer on a crafted log line. Refusing them costs two entries, so `names_a_device` takes them with the reserved names Microsoft lists, and `under`'s doc stays true as written. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- userland/symbolize/src/main.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/userland/symbolize/src/main.rs b/userland/symbolize/src/main.rs index 6b8d7537bdc..f5ced7e32df 100644 --- a/userland/symbolize/src/main.rs +++ b/userland/symbolize/src/main.rs @@ -136,9 +136,10 @@ fn under(root: &Path, file: &str) -> Option { /// Whether Win32 may open `part` as a device rather than a file: the reserved /// names of learn.microsoft.com/windows/win32/fileio/naming-a-file, "Naming -/// Conventions", in any case and followed by any extension. +/// Conventions", and the console's `CONIN$` and `CONOUT$`, in any case and +/// followed by any extension. fn names_a_device(part: &str) -> bool { - const RESERVED: [&str; 4] = ["CON", "PRN", "AUX", "NUL"]; + const RESERVED: [&str; 6] = ["CON", "PRN", "AUX", "NUL", "CONIN$", "CONOUT$"]; let stem = part.split_once('.').map_or(part, |(stem, _)| stem).trim_end_matches(' '); let numbered = |prefix: &str| { stem.get(..3).is_some_and(|head| head.eq_ignore_ascii_case(prefix)) @@ -171,10 +172,10 @@ mod tests { #[test] fn a_name_windows_reserves_for_a_device_is_refused() { - for file in ["/home/CON", "/home/nul.txt", "/home/Aux .tar.gz", "/home/com1", "/home/LPT\u{b9}"] { + for file in ["/home/CON", "/home/nul.txt", "/home/Aux .tar.gz", "/home/com1", "/home/LPT\u{b9}", "/home/CONIN$", "/home/conout$.log"] { assert_eq!(under(Path::new("img"), file), None, "{file}"); } - for file in ["/home/CONSOLE", "/home/com10", "/home/nulls.txt", "/home/LPT"] { + for file in ["/home/CONSOLE", "/home/com10", "/home/nulls.txt", "/home/LPT", "/home/CONIN"] { assert!(under(Path::new("img"), file).is_some(), "{file}"); } }