From 42089bbb6e295af48086d3caedeb8fdd38256c9a Mon Sep 17 00:00:00 2001 From: James Munns Date: Sat, 4 Jul 2026 18:23:27 +0200 Subject: [PATCH 01/35] Split out `elf` and `stack` crates from `xtask` I intend to do some rework of the max-stack analysis code, and would like to break out the max-stack analysis into its own crate to make testing this in isolation easier. As this code also depends on the `xtask::elf` module, I broke that out to avoid circular deps. This is the minimum change necessary to split things out before any refactoring. --- Cargo.lock | 26 +- build/elf/Cargo.toml | 12 + build/{xtask/src/elf.rs => elf/src/lib.rs} | 0 build/stack/Cargo.toml | 16 ++ build/stack/src/lib.rs | 269 ++++++++++++++++++++ build/xtask/Cargo.toml | 5 +- build/xtask/src/caboose_pos.rs | 3 +- build/xtask/src/dist.rs | 271 +-------------------- build/xtask/src/main.rs | 1 - build/xtask/src/sizes.rs | 11 +- build/xtask/src/task_slot.rs | 3 +- 11 files changed, 337 insertions(+), 280 deletions(-) create mode 100644 build/elf/Cargo.toml rename build/{xtask/src/elf.rs => elf/src/lib.rs} (100%) create mode 100644 build/stack/Cargo.toml create mode 100644 build/stack/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 5c480a443b..aecd3f2322 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3040,6 +3040,15 @@ version = "1.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "90e5c1c8368803113bf0c9584fc495a58b86dc8a29edbf8fe877d21d9507e797" +[[package]] +name = "elf" +version = "0.1.0" +dependencies = [ + "anyhow", + "goblin", + "scroll", +] + [[package]] name = "elliptic-curve" version = "0.13.8" @@ -5856,6 +5865,18 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" +[[package]] +name = "stack" +version = "0.1.0" +dependencies = [ + "anyhow", + "capstone", + "elf", + "goblin", + "leb128", + "rustc-demangle", +] + [[package]] name = "stage0-handoff" version = "0.1.0" @@ -8040,7 +8061,6 @@ dependencies = [ "build-kconfig", "byteorder", "bzip2-rs", - "capstone", "cargo_metadata", "clap 3.2.23", "colored", @@ -8048,6 +8068,7 @@ dependencies = [ "digest-io", "directories", "dunce", + "elf", "filetime", "fnv", "glob", @@ -8057,7 +8078,6 @@ dependencies = [ "hex", "hubtools", "indexmap 1.9.1", - "leb128", "lpc55-rom-data", "lpc55_sign", "memchr", @@ -8068,12 +8088,12 @@ dependencies = [ "rats-corim", "regex", "ron 0.8.0", - "rustc-demangle", "scroll", "serde", "serde_json", "sha2 0.11.0", "sha3 0.11.0", + "stack", "strsim 0.10.0", "tlvc", "tlvc-text", diff --git a/build/elf/Cargo.toml b/build/elf/Cargo.toml new file mode 100644 index 0000000000..cb92d1e6bf --- /dev/null +++ b/build/elf/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "elf" +version = "0.1.0" +edition = "2024" + +[dependencies] +anyhow = { workspace = true } +goblin = { workspace = true } +scroll = { workspace = true } + +[lints] +workspace = true diff --git a/build/xtask/src/elf.rs b/build/elf/src/lib.rs similarity index 100% rename from build/xtask/src/elf.rs rename to build/elf/src/lib.rs diff --git a/build/stack/Cargo.toml b/build/stack/Cargo.toml new file mode 100644 index 0000000000..011db7a9a8 --- /dev/null +++ b/build/stack/Cargo.toml @@ -0,0 +1,16 @@ +[package] +name = "stack" +version = "0.1.0" +edition = "2024" + +[dependencies] +anyhow = { workspace = true } +goblin = { workspace = true } +capstone = { workspace = true } +rustc-demangle = { workspace = true } +leb128 = { workspace = true } + +elf = { path = "../elf" } + +[lints] +workspace = true diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs new file mode 100644 index 0000000000..70617f91f9 --- /dev/null +++ b/build/stack/src/lib.rs @@ -0,0 +1,269 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use std::{ + collections::{BTreeMap, BTreeSet}, + path::Path, +}; + +use anyhow::{Context, Result, anyhow, bail}; + +/// Estimates the maximum stack size for the given task +/// +/// This does not take dynamic function calls into account, which could cause +/// underestimation. Overestimation is less likely, but still may happen if +/// there are logically impossible call trees (e.g. `A -> B` and `B -> C`, but +/// `B` never calls `C` if called by `A`). +pub fn get_max_stack( + elf: &Path, + task_name: &str, + verbose: bool, +) -> Result> { + // Open the statically-linked ELF file + let data = std::fs::read(elf).context("could not open ELF file")?; + let elf = goblin::elf::Elf::parse(&data)?; + + // Read the .stack_sizes section, which is an array of + // `(address: u32, stack size: unsigned leb128)` tuples + let sizes = elf::get_section_by_name(&elf, ".stack_sizes") + .context("could not get .stack_sizes")?; + let mut sizes = &data[sizes.sh_offset as usize..][..sizes.sh_size as usize]; + let mut addr_to_frame_size = BTreeMap::new(); + while !sizes.is_empty() { + let (addr, rest) = sizes.split_at(4); + let addr = u32::from_le_bytes(addr.try_into().unwrap()); + sizes = rest; + let size = leb128::read::unsigned(&mut sizes)?; + addr_to_frame_size.insert(addr, size); + } + + // There are `$t` and `$d` symbols which indicate the beginning of text + // versus data in the `.text` region. We collect them into a `BTreeMap` + // here so that we can avoid trying to decode inline data words. + let mut text_regions = BTreeMap::new(); + for sym in elf.syms.iter() { + if sym.st_name == 0 + || sym.st_size != 0 + || sym.st_type() != goblin::elf::sym::STT_NOTYPE + { + continue; + } + + let addr = sym.st_value as u32; + let is_text = match elf.strtab.get_at(sym.st_name) { + Some("$t") => true, + Some("$d") => false, + Some(_) => continue, + None => { + bail!("bad symbol in {task_name}: {}", sym.st_name); + } + }; + text_regions.insert(addr, is_text); + } + let is_code = |addr| { + let mut iter = text_regions.range(..=addr); + *iter.next_back().unwrap().1 + }; + + // We'll be packing everything into this data structure + #[derive(Debug)] + struct FunctionData { + name: String, + short_name: String, + frame_size: Option, + calls: BTreeSet, + } + + let text = elf::get_section_by_name(&elf, ".text") + .context("could not get .text")?; + + use capstone::{ + Capstone, InsnGroupId, InsnGroupType, + arch::{ArchOperand, BuildsCapstone, BuildsCapstoneExtraMode, arm}, + }; + let cs = Capstone::new() + .arm() + .mode(arm::ArchMode::Thumb) + .extra_mode(std::iter::once(arm::ArchExtraMode::MClass)) + .detail(true) + .build() + .map_err(|e| anyhow!("failed to initialize disassembler: {e:?}"))?; + + // Disassemble each function, building a map of its call sites + let mut fns = BTreeMap::new(); + for sym in elf.syms.iter() { + // We only care about named function symbols here + if sym.st_name == 0 || !sym.is_function() || sym.st_size == 0 { + continue; + } + + let Some(name) = elf.strtab.get_at(sym.st_name) else { + bail!("bad symbol in {task_name}: {}", sym.st_name); + }; + + // Clear the lowest bit, which indicates that the function contains + // thumb instructions (always true for our systems!) + let val = sym.st_value & !1; + let base_addr = val as u32; + + // Get the text region for this function + let offset = (val - text.sh_addr + text.sh_offset) as usize; + let text = &data[offset..][..sym.st_size as usize]; + + // Split the text region into instruction-only chunks + let mut chunks = vec![]; + let mut chunk = None; + for (i, b) in text.iter().enumerate() { + let addr = base_addr + i as u32; + if is_code(addr) { + chunk.get_or_insert((addr, vec![])).1.push(*b); + } else { + chunks.extend(chunk.take()); + } + } + chunks.extend(chunk); // don't forget the trailing chunk! + + let frame_size = addr_to_frame_size.get(&base_addr).copied(); + let mut calls = BTreeSet::new(); + for (addr, chunk) in chunks { + let instrs = cs + .disasm_all(&chunk, addr.into()) + .map_err(|e| anyhow!("disassembly failed: {e:?}"))?; + for (i, instr) in instrs.iter().enumerate() { + let detail = cs.insn_detail(instr).map_err(|e| { + anyhow!("could not get instruction details: {e}") + })?; + + // Detect tail calls, which are jumps at the final instruction + // when the function itself has no stack frame. + let can_tail = frame_size == Some(0) && i == instrs.len() - 1; + if detail.groups().iter().any(|g| { + g == &InsnGroupId(InsnGroupType::CS_GRP_CALL as u8) + || (g == &InsnGroupId(InsnGroupType::CS_GRP_JUMP as u8) + && can_tail) + }) { + let arch = detail.arch_detail(); + let ops = arch.operands(); + let op = ops.last().unwrap_or_else(|| { + panic!("missing operand!"); + }); + + let ArchOperand::ArmOperand(op) = op else { + panic!("bad operand type: {op:?}"); + }; + // We can't resolve indirect calls, alas + let arm::ArmOperandType::Imm(target) = op.op_type else { + continue; + }; + let target = u32::try_from(target).unwrap(); + + // Avoid recursive calls into the same function (or midway + // into the function, which is a thing we've seen before! + // it's weird!) + if !(base_addr..base_addr + sym.st_size as u32) + .contains(&target) + { + calls.insert(target); + } + } + } + } + + let name = rustc_demangle::demangle(name).to_string(); + + // Strip the trailing hash from the name for ease of printing + let short_name = if let Some(i) = name.rfind("::") { + &name[..i] + } else { + &name + } + .to_owned(); + + fns.insert( + base_addr, + FunctionData { + name, + short_name, + frame_size, + calls, + }, + ); + } + + fn recurse( + call_stack: &mut Vec, + recurse_depth: usize, + mut stack_depth: u64, + fns: &BTreeMap, + deepest: &mut Option<(u64, Vec)>, + verbose: bool, + ) { + let addr = *call_stack.last().unwrap(); + let Some(f) = fns.get(&addr) else { + panic!("found jump to unknown function at {call_stack:08x?}"); + }; + let frame_size = f.frame_size.unwrap_or(0); + stack_depth += frame_size; + if verbose { + let indent = recurse_depth * 2; + println!( + " {:indent$}{addr:08x}: {} [+{frame_size} => {stack_depth}]", + "", + f.short_name, + indent = indent + ); + } + + if deepest + .as_ref() + .map(|(max_depth, _)| stack_depth > *max_depth) + .unwrap_or(true) + { + *deepest = Some((stack_depth, call_stack.to_owned())); + } + for j in &f.calls { + if call_stack.contains(j) { + // Skip recursive / mutually recursive calls, because we can't + // reason about them. + continue; + } else { + call_stack.push(*j); + recurse( + call_stack, + recurse_depth + 1, + stack_depth, + fns, + deepest, + verbose, + ); + call_stack.pop(); + } + } + } + + // Find stack sizes by traversing the graph + if verbose { + println!("finding stack sizes for {task_name}"); + } + let start_addr = fns + .iter() + .find(|(_addr, v)| v.name.as_str() == "_start") + .map(|(addr, _v)| *addr) + .ok_or_else(|| anyhow!("could not find _start"))?; + let mut deepest = None; + recurse(&mut vec![start_addr], 0, 0, &fns, &mut deepest, verbose); + + // Check against our configured task stack size + let Some((_max_depth, max_stack)) = deepest else { + unreachable!("must have at least one call stack"); + }; + + let mut out = vec![]; + for m in max_stack { + let f = fns.get(&m).unwrap(); + let name = &f.short_name; + out.push((f.frame_size.unwrap_or(0), name.clone())); + } + Ok(out) +} diff --git a/build/xtask/Cargo.toml b/build/xtask/Cargo.toml index 79d899a978..5ccd9b8e05 100644 --- a/build/xtask/Cargo.toml +++ b/build/xtask/Cargo.toml @@ -17,11 +17,11 @@ toml_edit = { workspace = true } # for dist byteorder = { workspace = true } bzip2-rs = { workspace = true } -capstone = { workspace = true } ctrlc = { workspace = true } directories = { workspace = true } digest-io = { workspace = true } dunce = { workspace = true } +elf = { path = "../elf" } filetime = { workspace = true } fnv = { workspace = true } glob = { workspace = true } @@ -30,7 +30,6 @@ guppy = { workspace = true } hex = "0.4" hubtools = { workspace = true } indexmap = { workspace = true } -leb128 = { workspace = true } multimap = { workspace = true } path-slash = { workspace = true } rangemap = { workspace = true } @@ -41,7 +40,7 @@ scroll = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } sha3 = { workspace = true } -rustc-demangle = { workspace = true } +stack = { path = "../stack" } tlvc = { workspace = true } tlvc-text = { workspace = true } toml = { workspace = true } diff --git a/build/xtask/src/caboose_pos.rs b/build/xtask/src/caboose_pos.rs index a0ec6287a2..40cae4b642 100644 --- a/build/xtask/src/caboose_pos.rs +++ b/build/xtask/src/caboose_pos.rs @@ -2,7 +2,6 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use crate::elf; use anyhow::{Context, Result, bail}; use scroll::Pread; @@ -33,7 +32,7 @@ impl scroll::ctx::TryFromCtx<'_, &goblin::elf::Elf<'_>> }; let caboose_pos_file_offset = - crate::elf::get_file_offset_by_vma(elf, caboose_pos_address) + elf::get_file_offset_by_vma(elf, caboose_pos_address) .context("could not get caboose pos file offset")?; Ok(( diff --git a/build/xtask/src/dist.rs b/build/xtask/src/dist.rs index e4a742d2d9..cfa522cf7e 100644 --- a/build/xtask/src/dist.rs +++ b/build/xtask/src/dist.rs @@ -16,12 +16,12 @@ use atty::Stream; use indexmap::IndexMap; use multimap::MultiMap; use path_slash::{PathBufExt, PathExt}; +use stack::get_max_stack; use zerocopy::IntoBytes; use crate::{ caboose_pos, config::{BuildConfig, CabooseConfig, Config, DEFAULT_RAM_NAME}, - elf, sizes::load_task_size, task_slot, }; @@ -1433,7 +1433,11 @@ fn task_can_overflow( task_name: &str, verbose: bool, ) -> Result { - let max_stack = get_max_stack(toml, task_name, verbose)?; + let f = Path::new("target") + .join(&toml.name) + .join("dist") + .join(format!("{task_name}.tmp")); + let max_stack = get_max_stack(&f, task_name, verbose)?; let max_depth: u64 = max_stack.iter().map(|(d, _)| *d).sum(); let task_stack_size = toml.tasks[task_name] @@ -1462,269 +1466,6 @@ fn task_can_overflow( } } -/// Estimates the maximum stack size for the given task -/// -/// This does not take dynamic function calls into account, which could cause -/// underestimation. Overestimation is less likely, but still may happen if -/// there are logically impossible call trees (e.g. `A -> B` and `B -> C`, but -/// `B` never calls `C` if called by `A`). -pub fn get_max_stack( - toml: &Config, - task_name: &str, - verbose: bool, -) -> Result> { - // Open the statically-linked ELF file - let f = Path::new("target") - .join(&toml.name) - .join("dist") - .join(format!("{task_name}.tmp")); - let data = std::fs::read(f).context("could not open ELF file")?; - let elf = goblin::elf::Elf::parse(&data)?; - - // Read the .stack_sizes section, which is an array of - // `(address: u32, stack size: unsigned leb128)` tuples - let sizes = crate::elf::get_section_by_name(&elf, ".stack_sizes") - .context("could not get .stack_sizes")?; - let mut sizes = &data[sizes.sh_offset as usize..][..sizes.sh_size as usize]; - let mut addr_to_frame_size = BTreeMap::new(); - while !sizes.is_empty() { - let (addr, rest) = sizes.split_at(4); - let addr = u32::from_le_bytes(addr.try_into().unwrap()); - sizes = rest; - let size = leb128::read::unsigned(&mut sizes)?; - addr_to_frame_size.insert(addr, size); - } - - // There are `$t` and `$d` symbols which indicate the beginning of text - // versus data in the `.text` region. We collect them into a `BTreeMap` - // here so that we can avoid trying to decode inline data words. - let mut text_regions = BTreeMap::new(); - for sym in elf.syms.iter() { - if sym.st_name == 0 - || sym.st_size != 0 - || sym.st_type() != goblin::elf::sym::STT_NOTYPE - { - continue; - } - - let addr = sym.st_value as u32; - let is_text = match elf.strtab.get_at(sym.st_name) { - Some("$t") => true, - Some("$d") => false, - Some(_) => continue, - None => { - bail!("bad symbol in {task_name}: {}", sym.st_name); - } - }; - text_regions.insert(addr, is_text); - } - let is_code = |addr| { - let mut iter = text_regions.range(..=addr); - *iter.next_back().unwrap().1 - }; - - // We'll be packing everything into this data structure - #[derive(Debug)] - struct FunctionData { - name: String, - short_name: String, - frame_size: Option, - calls: BTreeSet, - } - - let text = crate::elf::get_section_by_name(&elf, ".text") - .context("could not get .text")?; - - use capstone::{ - Capstone, InsnGroupId, InsnGroupType, - arch::{ArchOperand, BuildsCapstone, BuildsCapstoneExtraMode, arm}, - }; - let cs = Capstone::new() - .arm() - .mode(arm::ArchMode::Thumb) - .extra_mode(std::iter::once(arm::ArchExtraMode::MClass)) - .detail(true) - .build() - .map_err(|e| anyhow!("failed to initialize disassembler: {e:?}"))?; - - // Disassemble each function, building a map of its call sites - let mut fns = BTreeMap::new(); - for sym in elf.syms.iter() { - // We only care about named function symbols here - if sym.st_name == 0 || !sym.is_function() || sym.st_size == 0 { - continue; - } - - let Some(name) = elf.strtab.get_at(sym.st_name) else { - bail!("bad symbol in {task_name}: {}", sym.st_name); - }; - - // Clear the lowest bit, which indicates that the function contains - // thumb instructions (always true for our systems!) - let val = sym.st_value & !1; - let base_addr = val as u32; - - // Get the text region for this function - let offset = (val - text.sh_addr + text.sh_offset) as usize; - let text = &data[offset..][..sym.st_size as usize]; - - // Split the text region into instruction-only chunks - let mut chunks = vec![]; - let mut chunk = None; - for (i, b) in text.iter().enumerate() { - let addr = base_addr + i as u32; - if is_code(addr) { - chunk.get_or_insert((addr, vec![])).1.push(*b); - } else { - chunks.extend(chunk.take()); - } - } - chunks.extend(chunk); // don't forget the trailing chunk! - - let frame_size = addr_to_frame_size.get(&base_addr).copied(); - let mut calls = BTreeSet::new(); - for (addr, chunk) in chunks { - let instrs = cs - .disasm_all(&chunk, addr.into()) - .map_err(|e| anyhow!("disassembly failed: {e:?}"))?; - for (i, instr) in instrs.iter().enumerate() { - let detail = cs.insn_detail(instr).map_err(|e| { - anyhow!("could not get instruction details: {e}") - })?; - - // Detect tail calls, which are jumps at the final instruction - // when the function itself has no stack frame. - let can_tail = frame_size == Some(0) && i == instrs.len() - 1; - if detail.groups().iter().any(|g| { - g == &InsnGroupId(InsnGroupType::CS_GRP_CALL as u8) - || (g == &InsnGroupId(InsnGroupType::CS_GRP_JUMP as u8) - && can_tail) - }) { - let arch = detail.arch_detail(); - let ops = arch.operands(); - let op = ops.last().unwrap_or_else(|| { - panic!("missing operand!"); - }); - - let ArchOperand::ArmOperand(op) = op else { - panic!("bad operand type: {op:?}"); - }; - // We can't resolve indirect calls, alas - let arm::ArmOperandType::Imm(target) = op.op_type else { - continue; - }; - let target = u32::try_from(target).unwrap(); - - // Avoid recursive calls into the same function (or midway - // into the function, which is a thing we've seen before! - // it's weird!) - if !(base_addr..base_addr + sym.st_size as u32) - .contains(&target) - { - calls.insert(target); - } - } - } - } - - let name = rustc_demangle::demangle(name).to_string(); - - // Strip the trailing hash from the name for ease of printing - let short_name = if let Some(i) = name.rfind("::") { - &name[..i] - } else { - &name - } - .to_owned(); - - fns.insert( - base_addr, - FunctionData { - name, - short_name, - frame_size, - calls, - }, - ); - } - - fn recurse( - call_stack: &mut Vec, - recurse_depth: usize, - mut stack_depth: u64, - fns: &BTreeMap, - deepest: &mut Option<(u64, Vec)>, - verbose: bool, - ) { - let addr = *call_stack.last().unwrap(); - let Some(f) = fns.get(&addr) else { - panic!("found jump to unknown function at {call_stack:08x?}"); - }; - let frame_size = f.frame_size.unwrap_or(0); - stack_depth += frame_size; - if verbose { - let indent = recurse_depth * 2; - println!( - " {:indent$}{addr:08x}: {} [+{frame_size} => {stack_depth}]", - "", - f.short_name, - indent = indent - ); - } - - if deepest - .as_ref() - .map(|(max_depth, _)| stack_depth > *max_depth) - .unwrap_or(true) - { - *deepest = Some((stack_depth, call_stack.to_owned())); - } - for j in &f.calls { - if call_stack.contains(j) { - // Skip recursive / mutually recursive calls, because we can't - // reason about them. - continue; - } else { - call_stack.push(*j); - recurse( - call_stack, - recurse_depth + 1, - stack_depth, - fns, - deepest, - verbose, - ); - call_stack.pop(); - } - } - } - - // Find stack sizes by traversing the graph - if verbose { - println!("finding stack sizes for {task_name}"); - } - let start_addr = fns - .iter() - .find(|(_addr, v)| v.name.as_str() == "_start") - .map(|(addr, _v)| *addr) - .ok_or_else(|| anyhow!("could not find _start"))?; - let mut deepest = None; - recurse(&mut vec![start_addr], 0, 0, &fns, &mut deepest, verbose); - - // Check against our configured task stack size - let Some((_max_depth, max_stack)) = deepest else { - unreachable!("must have at least one call stack"); - }; - - let mut out = vec![]; - for m in max_stack { - let f = fns.get(&m).unwrap(); - let name = &f.short_name; - out.push((f.frame_size.unwrap_or(0), name.clone())); - } - Ok(out) -} - /// Link a specific task fn link_task( cfg: &PackageConfig, diff --git a/build/xtask/src/main.rs b/build/xtask/src/main.rs index 374d2f451f..6a9141f066 100644 --- a/build/xtask/src/main.rs +++ b/build/xtask/src/main.rs @@ -13,7 +13,6 @@ mod auxflash; mod caboose_pos; mod config; mod dist; -mod elf; mod flash; mod gha_prepare_artifacts; mod graph; diff --git a/build/xtask/src/sizes.rs b/build/xtask/src/sizes.rs index 7f43195c0d..381b0b18e8 100644 --- a/build/xtask/src/sizes.rs +++ b/build/xtask/src/sizes.rs @@ -16,10 +16,9 @@ use indexmap::map::Entry; use crate::{ Config, - dist::{ - Allocations, ContiguousRanges, DEFAULT_KERNEL_STACK, get_max_stack, - }, + dist::{Allocations, ContiguousRanges, DEFAULT_KERNEL_STACK}, }; +use stack::get_max_stack; #[derive(Debug)] struct TaskSizes<'a> { @@ -437,7 +436,11 @@ fn print_task_stacks(toml: &Config) -> Result<()> { let task_stack_size = task.stacksize.unwrap_or_else(|| toml.stacksize.unwrap()); - let max_stack = get_max_stack(toml, task_name, false)?; + let f = Path::new("target") + .join(&toml.name) + .join("dist") + .join(format!("{task_name}.tmp")); + let max_stack = get_max_stack(&f, task_name, false)?; let total: u64 = max_stack.iter().map(|(n, _)| *n).sum(); println!("{task_name}: {total} bytes (limit is {task_stack_size})"); for (frame_size, name) in max_stack { diff --git a/build/xtask/src/task_slot.rs b/build/xtask/src/task_slot.rs index fc473e17a7..3819625312 100644 --- a/build/xtask/src/task_slot.rs +++ b/build/xtask/src/task_slot.rs @@ -2,7 +2,6 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use crate::elf; use anyhow::{Context, Result, bail}; use scroll::Pread; use std::path::Path; @@ -46,7 +45,7 @@ impl<'a> scroll::ctx::TryFromCtx<'a, &goblin::elf::Elf<'a>> )?; let taskidx_file_offset = - crate::elf::get_file_offset_by_vma(elf, taskidx_address).context( + elf::get_file_offset_by_vma(elf, taskidx_address).context( format!("slot '{slot_name}' points to non-existent address"), )?; From 156ba534c33ee2ff82da6e38443096d99770f40c Mon Sep 17 00:00:00 2001 From: James Munns Date: Sat, 4 Jul 2026 21:55:29 +0200 Subject: [PATCH 02/35] Split out some nested items from the function --- build/stack/src/lib.rs | 128 ++++++++++++++++++++--------------------- 1 file changed, 64 insertions(+), 64 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 70617f91f9..f695713582 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -8,6 +8,19 @@ use std::{ }; use anyhow::{Context, Result, anyhow, bail}; +use capstone::{ + Capstone, InsnGroupId, InsnGroupType, + arch::{ArchOperand, BuildsCapstone, BuildsCapstoneExtraMode, arm}, +}; + +// We'll be packing everything into this data structure +#[derive(Debug)] +struct FunctionData { + name: String, + short_name: String, + frame_size: Option, + calls: BTreeSet, +} /// Estimates the maximum stack size for the given task /// @@ -66,22 +79,9 @@ pub fn get_max_stack( *iter.next_back().unwrap().1 }; - // We'll be packing everything into this data structure - #[derive(Debug)] - struct FunctionData { - name: String, - short_name: String, - frame_size: Option, - calls: BTreeSet, - } - let text = elf::get_section_by_name(&elf, ".text") .context("could not get .text")?; - use capstone::{ - Capstone, InsnGroupId, InsnGroupType, - arch::{ArchOperand, BuildsCapstone, BuildsCapstoneExtraMode, arm}, - }; let cs = Capstone::new() .arm() .mode(arm::ArchMode::Thumb) @@ -191,57 +191,6 @@ pub fn get_max_stack( ); } - fn recurse( - call_stack: &mut Vec, - recurse_depth: usize, - mut stack_depth: u64, - fns: &BTreeMap, - deepest: &mut Option<(u64, Vec)>, - verbose: bool, - ) { - let addr = *call_stack.last().unwrap(); - let Some(f) = fns.get(&addr) else { - panic!("found jump to unknown function at {call_stack:08x?}"); - }; - let frame_size = f.frame_size.unwrap_or(0); - stack_depth += frame_size; - if verbose { - let indent = recurse_depth * 2; - println!( - " {:indent$}{addr:08x}: {} [+{frame_size} => {stack_depth}]", - "", - f.short_name, - indent = indent - ); - } - - if deepest - .as_ref() - .map(|(max_depth, _)| stack_depth > *max_depth) - .unwrap_or(true) - { - *deepest = Some((stack_depth, call_stack.to_owned())); - } - for j in &f.calls { - if call_stack.contains(j) { - // Skip recursive / mutually recursive calls, because we can't - // reason about them. - continue; - } else { - call_stack.push(*j); - recurse( - call_stack, - recurse_depth + 1, - stack_depth, - fns, - deepest, - verbose, - ); - call_stack.pop(); - } - } - } - // Find stack sizes by traversing the graph if verbose { println!("finding stack sizes for {task_name}"); @@ -267,3 +216,54 @@ pub fn get_max_stack( } Ok(out) } + +fn recurse( + call_stack: &mut Vec, + recurse_depth: usize, + mut stack_depth: u64, + fns: &BTreeMap, + deepest: &mut Option<(u64, Vec)>, + verbose: bool, +) { + let addr = *call_stack.last().unwrap(); + let Some(f) = fns.get(&addr) else { + panic!("found jump to unknown function at {call_stack:08x?}"); + }; + let frame_size = f.frame_size.unwrap_or(0); + stack_depth += frame_size; + if verbose { + let indent = recurse_depth * 2; + println!( + " {:indent$}{addr:08x}: {} [+{frame_size} => {stack_depth}]", + "", + f.short_name, + indent = indent + ); + } + + if deepest + .as_ref() + .map(|(max_depth, _)| stack_depth > *max_depth) + .unwrap_or(true) + { + *deepest = Some((stack_depth, call_stack.to_owned())); + } + for j in &f.calls { + if call_stack.contains(j) { + // Skip recursive / mutually recursive calls, because we can't + // reason about them. + continue; + } else { + call_stack.push(*j); + recurse( + call_stack, + recurse_depth + 1, + stack_depth, + fns, + deepest, + verbose, + ); + call_stack.pop(); + } + } +} From e087d34402e3c3c44c903009c6f2456b47259944 Mon Sep 17 00:00:00 2001 From: James Munns Date: Sat, 4 Jul 2026 23:52:18 +0200 Subject: [PATCH 03/35] Start extracting portions of `get_max_stack` No functional changes expected. --- build/stack/src/lib.rs | 179 ++++++++++++++++++++++++++++------------- 1 file changed, 125 insertions(+), 54 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index f695713582..885f1efba7 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -12,6 +12,7 @@ use capstone::{ Capstone, InsnGroupId, InsnGroupType, arch::{ArchOperand, BuildsCapstone, BuildsCapstoneExtraMode, arm}, }; +use goblin::elf::{Elf, SectionHeader, Sym}; // We'll be packing everything into this data structure #[derive(Debug)] @@ -22,26 +23,20 @@ struct FunctionData { calls: BTreeSet, } -/// Estimates the maximum stack size for the given task -/// -/// This does not take dynamic function calls into account, which could cause -/// underestimation. Overestimation is less likely, but still may happen if -/// there are logically impossible call trees (e.g. `A -> B` and `B -> C`, but -/// `B` never calls `C` if called by `A`). -pub fn get_max_stack( - elf: &Path, - task_name: &str, - verbose: bool, -) -> Result> { - // Open the statically-linked ELF file - let data = std::fs::read(elf).context("could not open ELF file")?; - let elf = goblin::elf::Elf::parse(&data)?; +//////////////////////////////////////////////////////////////////////////////// +// Pulling data from the elf file +//////////////////////////////////////////////////////////////////////////////// - // Read the .stack_sizes section, which is an array of - // `(address: u32, stack size: unsigned leb128)` tuples - let sizes = elf::get_section_by_name(&elf, ".stack_sizes") +/// Read the .stack_sizes section, which is an array of +/// `(address: u32, stack size: unsigned leb128)` tuples. +fn extract_stack_sizes_section( + raw_elf: &[u8], + parsed_elf: &Elf<'_>, +) -> Result> { + let sizes = elf::get_section_by_name(parsed_elf, ".stack_sizes") .context("could not get .stack_sizes")?; - let mut sizes = &data[sizes.sh_offset as usize..][..sizes.sh_size as usize]; + let mut sizes = + &raw_elf[sizes.sh_offset as usize..][..sizes.sh_size as usize]; let mut addr_to_frame_size = BTreeMap::new(); while !sizes.is_empty() { let (addr, rest) = sizes.split_at(4); @@ -51,11 +46,18 @@ pub fn get_max_stack( addr_to_frame_size.insert(addr, size); } - // There are `$t` and `$d` symbols which indicate the beginning of text - // versus data in the `.text` region. We collect them into a `BTreeMap` - // here so that we can avoid trying to decode inline data words. + Ok(addr_to_frame_size) +} + +// There are `$t` and `$d` symbols which indicate the beginning of text +// versus data in the `.text` region. We collect them into a `BTreeMap` +// here so that we can avoid trying to decode inline data words. +fn extract_text_regions( + parsed_elf: &Elf<'_>, + task_name: &str, +) -> Result> { let mut text_regions = BTreeMap::new(); - for sym in elf.syms.iter() { + for sym in parsed_elf.syms.iter() { if sym.st_name == 0 || sym.st_size != 0 || sym.st_type() != goblin::elf::sym::STT_NOTYPE @@ -64,7 +66,7 @@ pub fn get_max_stack( } let addr = sym.st_value as u32; - let is_text = match elf.strtab.get_at(sym.st_name) { + let is_text = match parsed_elf.strtab.get_at(sym.st_name) { Some("$t") => true, Some("$d") => false, Some(_) => continue, @@ -74,6 +76,98 @@ pub fn get_max_stack( }; text_regions.insert(addr, is_text); } + Ok(text_regions) +} + +struct SymbolItem<'a> { + sym: Sym, + name: &'a str, + base_addr: u64, + text_region: &'a [u8], +} + +struct ChunkItem { + code: Vec, + addr: u64, +} + +impl SymbolItem<'_> { + // TODO: return `Vec<(u32, &[u8])>? + fn extract_instruction_chunks(&self, is_code: F) -> Vec + where + F: Fn(u32) -> bool, + { + // Split the text region into instruction-only chunks + let mut chunks = vec![]; + let mut chunk = None; + for (i, b) in self.text_region.iter().enumerate() { + let addr = self.base_addr + i as u64; + if is_code(addr as u32) { + chunk + .get_or_insert(ChunkItem { addr, code: vec![] }) + .code + .push(*b); + } else if let Some(c) = chunk.take() { + chunks.push(c); + } + } + chunks.extend(chunk); // don't forget the trailing chunk! + chunks + } +} + +fn fn_symbol_iter<'a>( + parsed_elf: &Elf<'a>, + text_section: &SectionHeader, + raw_elf: &'a [u8], +) -> impl Iterator> { + parsed_elf + .syms + .iter() + // We only care about named function symbols here + .filter(|s| s.st_name != 0) + .filter(|s| s.is_function()) + .filter(|s| s.st_size != 0) + // TODO: assert? + .filter_map(|s| { + // Clear the lowest bit, which indicates that the function contains + // thumb instructions (always true for our systems!) + let base_addr = s.st_value & !1; + + // Get the text region for this function + let offset = (base_addr - text_section.sh_addr + + text_section.sh_offset) as usize; + let text_region = &raw_elf[offset..][..s.st_size as usize]; + + // Bake into a handy collected symbol item + Some(SymbolItem { + sym: s, + name: parsed_elf.strtab.get_at(s.st_name)?, + base_addr, + text_region, + }) + }) +} + +/// Estimates the maximum stack size for the given task +/// +/// This does not take dynamic function calls into account, which could cause +/// underestimation. Overestimation is less likely, but still may happen if +/// there are logically impossible call trees (e.g. `A -> B` and `B -> C`, but +/// `B` never calls `C` if called by `A`). +pub fn get_max_stack( + elf: &Path, + task_name: &str, + verbose: bool, +) -> Result> { + // Open the statically-linked ELF file + let data = std::fs::read(elf).context("could not open ELF file")?; + let elf = goblin::elf::Elf::parse(&data)?; + + // Get sizes of stack frames by addr from the elf + let addr_to_frame_size = extract_stack_sizes_section(&data, &elf)?; + + let text_regions = extract_text_regions(&elf, task_name)?; let is_code = |addr| { let mut iter = text_regions.range(..=addr); *iter.next_back().unwrap().1 @@ -92,43 +186,20 @@ pub fn get_max_stack( // Disassemble each function, building a map of its call sites let mut fns = BTreeMap::new(); - for sym in elf.syms.iter() { - // We only care about named function symbols here - if sym.st_name == 0 || !sym.is_function() || sym.st_size == 0 { - continue; - } - - let Some(name) = elf.strtab.get_at(sym.st_name) else { - bail!("bad symbol in {task_name}: {}", sym.st_name); - }; - - // Clear the lowest bit, which indicates that the function contains - // thumb instructions (always true for our systems!) - let val = sym.st_value & !1; - let base_addr = val as u32; - - // Get the text region for this function - let offset = (val - text.sh_addr + text.sh_offset) as usize; - let text = &data[offset..][..sym.st_size as usize]; + for sym_item in fn_symbol_iter(&elf, text, &data) { + // TODO + let sym = sym_item.sym; + let base_addr = sym_item.base_addr as u32; + let name = sym_item.name; // Split the text region into instruction-only chunks - let mut chunks = vec![]; - let mut chunk = None; - for (i, b) in text.iter().enumerate() { - let addr = base_addr + i as u32; - if is_code(addr) { - chunk.get_or_insert((addr, vec![])).1.push(*b); - } else { - chunks.extend(chunk.take()); - } - } - chunks.extend(chunk); // don't forget the trailing chunk! + let chunks = sym_item.extract_instruction_chunks(is_code); let frame_size = addr_to_frame_size.get(&base_addr).copied(); let mut calls = BTreeSet::new(); - for (addr, chunk) in chunks { + for chunk_item in chunks { let instrs = cs - .disasm_all(&chunk, addr.into()) + .disasm_all(&chunk_item.code, chunk_item.addr) .map_err(|e| anyhow!("disassembly failed: {e:?}"))?; for (i, instr) in instrs.iter().enumerate() { let detail = cs.insn_detail(instr).map_err(|e| { From d340c3edad5d6bf4dcaf62d03c6c6a9dc6d92aeb Mon Sep 17 00:00:00 2001 From: James Munns Date: Sun, 5 Jul 2026 01:52:02 +0200 Subject: [PATCH 04/35] Some more refactoring --- build/stack/src/lib.rs | 118 ++++++++++++++++++++++++++--------------- 1 file changed, 75 insertions(+), 43 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 885f1efba7..ddc83ba43d 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -10,7 +10,10 @@ use std::{ use anyhow::{Context, Result, anyhow, bail}; use capstone::{ Capstone, InsnGroupId, InsnGroupType, - arch::{ArchOperand, BuildsCapstone, BuildsCapstoneExtraMode, arm}, + arch::{ + ArchDetail, BuildsCapstone, BuildsCapstoneExtraMode, DetailsArchInsn, + arm, + }, }; use goblin::elf::{Elf, SectionHeader, Sym}; @@ -57,14 +60,14 @@ fn extract_text_regions( task_name: &str, ) -> Result> { let mut text_regions = BTreeMap::new(); - for sym in parsed_elf.syms.iter() { - if sym.st_name == 0 - || sym.st_size != 0 - || sym.st_type() != goblin::elf::sym::STT_NOTYPE - { - continue; - } + let relevant = parsed_elf + .syms + .iter() + .filter(|s| s.st_name != 0) + .filter(|s| s.st_size == 0) + .filter(|s| s.st_type() == goblin::elf::sym::STT_NOTYPE); + for sym in relevant { let addr = sym.st_value as u32; let is_text = match parsed_elf.strtab.get_at(sym.st_name) { Some("$t") => true, @@ -128,7 +131,6 @@ fn fn_symbol_iter<'a>( .filter(|s| s.st_name != 0) .filter(|s| s.is_function()) .filter(|s| s.st_size != 0) - // TODO: assert? .filter_map(|s| { // Clear the lowest bit, which indicates that the function contains // thumb instructions (always true for our systems!) @@ -142,24 +144,19 @@ fn fn_symbol_iter<'a>( // Bake into a handy collected symbol item Some(SymbolItem { sym: s, - name: parsed_elf.strtab.get_at(s.st_name)?, + // TODO: assert? + name: parsed_elf.strtab.get_at(s.st_name).unwrap(), base_addr, text_region, }) }) } -/// Estimates the maximum stack size for the given task -/// -/// This does not take dynamic function calls into account, which could cause -/// underestimation. Overestimation is less likely, but still may happen if -/// there are logically impossible call trees (e.g. `A -> B` and `B -> C`, but -/// `B` never calls `C` if called by `A`). -pub fn get_max_stack( +fn extract_function_items( elf: &Path, task_name: &str, verbose: bool, -) -> Result> { +) -> Result> { // Open the statically-linked ELF file let data = std::fs::read(elf).context("could not open ELF file")?; let elf = goblin::elf::Elf::parse(&data)?; @@ -196,35 +193,67 @@ pub fn get_max_stack( let chunks = sym_item.extract_instruction_chunks(is_code); let frame_size = addr_to_frame_size.get(&base_addr).copied(); + let function_range = base_addr..base_addr + sym.st_size as u32; + + let name = rustc_demangle::demangle(name).to_string(); + + // Strip the trailing hash from the name for ease of printing + let short_name = if let Some(i) = name.rfind("::") { + &name[..i] + } else { + &name + } + .to_owned(); + + // Walk through each "chunk", which is an island of executable code + // inside of each function, and collect all the out-bound calls let mut calls = BTreeSet::new(); for chunk_item in chunks { let instrs = cs .disasm_all(&chunk_item.code, chunk_item.addr) .map_err(|e| anyhow!("disassembly failed: {e:?}"))?; + + // Walk through each instruction inside of each chunk for (i, instr) in instrs.iter().enumerate() { + // We need to get details for the instruction, which we should + // always have for well-formed programs let detail = cs.insn_detail(instr).map_err(|e| { anyhow!("could not get instruction details: {e}") })?; + let can_tail = frame_size == Some(0) && i == instrs.len() - 1; + let is_grp_call = + |g| g == &InsnGroupId(InsnGroupType::CS_GRP_CALL as u8); + let is_grp_jump = + |g| g == &InsnGroupId(InsnGroupType::CS_GRP_JUMP as u8); + // Detect tail calls, which are jumps at the final instruction // when the function itself has no stack frame. - let can_tail = frame_size == Some(0) && i == instrs.len() - 1; - if detail.groups().iter().any(|g| { - g == &InsnGroupId(InsnGroupType::CS_GRP_CALL as u8) - || (g == &InsnGroupId(InsnGroupType::CS_GRP_JUMP as u8) - && can_tail) - }) { + let is_tail_call = |g| is_grp_jump(g) && can_tail; + + let is_branching_instr = detail + .groups() + .iter() + .any(|g| is_grp_call(g) || is_tail_call(g)); + + if is_branching_instr { + // On Arm/Thumb, a jump always has some kind of operand, + // which is where we are jumping to. Get the last operand so + // we can determine if we can follow this. let arch = detail.arch_detail(); - let ops = arch.operands(); - let op = ops.last().unwrap_or_else(|| { + let ArchDetail::ArmDetail(details) = arch else { + panic!("Unsupported arch"); + }; + let op = details.operands().last().unwrap_or_else(|| { panic!("missing operand!"); }); - - let ArchOperand::ArmOperand(op) = op else { - panic!("bad operand type: {op:?}"); - }; // We can't resolve indirect calls, alas let arm::ArmOperandType::Imm(target) = op.op_type else { + if verbose { + println!( + "Failed to resolve indirect call in {name}!" + ); + } continue; }; let target = u32::try_from(target).unwrap(); @@ -232,25 +261,13 @@ pub fn get_max_stack( // Avoid recursive calls into the same function (or midway // into the function, which is a thing we've seen before! // it's weird!) - if !(base_addr..base_addr + sym.st_size as u32) - .contains(&target) - { + if !function_range.contains(&target) { calls.insert(target); } } } } - let name = rustc_demangle::demangle(name).to_string(); - - // Strip the trailing hash from the name for ease of printing - let short_name = if let Some(i) = name.rfind("::") { - &name[..i] - } else { - &name - } - .to_owned(); - fns.insert( base_addr, FunctionData { @@ -262,6 +279,21 @@ pub fn get_max_stack( ); } + Ok(fns) +} + +/// Estimates the maximum stack size for the given task +/// +/// This does not take dynamic function calls into account, which could cause +/// underestimation. Overestimation is less likely, but still may happen if +/// there are logically impossible call trees (e.g. `A -> B` and `B -> C`, but +/// `B` never calls `C` if called by `A`). +pub fn get_max_stack( + elf: &Path, + task_name: &str, + verbose: bool, +) -> Result> { + let fns = extract_function_items(elf, task_name, verbose)?; // Find stack sizes by traversing the graph if verbose { println!("finding stack sizes for {task_name}"); From 50641645c7cc9306a8b27f2078f3e46610a60b4b Mon Sep 17 00:00:00 2001 From: James Munns Date: Sun, 5 Jul 2026 02:37:56 +0200 Subject: [PATCH 05/35] Add a small harness for stack --- build/stack/src/lib.rs | 57 ++++++++++++++++++++++++-------------- build/stack/src/main.rs | 61 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 20 deletions(-) create mode 100644 build/stack/src/main.rs diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index ddc83ba43d..8e6b9e2a56 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -19,11 +19,23 @@ use goblin::elf::{Elf, SectionHeader, Sym}; // We'll be packing everything into this data structure #[derive(Debug)] -struct FunctionData { - name: String, - short_name: String, - frame_size: Option, - calls: BTreeSet, +pub struct FunctionData { + pub name: String, + pub short_name: String, + pub frame_size: Option, + pub calls: BTreeSet, +} + +struct SymbolItem<'a> { + sym: Sym, + name: &'a str, + base_addr: u64, + text_region: &'a [u8], +} + +struct ChunkItem { + code: Vec, + addr: u64, } //////////////////////////////////////////////////////////////////////////////// @@ -82,18 +94,6 @@ fn extract_text_regions( Ok(text_regions) } -struct SymbolItem<'a> { - sym: Sym, - name: &'a str, - base_addr: u64, - text_region: &'a [u8], -} - -struct ChunkItem { - code: Vec, - addr: u64, -} - impl SymbolItem<'_> { // TODO: return `Vec<(u32, &[u8])>? fn extract_instruction_chunks(&self, is_code: F) -> Vec @@ -152,11 +152,17 @@ fn fn_symbol_iter<'a>( }) } -fn extract_function_items( +pub struct FunctionReport { + pub function_items: BTreeMap, + pub addr_to_frame_size: BTreeMap, + pub names: BTreeMap, +} + +pub fn extract_function_items( elf: &Path, task_name: &str, verbose: bool, -) -> Result> { +) -> Result { // Open the statically-linked ELF file let data = std::fs::read(elf).context("could not open ELF file")?; let elf = goblin::elf::Elf::parse(&data)?; @@ -183,6 +189,7 @@ fn extract_function_items( // Disassemble each function, building a map of its call sites let mut fns = BTreeMap::new(); + let mut fn_names = BTreeMap::new(); for sym_item in fn_symbol_iter(&elf, text, &data) { // TODO let sym = sym_item.sym; @@ -196,6 +203,7 @@ fn extract_function_items( let function_range = base_addr..base_addr + sym.st_size as u32; let name = rustc_demangle::demangle(name).to_string(); + fn_names.insert(base_addr, name.clone()); // Strip the trailing hash from the name for ease of printing let short_name = if let Some(i) = name.rfind("::") { @@ -279,7 +287,11 @@ fn extract_function_items( ); } - Ok(fns) + Ok(FunctionReport { + function_items: fns, + addr_to_frame_size, + names: fn_names, + }) } /// Estimates the maximum stack size for the given task @@ -294,6 +306,11 @@ pub fn get_max_stack( verbose: bool, ) -> Result> { let fns = extract_function_items(elf, task_name, verbose)?; + get_max_stack_inner(fns) +} + +pub fn get_max_stack_inner(fns: FunctionReport) -> Result> { + let fns = fns.function_items; // Find stack sizes by traversing the graph if verbose { println!("finding stack sizes for {task_name}"); diff --git a/build/stack/src/main.rs b/build/stack/src/main.rs new file mode 100644 index 0000000000..acf1b68236 --- /dev/null +++ b/build/stack/src/main.rs @@ -0,0 +1,61 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use std::path::Path; + +use stack::FunctionReport; + +fn main() { + let file = "../../target/gimlet-c/dist/host_sp_comms.tmp"; + let items = + stack::extract_function_items(Path::new(file), "host_sp_comms", false) + .unwrap(); + + let FunctionReport { + function_items, + addr_to_frame_size, + names, + } = items; + + for (addr, name) in names.iter() { + println!("{addr:08X} - {name}"); + } + + println!(); + + for (addr, size) in addr_to_frame_size.iter() { + println!("{addr:08X} - {size}"); + } + + println!(); + + for (addr, item) in function_items { + print!("{addr:08X} "); + + if let Some(n) = names.get(&addr) { + print!("{n} "); + } else { + print!("anon@{addr:08X} "); + } + if let Some(n) = addr_to_frame_size.get(&addr) { + println!("- {n} bytes"); + } else { + println!("- ??? bytes"); + } + + for subitem in item.calls { + print!(" => "); + if let Some(n) = names.get(&subitem) { + print!("{n} "); + } else { + print!("anon@{subitem:08X} "); + } + if let Some(n) = addr_to_frame_size.get(&subitem) { + println!("- {n} bytes"); + } else { + println!("- ??? bytes"); + } + } + } +} From c7990a9ac934283ef1b479148c8558d179a8fe37 Mon Sep 17 00:00:00 2001 From: James Munns Date: Sun, 5 Jul 2026 19:31:08 +0200 Subject: [PATCH 06/35] More touch-ups --- build/stack/src/lib.rs | 91 +++++++++++++++++++++++++---------------- build/stack/src/main.rs | 32 +++++++++++++-- 2 files changed, 85 insertions(+), 38 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 8e6b9e2a56..aed087ff05 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -4,12 +4,13 @@ use std::{ collections::{BTreeMap, BTreeSet}, + ops::Range, path::Path, }; use anyhow::{Context, Result, anyhow, bail}; use capstone::{ - Capstone, InsnGroupId, InsnGroupType, + Capstone, Insn, InsnDetail, InsnGroupId, InsnGroupType, arch::{ ArchDetail, BuildsCapstone, BuildsCapstoneExtraMode, DetailsArchInsn, arm, @@ -28,7 +29,7 @@ pub struct FunctionData { struct SymbolItem<'a> { sym: Sym, - name: &'a str, + mangled_name: &'a str, base_addr: u64, text_region: &'a [u8], } @@ -38,6 +39,12 @@ struct ChunkItem { addr: u64, } +pub struct FunctionReport { + pub function_items: BTreeMap, + pub addr_to_frame_size: BTreeMap, + pub names: BTreeMap, +} + //////////////////////////////////////////////////////////////////////////////// // Pulling data from the elf file //////////////////////////////////////////////////////////////////////////////// @@ -95,6 +102,11 @@ fn extract_text_regions( } impl SymbolItem<'_> { + fn addr_range(&self) -> Range { + let base_addr = self.base_addr as u32; + base_addr..base_addr + self.sym.st_size as u32 + } + // TODO: return `Vec<(u32, &[u8])>? fn extract_instruction_chunks(&self, is_code: F) -> Vec where @@ -145,19 +157,13 @@ fn fn_symbol_iter<'a>( Some(SymbolItem { sym: s, // TODO: assert? - name: parsed_elf.strtab.get_at(s.st_name).unwrap(), + mangled_name: parsed_elf.strtab.get_at(s.st_name).unwrap(), base_addr, text_region, }) }) } -pub struct FunctionReport { - pub function_items: BTreeMap, - pub addr_to_frame_size: BTreeMap, - pub names: BTreeMap, -} - pub fn extract_function_items( elf: &Path, task_name: &str, @@ -191,44 +197,45 @@ pub fn extract_function_items( let mut fns = BTreeMap::new(); let mut fn_names = BTreeMap::new(); for sym_item in fn_symbol_iter(&elf, text, &data) { - // TODO - let sym = sym_item.sym; let base_addr = sym_item.base_addr as u32; - let name = sym_item.name; - - // Split the text region into instruction-only chunks - let chunks = sym_item.extract_instruction_chunks(is_code); - + let name = sym_item.mangled_name; + // This is the stack frame size of the current function let frame_size = addr_to_frame_size.get(&base_addr).copied(); - let function_range = base_addr..base_addr + sym.st_size as u32; - + // This is the range of addresses comprising this function + let function_range = sym_item.addr_range(); + // Demangled and short name of this function let name = rustc_demangle::demangle(name).to_string(); + let short_name = name_shortener(&name); + fn_names.insert(base_addr, name.clone()); - // Strip the trailing hash from the name for ease of printing - let short_name = if let Some(i) = name.rfind("::") { - &name[..i] - } else { - &name - } - .to_owned(); + // Split the text region into instruction-only chunks + let chunks = sym_item.extract_instruction_chunks(is_code); // Walk through each "chunk", which is an island of executable code - // inside of each function, and collect all the out-bound calls + // inside of each function, and collect all the out-bound calls. We + // disassemble chunks rather than functions, as functions might contain + // puddles of inline data which we don't want to (mis)-disassemble. let mut calls = BTreeSet::new(); for chunk_item in chunks { let instrs = cs .disasm_all(&chunk_item.code, chunk_item.addr) .map_err(|e| anyhow!("disassembly failed: {e:?}"))?; - // Walk through each instruction inside of each chunk - for (i, instr) in instrs.iter().enumerate() { - // We need to get details for the instruction, which we should - // always have for well-formed programs - let detail = cs.insn_detail(instr).map_err(|e| { - anyhow!("could not get instruction details: {e}") - })?; + // We need to get details for the instruction, which we should + // always have for well-formed programs + let instrs: Vec<&Insn<'_>> = instrs.iter().collect(); + let details: Vec> = instrs + .iter() + .map(|instr| { + cs.insn_detail(instr).map_err(|e| { + anyhow!("could not get instruction details: {e}") + }) + }) + .collect::>()?; + // Walk through each instruction inside of each chunk + for (i, detail) in details.iter().enumerate() { let can_tail = frame_size == Some(0) && i == instrs.len() - 1; let is_grp_call = |g| g == &InsnGroupId(InsnGroupType::CS_GRP_CALL as u8); @@ -306,10 +313,14 @@ pub fn get_max_stack( verbose: bool, ) -> Result> { let fns = extract_function_items(elf, task_name, verbose)?; - get_max_stack_inner(fns) + get_max_stack_inner(fns, task_name, verbose) } -pub fn get_max_stack_inner(fns: FunctionReport) -> Result> { +pub fn get_max_stack_inner( + fns: FunctionReport, + task_name: &str, + verbose: bool, +) -> Result> { let fns = fns.function_items; // Find stack sizes by traversing the graph if verbose { @@ -337,6 +348,16 @@ pub fn get_max_stack_inner(fns: FunctionReport) -> Result> { Ok(out) } +fn name_shortener(name: &str) -> String { + // Strip the trailing hash from the name for ease of printing + if let Some(i) = name.rfind("::") { + &name[..i] + } else { + &name + } + .to_owned() +} + fn recurse( call_stack: &mut Vec, recurse_depth: usize, diff --git a/build/stack/src/main.rs b/build/stack/src/main.rs index acf1b68236..e4563cade7 100644 --- a/build/stack/src/main.rs +++ b/build/stack/src/main.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use std::path::Path; +use std::{collections::BTreeSet, path::Path}; use stack::FunctionReport; @@ -30,7 +30,7 @@ fn main() { println!(); - for (addr, item) in function_items { + for (addr, item) in function_items.iter() { print!("{addr:08X} "); if let Some(n) = names.get(&addr) { @@ -44,7 +44,7 @@ fn main() { println!("- ??? bytes"); } - for subitem in item.calls { + for subitem in item.calls.iter() { print!(" => "); if let Some(n) = names.get(&subitem) { print!("{n} "); @@ -58,4 +58,30 @@ fn main() { } } } + + let as_fi = function_items.keys().copied().collect::>(); + let as_a2f = addr_to_frame_size.keys().copied().collect::>(); + let as_names = names.keys().copied().collect::>(); + + let fi_a2f = as_fi.difference(&as_a2f); + let a2f_names = as_a2f.difference(&as_names); + let names_fi = as_names.difference(&as_fi); + + println!("fi_a2f:"); + for addr in fi_a2f { + println!("- {addr:08X}"); + } + println!(); + + println!("a2f_names:"); + for addr in a2f_names { + println!("- {addr:08X}"); + } + println!(); + + println!("names_fi:"); + for addr in names_fi { + println!("- {addr:08X}"); + } + println!(); } From aa26151f060c32145fd85588ae5a957d8abcb832 Mon Sep 17 00:00:00 2001 From: James Munns Date: Sun, 5 Jul 2026 21:24:36 +0200 Subject: [PATCH 07/35] Add fix for missing stack, add some tooling for catching more --- build/stack/src/lib.rs | 144 ++++++++++++++++++++++++++++++++++++++-- build/stack/src/main.rs | 112 ++++++++++++++++--------------- 2 files changed, 198 insertions(+), 58 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index aed087ff05..a524e79502 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -6,6 +6,7 @@ use std::{ collections::{BTreeMap, BTreeSet}, ops::Range, path::Path, + rc::Rc, }; use anyhow::{Context, Result, anyhow, bail}; @@ -241,15 +242,19 @@ pub fn extract_function_items( |g| g == &InsnGroupId(InsnGroupType::CS_GRP_CALL as u8); let is_grp_jump = |g| g == &InsnGroupId(InsnGroupType::CS_GRP_JUMP as u8); + let is_grp_rel = |g| { + g == &InsnGroupId( + InsnGroupType::CS_GRP_BRANCH_RELATIVE as u8, + ) + }; // Detect tail calls, which are jumps at the final instruction // when the function itself has no stack frame. let is_tail_call = |g| is_grp_jump(g) && can_tail; - let is_branching_instr = detail - .groups() - .iter() - .any(|g| is_grp_call(g) || is_tail_call(g)); + let is_branching_instr = detail.groups().iter().any(|g| { + is_grp_call(g) || is_grp_rel(g) || is_tail_call(g) + }); if is_branching_instr { // On Arm/Thumb, a jump always has some kind of operand, @@ -358,6 +363,137 @@ fn name_shortener(name: &str) -> String { .to_owned() } +pub struct ResolvedNode { + pub addr: u32, + pub name: String, + pub local_size: Option, + pub max_children: u64, + pub children: BTreeMap>, +} + +impl ResolvedNode { + pub fn debug_all(&self) { + self.debug_all_depth(0, 0); + } + + pub fn debug_all_depth(&self, depth: usize, current_stack: u64) { + let frame_size = self.local_size.unwrap_or(0); + let stack_depth = current_stack + frame_size; + for _ in 0..depth { + print!(" "); + } + println!( + "- 0x{:08X} {} [+{frame_size} => {stack_depth}]", + self.addr, self.name, + ); + for (_addr, child) in self.children.iter() { + child.debug_all_depth(depth + 1, current_stack + frame_size); + } + } + + /// Used to determine if `other` should be discarded + pub fn is_same_or_child_of(&self, other: &Self) -> bool { + if other.addr == self.addr { + return true; + } + for (_caddr, child) in other.children.iter() { + if self.is_same_or_child_of(child) { + return true; + } + } + false + } + + pub fn max_stack(&self) -> u64 { + self.local_size.unwrap_or(0) + self.max_children + } + + pub fn worst_chain(self: &Rc) -> Vec> { + let mut chain = vec![]; + self.worst_chain_inner(&mut chain); + chain + } + + fn worst_chain_inner(self: &Rc, chain: &mut Vec>) { + chain.push(self.clone()); + if let Some(child) = self + .children + .iter() + .find(|c| c.1.max_stack() == self.max_children) + { + child.1.worst_chain_inner(chain) + } + } +} + +pub struct Resolver { + pub call_stack: Vec, + pub all_resolved: BTreeMap>, + pub fn_items: BTreeMap, +} + +impl Resolver { + pub fn new(fn_items: BTreeMap) -> Self { + Self { + call_stack: vec![], + all_resolved: BTreeMap::new(), + fn_items, + } + } + + pub fn resolve_by_name(&mut self, entry: &str) -> Result> { + let Some(item) = self.fn_items.iter().find(|(_k, v)| &v.name == entry) + else { + bail!("Not found"); + }; + let addr = *(item.0); + self.resolve_addr(addr) + } + + pub fn resolve_addr(&mut self, addr: u32) -> Result> { + // Have we already resolved this node? + if let Some(node) = self.all_resolved.get(&addr) { + return Ok(node.clone()); + } + + // no, we havent. Get the node info from the fn data + let Some(item) = self.fn_items.get(&addr) else { + bail!("no function data"); + }; + self.call_stack.push(addr); + let children = item.calls.clone(); + let name = item.name.clone(); + let local_size = item.frame_size; + + let mut res_children = BTreeMap::new(); + let mut max_children = 0; + for child in children { + if self.call_stack.contains(&child) { + bail!("Refusing to handle recursive"); + } + + // Resolve the child + let rchild = self.resolve_addr(child)?; + + let ttl_child = + rchild.local_size.unwrap_or(0) + rchild.max_children; + max_children = max_children.max(ttl_child); + res_children.insert(child, rchild); + } + + let new_node = Rc::new(ResolvedNode { + addr, + name, + local_size, + max_children, + children: res_children, + }); + self.all_resolved.insert(addr, new_node.clone()); + self.call_stack.pop(); + Ok(new_node) + } +} + fn recurse( call_stack: &mut Vec, recurse_depth: usize, diff --git a/build/stack/src/main.rs b/build/stack/src/main.rs index e4563cade7..730535e37e 100644 --- a/build/stack/src/main.rs +++ b/build/stack/src/main.rs @@ -2,9 +2,9 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use std::{collections::BTreeSet, path::Path}; +use std::{path::Path, rc::Rc}; -use stack::FunctionReport; +use stack::{FunctionReport, ResolvedNode, Resolver}; fn main() { let file = "../../target/gimlet-c/dist/host_sp_comms.tmp"; @@ -18,70 +18,74 @@ fn main() { names, } = items; - for (addr, name) in names.iter() { - println!("{addr:08X} - {name}"); - } - - println!(); + let mut resolver = Resolver::new(function_items); + let node = resolver.resolve_by_name("_start").unwrap(); + node.debug_all(); + let mut missing = vec![]; for (addr, size) in addr_to_frame_size.iter() { - println!("{addr:08X} - {size}"); + let name = names.get(addr).unwrap(); + if !resolver.all_resolved.contains_key(addr) { + println!("WARN: missing {name} => {size}"); + missing.push((*addr, name)); + } } - println!(); - - for (addr, item) in function_items.iter() { - print!("{addr:08X} "); - - if let Some(n) = names.get(&addr) { - print!("{n} "); - } else { - print!("anon@{addr:08X} "); - } - if let Some(n) = addr_to_frame_size.get(&addr) { - println!("- {n} bytes"); - } else { - println!("- ??? bytes"); - } + let mut found = vec![]; + for (addr, name) in missing { + println!("probing {name}..."); + let node = resolver.resolve_addr(addr).unwrap(); + println!(" {:?} + {}", node.local_size, node.max_children); + node.debug_all(); + println!("---"); + found.push(node); + } - for subitem in item.calls.iter() { - print!(" => "); - if let Some(n) = names.get(&subitem) { - print!("{n} "); + println!("Shaking down..."); + let mut last_len = found.len(); + loop { + let mut to_keep = vec![]; + while let Some(val) = found.pop() { + if found + .iter() + .any(|f: &Rc| f.is_same_or_child_of(&val)) + || to_keep + .iter() + .any(|f: &Rc| f.is_same_or_child_of(&val)) + { + continue; } else { - print!("anon@{subitem:08X} "); - } - if let Some(n) = addr_to_frame_size.get(&subitem) { - println!("- {n} bytes"); - } else { - println!("- ??? bytes"); + to_keep.push(val); } } + found = to_keep; + if found.len() == last_len { + break; + } else { + last_len = found.len(); + } } - let as_fi = function_items.keys().copied().collect::>(); - let as_a2f = addr_to_frame_size.keys().copied().collect::>(); - let as_names = names.keys().copied().collect::>(); - - let fi_a2f = as_fi.difference(&as_a2f); - let a2f_names = as_a2f.difference(&as_names); - let names_fi = as_names.difference(&as_fi); - - println!("fi_a2f:"); - for addr in fi_a2f { - println!("- {addr:08X}"); + println!("--- shook ---"); + let mut sum = 0; + for n in found.iter() { + println!("{} - {:?} + {}", n.name, n.local_size, n.max_children); + n.debug_all(); + println!("---"); + sum += n.max_stack(); } - println!(); - println!("a2f_names:"); - for addr in a2f_names { - println!("- {addr:08X}"); - } println!(); - - println!("names_fi:"); - for addr in names_fi { - println!("- {addr:08X}"); + println!("Worst chain:"); + let chain = node.worst_chain(); + for n in chain { + println!("{} - {}", n.name, n.max_stack()); } - println!(); + + println!( + "max stack: {} + fudge ({}) = {}", + node.max_stack(), + sum, + node.max_stack() + sum + ); } From 184b17acc60124a18a99f456422c3bfc4cf1b074 Mon Sep 17 00:00:00 2001 From: James Munns Date: Mon, 6 Jul 2026 01:42:56 +0200 Subject: [PATCH 08/35] Wire up more of the new resolver --- build/stack/src/lib.rs | 109 ++++++++++++++++++++++++++++++++++++---- build/stack/src/main.rs | 20 +++++--- 2 files changed, 112 insertions(+), 17 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index a524e79502..6c49fd85f6 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -236,7 +236,9 @@ pub fn extract_function_items( .collect::>()?; // Walk through each instruction inside of each chunk - for (i, detail) in details.iter().enumerate() { + for (i, (_instr, detail)) in + instrs.iter().zip(details.iter()).enumerate() + { let can_tail = frame_size == Some(0) && i == instrs.len() - 1; let is_grp_call = |g| g == &InsnGroupId(InsnGroupType::CS_GRP_CALL as u8); @@ -251,8 +253,12 @@ pub fn extract_function_items( // Detect tail calls, which are jumps at the final instruction // when the function itself has no stack frame. let is_tail_call = |g| is_grp_jump(g) && can_tail; - let is_branching_instr = detail.groups().iter().any(|g| { + // Check if this is now not needed anymore + if !is_grp_rel(g) && is_tail_call(g) { + panic!("this check is NOT obsolete!"); + } + is_grp_call(g) || is_grp_rel(g) || is_tail_call(g) }); @@ -268,6 +274,10 @@ pub fn extract_function_items( panic!("missing operand!"); }); // We can't resolve indirect calls, alas + // + // TODO: We could consider keeping track of register ops + // and potentially figure out the location of the register + // based jump here let arm::ArmOperandType::Imm(target) = op.op_type else { if verbose { println!( @@ -316,6 +326,83 @@ pub fn get_max_stack( elf: &Path, task_name: &str, verbose: bool, +) -> Result> { + let fns = extract_function_items(elf, task_name, verbose)?; + let mut resolver = Resolver::new(fns.function_items); + let node = resolver.resolve_by_name("_start")?; + let chain = node.worst_chain(); + let mut chain_compat = chain + .into_iter() + .map(|n| (n.local_size.unwrap_or(0), n.name.clone())) + .collect::>(); + + let missing = find_missing_nodes( + &mut resolver, + fns.addr_to_frame_size.keys().copied(), + ); + + let fake_items = missing + .iter() + .map(|n| (n.max_stack(), format!("missing::{}", n.name))); + chain_compat.extend(fake_items); + + Ok(chain_compat) +} + +fn find_missing_nodes( + resolver: &mut Resolver, + all_addrs: impl Iterator, +) -> Vec> { + let mut missing = vec![]; + for addr in all_addrs { + if !resolver.all_resolved.contains_key(&addr) { + missing.push(addr); + } + } + + let mut found = vec![]; + for addr in missing { + let node = resolver.resolve_addr(addr).unwrap(); + found.push(node); + } + + let mut last_len = found.len(); + loop { + let mut to_keep = vec![]; + while let Some(val) = found.pop() { + if found + .iter() + .any(|f: &Rc| f.is_same_or_child_of(&val)) + || to_keep + .iter() + .any(|f: &Rc| f.is_same_or_child_of(&val)) + { + continue; + } else { + to_keep.push(val); + } + } + found = to_keep; + if found.len() == last_len { + break; + } else { + last_len = found.len(); + } + } + + found +} + +/// Estimates the maximum stack size for the given task +/// +/// This does not take dynamic function calls into account, which could cause +/// underestimation. Overestimation is less likely, but still may happen if +/// there are logically impossible call trees (e.g. `A -> B` and `B -> C`, but +/// `B` never calls `C` if called by `A`). +pub fn get_max_stack_old( + elf: &Path, + task_name: &str, + verbose: bool, ) -> Result> { let fns = extract_function_items(elf, task_name, verbose)?; get_max_stack_inner(fns, task_name, verbose) @@ -377,21 +464,24 @@ impl ResolvedNode { } pub fn debug_all_depth(&self, depth: usize, current_stack: u64) { - let frame_size = self.local_size.unwrap_or(0); - let stack_depth = current_stack + frame_size; + let frame_size = self.local_size; + let stack_depth = current_stack + frame_size.unwrap_or(0); for _ in 0..depth { print!(" "); } println!( - "- 0x{:08X} {} [+{frame_size} => {stack_depth}]", + "- 0x{:08X} {} [+{frame_size:?} => {stack_depth}]", self.addr, self.name, ); for (_addr, child) in self.children.iter() { - child.debug_all_depth(depth + 1, current_stack + frame_size); + child.debug_all_depth( + depth + 1, + current_stack + frame_size.unwrap_or(0), + ); } } - /// Used to determine if `other` should be discarded + /// Used to determine if `self` could be potentially discarded as a dupe pub fn is_same_or_child_of(&self, other: &Self) -> bool { if other.addr == self.addr { return true; @@ -469,14 +559,13 @@ impl Resolver { let mut max_children = 0; for child in children { if self.call_stack.contains(&child) { - bail!("Refusing to handle recursive"); + bail!("Refusing to handle recursion"); } // Resolve the child let rchild = self.resolve_addr(child)?; - let ttl_child = - rchild.local_size.unwrap_or(0) + rchild.max_children; + let ttl_child = rchild.max_stack(); max_children = max_children.max(ttl_child); res_children.insert(child, rchild); } diff --git a/build/stack/src/main.rs b/build/stack/src/main.rs index 730535e37e..4d09dc18a6 100644 --- a/build/stack/src/main.rs +++ b/build/stack/src/main.rs @@ -20,8 +20,10 @@ fn main() { let mut resolver = Resolver::new(function_items); let node = resolver.resolve_by_name("_start").unwrap(); - node.debug_all(); + // node.debug_all(); + println!(); + println!(); let mut missing = vec![]; for (addr, size) in addr_to_frame_size.iter() { let name = names.get(addr).unwrap(); @@ -31,13 +33,11 @@ fn main() { } } + println!(); let mut found = vec![]; for (addr, name) in missing { println!("probing {name}..."); let node = resolver.resolve_addr(addr).unwrap(); - println!(" {:?} + {}", node.local_size, node.max_children); - node.debug_all(); - println!("---"); found.push(node); } @@ -70,22 +70,28 @@ fn main() { let mut sum = 0; for n in found.iter() { println!("{} - {:?} + {}", n.name, n.local_size, n.max_children); - n.debug_all(); - println!("---"); sum += n.max_stack(); } println!(); + println!("------------"); println!("Worst chain:"); + println!("------------"); let chain = node.worst_chain(); + let mut worst_sum = 0; for n in chain { - println!("{} - {}", n.name, n.max_stack()); + worst_sum += n.local_size.unwrap_or(0); + println!("{} - [+{:?} => {}]", n.name, n.local_size, worst_sum); } + println!(); println!( "max stack: {} + fudge ({}) = {}", node.max_stack(), sum, node.max_stack() + sum ); + + println!(); + println!("{} functions resolved.", resolver.all_resolved.len()); } From 24c2dbe9d7b038ba5ae27d3a7c9d897c23e88dcc Mon Sep 17 00:00:00 2001 From: James Munns Date: Mon, 6 Jul 2026 16:52:09 +0200 Subject: [PATCH 09/35] Final docs/refactoring/reorg pass --- build/stack/src/lib.rs | 660 +++++++++++++++++++---------------------- 1 file changed, 307 insertions(+), 353 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 6c49fd85f6..2963a9b7d5 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -19,95 +19,252 @@ use capstone::{ }; use goblin::elf::{Elf, SectionHeader, Sym}; -// We'll be packing everything into this data structure +/// Information derived about a given function #[derive(Debug)] pub struct FunctionData { pub name: String, pub short_name: String, pub frame_size: Option, pub calls: BTreeSet, + pub missing_calls: usize, } -struct SymbolItem<'a> { +/// The resulting report of all metadata obtained for functions after +/// extracting from the elf, but before attempting to "resolve" or visit each +/// function to determine max stack usage. +pub struct FunctionReport { + pub function_items: BTreeMap, + pub addr_to_frame_size: BTreeMap, + pub names: BTreeMap, +} + +/// A function that has been visited and "filled in" by the [`Resolver`]. +/// +/// Typically stored in an [`Rc`] as each node may appear multiple times across +/// the call graph of a program. +pub struct ResolvedNode { + pub addr: u32, + pub name: String, + pub local_size: Option, + pub max_children: u64, + pub children: BTreeMap>, +} + +/// A tool for turning a [`FunctionReport`] into a hydrated call graph. +/// +/// This visits a call graph of [`FunctionData`] recursively, turning each node +/// into a memoized `Rc`, and keeps a mapping of all nodes by +/// address. +pub struct Resolver { + pub call_stack: Vec, + pub all_resolved: BTreeMap>, + pub fn_items: BTreeMap, +} + +/// Information derived about a given symbol +struct SymbolData<'a> { sym: Sym, mangled_name: &'a str, base_addr: u64, text_region: &'a [u8], } +/// Information about a "chunk", which is a portion of a function containing +/// executable code (and not inlined data) struct ChunkItem { code: Vec, addr: u64, } -pub struct FunctionReport { - pub function_items: BTreeMap, - pub addr_to_frame_size: BTreeMap, - pub names: BTreeMap, -} +impl ResolvedNode { + /// Print the entire stack trace to stdout + pub fn debug_all(&self) { + self.debug_all_depth(0, 0); + } -//////////////////////////////////////////////////////////////////////////////// -// Pulling data from the elf file -//////////////////////////////////////////////////////////////////////////////// + /// Print the entire stack trace to stdout, with a given starting depth and + /// current stack usage at the present depth + pub fn debug_all_depth(&self, depth: usize, current_stack: u64) { + let frame_size = self.local_size; + let stack_depth = current_stack + frame_size.unwrap_or(0); + for _ in 0..depth { + print!(" "); + } + println!( + "- 0x{:08X} {} [+{frame_size:?} => {stack_depth}]", + self.addr, self.name, + ); + for (_addr, child) in self.children.iter() { + child.debug_all_depth( + depth + 1, + current_stack + frame_size.unwrap_or(0), + ); + } + } -/// Read the .stack_sizes section, which is an array of -/// `(address: u32, stack size: unsigned leb128)` tuples. -fn extract_stack_sizes_section( - raw_elf: &[u8], - parsed_elf: &Elf<'_>, -) -> Result> { - let sizes = elf::get_section_by_name(parsed_elf, ".stack_sizes") - .context("could not get .stack_sizes")?; - let mut sizes = - &raw_elf[sizes.sh_offset as usize..][..sizes.sh_size as usize]; - let mut addr_to_frame_size = BTreeMap::new(); - while !sizes.is_empty() { - let (addr, rest) = sizes.split_at(4); - let addr = u32::from_le_bytes(addr.try_into().unwrap()); - sizes = rest; - let size = leb128::read::unsigned(&mut sizes)?; - addr_to_frame_size.insert(addr, size); + /// Used to determine if `self` could be potentially discarded as a dupe + pub fn is_same_or_child_of(&self, other: &Self) -> bool { + if other.addr == self.addr { + return true; + } + for (_caddr, child) in other.children.iter() { + if self.is_same_or_child_of(child) { + return true; + } + } + false } - Ok(addr_to_frame_size) + /// The max stack used by this node, considering this node's stack usage + /// (if known), and the max stack usage of this node's largest child. + pub fn max_stack(&self) -> u64 { + self.local_size.unwrap_or(0) + self.max_children + } + + /// The worst case call chain starting from this node + pub fn worst_chain(self: &Rc) -> Vec> { + let mut chain = vec![]; + self.worst_chain_inner(&mut chain); + chain + } + + /// used for recursively calculating [`Self::worst_chain_inner`] + fn worst_chain_inner(self: &Rc, chain: &mut Vec>) { + chain.push(self.clone()); + if let Some(child) = self + .children + .iter() + .find(|c| c.1.max_stack() == self.max_children) + { + child.1.worst_chain_inner(chain) + } + } } -// There are `$t` and `$d` symbols which indicate the beginning of text -// versus data in the `.text` region. We collect them into a `BTreeMap` -// here so that we can avoid trying to decode inline data words. -fn extract_text_regions( - parsed_elf: &Elf<'_>, - task_name: &str, -) -> Result> { - let mut text_regions = BTreeMap::new(); - let relevant = parsed_elf - .syms - .iter() - .filter(|s| s.st_name != 0) - .filter(|s| s.st_size == 0) - .filter(|s| s.st_type() == goblin::elf::sym::STT_NOTYPE); +impl Resolver { + /// Create a new resolver from the given map of [`FunctionData`] items + pub fn new(fn_items: BTreeMap) -> Self { + Self { + call_stack: vec![], + all_resolved: BTreeMap::new(), + fn_items, + } + } - for sym in relevant { - let addr = sym.st_value as u32; - let is_text = match parsed_elf.strtab.get_at(sym.st_name) { - Some("$t") => true, - Some("$d") => false, - Some(_) => continue, - None => { - bail!("bad symbol in {task_name}: {}", sym.st_name); - } + /// Attempt to resolve a function by name into a [`ResolvedNode`]. + pub fn resolve_by_name(&mut self, entry: &str) -> Result> { + let Some(item) = self.fn_items.iter().find(|(_k, v)| &v.name == entry) + else { + bail!("Not found"); }; - text_regions.insert(addr, is_text); + let addr = *(item.0); + self.resolve_addr(addr) + } + + /// Resolve a function by address into a [`ResolvedNode`]. + pub fn resolve_addr(&mut self, addr: u32) -> Result> { + // Have we already resolved this node? + if let Some(node) = self.all_resolved.get(&addr) { + return Ok(node.clone()); + } + + // no, we havent. Get the node info from the fn data + let Some(item) = self.fn_items.get(&addr) else { + bail!("no function data"); + }; + self.call_stack.push(addr); + let children = item.calls.clone(); + let name = item.name.clone(); + let local_size = item.frame_size; + + let mut res_children = BTreeMap::new(); + let mut max_children = 0; + for child in children { + if self.call_stack.contains(&child) { + bail!("Refusing to handle recursion"); + } + + // Resolve the child + let rchild = self.resolve_addr(child)?; + + let ttl_child = rchild.max_stack(); + max_children = max_children.max(ttl_child); + res_children.insert(child, rchild); + } + + let new_node = Rc::new(ResolvedNode { + addr, + name, + local_size, + max_children, + children: res_children, + }); + self.all_resolved.insert(addr, new_node.clone()); + self.call_stack.pop(); + Ok(new_node) + } + + /// Finds all functions *not* already resolved by the given [`Resolver`] and + /// attempts to resolve them. + /// + /// This finds functions that exist in the elf file, but were not visited + /// while resolving the call graph, likely due to being called indirectly or + /// through vtable methods. + fn find_missing_nodes( + &mut self, + all_addrs: impl Iterator, + ) -> Vec> { + let mut missing = vec![]; + for addr in all_addrs { + if !self.all_resolved.contains_key(&addr) { + missing.push(addr); + } + } + + let mut found = vec![]; + for addr in missing { + let node = self.resolve_addr(addr).unwrap(); + found.push(node); + } + + let mut last_len = found.len(); + loop { + let mut to_keep = vec![]; + while let Some(val) = found.pop() { + if found + .iter() + .any(|f: &Rc| f.is_same_or_child_of(&val)) + || to_keep + .iter() + .any(|f: &Rc| f.is_same_or_child_of(&val)) + { + continue; + } else { + to_keep.push(val); + } + } + found = to_keep; + if found.len() == last_len { + break; + } else { + last_len = found.len(); + } + } + + found } - Ok(text_regions) } -impl SymbolItem<'_> { +impl SymbolData<'_> { + /// The range of addresses contained within this symbol fn addr_range(&self) -> Range { let base_addr = self.base_addr as u32; base_addr..base_addr + self.sym.st_size as u32 } + /// An array of "Chunks", which are each of the executable portions of this + /// symbol. + // // TODO: return `Vec<(u32, &[u8])>? fn extract_instruction_chunks(&self, is_code: F) -> Vec where @@ -132,39 +289,12 @@ impl SymbolItem<'_> { } } -fn fn_symbol_iter<'a>( - parsed_elf: &Elf<'a>, - text_section: &SectionHeader, - raw_elf: &'a [u8], -) -> impl Iterator> { - parsed_elf - .syms - .iter() - // We only care about named function symbols here - .filter(|s| s.st_name != 0) - .filter(|s| s.is_function()) - .filter(|s| s.st_size != 0) - .filter_map(|s| { - // Clear the lowest bit, which indicates that the function contains - // thumb instructions (always true for our systems!) - let base_addr = s.st_value & !1; - - // Get the text region for this function - let offset = (base_addr - text_section.sh_addr - + text_section.sh_offset) as usize; - let text_region = &raw_elf[offset..][..s.st_size as usize]; - - // Bake into a handy collected symbol item - Some(SymbolItem { - sym: s, - // TODO: assert? - mangled_name: parsed_elf.strtab.get_at(s.st_name).unwrap(), - base_addr, - text_region, - }) - }) -} +//////////////////////////////////////////////////////////////////////////////// +// Public methods +//////////////////////////////////////////////////////////////////////////////// +/// Load and parse the `elf` file at the given path, producing a report of +/// metadata about all functions in the `elf` file. pub fn extract_function_items( elf: &Path, task_name: &str, @@ -218,6 +348,7 @@ pub fn extract_function_items( // disassemble chunks rather than functions, as functions might contain // puddles of inline data which we don't want to (mis)-disassemble. let mut calls = BTreeSet::new(); + let mut missing_calls = 0; for chunk_item in chunks { let instrs = cs .disasm_all(&chunk_item.code, chunk_item.addr) @@ -284,6 +415,14 @@ pub fn extract_function_items( "Failed to resolve indirect call in {name}!" ); } + // TODO: we record that we are missing a call, and + // ideally we would plug the worst of the missing funcs + // here. Unfortunately, that means we end up needing to + // invalidate the RC, which would be disappointing. We + // could potentially add an AtomicU64 here to add + // "bonus" items, but that would again invalidate the + // pre-computed "max stack" numbers. + missing_calls += 1; continue; }; let target = u32::try_from(target).unwrap(); @@ -305,6 +444,7 @@ pub fn extract_function_items( short_name, frame_size, calls, + missing_calls, }, ); } @@ -336,10 +476,8 @@ pub fn get_max_stack( .map(|n| (n.local_size.unwrap_or(0), n.name.clone())) .collect::>(); - let missing = find_missing_nodes( - &mut resolver, - fns.addr_to_frame_size.keys().copied(), - ); + let missing = + resolver.find_missing_nodes(fns.addr_to_frame_size.keys().copied()); let fake_items = missing .iter() @@ -349,95 +487,95 @@ pub fn get_max_stack( Ok(chain_compat) } -fn find_missing_nodes( - resolver: &mut Resolver, - all_addrs: impl Iterator, -) -> Vec> { - let mut missing = vec![]; - for addr in all_addrs { - if !resolver.all_resolved.contains_key(&addr) { - missing.push(addr); - } - } - - let mut found = vec![]; - for addr in missing { - let node = resolver.resolve_addr(addr).unwrap(); - found.push(node); - } +//////////////////////////////////////////////////////////////////////////////// +// Pulling data from the elf file +//////////////////////////////////////////////////////////////////////////////// - let mut last_len = found.len(); - loop { - let mut to_keep = vec![]; - while let Some(val) = found.pop() { - if found - .iter() - .any(|f: &Rc| f.is_same_or_child_of(&val)) - || to_keep - .iter() - .any(|f: &Rc| f.is_same_or_child_of(&val)) - { - continue; - } else { - to_keep.push(val); - } - } - found = to_keep; - if found.len() == last_len { - break; - } else { - last_len = found.len(); - } +/// Read the .stack_sizes section, which is an array of +/// `(address: u32, stack size: unsigned leb128)` tuples. +fn extract_stack_sizes_section( + raw_elf: &[u8], + parsed_elf: &Elf<'_>, +) -> Result> { + let sizes = elf::get_section_by_name(parsed_elf, ".stack_sizes") + .context("could not get .stack_sizes")?; + let mut sizes = + &raw_elf[sizes.sh_offset as usize..][..sizes.sh_size as usize]; + let mut addr_to_frame_size = BTreeMap::new(); + while !sizes.is_empty() { + let (addr, rest) = sizes.split_at(4); + let addr = u32::from_le_bytes(addr.try_into().unwrap()); + sizes = rest; + let size = leb128::read::unsigned(&mut sizes)?; + addr_to_frame_size.insert(addr, size); } - found + Ok(addr_to_frame_size) } -/// Estimates the maximum stack size for the given task -/// -/// This does not take dynamic function calls into account, which could cause -/// underestimation. Overestimation is less likely, but still may happen if -/// there are logically impossible call trees (e.g. `A -> B` and `B -> C`, but -/// `B` never calls `C` if called by `A`). -pub fn get_max_stack_old( - elf: &Path, +/// There are `$t` and `$d` symbols which indicate the beginning of text +/// versus data in the `.text` region. We collect them into a `BTreeMap` +/// here so that we can avoid trying to decode inline data words. +fn extract_text_regions( + parsed_elf: &Elf<'_>, task_name: &str, - verbose: bool, -) -> Result> { - let fns = extract_function_items(elf, task_name, verbose)?; - get_max_stack_inner(fns, task_name, verbose) -} +) -> Result> { + let mut text_regions = BTreeMap::new(); + let relevant = parsed_elf + .syms + .iter() + .filter(|s| s.st_name != 0) + .filter(|s| s.st_size == 0) + .filter(|s| s.st_type() == goblin::elf::sym::STT_NOTYPE); -pub fn get_max_stack_inner( - fns: FunctionReport, - task_name: &str, - verbose: bool, -) -> Result> { - let fns = fns.function_items; - // Find stack sizes by traversing the graph - if verbose { - println!("finding stack sizes for {task_name}"); + for sym in relevant { + let addr = sym.st_value as u32; + let is_text = match parsed_elf.strtab.get_at(sym.st_name) { + Some("$t") => true, + Some("$d") => false, + Some(_) => continue, + None => { + bail!("bad symbol in {task_name}: {}", sym.st_name); + } + }; + text_regions.insert(addr, is_text); } - let start_addr = fns + Ok(text_regions) +} + +/// Returns an iterator over symbols inside the elf file that represent a named +/// function. +fn fn_symbol_iter<'a>( + parsed_elf: &Elf<'a>, + text_section: &SectionHeader, + raw_elf: &'a [u8], +) -> impl Iterator> { + parsed_elf + .syms .iter() - .find(|(_addr, v)| v.name.as_str() == "_start") - .map(|(addr, _v)| *addr) - .ok_or_else(|| anyhow!("could not find _start"))?; - let mut deepest = None; - recurse(&mut vec![start_addr], 0, 0, &fns, &mut deepest, verbose); - - // Check against our configured task stack size - let Some((_max_depth, max_stack)) = deepest else { - unreachable!("must have at least one call stack"); - }; + // We only care about named function symbols here + .filter(|s| s.st_name != 0) + .filter(|s| s.is_function()) + .filter(|s| s.st_size != 0) + .filter_map(|s| { + // Clear the lowest bit, which indicates that the function contains + // thumb instructions (always true for our systems!) + let base_addr = s.st_value & !1; - let mut out = vec![]; - for m in max_stack { - let f = fns.get(&m).unwrap(); - let name = &f.short_name; - out.push((f.frame_size.unwrap_or(0), name.clone())); - } - Ok(out) + // Get the text region for this function + let offset = (base_addr - text_section.sh_addr + + text_section.sh_offset) as usize; + let text_region = &raw_elf[offset..][..s.st_size as usize]; + + // Bake into a handy collected symbol item + Some(SymbolData { + sym: s, + // TODO: assert? + mangled_name: parsed_elf.strtab.get_at(s.st_name).unwrap(), + base_addr, + text_region, + }) + }) } fn name_shortener(name: &str) -> String { @@ -449,187 +587,3 @@ fn name_shortener(name: &str) -> String { } .to_owned() } - -pub struct ResolvedNode { - pub addr: u32, - pub name: String, - pub local_size: Option, - pub max_children: u64, - pub children: BTreeMap>, -} - -impl ResolvedNode { - pub fn debug_all(&self) { - self.debug_all_depth(0, 0); - } - - pub fn debug_all_depth(&self, depth: usize, current_stack: u64) { - let frame_size = self.local_size; - let stack_depth = current_stack + frame_size.unwrap_or(0); - for _ in 0..depth { - print!(" "); - } - println!( - "- 0x{:08X} {} [+{frame_size:?} => {stack_depth}]", - self.addr, self.name, - ); - for (_addr, child) in self.children.iter() { - child.debug_all_depth( - depth + 1, - current_stack + frame_size.unwrap_or(0), - ); - } - } - - /// Used to determine if `self` could be potentially discarded as a dupe - pub fn is_same_or_child_of(&self, other: &Self) -> bool { - if other.addr == self.addr { - return true; - } - for (_caddr, child) in other.children.iter() { - if self.is_same_or_child_of(child) { - return true; - } - } - false - } - - pub fn max_stack(&self) -> u64 { - self.local_size.unwrap_or(0) + self.max_children - } - - pub fn worst_chain(self: &Rc) -> Vec> { - let mut chain = vec![]; - self.worst_chain_inner(&mut chain); - chain - } - - fn worst_chain_inner(self: &Rc, chain: &mut Vec>) { - chain.push(self.clone()); - if let Some(child) = self - .children - .iter() - .find(|c| c.1.max_stack() == self.max_children) - { - child.1.worst_chain_inner(chain) - } - } -} - -pub struct Resolver { - pub call_stack: Vec, - pub all_resolved: BTreeMap>, - pub fn_items: BTreeMap, -} - -impl Resolver { - pub fn new(fn_items: BTreeMap) -> Self { - Self { - call_stack: vec![], - all_resolved: BTreeMap::new(), - fn_items, - } - } - - pub fn resolve_by_name(&mut self, entry: &str) -> Result> { - let Some(item) = self.fn_items.iter().find(|(_k, v)| &v.name == entry) - else { - bail!("Not found"); - }; - let addr = *(item.0); - self.resolve_addr(addr) - } - - pub fn resolve_addr(&mut self, addr: u32) -> Result> { - // Have we already resolved this node? - if let Some(node) = self.all_resolved.get(&addr) { - return Ok(node.clone()); - } - - // no, we havent. Get the node info from the fn data - let Some(item) = self.fn_items.get(&addr) else { - bail!("no function data"); - }; - self.call_stack.push(addr); - let children = item.calls.clone(); - let name = item.name.clone(); - let local_size = item.frame_size; - - let mut res_children = BTreeMap::new(); - let mut max_children = 0; - for child in children { - if self.call_stack.contains(&child) { - bail!("Refusing to handle recursion"); - } - - // Resolve the child - let rchild = self.resolve_addr(child)?; - - let ttl_child = rchild.max_stack(); - max_children = max_children.max(ttl_child); - res_children.insert(child, rchild); - } - - let new_node = Rc::new(ResolvedNode { - addr, - name, - local_size, - max_children, - children: res_children, - }); - self.all_resolved.insert(addr, new_node.clone()); - self.call_stack.pop(); - Ok(new_node) - } -} - -fn recurse( - call_stack: &mut Vec, - recurse_depth: usize, - mut stack_depth: u64, - fns: &BTreeMap, - deepest: &mut Option<(u64, Vec)>, - verbose: bool, -) { - let addr = *call_stack.last().unwrap(); - let Some(f) = fns.get(&addr) else { - panic!("found jump to unknown function at {call_stack:08x?}"); - }; - let frame_size = f.frame_size.unwrap_or(0); - stack_depth += frame_size; - if verbose { - let indent = recurse_depth * 2; - println!( - " {:indent$}{addr:08x}: {} [+{frame_size} => {stack_depth}]", - "", - f.short_name, - indent = indent - ); - } - - if deepest - .as_ref() - .map(|(max_depth, _)| stack_depth > *max_depth) - .unwrap_or(true) - { - *deepest = Some((stack_depth, call_stack.to_owned())); - } - for j in &f.calls { - if call_stack.contains(j) { - // Skip recursive / mutually recursive calls, because we can't - // reason about them. - continue; - } else { - call_stack.push(*j); - recurse( - call_stack, - recurse_depth + 1, - stack_depth, - fns, - deepest, - verbose, - ); - call_stack.pop(); - } - } -} From a8bdeeb7dd4fe105a9736eea2c99e379c315637a Mon Sep 17 00:00:00 2001 From: James Munns Date: Mon, 6 Jul 2026 17:28:51 +0200 Subject: [PATCH 10/35] Add docs from the issue writeup, and tweak recursion detection --- build/stack/src/lib.rs | 65 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 2963a9b7d5..407b54e602 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -2,6 +2,58 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. +//! ## Static Stack Analysis +//! +//! The current static analysis implemented works roughly as follows: +//! +//! 1. We configure LLVM to emit `.stack_sizes` debug information using a `-Z` +//! flag, which means that the `elf` file compiled for each task contains a +//! debuginfo section listing the size of stack frames on a per-function +//! basis. +//! 2. We use the `goblin` crate to parse the compiled `elf` files, and obtain: +//! 1. The per-function stack frame sizes: [`extract_stack_sizes_section()`] +//! 2. The list of all function symbols contained in the elf file, see +//! [`fn_symbol_iter()`] +//! 3. The `.text` section of the elf file, which contains executable +//! instructions, inside of [`extract_function_items()`] +//! 4. Information about where data is inlined within the `.text` section, +//! see [`extract_text_regions()`] +//! 3. For each function in the elf file, we: +//! 1. Use the [Capstone library](https://www.capstone-engine.org/) (by way +//! of the [`capstone`](https://docs.rs/capstone/latest/capstone/) +//! FFI crate) to disassemble the instruction code +//! 2. Iterate through each instruction in the function, and determine all +//! "outgoing" branches which are calls to other functions +//! 3. Produce a report called [`FunctionData`] for that function which +//! contains the name, local stack usage, and a list of all called +//! functions by that function +//! 4. We then start from the entrypoint of the task, `_start`, and recurse +//! through each node, to calculate the deepest stack usage of any chain of +//! function calls. See [`get_max_stack()`] and [`Resolver`]'s `resolve_*` +//! methods. +//! 5. This "max stack" usage is compared against the `stacksize` for the task +//! in the manifest, and compilation fails if the calculated max stack +//! exceeds the written stacksize (outside of this crate). +//! +//! ## Known Limitations +//! +//! This approach already had a couple of known limitations, as they are +//! typical for this approach of static analysis. +//! +//! 1. This approach does not handle recursion, as we have no way to annotate a +//! potential upper bound of recursive iterations. Currently, the code +//! counts the number of direct recursion instances detected (e.g. self-calls +//! of a function), and refuses to resolve call stacks with cycles. +//! 2. This approach does not handle "indirect" branching, which looks something +//! like `blx r5` in assembly, and is often (but not exclusively) generated +//! when calling a function through a vtable method, like `dyn Format`. +//! Currently, the code silently skips considering instances of indirect +//! branching found. +//! 3. This approach does not handle functions without `.stack_sizes` metadata. +//! This is commonly present in assembly functions such as +//! `userlib::sys_send_stub`, or in `compiler-rt`/`llvm` builtins like +//! `__aeabi_memcpy`. These functions are silently assumed to use zero stack. + use std::{ collections::{BTreeMap, BTreeSet}, ops::Range, @@ -27,6 +79,7 @@ pub struct FunctionData { pub frame_size: Option, pub calls: BTreeSet, pub missing_calls: usize, + pub recursive_calls: usize, } /// The resulting report of all metadata obtained for functions after @@ -349,6 +402,7 @@ pub fn extract_function_items( // puddles of inline data which we don't want to (mis)-disassemble. let mut calls = BTreeSet::new(); let mut missing_calls = 0; + let mut recursive_calls = 0; for chunk_item in chunks { let instrs = cs .disasm_all(&chunk_item.code, chunk_item.addr) @@ -427,10 +481,12 @@ pub fn extract_function_items( }; let target = u32::try_from(target).unwrap(); - // Avoid recursive calls into the same function (or midway - // into the function, which is a thing we've seen before! - // it's weird!) - if !function_range.contains(&target) { + // Avoid recursive calls to the same function, and ignore + // any control flow that directs back inside this function. + // Any other jumps should be recorded as a call. + if target == base_addr { + recursive_calls += 1; + } else if !function_range.contains(&target) { calls.insert(target); } } @@ -445,6 +501,7 @@ pub fn extract_function_items( frame_size, calls, missing_calls, + recursive_calls, }, ); } From fcb73aa19f57d5ae0728920cc6a87af9184950d4 Mon Sep 17 00:00:00 2001 From: James Munns Date: Mon, 6 Jul 2026 17:35:47 +0200 Subject: [PATCH 11/35] Tweak fudge factor to "max" instead of "sum" of missing items --- build/stack/src/lib.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 407b54e602..b0e85040f2 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -536,10 +536,12 @@ pub fn get_max_stack( let missing = resolver.find_missing_nodes(fns.addr_to_frame_size.keys().copied()); - let fake_items = missing - .iter() - .map(|n| (n.max_stack(), format!("missing::{}", n.name))); - chain_compat.extend(fake_items); + // Find the largest missing item so that we can add it to the call stack as + // a "fudge factor" for unresolved dynamic/indirect dispatch. + let largest = missing.iter().map(|n| n.max_stack()).max().unwrap_or(0); + if let Some(node) = missing.iter().find(|n| n.max_stack() == largest) { + chain_compat.push((largest, format!("missing::{}", node.name))); + } Ok(chain_compat) } From e7bf246a6cfece01e4414f6962e356e84b1dd7a3 Mon Sep 17 00:00:00 2001 From: James Munns Date: Mon, 6 Jul 2026 17:52:55 +0200 Subject: [PATCH 12/35] Update stack limits --- app/cosmo/base.toml | 5 +++-- app/gimlet/base.toml | 6 +++--- app/gimletlet/app.toml | 2 +- app/grapefruit/base.toml | 1 + app/grapefruit/standalone.toml | 2 +- app/observer/base.toml | 2 +- app/psc/base.toml | 2 +- app/sidecar/base.toml | 2 +- 8 files changed, 12 insertions(+), 10 deletions(-) diff --git a/app/cosmo/base.toml b/app/cosmo/base.toml index 5dd1d635ae..427cd002f8 100644 --- a/app/cosmo/base.toml +++ b/app/cosmo/base.toml @@ -132,7 +132,7 @@ notifications = ["i2c1-irq", "i2c2-irq", "i2c3-irq", "i2c4-irq"] [tasks.packrat] name = "task-packrat" priority = 1 -stacksize = 1400 +stacksize = 1496 start = true task-slots = ["jefe"] features = ["cosmo", "ereport"] @@ -259,7 +259,7 @@ uses = ["usart6", "dbgmcu"] interrupts = {"usart6.irq" = "usart-irq"} priority = 9 max-sizes = {flash = 74000, ram = 65536} -stacksize = 5400 +stacksize = 7632 start = true task-slots = ["sys", { cpu_seq = "cosmo_seq" }, "hf", "control_plane_agent", "net", "packrat", "i2c_driver", { spi_driver = "spi2_driver" }, "sprot", "auxflash"] notifications = ["jefe-state-change", "usart-irq", "multitimer", "control-plane-agent"] @@ -385,6 +385,7 @@ start = true notifications = ["jefe-state-change", "timer"] task-slots = ["jefe", "spartan7_loader", "packrat", "sensor"] uses = ["mmio_dimms"] +stacksize = 904 [tasks.auxflash] name = "drv-auxflash-server" diff --git a/app/gimlet/base.toml b/app/gimlet/base.toml index 2086dfb926..1dad584033 100644 --- a/app/gimlet/base.toml +++ b/app/gimlet/base.toml @@ -125,7 +125,7 @@ notifications = ["i2c1-irq", "jefe-state-change"] [tasks.packrat] name = "task-packrat" priority = 1 -stacksize = 1400 +stacksize = 1496 start = true task-slots = ["jefe"] features = ["gimlet", "ereport"] @@ -166,7 +166,7 @@ name = "drv-gimlet-seq-server" features = ["h753"] priority = 4 max-sizes = {flash = 131072, ram = 16384 } -stacksize = 2600 +stacksize = 2792 start = true task-slots = ["sys", "i2c_driver", {spi_driver = "spi2_driver"}, "hf", "jefe", "packrat"] notifications = ["timer", "vcore"] @@ -244,7 +244,7 @@ uses = ["uart7", "dbgmcu"] interrupts = {"uart7.irq" = "usart-irq"} priority = 8 max-sizes = {flash = 72000, ram = 65536} -stacksize = 5376 +stacksize = 7280 start = true task-slots = ["sys", { cpu_seq = "gimlet_seq" }, "hf", "control_plane_agent", "net", "packrat", "i2c_driver", { spi_driver = "spi2_driver" }, "sprot"] notifications = ["jefe-state-change", "usart-irq", "multitimer", "control-plane-agent"] diff --git a/app/gimletlet/app.toml b/app/gimletlet/app.toml index eea7ac1003..cdc9e40ba9 100644 --- a/app/gimletlet/app.toml +++ b/app/gimletlet/app.toml @@ -52,7 +52,7 @@ name = "task-packrat" priority = 1 start = true task-slots = ["jefe"] -stacksize = 1400 +stacksize = 1480 features = ["ereport"] notifications = ["task-faulted"] diff --git a/app/grapefruit/base.toml b/app/grapefruit/base.toml index 6eaeea5f78..49e173abf4 100644 --- a/app/grapefruit/base.toml +++ b/app/grapefruit/base.toml @@ -111,6 +111,7 @@ start = true # task-slots is explicitly empty: packrat should not send IPCs! task-slots = [] features = ["grapefruit", "boot-kmdb"] +stacksize = 1496 [tasks.hiffy] name = "task-hiffy" diff --git a/app/grapefruit/standalone.toml b/app/grapefruit/standalone.toml index a30c4b4277..5157618613 100644 --- a/app/grapefruit/standalone.toml +++ b/app/grapefruit/standalone.toml @@ -11,7 +11,7 @@ interrupts = {"uart8.irq" = "usart-irq"} # Ereport stuff [tasks.packrat] -stacksize = 1400 +stacksize = 1496 task-slots = ["jefe"] features = ["ereport"] notifications = ["task-faulted"] diff --git a/app/observer/base.toml b/app/observer/base.toml index 5b29df3495..5a70921a5d 100644 --- a/app/observer/base.toml +++ b/app/observer/base.toml @@ -120,7 +120,7 @@ notifications = ["i2c2-irq", "i2c3-irq", "i2c4-irq"] [tasks.packrat] name = "task-packrat" priority = 1 -stacksize = 1400 +stacksize = 1480 start = true task-slots = ["jefe"] features = ["ereport"] diff --git a/app/psc/base.toml b/app/psc/base.toml index a57664ec8d..f55348fe85 100644 --- a/app/psc/base.toml +++ b/app/psc/base.toml @@ -121,7 +121,7 @@ notifications = ["i2c2-irq", "i2c3-irq"] [tasks.packrat] name = "task-packrat" priority = 1 -stacksize = 1400 +stacksize = 1480 start = true task-slots = ["jefe"] features = ["ereport"] diff --git a/app/sidecar/base.toml b/app/sidecar/base.toml index c1d1f1e5fc..f8dcf9ced2 100644 --- a/app/sidecar/base.toml +++ b/app/sidecar/base.toml @@ -272,7 +272,7 @@ notifications = ["timer"] [tasks.packrat] name = "task-packrat" priority = 1 -stacksize = 1400 +stacksize = 1480 start = true task-slots = ["jefe"] features = ["ereport"] From 3aa8112f7c4ab46b88cb9964b37f2a3f6f9bf2ff Mon Sep 17 00:00:00 2001 From: James Munns Date: Tue, 7 Jul 2026 16:04:38 +0200 Subject: [PATCH 13/35] Fix bad merge --- Cargo.lock | 23 ----------------------- build/stack/src/lib.rs | 4 ++-- build/xtask/src/dist.rs | 1 - 3 files changed, 2 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6d76de3ed0..0bdf776758 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3061,15 +3061,6 @@ version = "1.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "90e5c1c8368803113bf0c9584fc495a58b86dc8a29edbf8fe877d21d9507e797" -[[package]] -name = "elf" -version = "0.1.0" -dependencies = [ - "anyhow", - "goblin", - "scroll", -] - [[package]] name = "elliptic-curve" version = "0.13.8" @@ -5886,18 +5877,6 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" -[[package]] -name = "stack" -version = "0.1.0" -dependencies = [ - "anyhow", - "capstone", - "elf", - "goblin", - "leb128", - "rustc-demangle", -] - [[package]] name = "stage0-handoff" version = "0.1.0" @@ -8091,7 +8070,6 @@ dependencies = [ "digest-io", "directories", "dunce", - "elf", "filetime", "fnv", "glob", @@ -8116,7 +8094,6 @@ dependencies = [ "serde_json", "sha2 0.11.0", "sha3 0.11.0", - "stack", "strsim 0.10.0", "tlvc", "tlvc-text", diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index b0e85040f2..87b1d2f09f 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -366,7 +366,7 @@ pub fn extract_function_items( *iter.next_back().unwrap().1 }; - let text = elf::get_section_by_name(&elf, ".text") + let text = build_elf::get_section_by_name(&elf, ".text") .context("could not get .text")?; let cs = Capstone::new() @@ -556,7 +556,7 @@ fn extract_stack_sizes_section( raw_elf: &[u8], parsed_elf: &Elf<'_>, ) -> Result> { - let sizes = elf::get_section_by_name(parsed_elf, ".stack_sizes") + let sizes = build_elf::get_section_by_name(parsed_elf, ".stack_sizes") .context("could not get .stack_sizes")?; let mut sizes = &raw_elf[sizes.sh_offset as usize..][..sizes.sh_size as usize]; diff --git a/build/xtask/src/dist.rs b/build/xtask/src/dist.rs index d6ad5430f1..259ddcb9ae 100644 --- a/build/xtask/src/dist.rs +++ b/build/xtask/src/dist.rs @@ -17,7 +17,6 @@ use build_stack::get_max_stack; use indexmap::IndexMap; use multimap::MultiMap; use path_slash::{PathBufExt, PathExt}; -use stack::get_max_stack; use zerocopy::IntoBytes; use crate::{ From a8454397a985a126e98f9e2e26f326d940699265 Mon Sep 17 00:00:00 2001 From: James Munns Date: Wed, 8 Jul 2026 20:09:32 +0200 Subject: [PATCH 14/35] Work on new chunking logic --- build/stack/src/lib.rs | 285 ++++++++++++++++++++++++++++++++++++++++ build/stack/src/main.rs | 160 +++++++++++----------- 2 files changed, 366 insertions(+), 79 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 87b1d2f09f..66af527a6e 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -346,6 +346,291 @@ impl SymbolData<'_> { // Public methods //////////////////////////////////////////////////////////////////////////////// +pub fn chunkify_text(elf: &Path) -> Result<()> { + // Open the statically-linked ELF file + let data = std::fs::read(elf).context("could not open ELF file")?; + let elf = goblin::elf::Elf::parse(&data)?; + + chunkify_text_inner(&elf) +} + +/// Describes where a symbol lives. +#[derive(Debug)] +enum SymSection<'a> { + /// Defined in a real section; carries the section name. + Section(&'a str), + /// Undefined (imported / external) — st_shndx == SHN_UNDEF. + Undefined, + /// Absolute value, not tied to a section — SHN_ABS. + Absolute, + /// Common block (tentative definition) — SHN_COMMON. + Common, + /// Real index lives in a SHT_SYMTAB_SHNDX section — SHN_XINDEX. + Extended, +} + +fn section_of<'a>(elf: &'a Elf<'_>, sym: &Sym) -> SymSection<'a> { + match sym.st_shndx as u32 { + goblin::elf::section_header::SHN_UNDEF => SymSection::Undefined, + goblin::elf::section_header::SHN_ABS => SymSection::Absolute, + goblin::elf::section_header::SHN_COMMON => SymSection::Common, + goblin::elf::section_header::SHN_XINDEX => SymSection::Extended, + idx => { + match elf.section_headers.get(idx as usize) { + Some(shdr) => elf + .shdr_strtab + .get_at(shdr.sh_name) + .map(SymSection::Section) + .unwrap_or(SymSection::Undefined), + None => SymSection::Undefined, // out-of-range / malformed + } + } + } +} + +#[derive(Debug)] +struct NeoChunk { + name: String, + base_addr: u32, + header_size: Option, + calc_size: u64, + text_ranges: Vec, +} + +impl NeoChunk { + pub fn new(entry: Sym, parsed_elf: &Elf<'_>, base_addr: u32) -> Self { + // Get the name, if there is one in the symbol table, otherwise + // make one up. + let name = if entry.st_name == 0 { + None + } else { + parsed_elf + .strtab + .get_at(entry.st_name) + .map(|n| rustc_demangle::demangle(n).to_string()) + }; + let name = name.unwrap_or_else(|| format!("anon_fn_0x{base_addr:08X}")); + + // Get the size, if there is one + let size = if entry.st_size == 0 { + None + } else { + Some(entry.st_size) + }; + + NeoChunk { + name, + base_addr, + header_size: size, + text_ranges: vec![], + + // For now, we don't calculate the size + calc_size: 0, + } + } + + pub fn push_text_end(&mut self, addr: u32) -> Result<()> { + match self.text_ranges.last_mut() { + None => { + bail!("Unexpected data before text at {addr:08X}") + } + Some(val) => match val { + TextRange::TextStart { start } => { + let start = *start; + *val = TextRange::TextRange { start, end: addr }; + } + TextRange::TextRange { start: _, end } => { + bail!("Unexpected repeated data {end:08X} -> {addr:08X}") + } + }, + } + Ok(()) + } + + pub fn push_text_start(&mut self, addr: u32) -> Result<()> { + if let Some(TextRange::TextStart { start: old }) = + self.text_ranges.last() + { + bail!("Back to back Text Starts! {old:08X} => {addr:08X}"); + } + self.text_ranges.push(TextRange::TextStart { start: addr }); + Ok(()) + } +} + +fn chunkify_text_inner(parsed_elf: &Elf<'_>) -> Result<()> { + let text = build_elf::get_section_by_name(parsed_elf, ".text") + .context("could not get .text")?; + + let neo_chunks = neochunkify(parsed_elf, &text)?; + + for nc in neo_chunks { + println!("{nc:08X?}"); + } + panic!(); +} + +#[derive(Debug)] +enum TextRange { + TextStart { start: u32 }, + TextRange { start: u32, end: u32 }, +} + +impl TextRange { + fn text_data<'a>( + &self, + raw_elf: &'a [u8], + text_section: &SectionHeader, + ) -> Result<&'a [u8]> { + let (start, end) = match self { + TextRange::TextStart { start } => { + bail!("Missing text end: {start:08X}") + } + TextRange::TextRange { start, end } => (*start, *end), + }; + // Get the text region for this function + let start = (start as u64 - text_section.sh_addr + + text_section.sh_offset) as usize; + let end = (end as u64 - text_section.sh_addr + text_section.sh_offset) + as usize; + Ok(&raw_elf[start..end]) + } +} + +fn neochunkify( + parsed_elf: &Elf<'_>, + text_section: &SectionHeader, +) -> Result> { + // We're going to walk through each of the symbols in the symbol table, + // pulling any item that is part of the `.text` or executable section. + // This section may include multiple entries for the same address, we'll + // explain that more shortly! But for now, "bunch" them into buckets of + // symbols at the same address. We're *also* doing this because the symbols + // may not necessarily be in-order in the symbol table. + let mut buckets = BTreeMap::>::new(); + for sym in parsed_elf.syms.iter() { + // If it's not in the `.text` table, we're not interested! + let section = section_of(parsed_elf, &sym); + let SymSection::Section(".text") = section else { + continue; + }; + + // Mask the lowest bit off, this is because thumb functions are listed + // with this bit set, because that controls the "mode" on jump in arm + // processors. + let base_addr = sym.st_value & !1; + + // Get the existing bucket for this address, or make a new bucket. + let syms = buckets.entry(base_addr as u32).or_default(); + syms.push(sym); + } + + // Great, now we have a bunch of buckets! We expect to see something that + // looks like this, if you were to look at it with `objdump`: + // + // ```text + // 00012078 l .text 00000000 $t + // 00012078 l F .text 00000064 func_one + // 000120d0 l .text 00000000 $d + // 000120dc l .text 00000000 $t + // 000120dc l F .text 00000014 func_two + // ``` + // + // This is a bit harder to read, because it's out of order, as mentioned + // above, but we would expect three buckets in our code now: + // + // * 00012078 with two items ($t and func_one) + // * 000120d0 with one item ($d) + // * 000120dc with two items ($t and func_two) + // + // This might make more sense if we arrange this correctly: + // + // ```text + // 00012078 l F .text 00000064 func_one + // 00012078 l .text 00000000 $t + // 000120d0 l .text 00000000 $d + // 000120dc l F .text 00000014 func_two + // 000120dc l .text 00000000 $t + // ``` + // + // What we actually have here is two functions, `func_one` at 00012078, and + // `func_two` at `000120dc`. `func_one` has three symbols: The one marked + // `F`, which is the actual function symbol, showing it has a length of 64, + // and the name "func_one". Then it has `$t` at the same address, which + // shows that it starts with "executable code" first. Later, it has a `$d` + // symbol, which notes that inline data is here at the end of the function. + // After this, `func_two` begins. + // + // The following code is a small state machine that tries to extract all the + // symbols related to a single function into an aggregated "neochunk" + // report, which we will later use. + let mut neo_chunks = BTreeMap::::new(); + let mut current: Option = None; + + // For each bucket... + for (base_addr, mut syms) in buckets { + // ... while there is still any contents in this bucket... + while !syms.is_empty() { + // ..see if there is a function entry still in this bucket. If there + // is, remove it from the bucket and process it. + if let Some(entry) = { + syms.iter() + .position(|s| s.is_function()) + .map(|idx| syms.remove(idx)) + } { + // Found one, if there was a pending neochunk, "commit" it, + // using the current address to calculate the size, in case the + // function item was missing "function size" metadata, which we + // have observed with some externally linked static libraries, + // like `salty`. + if let Some(mut ch) = current.take() { + let calc_size = (base_addr - ch.base_addr) as u64; + ch.calc_size = calc_size; + ch.push_text_end(base_addr); + neo_chunks.insert(ch.base_addr, ch); + } + + // Store off the current chunk + current = Some(NeoChunk::new(entry, parsed_elf, base_addr)); + continue; + } + + // We know `syms` is non-empty, take some item in it + let entry = syms.pop().unwrap(); + + // Nope, see if we can extract $d/$t symbols to the current fn. + // We *should* always have a function entry by now, but if not, + // just log a warning. + let Some(current) = current.as_mut() else { + bail!("Discarded {entry:?} at {base_addr:08X}"); + }; + + match parsed_elf.strtab.get_at(entry.st_name) { + Some("$t") => current.push_text_start(base_addr)?, + Some("$d") => current.push_text_end(base_addr)?, + other => { + println!( + "WARN: Discarding unexpected '{other:?}' \ + {entry:?} at {base_addr:08X}" + ) + } + } + } + } + + // Finally push the last chunk onto the list + if let Some(mut cur) = current.take() { + // TODO! We need to figure out the end of the section here to fill in + // calculated length! + let text_end = text_section.sh_addr + text_section.sh_size; + cur.calc_size = text_end - cur.base_addr as u64; + cur.push_text_end(text_end as u32); + neo_chunks.insert(cur.base_addr, cur); + } + + Ok(neo_chunks) +} + /// Load and parse the `elf` file at the given path, producing a report of /// metadata about all functions in the `elf` file. pub fn extract_function_items( diff --git a/build/stack/src/main.rs b/build/stack/src/main.rs index 4d09dc18a6..d206f906b9 100644 --- a/build/stack/src/main.rs +++ b/build/stack/src/main.rs @@ -4,94 +4,96 @@ use std::{path::Path, rc::Rc}; -use stack::{FunctionReport, ResolvedNode, Resolver}; +use build_stack::{FunctionReport, ResolvedNode, Resolver}; fn main() { - let file = "../../target/gimlet-c/dist/host_sp_comms.tmp"; - let items = - stack::extract_function_items(Path::new(file), "host_sp_comms", false) - .unwrap(); + let file = "target/oxide-rot-1-selfsigned/dist/attest.tmp"; + build_stack::chunkify_text(Path::new(file)); + // let items = + // stack::extract_function_items( + // Path::new(file), "host_sp_comms", false) + // .unwrap(); - let FunctionReport { - function_items, - addr_to_frame_size, - names, - } = items; + // let FunctionReport { + // function_items, + // addr_to_frame_size, + // names, + // } = items; - let mut resolver = Resolver::new(function_items); - let node = resolver.resolve_by_name("_start").unwrap(); - // node.debug_all(); + // let mut resolver = Resolver::new(function_items); + // let node = resolver.resolve_by_name("_start").unwrap(); + // // node.debug_all(); - println!(); - println!(); - let mut missing = vec![]; - for (addr, size) in addr_to_frame_size.iter() { - let name = names.get(addr).unwrap(); - if !resolver.all_resolved.contains_key(addr) { - println!("WARN: missing {name} => {size}"); - missing.push((*addr, name)); - } - } + // println!(); + // println!(); + // let mut missing = vec![]; + // for (addr, size) in addr_to_frame_size.iter() { + // let name = names.get(addr).unwrap(); + // if !resolver.all_resolved.contains_key(addr) { + // println!("WARN: missing {name} => {size}"); + // missing.push((*addr, name)); + // } + // } - println!(); - let mut found = vec![]; - for (addr, name) in missing { - println!("probing {name}..."); - let node = resolver.resolve_addr(addr).unwrap(); - found.push(node); - } + // println!(); + // let mut found = vec![]; + // for (addr, name) in missing { + // println!("probing {name}..."); + // let node = resolver.resolve_addr(addr).unwrap(); + // found.push(node); + // } - println!("Shaking down..."); - let mut last_len = found.len(); - loop { - let mut to_keep = vec![]; - while let Some(val) = found.pop() { - if found - .iter() - .any(|f: &Rc| f.is_same_or_child_of(&val)) - || to_keep - .iter() - .any(|f: &Rc| f.is_same_or_child_of(&val)) - { - continue; - } else { - to_keep.push(val); - } - } - found = to_keep; - if found.len() == last_len { - break; - } else { - last_len = found.len(); - } - } + // println!("Shaking down..."); + // let mut last_len = found.len(); + // loop { + // let mut to_keep = vec![]; + // while let Some(val) = found.pop() { + // if found + // .iter() + // .any(|f: &Rc| f.is_same_or_child_of(&val)) + // || to_keep + // .iter() + // .any(|f: &Rc| f.is_same_or_child_of(&val)) + // { + // continue; + // } else { + // to_keep.push(val); + // } + // } + // found = to_keep; + // if found.len() == last_len { + // break; + // } else { + // last_len = found.len(); + // } + // } - println!("--- shook ---"); - let mut sum = 0; - for n in found.iter() { - println!("{} - {:?} + {}", n.name, n.local_size, n.max_children); - sum += n.max_stack(); - } + // println!("--- shook ---"); + // let mut sum = 0; + // for n in found.iter() { + // println!("{} - {:?} + {}", n.name, n.local_size, n.max_children); + // sum += n.max_stack(); + // } - println!(); - println!("------------"); - println!("Worst chain:"); - println!("------------"); - let chain = node.worst_chain(); - let mut worst_sum = 0; - for n in chain { - worst_sum += n.local_size.unwrap_or(0); - println!("{} - [+{:?} => {}]", n.name, n.local_size, worst_sum); - } + // println!(); + // println!("------------"); + // println!("Worst chain:"); + // println!("------------"); + // let chain = node.worst_chain(); + // let mut worst_sum = 0; + // for n in chain { + // worst_sum += n.local_size.unwrap_or(0); + // println!("{} - [+{:?} => {}]", n.name, n.local_size, worst_sum); + // } - println!(); - println!( - "max stack: {} + fudge ({}) = {}", - node.max_stack(), - sum, - node.max_stack() + sum - ); + // println!(); + // println!( + // "max stack: {} + fudge ({}) = {}", + // node.max_stack(), + // sum, + // node.max_stack() + sum + // ); - println!(); - println!("{} functions resolved.", resolver.all_resolved.len()); + // println!(); + // println!("{} functions resolved.", resolver.all_resolved.len()); } From 8df02ed6b76ef9a47551418db4bdb9c7409f0c08 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 15:37:31 +0200 Subject: [PATCH 15/35] Handle missing size info --- build/stack/src/lib.rs | 458 ++++++++++++++++------------------------ build/stack/src/main.rs | 158 +++++++------- 2 files changed, 257 insertions(+), 359 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 66af527a6e..54662a6c48 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -72,7 +72,7 @@ use capstone::{ use goblin::elf::{Elf, SectionHeader, Sym}; /// Information derived about a given function -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct FunctionData { pub name: String, pub short_name: String, @@ -88,7 +88,6 @@ pub struct FunctionData { pub struct FunctionReport { pub function_items: BTreeMap, pub addr_to_frame_size: BTreeMap, - pub names: BTreeMap, } /// A function that has been visited and "filled in" by the [`Resolver`]. @@ -114,21 +113,6 @@ pub struct Resolver { pub fn_items: BTreeMap, } -/// Information derived about a given symbol -struct SymbolData<'a> { - sym: Sym, - mangled_name: &'a str, - base_addr: u64, - text_region: &'a [u8], -} - -/// Information about a "chunk", which is a portion of a function containing -/// executable code (and not inlined data) -struct ChunkItem { - code: Vec, - addr: u64, -} - impl ResolvedNode { /// Print the entire stack trace to stdout pub fn debug_all(&self) { @@ -223,7 +207,7 @@ impl Resolver { // no, we havent. Get the node info from the fn data let Some(item) = self.fn_items.get(&addr) else { - bail!("no function data"); + bail!("{addr:08X}: no function data"); }; self.call_stack.push(addr); let children = item.calls.clone(); @@ -308,52 +292,10 @@ impl Resolver { } } -impl SymbolData<'_> { - /// The range of addresses contained within this symbol - fn addr_range(&self) -> Range { - let base_addr = self.base_addr as u32; - base_addr..base_addr + self.sym.st_size as u32 - } - - /// An array of "Chunks", which are each of the executable portions of this - /// symbol. - // - // TODO: return `Vec<(u32, &[u8])>? - fn extract_instruction_chunks(&self, is_code: F) -> Vec - where - F: Fn(u32) -> bool, - { - // Split the text region into instruction-only chunks - let mut chunks = vec![]; - let mut chunk = None; - for (i, b) in self.text_region.iter().enumerate() { - let addr = self.base_addr + i as u64; - if is_code(addr as u32) { - chunk - .get_or_insert(ChunkItem { addr, code: vec![] }) - .code - .push(*b); - } else if let Some(c) = chunk.take() { - chunks.push(c); - } - } - chunks.extend(chunk); // don't forget the trailing chunk! - chunks - } -} - //////////////////////////////////////////////////////////////////////////////// // Public methods //////////////////////////////////////////////////////////////////////////////// -pub fn chunkify_text(elf: &Path) -> Result<()> { - // Open the statically-linked ELF file - let data = std::fs::read(elf).context("could not open ELF file")?; - let elf = goblin::elf::Elf::parse(&data)?; - - chunkify_text_inner(&elf) -} - /// Describes where a symbol lives. #[derive(Debug)] enum SymSection<'a> { @@ -429,18 +371,27 @@ impl NeoChunk { } } - pub fn push_text_end(&mut self, addr: u32) -> Result<()> { + pub fn push_text_end(&mut self, mut addr: u32) -> Result<()> { + // We may push the text end when we reach the end of the function. If + // there is padding, this may be a garbage D4D4 instruction inserted by + // the linker! + if let Some(size) = self.header_size.as_ref() { + addr = addr.min(self.base_addr + (*size as u32)); + } + match self.text_ranges.last_mut() { None => { - bail!("Unexpected data before text at {addr:08X}") + bail!("Unexpected data/end of fn before text at {addr:08X}") } Some(val) => match val { TextRange::TextStart { start } => { let start = *start; *val = TextRange::TextRange { start, end: addr }; } - TextRange::TextRange { start: _, end } => { - bail!("Unexpected repeated data {end:08X} -> {addr:08X}") + TextRange::TextRange { .. } => { + // This could happen if we had text, then data, then the + // next function, because we call `push_text_end` both when + // we see a `$d` record or when the function ends. } }, } @@ -456,18 +407,16 @@ impl NeoChunk { self.text_ranges.push(TextRange::TextStart { start: addr }); Ok(()) } -} -fn chunkify_text_inner(parsed_elf: &Elf<'_>) -> Result<()> { - let text = build_elf::get_section_by_name(parsed_elf, ".text") - .context("could not get .text")?; - - let neo_chunks = neochunkify(parsed_elf, &text)?; - - for nc in neo_chunks { - println!("{nc:08X?}"); + pub fn function_range(&self) -> Range { + let start = self.base_addr; + let size = if let Some(addr) = self.header_size.as_ref() { + *addr + } else { + self.calc_size + } as u32; + start..(start + size) } - panic!(); } #[derive(Debug)] @@ -481,7 +430,7 @@ impl TextRange { &self, raw_elf: &'a [u8], text_section: &SectionHeader, - ) -> Result<&'a [u8]> { + ) -> Result<(u64, &'a [u8])> { let (start, end) = match self { TextRange::TextStart { start } => { bail!("Missing text end: {start:08X}") @@ -489,11 +438,11 @@ impl TextRange { TextRange::TextRange { start, end } => (*start, *end), }; // Get the text region for this function - let start = (start as u64 - text_section.sh_addr + let start_offset = (start as u64 - text_section.sh_addr + + text_section.sh_offset) as usize; + let end_offset = (end as u64 - text_section.sh_addr + text_section.sh_offset) as usize; - let end = (end as u64 - text_section.sh_addr + text_section.sh_offset) - as usize; - Ok(&raw_elf[start..end]) + Ok((start as u64, &raw_elf[start_offset..end_offset])) } } @@ -586,7 +535,7 @@ fn neochunkify( if let Some(mut ch) = current.take() { let calc_size = (base_addr - ch.base_addr) as u64; ch.calc_size = calc_size; - ch.push_text_end(base_addr); + ch.push_text_end(base_addr)?; neo_chunks.insert(ch.base_addr, ch); } @@ -624,18 +573,135 @@ fn neochunkify( // calculated length! let text_end = text_section.sh_addr + text_section.sh_size; cur.calc_size = text_end - cur.base_addr as u64; - cur.push_text_end(text_end as u32); + cur.push_text_end(text_end as u32)?; neo_chunks.insert(cur.base_addr, cur); } Ok(neo_chunks) } +struct FunctionCollector<'a> { + cs: &'a Capstone, + name: &'a str, + base_addr: u32, + function_range: Range, + frame_size: Option, + verbose: bool, + missing_calls: usize, + recursive_calls: usize, + calls: BTreeSet, +} + +impl FunctionCollector<'_> { + fn extract_calls(&mut self, data: &[u8], data_addr: u64) -> Result<()> { + let instrs = self + .cs + .disasm_all(data, data_addr) + .map_err(|e| anyhow!("disassembly failed: {e:?}"))?; + + let Some((last, all_except_last)) = instrs.split_last() else { + bail!("Empty chunk in {} at {:08X}", self.name, data_addr); + }; + + let to_consider = if last.bytes() == [0xD4, 0xD4] { + all_except_last + } else { + &instrs + }; + + // Walk through each instruction inside of each chunk + for (i, instr) in to_consider.iter().enumerate() { + // We need to get details for the instruction, which we should + // always have for well-formed programs + let detail = self.cs.insn_detail(instr).map_err(|e| { + anyhow!("could not get instruction details: {e}") + })?; + self.process_one(instr, &detail, i == (to_consider.len() - 1))?; + } + + // And process the last instruction carefully + // + Ok(()) + } + + fn process_one( + &mut self, + instr: &Insn<'_>, + detail: &InsnDetail<'_>, + last_in_chunk: bool, + ) -> Result<()> { + let can_tail = self.frame_size == Some(0) && last_in_chunk; + let is_grp_call = + |g| g == &InsnGroupId(InsnGroupType::CS_GRP_CALL as u8); + let is_grp_jump = + |g| g == &InsnGroupId(InsnGroupType::CS_GRP_JUMP as u8); + let is_grp_rel = + |g| g == &InsnGroupId(InsnGroupType::CS_GRP_BRANCH_RELATIVE as u8); + + // Detect tail calls, which are jumps at the final instruction + // when the function itself has no stack frame. + let is_tail_call = |g| is_grp_jump(g) && can_tail; + let is_branching_instr = detail.groups().iter().any(|g| { + // Check if this is now not needed anymore + if !is_grp_rel(g) && is_tail_call(g) { + panic!("this check is NOT obsolete!"); + } + + is_grp_call(g) || is_grp_rel(g) || is_tail_call(g) + }); + + if is_branching_instr { + // On Arm/Thumb, a jump always has some kind of operand, + // which is where we are jumping to. Get the last operand so + // we can determine if we can follow this. + let arch = detail.arch_detail(); + let ArchDetail::ArmDetail(details) = arch else { + panic!("Unsupported arch"); + }; + let op = details.operands().last().unwrap_or_else(|| { + panic!("missing operand!"); + }); + // We can't resolve indirect calls, alas + // + // TODO: We could consider keeping track of register ops + // and potentially figure out the location of the register + // based jump here + let arm::ArmOperandType::Imm(target) = op.op_type else { + if self.verbose { + println!( + "Failed to resolve indirect call in {}!", + self.name + ); + } + // TODO: we record that we are missing a call, and + // ideally we would plug the worst of the missing funcs + // here. Unfortunately, that means we end up needing to + // invalidate the RC, which would be disappointing. We + // could potentially add an AtomicU64 here to add + // "bonus" items, but that would again invalidate the + // pre-computed "max stack" numbers. + self.missing_calls += 1; + return Ok(()); + }; + let target = u32::try_from(target).unwrap(); + + // Avoid recursive calls to the same function, and ignore + // any control flow that directs back inside this function. + // Any other jumps should be recorded as a call. + if target == self.base_addr { + self.recursive_calls += 1; + } else if !self.function_range.contains(&target) { + self.calls.insert(target); + } + } + Ok(()) + } +} + /// Load and parse the `elf` file at the given path, producing a report of /// metadata about all functions in the `elf` file. pub fn extract_function_items( elf: &Path, - task_name: &str, verbose: bool, ) -> Result { // Open the statically-linked ELF file @@ -645,12 +711,6 @@ pub fn extract_function_items( // Get sizes of stack frames by addr from the elf let addr_to_frame_size = extract_stack_sizes_section(&data, &elf)?; - let text_regions = extract_text_regions(&elf, task_name)?; - let is_code = |addr| { - let mut iter = text_regions.range(..=addr); - *iter.next_back().unwrap().1 - }; - let text = build_elf::get_section_by_name(&elf, ".text") .context("could not get .text")?; @@ -664,129 +724,47 @@ pub fn extract_function_items( // Disassemble each function, building a map of its call sites let mut fns = BTreeMap::new(); - let mut fn_names = BTreeMap::new(); - for sym_item in fn_symbol_iter(&elf, text, &data) { - let base_addr = sym_item.base_addr as u32; - let name = sym_item.mangled_name; - // This is the stack frame size of the current function - let frame_size = addr_to_frame_size.get(&base_addr).copied(); - // This is the range of addresses comprising this function - let function_range = sym_item.addr_range(); - // Demangled and short name of this function - let name = rustc_demangle::demangle(name).to_string(); - let short_name = name_shortener(&name); - - fn_names.insert(base_addr, name.clone()); - // Split the text region into instruction-only chunks - let chunks = sym_item.extract_instruction_chunks(is_code); + let chunks = neochunkify(&elf, text)?; + for (base_addr, nchunk) in chunks { // Walk through each "chunk", which is an island of executable code // inside of each function, and collect all the out-bound calls. We // disassemble chunks rather than functions, as functions might contain // puddles of inline data which we don't want to (mis)-disassemble. - let mut calls = BTreeSet::new(); - let mut missing_calls = 0; - let mut recursive_calls = 0; - for chunk_item in chunks { - let instrs = cs - .disasm_all(&chunk_item.code, chunk_item.addr) - .map_err(|e| anyhow!("disassembly failed: {e:?}"))?; + // + // This is the stack frame size of the current function + let frame_size = addr_to_frame_size.get(&base_addr).copied(); + let function_range = nchunk.function_range(); - // We need to get details for the instruction, which we should - // always have for well-formed programs - let instrs: Vec<&Insn<'_>> = instrs.iter().collect(); - let details: Vec> = instrs - .iter() - .map(|instr| { - cs.insn_detail(instr).map_err(|e| { - anyhow!("could not get instruction details: {e}") - }) - }) - .collect::>()?; - - // Walk through each instruction inside of each chunk - for (i, (_instr, detail)) in - instrs.iter().zip(details.iter()).enumerate() - { - let can_tail = frame_size == Some(0) && i == instrs.len() - 1; - let is_grp_call = - |g| g == &InsnGroupId(InsnGroupType::CS_GRP_CALL as u8); - let is_grp_jump = - |g| g == &InsnGroupId(InsnGroupType::CS_GRP_JUMP as u8); - let is_grp_rel = |g| { - g == &InsnGroupId( - InsnGroupType::CS_GRP_BRANCH_RELATIVE as u8, - ) - }; - - // Detect tail calls, which are jumps at the final instruction - // when the function itself has no stack frame. - let is_tail_call = |g| is_grp_jump(g) && can_tail; - let is_branching_instr = detail.groups().iter().any(|g| { - // Check if this is now not needed anymore - if !is_grp_rel(g) && is_tail_call(g) { - panic!("this check is NOT obsolete!"); - } - - is_grp_call(g) || is_grp_rel(g) || is_tail_call(g) - }); - - if is_branching_instr { - // On Arm/Thumb, a jump always has some kind of operand, - // which is where we are jumping to. Get the last operand so - // we can determine if we can follow this. - let arch = detail.arch_detail(); - let ArchDetail::ArmDetail(details) = arch else { - panic!("Unsupported arch"); - }; - let op = details.operands().last().unwrap_or_else(|| { - panic!("missing operand!"); - }); - // We can't resolve indirect calls, alas - // - // TODO: We could consider keeping track of register ops - // and potentially figure out the location of the register - // based jump here - let arm::ArmOperandType::Imm(target) = op.op_type else { - if verbose { - println!( - "Failed to resolve indirect call in {name}!" - ); - } - // TODO: we record that we are missing a call, and - // ideally we would plug the worst of the missing funcs - // here. Unfortunately, that means we end up needing to - // invalidate the RC, which would be disappointing. We - // could potentially add an AtomicU64 here to add - // "bonus" items, but that would again invalidate the - // pre-computed "max stack" numbers. - missing_calls += 1; - continue; - }; - let target = u32::try_from(target).unwrap(); - - // Avoid recursive calls to the same function, and ignore - // any control flow that directs back inside this function. - // Any other jumps should be recorded as a call. - if target == base_addr { - recursive_calls += 1; - } else if !function_range.contains(&target) { - calls.insert(target); - } - } - } + // Gather up all the information we need to find all the calls in this + // function + let mut fc = FunctionCollector { + cs: &cs, + name: &nchunk.name, + base_addr, + function_range, + frame_size, + verbose, + missing_calls: 0, + recursive_calls: 0, + calls: BTreeSet::new(), + }; + + for chunk in nchunk.text_ranges.iter() { + let (data_addr, data) = chunk.text_data(&data, text)?; + fc.extract_calls(data, data_addr)?; } fns.insert( base_addr, FunctionData { - name, - short_name, + calls: fc.calls, + missing_calls: fc.missing_calls, + recursive_calls: fc.recursive_calls, + name: nchunk.name.clone(), + short_name: nchunk.name, frame_size, - calls, - missing_calls, - recursive_calls, }, ); } @@ -794,7 +772,6 @@ pub fn extract_function_items( Ok(FunctionReport { function_items: fns, addr_to_frame_size, - names: fn_names, }) } @@ -804,12 +781,8 @@ pub fn extract_function_items( /// underestimation. Overestimation is less likely, but still may happen if /// there are logically impossible call trees (e.g. `A -> B` and `B -> C`, but /// `B` never calls `C` if called by `A`). -pub fn get_max_stack( - elf: &Path, - task_name: &str, - verbose: bool, -) -> Result> { - let fns = extract_function_items(elf, task_name, verbose)?; +pub fn get_max_stack(elf: &Path, verbose: bool) -> Result> { + let fns = extract_function_items(elf, verbose)?; let mut resolver = Resolver::new(fns.function_items); let node = resolver.resolve_by_name("_start")?; let chain = node.worst_chain(); @@ -856,78 +829,3 @@ fn extract_stack_sizes_section( Ok(addr_to_frame_size) } - -/// There are `$t` and `$d` symbols which indicate the beginning of text -/// versus data in the `.text` region. We collect them into a `BTreeMap` -/// here so that we can avoid trying to decode inline data words. -fn extract_text_regions( - parsed_elf: &Elf<'_>, - task_name: &str, -) -> Result> { - let mut text_regions = BTreeMap::new(); - let relevant = parsed_elf - .syms - .iter() - .filter(|s| s.st_name != 0) - .filter(|s| s.st_size == 0) - .filter(|s| s.st_type() == goblin::elf::sym::STT_NOTYPE); - - for sym in relevant { - let addr = sym.st_value as u32; - let is_text = match parsed_elf.strtab.get_at(sym.st_name) { - Some("$t") => true, - Some("$d") => false, - Some(_) => continue, - None => { - bail!("bad symbol in {task_name}: {}", sym.st_name); - } - }; - text_regions.insert(addr, is_text); - } - Ok(text_regions) -} - -/// Returns an iterator over symbols inside the elf file that represent a named -/// function. -fn fn_symbol_iter<'a>( - parsed_elf: &Elf<'a>, - text_section: &SectionHeader, - raw_elf: &'a [u8], -) -> impl Iterator> { - parsed_elf - .syms - .iter() - // We only care about named function symbols here - .filter(|s| s.st_name != 0) - .filter(|s| s.is_function()) - .filter(|s| s.st_size != 0) - .filter_map(|s| { - // Clear the lowest bit, which indicates that the function contains - // thumb instructions (always true for our systems!) - let base_addr = s.st_value & !1; - - // Get the text region for this function - let offset = (base_addr - text_section.sh_addr - + text_section.sh_offset) as usize; - let text_region = &raw_elf[offset..][..s.st_size as usize]; - - // Bake into a handy collected symbol item - Some(SymbolData { - sym: s, - // TODO: assert? - mangled_name: parsed_elf.strtab.get_at(s.st_name).unwrap(), - base_addr, - text_region, - }) - }) -} - -fn name_shortener(name: &str) -> String { - // Strip the trailing hash from the name for ease of printing - if let Some(i) = name.rfind("::") { - &name[..i] - } else { - &name - } - .to_owned() -} diff --git a/build/stack/src/main.rs b/build/stack/src/main.rs index d206f906b9..2c7c9ba4f1 100644 --- a/build/stack/src/main.rs +++ b/build/stack/src/main.rs @@ -8,92 +8,92 @@ use build_stack::{FunctionReport, ResolvedNode, Resolver}; fn main() { let file = "target/oxide-rot-1-selfsigned/dist/attest.tmp"; - build_stack::chunkify_text(Path::new(file)); - // let items = - // stack::extract_function_items( - // Path::new(file), "host_sp_comms", false) - // .unwrap(); + // build_stack::chunkify_text(Path::new(file)); + let items = + build_stack::extract_function_items(Path::new(file), false).unwrap(); - // let FunctionReport { - // function_items, - // addr_to_frame_size, - // names, - // } = items; + let FunctionReport { + function_items, + addr_to_frame_size: _, + } = items; - // let mut resolver = Resolver::new(function_items); - // let node = resolver.resolve_by_name("_start").unwrap(); - // // node.debug_all(); + for (addr, item) in function_items.iter() { + println!("{addr:08X}, {item:08X?}"); + } - // println!(); - // println!(); - // let mut missing = vec![]; - // for (addr, size) in addr_to_frame_size.iter() { - // let name = names.get(addr).unwrap(); - // if !resolver.all_resolved.contains_key(addr) { - // println!("WARN: missing {name} => {size}"); - // missing.push((*addr, name)); - // } - // } + let mut resolver = Resolver::new(function_items); + let node = resolver.resolve_by_name("_start").unwrap(); + // node.debug_all(); - // println!(); - // let mut found = vec![]; - // for (addr, name) in missing { - // println!("probing {name}..."); - // let node = resolver.resolve_addr(addr).unwrap(); - // found.push(node); - // } + println!(); + println!(); + let mut missing = vec![]; + for (addr, fd) in resolver.fn_items.clone() { + if !resolver.all_resolved.contains_key(&addr) { + println!("WARN: missing {} => {:?}", fd.name, fd.frame_size); + missing.push((addr, fd)); + } + } - // println!("Shaking down..."); - // let mut last_len = found.len(); - // loop { - // let mut to_keep = vec![]; - // while let Some(val) = found.pop() { - // if found - // .iter() - // .any(|f: &Rc| f.is_same_or_child_of(&val)) - // || to_keep - // .iter() - // .any(|f: &Rc| f.is_same_or_child_of(&val)) - // { - // continue; - // } else { - // to_keep.push(val); - // } - // } - // found = to_keep; - // if found.len() == last_len { - // break; - // } else { - // last_len = found.len(); - // } - // } + println!(); + let mut found = vec![]; + for (addr, fd) in missing { + println!("probing {}...", fd.name); + let node = resolver.resolve_addr(addr).unwrap(); + found.push(node); + } - // println!("--- shook ---"); - // let mut sum = 0; - // for n in found.iter() { - // println!("{} - {:?} + {}", n.name, n.local_size, n.max_children); - // sum += n.max_stack(); - // } + println!("Shaking down..."); + let mut last_len = found.len(); + loop { + let mut to_keep = vec![]; + while let Some(val) = found.pop() { + if found + .iter() + .any(|f: &Rc| f.is_same_or_child_of(&val)) + || to_keep + .iter() + .any(|f: &Rc| f.is_same_or_child_of(&val)) + { + continue; + } else { + to_keep.push(val); + } + } + found = to_keep; + if found.len() == last_len { + break; + } else { + last_len = found.len(); + } + } - // println!(); - // println!("------------"); - // println!("Worst chain:"); - // println!("------------"); - // let chain = node.worst_chain(); - // let mut worst_sum = 0; - // for n in chain { - // worst_sum += n.local_size.unwrap_or(0); - // println!("{} - [+{:?} => {}]", n.name, n.local_size, worst_sum); - // } + println!("--- shook ---"); + let mut sum = 0; + for n in found.iter() { + println!("{} - {:?} + {}", n.name, n.local_size, n.max_children); + sum += n.max_stack(); + } - // println!(); - // println!( - // "max stack: {} + fudge ({}) = {}", - // node.max_stack(), - // sum, - // node.max_stack() + sum - // ); + println!(); + println!("------------"); + println!("Worst chain:"); + println!("------------"); + let chain = node.worst_chain(); + let mut worst_sum = 0; + for n in chain { + worst_sum += n.local_size.unwrap_or(0); + println!("{} - [+{:?} => {}]", n.name, n.local_size, worst_sum); + } - // println!(); - // println!("{} functions resolved.", resolver.all_resolved.len()); + println!(); + println!( + "max stack: {} + fudge ({}) = {}", + node.max_stack(), + sum, + node.max_stack() + sum + ); + + println!(); + println!("{} functions resolved.", resolver.all_resolved.len()); } From 706be11045c8fb2c1a995964eee9ec7ff5360346 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 16:24:16 +0200 Subject: [PATCH 16/35] Add allow-list of recursion sites --- build/stack/src/lib.rs | 78 ++++++++++++++++++++++++++-------------- build/xtask/src/dist.rs | 9 ++++- build/xtask/src/sizes.rs | 6 +++- 3 files changed, 65 insertions(+), 28 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 54662a6c48..1634b60473 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -63,7 +63,7 @@ use std::{ use anyhow::{Context, Result, anyhow, bail}; use capstone::{ - Capstone, Insn, InsnDetail, InsnGroupId, InsnGroupType, + Capstone, InsnDetail, InsnGroupId, InsnGroupType, arch::{ ArchDetail, BuildsCapstone, BuildsCapstoneExtraMode, DetailsArchInsn, arm, @@ -71,6 +71,11 @@ use capstone::{ }; use goblin::elf::{Elf, SectionHeader, Sym}; +pub const KNOWN_RECURSORS: &[&str] = &[ + // slice_error_fail calls slice_error_fail_rt which calls slice_error_fail + "str::slice_error_fail", +]; + /// Information derived about a given function #[derive(Debug, Clone)] pub struct FunctionData { @@ -111,6 +116,7 @@ pub struct Resolver { pub call_stack: Vec, pub all_resolved: BTreeMap>, pub fn_items: BTreeMap, + pub allowed_recurses: Vec, } impl ResolvedNode { @@ -180,11 +186,15 @@ impl ResolvedNode { impl Resolver { /// Create a new resolver from the given map of [`FunctionData`] items - pub fn new(fn_items: BTreeMap) -> Self { + pub fn new( + fn_items: BTreeMap, + allowed_recurses: Vec, + ) -> Self { Self { call_stack: vec![], all_resolved: BTreeMap::new(), fn_items, + allowed_recurses, } } @@ -218,7 +228,30 @@ impl Resolver { let mut max_children = 0; for child in children { if self.call_stack.contains(&child) { - bail!("Refusing to handle recursion"); + let Some(child_info) = self.fn_items.get(&child) else { + bail!("{child:08X}: no child function data"); + }; + let allow_match = self + .allowed_recurses + .iter() + .find(|allow| child_info.name.contains(allow.as_str())); + + if let Some(am) = allow_match { + // For now, pragmatically, we'll just ignore this recursive + // call site. + println!( + "WARN: Allowing {} to recurse, matching {}", + child_info.name, am + ); + continue; + } + + bail!( + "Refusing to handle recursion of {}, {:08X?} + {:08X}", + child_info.name, + self.call_stack, + child + ); } // Resolve the child @@ -247,20 +280,17 @@ impl Resolver { /// This finds functions that exist in the elf file, but were not visited /// while resolving the call graph, likely due to being called indirectly or /// through vtable methods. - fn find_missing_nodes( - &mut self, - all_addrs: impl Iterator, - ) -> Vec> { + fn find_missing_nodes(&mut self) -> Result>> { let mut missing = vec![]; - for addr in all_addrs { - if !self.all_resolved.contains_key(&addr) { - missing.push(addr); + for addr in self.fn_items.keys() { + if !self.all_resolved.contains_key(addr) { + missing.push(*addr); } } let mut found = vec![]; for addr in missing { - let node = self.resolve_addr(addr).unwrap(); + let node = self.resolve_addr(addr)?; found.push(node); } @@ -288,7 +318,7 @@ impl Resolver { } } - found + Ok(found) } } @@ -557,11 +587,8 @@ fn neochunkify( match parsed_elf.strtab.get_at(entry.st_name) { Some("$t") => current.push_text_start(base_addr)?, Some("$d") => current.push_text_end(base_addr)?, - other => { - println!( - "WARN: Discarding unexpected '{other:?}' \ - {entry:?} at {base_addr:08X}" - ) + _other => { + // Ignore other symbols, like `_stext`, etc. } } } @@ -616,17 +643,13 @@ impl FunctionCollector<'_> { let detail = self.cs.insn_detail(instr).map_err(|e| { anyhow!("could not get instruction details: {e}") })?; - self.process_one(instr, &detail, i == (to_consider.len() - 1))?; + self.process_one(&detail, i == (to_consider.len() - 1))?; } - - // And process the last instruction carefully - // Ok(()) } fn process_one( &mut self, - instr: &Insn<'_>, detail: &InsnDetail<'_>, last_in_chunk: bool, ) -> Result<()> { @@ -781,9 +804,13 @@ pub fn extract_function_items( /// underestimation. Overestimation is less likely, but still may happen if /// there are logically impossible call trees (e.g. `A -> B` and `B -> C`, but /// `B` never calls `C` if called by `A`). -pub fn get_max_stack(elf: &Path, verbose: bool) -> Result> { +pub fn get_max_stack( + elf: &Path, + verbose: bool, + allowed_recurses: Vec, +) -> Result> { let fns = extract_function_items(elf, verbose)?; - let mut resolver = Resolver::new(fns.function_items); + let mut resolver = Resolver::new(fns.function_items, allowed_recurses); let node = resolver.resolve_by_name("_start")?; let chain = node.worst_chain(); let mut chain_compat = chain @@ -791,8 +818,7 @@ pub fn get_max_stack(elf: &Path, verbose: bool) -> Result> { .map(|n| (n.local_size.unwrap_or(0), n.name.clone())) .collect::>(); - let missing = - resolver.find_missing_nodes(fns.addr_to_frame_size.keys().copied()); + let missing = resolver.find_missing_nodes()?; // Find the largest missing item so that we can add it to the call stack as // a "fudge factor" for unresolved dynamic/indirect dispatch. diff --git a/build/xtask/src/dist.rs b/build/xtask/src/dist.rs index 259ddcb9ae..f9541e6c3c 100644 --- a/build/xtask/src/dist.rs +++ b/build/xtask/src/dist.rs @@ -1437,7 +1437,14 @@ fn task_can_overflow( .join(&toml.name) .join("dist") .join(format!("{task_name}.tmp")); - let max_stack = get_max_stack(&f, task_name, verbose)?; + let allowed_recurses = build_stack::KNOWN_RECURSORS + .iter() + .map(|s| s.to_string()) + .collect(); + let max_stack = + get_max_stack(&f, verbose, allowed_recurses).with_context(|| { + format!("Failed getting max stack for task: {task_name}") + })?; let max_depth: u64 = max_stack.iter().map(|(d, _)| *d).sum(); let task_stack_size = toml.tasks[task_name] diff --git a/build/xtask/src/sizes.rs b/build/xtask/src/sizes.rs index 01ae9fc590..3a7a3050f9 100644 --- a/build/xtask/src/sizes.rs +++ b/build/xtask/src/sizes.rs @@ -440,7 +440,11 @@ fn print_task_stacks(toml: &Config) -> Result<()> { .join(&toml.name) .join("dist") .join(format!("{task_name}.tmp")); - let max_stack = get_max_stack(&f, task_name, false)?; + let allowed_recurses = build_stack::KNOWN_RECURSORS + .iter() + .map(|s| s.to_string()) + .collect(); + let max_stack = get_max_stack(&f, false, allowed_recurses)?; let total: u64 = max_stack.iter().map(|(n, _)| *n).sum(); println!("{task_name}: {total} bytes (limit is {task_stack_size})"); for (frame_size, name) in max_stack { From 928e20a232797e551e0e20e349680ff2010f6e9e Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 16:28:00 +0200 Subject: [PATCH 17/35] danke clippy --- build/stack/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 1634b60473..69fe82cb8d 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -200,7 +200,7 @@ impl Resolver { /// Attempt to resolve a function by name into a [`ResolvedNode`]. pub fn resolve_by_name(&mut self, entry: &str) -> Result> { - let Some(item) = self.fn_items.iter().find(|(_k, v)| &v.name == entry) + let Some(item) = self.fn_items.iter().find(|(_k, v)| v.name == entry) else { bail!("Not found"); }; From 47502cc0eee76ad2c90f1b62ea4c817e87398954 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 16:43:37 +0200 Subject: [PATCH 18/35] Disallow self-recursion too, unless allowed --- build/stack/src/lib.rs | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 69fe82cb8d..87745aefba 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -74,6 +74,14 @@ use goblin::elf::{Elf, SectionHeader, Sym}; pub const KNOWN_RECURSORS: &[&str] = &[ // slice_error_fail calls slice_error_fail_rt which calls slice_error_fail "str::slice_error_fail", + // `smoltcp::iface::interface::InterfaceInner::dispatch_ip::` + // self-recurses. This fragment is a little weird because it looks like: + // + // ``` + // + // ::dispatch_ip:: + // ``` + "dispatch_ip", ]; /// Information derived about a given function @@ -255,7 +263,9 @@ impl Resolver { } // Resolve the child - let rchild = self.resolve_addr(child)?; + let rchild = self + .resolve_addr(child) + .with_context(|| format!("While resolving {}", name))?; let ttl_child = rchild.max_stack(); max_children = max_children.max(ttl_child); @@ -726,6 +736,7 @@ impl FunctionCollector<'_> { pub fn extract_function_items( elf: &Path, verbose: bool, + allowed_recurses: &[String], ) -> Result { // Open the statically-linked ELF file let data = std::fs::read(elf).context("could not open ELF file")?; @@ -779,6 +790,23 @@ pub fn extract_function_items( fc.extract_calls(data, data_addr)?; } + if fc.recursive_calls != 0 { + let allow_match = allowed_recurses + .iter() + .find(|allow| fc.name.contains(allow.as_str())); + + if let Some(am) = allow_match { + // For now, pragmatically, we'll just ignore this recursive + // call site. + println!( + "WARN: Allowing {} to self-recurse, matching {}", + fc.name, am + ); + } else { + bail!("Refusing to handle self-recursion of {}", fc.name); + } + } + fns.insert( base_addr, FunctionData { @@ -809,7 +837,7 @@ pub fn get_max_stack( verbose: bool, allowed_recurses: Vec, ) -> Result> { - let fns = extract_function_items(elf, verbose)?; + let fns = extract_function_items(elf, verbose, &allowed_recurses)?; let mut resolver = Resolver::new(fns.function_items, allowed_recurses); let node = resolver.resolve_by_name("_start")?; let chain = node.worst_chain(); From 53e26cc8e7b98ed2aefe6d40f75f34ee5141196a Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 17:15:31 +0200 Subject: [PATCH 19/35] Allow recursion --- build/stack/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 87745aefba..ff3686ac28 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -82,6 +82,9 @@ pub const KNOWN_RECURSORS: &[&str] = &[ // ::dispatch_ip:: // ``` "dispatch_ip", + // We have limited recursion here, see + // https://github.com/oxidecomputer/hubris/issues/2593 + "Vsc7448Spi", ]; /// Information derived about a given function From e509b3e1129c0f399dad92d7206101e36c0dfcc0 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 17:29:50 +0200 Subject: [PATCH 20/35] A few review comments, bump demo app task size --- app/demo-stm32h7-nucleo/app-h743.toml | 2 +- build/stack/src/lib.rs | 44 ++++++------ build/stack/src/main.rs | 99 --------------------------- 3 files changed, 23 insertions(+), 122 deletions(-) delete mode 100644 build/stack/src/main.rs diff --git a/app/demo-stm32h7-nucleo/app-h743.toml b/app/demo-stm32h7-nucleo/app-h743.toml index d7077f216c..da0d4de5ad 100644 --- a/app/demo-stm32h7-nucleo/app-h743.toml +++ b/app/demo-stm32h7-nucleo/app-h743.toml @@ -152,7 +152,7 @@ priority = 4 max-sizes = {flash = 32768, ram = 2048 } start = true task-slots = ["jefe"] -stacksize = 1200 +stacksize = 1264 extern-regions = [ "sram1", "sram2", "sram3", "sram4" ] [config] diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index ff3686ac28..856f966be3 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -91,7 +91,6 @@ pub const KNOWN_RECURSORS: &[&str] = &[ #[derive(Debug, Clone)] pub struct FunctionData { pub name: String, - pub short_name: String, pub frame_size: Option, pub calls: BTreeSet, pub missing_calls: usize, @@ -213,7 +212,7 @@ impl Resolver { pub fn resolve_by_name(&mut self, entry: &str) -> Result> { let Some(item) = self.fn_items.iter().find(|(_k, v)| v.name == entry) else { - bail!("Not found"); + bail!("function '{entry}' not found"); }; let addr = *(item.0); self.resolve_addr(addr) @@ -228,7 +227,7 @@ impl Resolver { // no, we havent. Get the node info from the fn data let Some(item) = self.fn_items.get(&addr) else { - bail!("{addr:08X}: no function data"); + bail!("no function data for {addr:08X}"); }; self.call_stack.push(addr); let children = item.calls.clone(); @@ -270,8 +269,7 @@ impl Resolver { .resolve_addr(child) .with_context(|| format!("While resolving {}", name))?; - let ttl_child = rchild.max_stack(); - max_children = max_children.max(ttl_child); + max_children = max_children.max(rchild.max_stack()); res_children.insert(child, rchild); } @@ -294,18 +292,18 @@ impl Resolver { /// while resolving the call graph, likely due to being called indirectly or /// through vtable methods. fn find_missing_nodes(&mut self) -> Result>> { - let mut missing = vec![]; - for addr in self.fn_items.keys() { - if !self.all_resolved.contains_key(addr) { - missing.push(*addr); - } - } - - let mut found = vec![]; - for addr in missing { - let node = self.resolve_addr(addr)?; - found.push(node); - } + // Find all of the functions we know about in `fn_items`, and find + // any that haven't already been resolved into `all_resolved` from + // previous resolution, usually the `_start` entry point + let mut found = self + .fn_items + .keys() + .copied() + .filter(|addr| !self.all_resolved.contains_key(&addr)) + .collect::>() // necessary to avoid double borrowing self + .into_iter() + .map(|addr| self.resolve_addr(addr)) + .collect::, _>>()?; let mut last_len = found.len(); loop { @@ -695,7 +693,7 @@ impl FunctionCollector<'_> { panic!("Unsupported arch"); }; let op = details.operands().last().unwrap_or_else(|| { - panic!("missing operand!"); + panic!("jumps on ARM should always have an operand!"); }); // We can't resolve indirect calls, alas // @@ -742,8 +740,11 @@ pub fn extract_function_items( allowed_recurses: &[String], ) -> Result { // Open the statically-linked ELF file - let data = std::fs::read(elf).context("could not open ELF file")?; - let elf = goblin::elf::Elf::parse(&data)?; + let data = std::fs::read(elf).with_context(|| { + format!("could not open ELF file: {}", elf.display()) + })?; + let elf = goblin::elf::Elf::parse(&data) + .with_context(|| format!("could not parse {}", elf.display()))?; // Get sizes of stack frames by addr from the elf let addr_to_frame_size = extract_stack_sizes_section(&data, &elf)?; @@ -816,8 +817,7 @@ pub fn extract_function_items( calls: fc.calls, missing_calls: fc.missing_calls, recursive_calls: fc.recursive_calls, - name: nchunk.name.clone(), - short_name: nchunk.name, + name: nchunk.name, frame_size, }, ); diff --git a/build/stack/src/main.rs b/build/stack/src/main.rs deleted file mode 100644 index 2c7c9ba4f1..0000000000 --- a/build/stack/src/main.rs +++ /dev/null @@ -1,99 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -use std::{path::Path, rc::Rc}; - -use build_stack::{FunctionReport, ResolvedNode, Resolver}; - -fn main() { - let file = "target/oxide-rot-1-selfsigned/dist/attest.tmp"; - // build_stack::chunkify_text(Path::new(file)); - let items = - build_stack::extract_function_items(Path::new(file), false).unwrap(); - - let FunctionReport { - function_items, - addr_to_frame_size: _, - } = items; - - for (addr, item) in function_items.iter() { - println!("{addr:08X}, {item:08X?}"); - } - - let mut resolver = Resolver::new(function_items); - let node = resolver.resolve_by_name("_start").unwrap(); - // node.debug_all(); - - println!(); - println!(); - let mut missing = vec![]; - for (addr, fd) in resolver.fn_items.clone() { - if !resolver.all_resolved.contains_key(&addr) { - println!("WARN: missing {} => {:?}", fd.name, fd.frame_size); - missing.push((addr, fd)); - } - } - - println!(); - let mut found = vec![]; - for (addr, fd) in missing { - println!("probing {}...", fd.name); - let node = resolver.resolve_addr(addr).unwrap(); - found.push(node); - } - - println!("Shaking down..."); - let mut last_len = found.len(); - loop { - let mut to_keep = vec![]; - while let Some(val) = found.pop() { - if found - .iter() - .any(|f: &Rc| f.is_same_or_child_of(&val)) - || to_keep - .iter() - .any(|f: &Rc| f.is_same_or_child_of(&val)) - { - continue; - } else { - to_keep.push(val); - } - } - found = to_keep; - if found.len() == last_len { - break; - } else { - last_len = found.len(); - } - } - - println!("--- shook ---"); - let mut sum = 0; - for n in found.iter() { - println!("{} - {:?} + {}", n.name, n.local_size, n.max_children); - sum += n.max_stack(); - } - - println!(); - println!("------------"); - println!("Worst chain:"); - println!("------------"); - let chain = node.worst_chain(); - let mut worst_sum = 0; - for n in chain { - worst_sum += n.local_size.unwrap_or(0); - println!("{} - [+{:?} => {}]", n.name, n.local_size, worst_sum); - } - - println!(); - println!( - "max stack: {} + fudge ({}) = {}", - node.max_stack(), - sum, - node.max_stack() + sum - ); - - println!(); - println!("{} functions resolved.", resolver.all_resolved.len()); -} From ca8a83159140318daba4b3a36b308288ee6e8683 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 17:34:16 +0200 Subject: [PATCH 21/35] One more clippy --- build/stack/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 856f966be3..1642419536 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -299,7 +299,7 @@ impl Resolver { .fn_items .keys() .copied() - .filter(|addr| !self.all_resolved.contains_key(&addr)) + .filter(|addr| !self.all_resolved.contains_key(addr)) .collect::>() // necessary to avoid double borrowing self .into_iter() .map(|addr| self.resolve_addr(addr)) From be21fa116f4bef5d25780da979b467f04f43b04f Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 19:16:30 +0200 Subject: [PATCH 22/35] Re-organize, cleanup docs --- build/stack/src/lib.rs | 782 +++++++++++++++++++++-------------------- 1 file changed, 408 insertions(+), 374 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 1642419536..511710d755 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -12,12 +12,13 @@ //! basis. //! 2. We use the `goblin` crate to parse the compiled `elf` files, and obtain: //! 1. The per-function stack frame sizes: [`extract_stack_sizes_section()`] -//! 2. The list of all function symbols contained in the elf file, see -//! [`fn_symbol_iter()`] -//! 3. The `.text` section of the elf file, which contains executable -//! instructions, inside of [`extract_function_items()`] -//! 4. Information about where data is inlined within the `.text` section, -//! see [`extract_text_regions()`] +//! 2. "Raw" function information per-function, see +//! [`extract_raw_function_data`], containing: +//! 1. The list of all function symbols contained in the elf file +//! 2. The `.text` section of the elf file, which contains executable +//! instructions +//! 3. Information about where data is inlined within the `.text` +//! section //! 3. For each function in the elf file, we: //! 1. Use the [Capstone library](https://www.capstone-engine.org/) (by way //! of the [`capstone`](https://docs.rs/capstone/latest/capstone/) @@ -129,6 +130,210 @@ pub struct Resolver { pub allowed_recurses: Vec, } +/// Describes where a symbol lives. +#[derive(Debug)] +enum SymSection<'a> { + /// Defined in a real section; carries the section name. + Section(&'a str), + /// Undefined (imported / external) — st_shndx == SHN_UNDEF. + Undefined, + /// Absolute value, not tied to a section — SHN_ABS. + Absolute, + /// Common block (tentative definition) — SHN_COMMON. + Common, + /// Real index lives in a SHT_SYMTAB_SHNDX section — SHN_XINDEX. + Extended, +} + +/// "Raw" function data, extracted from the elf metadata and section headers +#[derive(Debug)] +struct RawFunctionData { + /// Demangled name of the function + name: String, + /// Address of the function + base_addr: u32, + /// The size of the function, in bytes, according to the symbol table + symbol_table_size: Option, + /// The calculated size of the function, counting until the next symbol + /// starts (useful when the symbol table is missing size) + calc_size: u64, + /// The address ranges that contain executable code (and not inline data) + text_ranges: Vec, +} + +/// A range of executable "Text" data +#[derive(Debug)] +enum TextRange { + /// An open-ended range starting at the given address + TextStart { start: u32 }, + /// A closed-ended range `start..end`. + TextRange { start: u32, end: u32 }, +} + +/// Private helper type to group up all of the context information we need when +/// processing the raw assembly of each function to figure out which functions +/// are called by this function +struct FunctionCallCollector<'a> { + /// The Capstone disassembler + cs: &'a Capstone, + /// The name of this function + name: &'a str, + /// The base address of this function + base_addr: u32, + /// The range of addresses on the target that contain this function, as well + /// as any data sections within/after this function + function_range: Range, + /// The stack frame size for this function, as reported by LLVM's + /// `.stack_sizes` section + frame_size: Option, + /// Whether to print debugging info + verbose: bool, + /// The number of outgoing calls we were unable to resolve, likely because + /// of indirect addressing via function pointers/dyn Trait + missing_calls: usize, + /// The number of self-recursive calls, e.g. this function directly calling + /// itself + recursive_calls: usize, + /// All outgoing calls from this function + calls: BTreeSet, +} + +//////////////////////////////////////////////////////////////////////////////// +// Public methods +//////////////////////////////////////////////////////////////////////////////// + +/// Estimates the maximum stack size for the given task +/// +/// This does not take dynamic function calls into account, which could cause +/// underestimation. Overestimation is less likely, but still may happen if +/// there are logically impossible call trees (e.g. `A -> B` and `B -> C`, but +/// `B` never calls `C` if called by `A`). +pub fn get_max_stack( + elf: &Path, + verbose: bool, + allowed_recurses: Vec, +) -> Result> { + let fns = extract_function_items(elf, verbose, &allowed_recurses)?; + let mut resolver = Resolver::new(fns.function_items, allowed_recurses); + let node = resolver.resolve_by_name("_start")?; + let chain = node.worst_chain(); + let mut chain_compat = chain + .into_iter() + .map(|n| (n.local_size.unwrap_or(0), n.name.clone())) + .collect::>(); + + let missing = resolver.find_missing_nodes()?; + + // Find the largest missing item so that we can add it to the call stack as + // a "fudge factor" for unresolved dynamic/indirect dispatch. + let largest = missing.iter().map(|n| n.max_stack()).max().unwrap_or(0); + if let Some(node) = missing.iter().find(|n| n.max_stack() == largest) { + chain_compat.push((largest, format!("missing::{}", node.name))); + } + + Ok(chain_compat) +} + +/// Load and parse the `elf` file at the given path, producing a report of +/// metadata about all functions in the `elf` file. +pub fn extract_function_items( + elf: &Path, + verbose: bool, + allowed_recurses: &[String], +) -> Result { + // Open the statically-linked ELF file + let data = std::fs::read(elf).with_context(|| { + format!("could not open ELF file: {}", elf.display()) + })?; + let elf = goblin::elf::Elf::parse(&data) + .with_context(|| format!("could not parse {}", elf.display()))?; + + // Get sizes of stack frames by addr from the elf + let addr_to_frame_size = extract_stack_sizes_section(&data, &elf)?; + + let text = build_elf::get_section_by_name(&elf, ".text") + .context("could not get .text")?; + + let cs = Capstone::new() + .arm() + .mode(arm::ArchMode::Thumb) + .extra_mode(std::iter::once(arm::ArchExtraMode::MClass)) + .detail(true) + .build() + .map_err(|e| anyhow!("failed to initialize disassembler: {e:?}"))?; + + // Disassemble each function, building a map of its call sites + let mut fns = BTreeMap::new(); + + let functions = extract_raw_function_data(&elf, text)?; + + for (base_addr, function) in functions { + // This is the stack frame size of the current function + let frame_size = addr_to_frame_size.get(&base_addr).copied(); + let function_range = function.function_range(); + + // Gather up all the information we need to find all the calls in this + // function + let mut fc = FunctionCallCollector { + cs: &cs, + name: &function.name, + base_addr, + function_range, + frame_size, + verbose, + missing_calls: 0, + recursive_calls: 0, + calls: BTreeSet::new(), + }; + + // Walk through each "text range", which is an island of executable code + // inside of each function, and collect all the out-bound calls. We + // disassemble ranges rather than functions, as functions might contain + // puddles of inline data which we don't want to (mis)-disassemble. + for chunk in function.text_ranges.iter() { + let (data_addr, data) = chunk.text_data(&data, text)?; + fc.extract_calls(data, data_addr)?; + } + + if fc.recursive_calls != 0 { + let allow_match = allowed_recurses + .iter() + .find(|allow| fc.name.contains(allow.as_str())); + + if let Some(am) = allow_match { + // For now, pragmatically, we'll just ignore this recursive + // call site. + println!( + "WARN: Allowing {} to self-recurse, matching {}", + fc.name, am + ); + } else { + bail!("Refusing to handle self-recursion of {}", fc.name); + } + } + + fns.insert( + base_addr, + FunctionData { + calls: fc.calls, + missing_calls: fc.missing_calls, + recursive_calls: fc.recursive_calls, + name: function.name, + frame_size, + }, + ); + } + + Ok(FunctionReport { + function_items: fns, + addr_to_frame_size, + }) +} + +//////////////////////////////////////////////////////////////////////////////// +// impls +//////////////////////////////////////////////////////////////////////////////// + impl ResolvedNode { /// Print the entire stack trace to stdout pub fn debug_all(&self) { @@ -295,63 +500,18 @@ impl Resolver { // Find all of the functions we know about in `fn_items`, and find // any that haven't already been resolved into `all_resolved` from // previous resolution, usually the `_start` entry point - let mut found = self - .fn_items + self.fn_items .keys() .copied() .filter(|addr| !self.all_resolved.contains_key(addr)) .collect::>() // necessary to avoid double borrowing self .into_iter() .map(|addr| self.resolve_addr(addr)) - .collect::, _>>()?; - - let mut last_len = found.len(); - loop { - let mut to_keep = vec![]; - while let Some(val) = found.pop() { - if found - .iter() - .any(|f: &Rc| f.is_same_or_child_of(&val)) - || to_keep - .iter() - .any(|f: &Rc| f.is_same_or_child_of(&val)) - { - continue; - } else { - to_keep.push(val); - } - } - found = to_keep; - if found.len() == last_len { - break; - } else { - last_len = found.len(); - } - } - - Ok(found) + .collect::, _>>() } } -//////////////////////////////////////////////////////////////////////////////// -// Public methods -//////////////////////////////////////////////////////////////////////////////// - -/// Describes where a symbol lives. -#[derive(Debug)] -enum SymSection<'a> { - /// Defined in a real section; carries the section name. - Section(&'a str), - /// Undefined (imported / external) — st_shndx == SHN_UNDEF. - Undefined, - /// Absolute value, not tied to a section — SHN_ABS. - Absolute, - /// Common block (tentative definition) — SHN_COMMON. - Common, - /// Real index lives in a SHT_SYMTAB_SHNDX section — SHN_XINDEX. - Extended, -} - +/// Gets the linking section that the given symbol is a part of. fn section_of<'a>(elf: &'a Elf<'_>, sym: &Sym) -> SymSection<'a> { match sym.st_shndx as u32 { goblin::elf::section_header::SHN_UNDEF => SymSection::Undefined, @@ -371,16 +531,11 @@ fn section_of<'a>(elf: &'a Elf<'_>, sym: &Sym) -> SymSection<'a> { } } -#[derive(Debug)] -struct NeoChunk { - name: String, - base_addr: u32, - header_size: Option, - calc_size: u64, - text_ranges: Vec, -} - -impl NeoChunk { +impl RawFunctionData { + /// Obtain "Raw" function data for a given [`Sym`]. + /// + /// This starts with no text ranges yet, and requires calls to + /// `push_text_*`. pub fn new(entry: Sym, parsed_elf: &Elf<'_>, base_addr: u32) -> Self { // Get the name, if there is one in the symbol table, otherwise // make one up. @@ -401,10 +556,10 @@ impl NeoChunk { Some(entry.st_size) }; - NeoChunk { + RawFunctionData { name, base_addr, - header_size: size, + symbol_table_size: size, text_ranges: vec![], // For now, we don't calculate the size @@ -412,11 +567,12 @@ impl NeoChunk { } } + /// Note where a text section ends pub fn push_text_end(&mut self, mut addr: u32) -> Result<()> { // We may push the text end when we reach the end of the function. If // there is padding, this may be a garbage D4D4 instruction inserted by // the linker! - if let Some(size) = self.header_size.as_ref() { + if let Some(size) = self.symbol_table_size.as_ref() { addr = addr.min(self.base_addr + (*size as u32)); } @@ -439,6 +595,7 @@ impl NeoChunk { Ok(()) } + /// Not where a text section starts pub fn push_text_start(&mut self, addr: u32) -> Result<()> { if let Some(TextRange::TextStart { start: old }) = self.text_ranges.last() @@ -449,9 +606,11 @@ impl NeoChunk { Ok(()) } + /// Get the range of addresses contained by this function. This does + /// include any inline data. pub fn function_range(&self) -> Range { let start = self.base_addr; - let size = if let Some(addr) = self.header_size.as_ref() { + let size = if let Some(addr) = self.symbol_table_size.as_ref() { *addr } else { self.calc_size @@ -460,13 +619,8 @@ impl NeoChunk { } } -#[derive(Debug)] -enum TextRange { - TextStart { start: u32 }, - TextRange { start: u32, end: u32 }, -} - impl TextRange { + /// Extract the raw bytes of assembly for this text range fn text_data<'a>( &self, raw_elf: &'a [u8], @@ -487,179 +641,44 @@ impl TextRange { } } -fn neochunkify( - parsed_elf: &Elf<'_>, - text_section: &SectionHeader, -) -> Result> { - // We're going to walk through each of the symbols in the symbol table, - // pulling any item that is part of the `.text` or executable section. - // This section may include multiple entries for the same address, we'll - // explain that more shortly! But for now, "bunch" them into buckets of - // symbols at the same address. We're *also* doing this because the symbols - // may not necessarily be in-order in the symbol table. - let mut buckets = BTreeMap::>::new(); - for sym in parsed_elf.syms.iter() { - // If it's not in the `.text` table, we're not interested! - let section = section_of(parsed_elf, &sym); - let SymSection::Section(".text") = section else { - continue; +impl FunctionCallCollector<'_> { + // attempt to extract all outgoing calls from the given chunk of executable + // code within this function + fn extract_calls(&mut self, data: &[u8], data_addr: u64) -> Result<()> { + let instrs = self + .cs + .disasm_all(data, data_addr) + .map_err(|e| anyhow!("disassembly failed: {e:?}"))?; + + let Some((last, all_except_last)) = instrs.split_last() else { + bail!("Empty chunk in {} at {:08X}", self.name, data_addr); }; - // Mask the lowest bit off, this is because thumb functions are listed - // with this bit set, because that controls the "mode" on jump in arm - // processors. - let base_addr = sym.st_value & !1; + // See https://www.nmichaels.org/musings/d4d4/d4d4/ + let to_consider = if last.bytes() == [0xD4, 0xD4] { + all_except_last + } else { + &instrs + }; - // Get the existing bucket for this address, or make a new bucket. - let syms = buckets.entry(base_addr as u32).or_default(); - syms.push(sym); + // Walk through each instruction inside of each chunk + for (i, instr) in to_consider.iter().enumerate() { + // We need to get details for the instruction, which we should + // always have for well-formed programs + let detail = self.cs.insn_detail(instr).map_err(|e| { + anyhow!("could not get instruction details: {e}") + })?; + self.process_one_instruction( + &detail, + i == (to_consider.len() - 1), + )?; + } + Ok(()) } - // Great, now we have a bunch of buckets! We expect to see something that - // looks like this, if you were to look at it with `objdump`: - // - // ```text - // 00012078 l .text 00000000 $t - // 00012078 l F .text 00000064 func_one - // 000120d0 l .text 00000000 $d - // 000120dc l .text 00000000 $t - // 000120dc l F .text 00000014 func_two - // ``` - // - // This is a bit harder to read, because it's out of order, as mentioned - // above, but we would expect three buckets in our code now: - // - // * 00012078 with two items ($t and func_one) - // * 000120d0 with one item ($d) - // * 000120dc with two items ($t and func_two) - // - // This might make more sense if we arrange this correctly: - // - // ```text - // 00012078 l F .text 00000064 func_one - // 00012078 l .text 00000000 $t - // 000120d0 l .text 00000000 $d - // 000120dc l F .text 00000014 func_two - // 000120dc l .text 00000000 $t - // ``` - // - // What we actually have here is two functions, `func_one` at 00012078, and - // `func_two` at `000120dc`. `func_one` has three symbols: The one marked - // `F`, which is the actual function symbol, showing it has a length of 64, - // and the name "func_one". Then it has `$t` at the same address, which - // shows that it starts with "executable code" first. Later, it has a `$d` - // symbol, which notes that inline data is here at the end of the function. - // After this, `func_two` begins. - // - // The following code is a small state machine that tries to extract all the - // symbols related to a single function into an aggregated "neochunk" - // report, which we will later use. - let mut neo_chunks = BTreeMap::::new(); - let mut current: Option = None; - - // For each bucket... - for (base_addr, mut syms) in buckets { - // ... while there is still any contents in this bucket... - while !syms.is_empty() { - // ..see if there is a function entry still in this bucket. If there - // is, remove it from the bucket and process it. - if let Some(entry) = { - syms.iter() - .position(|s| s.is_function()) - .map(|idx| syms.remove(idx)) - } { - // Found one, if there was a pending neochunk, "commit" it, - // using the current address to calculate the size, in case the - // function item was missing "function size" metadata, which we - // have observed with some externally linked static libraries, - // like `salty`. - if let Some(mut ch) = current.take() { - let calc_size = (base_addr - ch.base_addr) as u64; - ch.calc_size = calc_size; - ch.push_text_end(base_addr)?; - neo_chunks.insert(ch.base_addr, ch); - } - - // Store off the current chunk - current = Some(NeoChunk::new(entry, parsed_elf, base_addr)); - continue; - } - - // We know `syms` is non-empty, take some item in it - let entry = syms.pop().unwrap(); - - // Nope, see if we can extract $d/$t symbols to the current fn. - // We *should* always have a function entry by now, but if not, - // just log a warning. - let Some(current) = current.as_mut() else { - bail!("Discarded {entry:?} at {base_addr:08X}"); - }; - - match parsed_elf.strtab.get_at(entry.st_name) { - Some("$t") => current.push_text_start(base_addr)?, - Some("$d") => current.push_text_end(base_addr)?, - _other => { - // Ignore other symbols, like `_stext`, etc. - } - } - } - } - - // Finally push the last chunk onto the list - if let Some(mut cur) = current.take() { - // TODO! We need to figure out the end of the section here to fill in - // calculated length! - let text_end = text_section.sh_addr + text_section.sh_size; - cur.calc_size = text_end - cur.base_addr as u64; - cur.push_text_end(text_end as u32)?; - neo_chunks.insert(cur.base_addr, cur); - } - - Ok(neo_chunks) -} - -struct FunctionCollector<'a> { - cs: &'a Capstone, - name: &'a str, - base_addr: u32, - function_range: Range, - frame_size: Option, - verbose: bool, - missing_calls: usize, - recursive_calls: usize, - calls: BTreeSet, -} - -impl FunctionCollector<'_> { - fn extract_calls(&mut self, data: &[u8], data_addr: u64) -> Result<()> { - let instrs = self - .cs - .disasm_all(data, data_addr) - .map_err(|e| anyhow!("disassembly failed: {e:?}"))?; - - let Some((last, all_except_last)) = instrs.split_last() else { - bail!("Empty chunk in {} at {:08X}", self.name, data_addr); - }; - - let to_consider = if last.bytes() == [0xD4, 0xD4] { - all_except_last - } else { - &instrs - }; - - // Walk through each instruction inside of each chunk - for (i, instr) in to_consider.iter().enumerate() { - // We need to get details for the instruction, which we should - // always have for well-formed programs - let detail = self.cs.insn_detail(instr).map_err(|e| { - anyhow!("could not get instruction details: {e}") - })?; - self.process_one(&detail, i == (to_consider.len() - 1))?; - } - Ok(()) - } - - fn process_one( + /// Process a single instruction, and determine if it is making an outgoing + /// call + fn process_one_instruction( &mut self, detail: &InsnDetail<'_>, last_in_chunk: bool, @@ -676,9 +695,16 @@ impl FunctionCollector<'_> { // when the function itself has no stack frame. let is_tail_call = |g| is_grp_jump(g) && can_tail; let is_branching_instr = detail.groups().iter().any(|g| { - // Check if this is now not needed anymore + // NOTE: `is_tail_call` was a check before we started checking + // `is_grp_rel`. As of 2026-07-09, James *thinks* the latter check + // is a superset of the former, and we can probably remove it, but + // isn't that sure about it, and hasn't had time to look deeper. + // For now, we leave this as a tombstone to see if we ever observe + // this in practice. if !is_grp_rel(g) && is_tail_call(g) { - panic!("this check is NOT obsolete!"); + panic!( + "this check is NOT obsolete, please @jamesmunns about this" + ); } is_grp_call(g) || is_grp_rel(g) || is_tail_call(g) @@ -719,9 +745,12 @@ impl FunctionCollector<'_> { }; let target = u32::try_from(target).unwrap(); - // Avoid recursive calls to the same function, and ignore + // Note any recursive calls to the same function, and ignore // any control flow that directs back inside this function. // Any other jumps should be recorded as a call. + // + // We'll check this at the end if there were any instances + // of self-recursion if target == self.base_addr { self.recursive_calls += 1; } else if !self.function_range.contains(&target) { @@ -732,135 +761,6 @@ impl FunctionCollector<'_> { } } -/// Load and parse the `elf` file at the given path, producing a report of -/// metadata about all functions in the `elf` file. -pub fn extract_function_items( - elf: &Path, - verbose: bool, - allowed_recurses: &[String], -) -> Result { - // Open the statically-linked ELF file - let data = std::fs::read(elf).with_context(|| { - format!("could not open ELF file: {}", elf.display()) - })?; - let elf = goblin::elf::Elf::parse(&data) - .with_context(|| format!("could not parse {}", elf.display()))?; - - // Get sizes of stack frames by addr from the elf - let addr_to_frame_size = extract_stack_sizes_section(&data, &elf)?; - - let text = build_elf::get_section_by_name(&elf, ".text") - .context("could not get .text")?; - - let cs = Capstone::new() - .arm() - .mode(arm::ArchMode::Thumb) - .extra_mode(std::iter::once(arm::ArchExtraMode::MClass)) - .detail(true) - .build() - .map_err(|e| anyhow!("failed to initialize disassembler: {e:?}"))?; - - // Disassemble each function, building a map of its call sites - let mut fns = BTreeMap::new(); - - let chunks = neochunkify(&elf, text)?; - - for (base_addr, nchunk) in chunks { - // Walk through each "chunk", which is an island of executable code - // inside of each function, and collect all the out-bound calls. We - // disassemble chunks rather than functions, as functions might contain - // puddles of inline data which we don't want to (mis)-disassemble. - // - // This is the stack frame size of the current function - let frame_size = addr_to_frame_size.get(&base_addr).copied(); - let function_range = nchunk.function_range(); - - // Gather up all the information we need to find all the calls in this - // function - let mut fc = FunctionCollector { - cs: &cs, - name: &nchunk.name, - base_addr, - function_range, - frame_size, - verbose, - missing_calls: 0, - recursive_calls: 0, - calls: BTreeSet::new(), - }; - - for chunk in nchunk.text_ranges.iter() { - let (data_addr, data) = chunk.text_data(&data, text)?; - fc.extract_calls(data, data_addr)?; - } - - if fc.recursive_calls != 0 { - let allow_match = allowed_recurses - .iter() - .find(|allow| fc.name.contains(allow.as_str())); - - if let Some(am) = allow_match { - // For now, pragmatically, we'll just ignore this recursive - // call site. - println!( - "WARN: Allowing {} to self-recurse, matching {}", - fc.name, am - ); - } else { - bail!("Refusing to handle self-recursion of {}", fc.name); - } - } - - fns.insert( - base_addr, - FunctionData { - calls: fc.calls, - missing_calls: fc.missing_calls, - recursive_calls: fc.recursive_calls, - name: nchunk.name, - frame_size, - }, - ); - } - - Ok(FunctionReport { - function_items: fns, - addr_to_frame_size, - }) -} - -/// Estimates the maximum stack size for the given task -/// -/// This does not take dynamic function calls into account, which could cause -/// underestimation. Overestimation is less likely, but still may happen if -/// there are logically impossible call trees (e.g. `A -> B` and `B -> C`, but -/// `B` never calls `C` if called by `A`). -pub fn get_max_stack( - elf: &Path, - verbose: bool, - allowed_recurses: Vec, -) -> Result> { - let fns = extract_function_items(elf, verbose, &allowed_recurses)?; - let mut resolver = Resolver::new(fns.function_items, allowed_recurses); - let node = resolver.resolve_by_name("_start")?; - let chain = node.worst_chain(); - let mut chain_compat = chain - .into_iter() - .map(|n| (n.local_size.unwrap_or(0), n.name.clone())) - .collect::>(); - - let missing = resolver.find_missing_nodes()?; - - // Find the largest missing item so that we can add it to the call stack as - // a "fudge factor" for unresolved dynamic/indirect dispatch. - let largest = missing.iter().map(|n| n.max_stack()).max().unwrap_or(0); - if let Some(node) = missing.iter().find(|n| n.max_stack() == largest) { - chain_compat.push((largest, format!("missing::{}", node.name))); - } - - Ok(chain_compat) -} - //////////////////////////////////////////////////////////////////////////////// // Pulling data from the elf file //////////////////////////////////////////////////////////////////////////////// @@ -886,3 +786,137 @@ fn extract_stack_sizes_section( Ok(addr_to_frame_size) } + +/// Extract "raw" function data, which contains information local to each +/// function, without yet considering call graph data. +fn extract_raw_function_data( + parsed_elf: &Elf<'_>, + text_section: &SectionHeader, +) -> Result> { + // We're going to walk through each of the symbols in the symbol table, + // pulling any item that is part of the `.text` or executable section. + // This section may include multiple entries for the same address, we'll + // explain that more shortly! But for now, "bunch" them into buckets of + // symbols at the same address. We're *also* doing this because the symbols + // may not necessarily be in-order in the symbol table. + let mut buckets = BTreeMap::>::new(); + for sym in parsed_elf.syms.iter() { + // If it's not in the `.text` table, we're not interested! + let section = section_of(parsed_elf, &sym); + let SymSection::Section(".text") = section else { + continue; + }; + + // Mask the lowest bit off, this is because thumb functions are listed + // with this bit set, because that controls the "mode" on jump in arm + // processors. + let base_addr = sym.st_value & !1; + + // Get the existing bucket for this address, or make a new bucket. + let syms = buckets.entry(base_addr as u32).or_default(); + syms.push(sym); + } + + // Great, now we have a bunch of buckets! We expect to see something that + // looks like this, if you were to look at it with `objdump`: + // + // ```text + // 00012078 l .text 00000000 $t + // 00012078 l F .text 00000064 func_one + // 000120d0 l .text 00000000 $d + // 000120dc l .text 00000000 $t + // 000120dc l F .text 00000014 func_two + // ``` + // + // This is a bit harder to read, because it's out of order, as mentioned + // above, but we would expect three buckets in our code now: + // + // * 00012078 with two items ($t and func_one) + // * 000120d0 with one item ($d) + // * 000120dc with two items ($t and func_two) + // + // This might make more sense if we arrange this correctly: + // + // ```text + // 00012078 l F .text 00000064 func_one + // 00012078 l .text 00000000 $t + // 000120d0 l .text 00000000 $d + // 000120dc l F .text 00000014 func_two + // 000120dc l .text 00000000 $t + // ``` + // + // What we actually have here is two functions, `func_one` at 00012078, and + // `func_two` at `000120dc`. `func_one` has three symbols: The one marked + // `F`, which is the actual function symbol, showing it has a length of 64, + // and the name "func_one". Then it has `$t` at the same address, which + // shows that it starts with "executable code" first. Later, it has a `$d` + // symbol, which notes that inline data is here at the end of the function. + // After this, `func_two` begins. + // + // The following code is a small state machine that tries to extract all the + // symbols related to a single function into an aggregated "ElfFunctionData" + // report, which we will later use. + let mut functions = BTreeMap::::new(); + let mut current: Option = None; + + // For each bucket... + for (base_addr, mut syms) in buckets { + // ... while there is still any contents in this bucket... + while !syms.is_empty() { + // ..see if there is a function entry still in this bucket. If there + // is, remove it from the bucket and process it. + if let Some(entry) = { + syms.iter() + .position(|s| s.is_function()) + .map(|idx| syms.remove(idx)) + } { + // Found one, if there was a pending ElfFunctionData, "commit" + // it, using the current address to calculate the size, in case + // the function item was missing "function size" metadata, which + // we have observed with some externally linked static + // libraries, like `salty`. + if let Some(mut ch) = current.take() { + let calc_size = (base_addr - ch.base_addr) as u64; + ch.calc_size = calc_size; + ch.push_text_end(base_addr)?; + functions.insert(ch.base_addr, ch); + } + + // Store off the current chunk + current = + Some(RawFunctionData::new(entry, parsed_elf, base_addr)); + continue; + } + + // We know `syms` is non-empty, take some item in it + let entry = syms.pop().unwrap(); + + // Nope, see if we can extract $d/$t symbols to the current fn. + // We *should* always have a function entry by now, but if not, + // just log a warning. + let Some(current) = current.as_mut() else { + bail!("Discarded {entry:?} at {base_addr:08X}"); + }; + + match parsed_elf.strtab.get_at(entry.st_name) { + Some("$t") => current.push_text_start(base_addr)?, + Some("$d") => current.push_text_end(base_addr)?, + _other => { + // Ignore other symbols, like `_stext`, etc. + } + } + } + } + + // Finally push the last chunk onto the list + if let Some(mut cur) = current.take() { + // TODO! We need to figure out the end of the section here to fill in + // calculated length! + let text_end = text_section.sh_addr + text_section.sh_size; + cur.calc_size = text_end - cur.base_addr as u64; + cur.push_text_end(text_end as u32)?; + functions.insert(cur.base_addr, cur); + } + + Ok(functions) +} From b3c9f09f49e9c639cb17a86ea149bb5cfbd1f353 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 20:28:04 +0200 Subject: [PATCH 23/35] Bump a bunch of stack sizes --- app/gimlet/base.toml | 4 ++-- app/observer/base.toml | 2 +- app/psc/base.toml | 2 +- app/sidecar/base.toml | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/gimlet/base.toml b/app/gimlet/base.toml index 1dad584033..f7cb4c3f69 100644 --- a/app/gimlet/base.toml +++ b/app/gimlet/base.toml @@ -125,7 +125,7 @@ notifications = ["i2c1-irq", "jefe-state-change"] [tasks.packrat] name = "task-packrat" priority = 1 -stacksize = 1496 +stacksize = 1512 start = true task-slots = ["jefe"] features = ["gimlet", "ereport"] @@ -244,7 +244,7 @@ uses = ["uart7", "dbgmcu"] interrupts = {"uart7.irq" = "usart-irq"} priority = 8 max-sizes = {flash = 72000, ram = 65536} -stacksize = 7280 +stacksize = 7312 start = true task-slots = ["sys", { cpu_seq = "gimlet_seq" }, "hf", "control_plane_agent", "net", "packrat", "i2c_driver", { spi_driver = "spi2_driver" }, "sprot"] notifications = ["jefe-state-change", "usart-irq", "multitimer", "control-plane-agent"] diff --git a/app/observer/base.toml b/app/observer/base.toml index 5a70921a5d..473a096a1c 100644 --- a/app/observer/base.toml +++ b/app/observer/base.toml @@ -120,7 +120,7 @@ notifications = ["i2c2-irq", "i2c3-irq", "i2c4-irq"] [tasks.packrat] name = "task-packrat" priority = 1 -stacksize = 1480 +stacksize = 1496 start = true task-slots = ["jefe"] features = ["ereport"] diff --git a/app/psc/base.toml b/app/psc/base.toml index f55348fe85..96720bb474 100644 --- a/app/psc/base.toml +++ b/app/psc/base.toml @@ -121,7 +121,7 @@ notifications = ["i2c2-irq", "i2c3-irq"] [tasks.packrat] name = "task-packrat" priority = 1 -stacksize = 1480 +stacksize = 1496 start = true task-slots = ["jefe"] features = ["ereport"] diff --git a/app/sidecar/base.toml b/app/sidecar/base.toml index f8dcf9ced2..6e2a341379 100644 --- a/app/sidecar/base.toml +++ b/app/sidecar/base.toml @@ -272,7 +272,7 @@ notifications = ["timer"] [tasks.packrat] name = "task-packrat" priority = 1 -stacksize = 1480 +stacksize = 1496 start = true task-slots = ["jefe"] features = ["ereport"] From 1593c5a82f8a56252eb7612f927bc81ab931c31c Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 20:34:32 +0200 Subject: [PATCH 24/35] lol --- build/stack/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 511710d755..a328be4bcc 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -86,6 +86,9 @@ pub const KNOWN_RECURSORS: &[&str] = &[ // We have limited recursion here, see // https://github.com/oxidecomputer/hubris/issues/2593 "Vsc7448Spi", + // Tests have a method called "stackblow" that, you guessed it, blows + // the stack (intentionally) + "stackblow", ]; /// Information derived about a given function From 1129ba34f424853e97e8936bfc1e69829720a1a2 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 20:37:08 +0200 Subject: [PATCH 25/35] Revert, adding `stackblow` to the recursion doesn't help --- build/stack/src/lib.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index a328be4bcc..511710d755 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -86,9 +86,6 @@ pub const KNOWN_RECURSORS: &[&str] = &[ // We have limited recursion here, see // https://github.com/oxidecomputer/hubris/issues/2593 "Vsc7448Spi", - // Tests have a method called "stackblow" that, you guessed it, blows - // the stack (intentionally) - "stackblow", ]; /// Information derived about a given function From 56072c946b3e1989b0107ac8137517457ecc5950 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 21:31:57 +0200 Subject: [PATCH 26/35] Add ignore list --- build/stack/src/lib.rs | 94 ++++++++++++++++++++++++++++++++-------- build/xtask/src/dist.rs | 12 ++--- build/xtask/src/sizes.rs | 7 +-- 3 files changed, 81 insertions(+), 32 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 511710d755..807a3de521 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -88,6 +88,20 @@ pub const KNOWN_RECURSORS: &[&str] = &[ "Vsc7448Spi", ]; +pub const KNOWN_TO_IGNORE: &[&str] = &["stackblow"]; + +/// Configuration for public methods +#[derive(Debug, Clone)] +#[non_exhaustive] +pub struct Config { + /// Function names containing any of these patterns will be allowed to + /// recurse + pub allowed_recurses: Vec, + + /// Functions names containing any of these patterns will be totally ignored + pub ignored_functions: Vec, +} + /// Information derived about a given function #[derive(Debug, Clone)] pub struct FunctionData { @@ -127,7 +141,7 @@ pub struct Resolver { pub call_stack: Vec, pub all_resolved: BTreeMap>, pub fn_items: BTreeMap, - pub allowed_recurses: Vec, + pub config: Config, } /// Describes where a symbol lives. @@ -209,12 +223,12 @@ struct FunctionCallCollector<'a> { /// there are logically impossible call trees (e.g. `A -> B` and `B -> C`, but /// `B` never calls `C` if called by `A`). pub fn get_max_stack( + config: Config, elf: &Path, verbose: bool, - allowed_recurses: Vec, ) -> Result> { - let fns = extract_function_items(elf, verbose, &allowed_recurses)?; - let mut resolver = Resolver::new(fns.function_items, allowed_recurses); + let fns = extract_function_items(elf, verbose, &config)?; + let mut resolver = Resolver::new(fns.function_items, &config); let node = resolver.resolve_by_name("_start")?; let chain = node.worst_chain(); let mut chain_compat = chain @@ -239,7 +253,7 @@ pub fn get_max_stack( pub fn extract_function_items( elf: &Path, verbose: bool, - allowed_recurses: &[String], + config: &Config, ) -> Result { // Open the statically-linked ELF file let data = std::fs::read(elf).with_context(|| { @@ -296,9 +310,7 @@ pub fn extract_function_items( } if fc.recursive_calls != 0 { - let allow_match = allowed_recurses - .iter() - .find(|allow| fc.name.contains(allow.as_str())); + let allow_match = config.match_allowed_recurses(&fc.name); if let Some(am) = allow_match { // For now, pragmatically, we'll just ignore this recursive @@ -334,6 +346,37 @@ pub fn extract_function_items( // impls //////////////////////////////////////////////////////////////////////////////// +impl Default for Config { + fn default() -> Self { + Self { + allowed_recurses: KNOWN_RECURSORS + .iter() + .map(|s| s.to_string()) + .collect(), + ignored_functions: KNOWN_TO_IGNORE + .iter() + .map(|s| s.to_string()) + .collect(), + } + } +} + +impl Config { + fn match_allowed_recurses(&self, name: &str) -> Option<&str> { + self.allowed_recurses + .iter() + .find(|am| name.contains(am.as_str())) + .map(String::as_str) + } + + fn match_ignored_functions(&self, name: &str) -> Option<&str> { + self.ignored_functions + .iter() + .find(|am| name.contains(am.as_str())) + .map(String::as_str) + } +} + impl ResolvedNode { /// Print the entire stack trace to stdout pub fn debug_all(&self) { @@ -401,15 +444,12 @@ impl ResolvedNode { impl Resolver { /// Create a new resolver from the given map of [`FunctionData`] items - pub fn new( - fn_items: BTreeMap, - allowed_recurses: Vec, - ) -> Self { + pub fn new(fn_items: BTreeMap, config: &Config) -> Self { Self { call_stack: vec![], all_resolved: BTreeMap::new(), fn_items, - allowed_recurses, + config: config.clone(), } } @@ -442,14 +482,13 @@ impl Resolver { let mut res_children = BTreeMap::new(); let mut max_children = 0; for child in children { + // Is this a recursive callsite? if self.call_stack.contains(&child) { let Some(child_info) = self.fn_items.get(&child) else { bail!("{child:08X}: no child function data"); }; - let allow_match = self - .allowed_recurses - .iter() - .find(|allow| child_info.name.contains(allow.as_str())); + let allow_match = + self.config.match_allowed_recurses(&child_info.name); if let Some(am) = allow_match { // For now, pragmatically, we'll just ignore this recursive @@ -474,6 +513,15 @@ impl Resolver { .resolve_addr(child) .with_context(|| format!("While resolving {}", name))?; + // Is this an ignored function? + let ignore_match = + self.config.match_ignored_functions(&rchild.name); + if let Some(ignore) = ignore_match { + println!("WARN: Ignoring {}, matching {}", rchild.name, ignore); + continue; + } + + // Keep it! max_children = max_children.max(rchild.max_stack()); res_children.insert(child, rchild); } @@ -500,14 +548,22 @@ impl Resolver { // Find all of the functions we know about in `fn_items`, and find // any that haven't already been resolved into `all_resolved` from // previous resolution, usually the `_start` entry point - self.fn_items + let mut found = self + .fn_items .keys() .copied() .filter(|addr| !self.all_resolved.contains_key(addr)) .collect::>() // necessary to avoid double borrowing self .into_iter() .map(|addr| self.resolve_addr(addr)) - .collect::, _>>() + .collect::, _>>()?; + + // Filter out any ignored functions + found.retain(|rn| { + self.config.match_ignored_functions(&rn.name).is_none() + }); + + Ok(found) } } diff --git a/build/xtask/src/dist.rs b/build/xtask/src/dist.rs index f9541e6c3c..2fd2c438ed 100644 --- a/build/xtask/src/dist.rs +++ b/build/xtask/src/dist.rs @@ -1437,14 +1437,10 @@ fn task_can_overflow( .join(&toml.name) .join("dist") .join(format!("{task_name}.tmp")); - let allowed_recurses = build_stack::KNOWN_RECURSORS - .iter() - .map(|s| s.to_string()) - .collect(); - let max_stack = - get_max_stack(&f, verbose, allowed_recurses).with_context(|| { - format!("Failed getting max stack for task: {task_name}") - })?; + let config = build_stack::Config::default(); + let max_stack = get_max_stack(config, &f, verbose).with_context(|| { + format!("Failed getting max stack for task: {task_name}") + })?; let max_depth: u64 = max_stack.iter().map(|(d, _)| *d).sum(); let task_stack_size = toml.tasks[task_name] diff --git a/build/xtask/src/sizes.rs b/build/xtask/src/sizes.rs index 3a7a3050f9..40892a019c 100644 --- a/build/xtask/src/sizes.rs +++ b/build/xtask/src/sizes.rs @@ -440,11 +440,8 @@ fn print_task_stacks(toml: &Config) -> Result<()> { .join(&toml.name) .join("dist") .join(format!("{task_name}.tmp")); - let allowed_recurses = build_stack::KNOWN_RECURSORS - .iter() - .map(|s| s.to_string()) - .collect(); - let max_stack = get_max_stack(&f, false, allowed_recurses)?; + let config = build_stack::Config::default(); + let max_stack = get_max_stack(config, &f, false)?; let total: u64 = max_stack.iter().map(|(n, _)| *n).sum(); println!("{task_name}: {total} bytes (limit is {task_stack_size})"); for (frame_size, name) in max_stack { From b4d1e4f55e8c740bc8621bf4fbbc90347e5f2e8a Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 21:36:47 +0200 Subject: [PATCH 27/35] thanks clippy --- build/stack/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 807a3de521..37eace3c96 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -310,7 +310,7 @@ pub fn extract_function_items( } if fc.recursive_calls != 0 { - let allow_match = config.match_allowed_recurses(&fc.name); + let allow_match = config.match_allowed_recurses(fc.name); if let Some(am) = allow_match { // For now, pragmatically, we'll just ignore this recursive From 03c834418b021d03bba6f95e6610beb19130816b Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 21:53:47 +0200 Subject: [PATCH 28/35] Bump grapefruit, remove extra notation --- app/grapefruit/base.toml | 1 - app/grapefruit/standalone.toml | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/grapefruit/base.toml b/app/grapefruit/base.toml index 49e173abf4..6eaeea5f78 100644 --- a/app/grapefruit/base.toml +++ b/app/grapefruit/base.toml @@ -111,7 +111,6 @@ start = true # task-slots is explicitly empty: packrat should not send IPCs! task-slots = [] features = ["grapefruit", "boot-kmdb"] -stacksize = 1496 [tasks.hiffy] name = "task-hiffy" diff --git a/app/grapefruit/standalone.toml b/app/grapefruit/standalone.toml index 5157618613..a4b3bf04d9 100644 --- a/app/grapefruit/standalone.toml +++ b/app/grapefruit/standalone.toml @@ -11,7 +11,7 @@ interrupts = {"uart8.irq" = "usart-irq"} # Ereport stuff [tasks.packrat] -stacksize = 1496 +stacksize = 1512 task-slots = ["jefe"] features = ["ereport"] notifications = ["task-faulted"] From 78230103eb89713f5119ad89c10b3db944d8fa6d Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 22:04:31 +0200 Subject: [PATCH 29/35] Bump gimletlet --- app/gimletlet/app.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/gimletlet/app.toml b/app/gimletlet/app.toml index cdc9e40ba9..d8ce565760 100644 --- a/app/gimletlet/app.toml +++ b/app/gimletlet/app.toml @@ -52,7 +52,7 @@ name = "task-packrat" priority = 1 start = true task-slots = ["jefe"] -stacksize = 1480 +stacksize = 1496 features = ["ereport"] notifications = ["task-faulted"] From cbd7610f5b9351053ccb035dbcde218c31ca5147 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 22:24:14 +0200 Subject: [PATCH 30/35] bump cosmo --- app/cosmo/base.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/cosmo/base.toml b/app/cosmo/base.toml index 427cd002f8..44b0404320 100644 --- a/app/cosmo/base.toml +++ b/app/cosmo/base.toml @@ -132,7 +132,7 @@ notifications = ["i2c1-irq", "i2c2-irq", "i2c3-irq", "i2c4-irq"] [tasks.packrat] name = "task-packrat" priority = 1 -stacksize = 1496 +stacksize = 1512 start = true task-slots = ["jefe"] features = ["cosmo", "ereport"] @@ -259,7 +259,7 @@ uses = ["usart6", "dbgmcu"] interrupts = {"usart6.irq" = "usart-irq"} priority = 9 max-sizes = {flash = 74000, ram = 65536} -stacksize = 7632 +stacksize = 7664 start = true task-slots = ["sys", { cpu_seq = "cosmo_seq" }, "hf", "control_plane_agent", "net", "packrat", "i2c_driver", { spi_driver = "spi2_driver" }, "sprot", "auxflash"] notifications = ["jefe-state-change", "usart-irq", "multitimer", "control-plane-agent"] @@ -385,7 +385,7 @@ start = true notifications = ["jefe-state-change", "timer"] task-slots = ["jefe", "spartan7_loader", "packrat", "sensor"] uses = ["mmio_dimms"] -stacksize = 904 +stacksize = 920 [tasks.auxflash] name = "drv-auxflash-server" From b702011f358a5ddfdfca3a2bd25feb1528deb71c Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 9 Jul 2026 22:33:24 +0200 Subject: [PATCH 31/35] bump dev --- app/cosmo/base.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/cosmo/base.toml b/app/cosmo/base.toml index 44b0404320..eb2764fd0f 100644 --- a/app/cosmo/base.toml +++ b/app/cosmo/base.toml @@ -172,7 +172,7 @@ name = "task-hiffy" features = ["h753", "stm32h7", "i2c", "gpio", "spi", "qspi", "hash", "sprot", "turbo"] priority = 7 max-sizes = {flash = 32768, ram = 65536 } -stacksize = 1200 +stacksize = 1232 start = true task-slots = ["sys", "hf", "i2c_driver", "hash_driver", "update_server", "sprot", "net"] notifications = ["timer"] From b4ad49713de7b2c074cd36f22ee615ee1d5499f9 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 16 Jul 2026 09:15:51 +0200 Subject: [PATCH 32/35] Bump PSC/Observer --- app/observer/base.toml | 2 +- app/psc/base.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/observer/base.toml b/app/observer/base.toml index 6b154a6439..ee30cff5e0 100644 --- a/app/observer/base.toml +++ b/app/observer/base.toml @@ -120,7 +120,7 @@ notifications = ["i2c2-irq", "i2c3-irq", "i2c4-irq"] [tasks.packrat] name = "task-packrat" priority = 1 -stacksize = 1496 +stacksize = 1520 start = true task-slots = ["jefe"] features = ["ereport"] diff --git a/app/psc/base.toml b/app/psc/base.toml index 98d131501d..83c3d984c1 100644 --- a/app/psc/base.toml +++ b/app/psc/base.toml @@ -121,7 +121,7 @@ notifications = ["i2c2-irq", "i2c3-irq"] [tasks.packrat] name = "task-packrat" priority = 1 -stacksize = 1496 +stacksize = 1520 start = true task-slots = ["jefe"] features = ["ereport"] From b7cee9ab4671a9ed1b824cc000ca702e503fe9d1 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 16 Jul 2026 12:41:29 +0200 Subject: [PATCH 33/35] Bump sidecar --- app/sidecar/base.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/sidecar/base.toml b/app/sidecar/base.toml index b27d9a6778..f9f502d0d2 100644 --- a/app/sidecar/base.toml +++ b/app/sidecar/base.toml @@ -272,7 +272,7 @@ notifications = ["timer"] [tasks.packrat] name = "task-packrat" priority = 1 -stacksize = 1496 +stacksize = 1520 start = true task-slots = ["jefe"] features = ["ereport"] From 5af9908a355cc43b5dce41f80e3861971f369a10 Mon Sep 17 00:00:00 2001 From: James Munns Date: Fri, 17 Jul 2026 01:23:17 +0200 Subject: [PATCH 34/35] Bump gimletlet --- app/gimletlet/app.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/gimletlet/app.toml b/app/gimletlet/app.toml index c2ef9aabd5..38aac965c7 100644 --- a/app/gimletlet/app.toml +++ b/app/gimletlet/app.toml @@ -52,7 +52,7 @@ name = "task-packrat" priority = 1 start = true task-slots = ["jefe"] -stacksize = 1496 +stacksize = 1520 features = ["ereport"] notifications = ["task-faulted"] From 3a0832942e29d21d782b40c90f545da616e54b5c Mon Sep 17 00:00:00 2001 From: James Munns Date: Fri, 17 Jul 2026 15:12:32 +0200 Subject: [PATCH 35/35] Improve docs --- build/stack/src/lib.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/build/stack/src/lib.rs b/build/stack/src/lib.rs index 37eace3c96..112db5f86c 100644 --- a/build/stack/src/lib.rs +++ b/build/stack/src/lib.rs @@ -44,7 +44,10 @@ //! 1. This approach does not handle recursion, as we have no way to annotate a //! potential upper bound of recursive iterations. Currently, the code //! counts the number of direct recursion instances detected (e.g. self-calls -//! of a function), and refuses to resolve call stacks with cycles. +//! of a function), and refuses to resolve call stacks with cycles, UNLESS +//! a member of the cycle is included on the "allowed_recurses" list or +//! "ignored_functions" list, in which case these items are skipped and not +//! counted. //! 2. This approach does not handle "indirect" branching, which looks something //! like `blx r5` in assembly, and is often (but not exclusively) generated //! when calling a function through a vtable method, like `dyn Format`. @@ -88,7 +91,12 @@ pub const KNOWN_RECURSORS: &[&str] = &[ "Vsc7448Spi", ]; -pub const KNOWN_TO_IGNORE: &[&str] = &["stackblow"]; +pub const KNOWN_TO_IGNORE: &[&str] = &[ + // Our on-target test framework has an explicit "stackblow" function that + // intentionally causes a stack overflow. This unsurprisingly makes the + // max stack analysis upset. + "stackblow", +]; /// Configuration for public methods #[derive(Debug, Clone)]