From e86dde362fc0fa3b581c4ec4db46dc75dace83db Mon Sep 17 00:00:00 2001 From: James Munns Date: Wed, 11 Nov 2026 17:27:51 +0100 Subject: [PATCH 1/4] ringbuf: Handle toolchains >= 1.96 Atomic types have changed, causing us to be unable to handle ringbufs and counters. --- cmd/ringbuf/src/lib.rs | 75 ++++++++++++++++++++++++++---------- humility-core/src/reflect.rs | 14 ++++++- humility-doppel/src/lib.rs | 14 ++++++- 3 files changed, 80 insertions(+), 23 deletions(-) diff --git a/cmd/ringbuf/src/lib.rs b/cmd/ringbuf/src/lib.rs index 49e4d0d36..fee80715e 100644 --- a/cmd/ringbuf/src/lib.rs +++ b/cmd/ringbuf/src/lib.rs @@ -62,14 +62,16 @@ //! See the [`ringbuf` //! documentation](https://github.com/oxidecomputer/hubris/blob/master/lib/ringbuf/src/lib.rs) for more details. -use anyhow::{Result, bail}; +use anyhow::{Context, Result, anyhow, bail}; use clap::Parser; use humility::core::Core; use humility::hubris::*; use humility::log::info; use humility::reflect::{self, Format, Load, Value}; use humility_cli::{ExecutionContext, humility_cmd}; -use humility_doppel::{CountedRingbuf, CounterVariant, Ringbuf, StaticCell}; +use humility_doppel::{ + CountedRingbuf, CounterVariant, Counters, Ringbuf, StaticCell, +}; #[derive(Parser, Debug)] #[clap(name = "ringbuf", about = env!("CARGO_PKG_DESCRIPTION"))] @@ -112,6 +114,55 @@ struct TotalsOptions { no_totals: bool, } +type LoadedRingbuf = (Option, Option); +type RingbufLoader = fn(&Value) -> Result; + +// There are three possible shapes of ringbufs, depending on the age of the +// firmware. +// - Raw Ringbuf that is not wrapped by anything. +// - Safe Ringbuf that is inside a StaticCell. +// - CountedRingbuf, which consists of a `StaticCell` and a set of +// entry counters. +// +// Here we will attempt to handle all three, keeping all the error context of +// all of them if none of them succeed. +fn load_ringbuf(ringbuf_val: &Value) -> Result { + // Pre-hubris#219 + fn raw(v: &Value) -> Result { + Ok((Some(Ringbuf::from_value(v)?), None)) + } + // Post-hubris#219 + fn static_cell(v: &Value) -> Result { + let cell: StaticCell = StaticCell::from_value(v)?; + Ok((Some(cell.cell.value), None)) + } + // Post-hubris#1621 + fn counted(v: &Value) -> Result { + let CountedRingbuf { ringbuf, counters } = + CountedRingbuf::from_value(v)?; + Ok((ringbuf, Some(counters))) + } + + let methods: [(&str, RingbufLoader); 3] = [ + ("CountedRingbuf", counted), + ("Ringbuf", raw), + ("StaticCell", static_cell), + ]; + let mut errs = vec![]; + for (name, method) in methods { + match method(ringbuf_val) { + Ok(r) => return Ok(r), + Err(e) => errs.push((name, e)), + } + } + + let mut err = Err(anyhow!("Failed to load ringbuf")); + for (name, e) in errs { + err = err.with_context(|| format!("Tried '{name}', got: {e:#}")); + } + err.context("Failed to load ringbuf using any known layout") +} + fn ringbuf_dump( subargs: &RingbufArgs, hubris: &HubrisArchive, @@ -127,28 +178,10 @@ fn ringbuf_dump( core.read_bulk(ringbuf_var.addr, buf.as_mut_slice())?; core.run()?; - // There are three possible shapes of ringbufs, depending on the age of the - // firmware. - // - Raw Ringbuf that is not wrapped by anything. - // - Safe Ringbuf that is inside a StaticCell. - // - CountedRingbuf, which consists of a `StaticCell` and a set of - // entry counters. - // - // Here we will attempt to handle all three -- first the counted ringbuf, - // then raw, then fallback. let ringbuf_val: Value = Value::Struct(reflect::load_struct(hubris, &buf, definition, 0)?); - let (ringbuf, counters) = CountedRingbuf::from_value(&ringbuf_val) - .map(|CountedRingbuf { ringbuf, counters }| (ringbuf, Some(counters))) - .or_else(|_e| { - Ringbuf::from_value(&ringbuf_val).map(|r| (Some(r), None)) - }) - .or_else(|_e| { - let cell: StaticCell = - StaticCell::from_value(&ringbuf_val)?; - Ok::<_, anyhow::Error>((Some(cell.cell.value), None)) - })?; + let (ringbuf, counters) = load_ringbuf(&ringbuf_val)?; if let (Some(mut counters), false) = (counters, no_totals) { const TOTAL_WIDTH: usize = 8; diff --git a/humility-core/src/reflect.rs b/humility-core/src/reflect.rs index 62dc421e0..d3f2d5ded 100644 --- a/humility-core/src/reflect.rs +++ b/humility-core/src/reflect.rs @@ -962,7 +962,19 @@ pub fn load_enum( /// That extra level is why images built by the newer toolchains stopped /// decoding. We peel all of these, so the payload can be presented the same way /// whichever toolchain built the image. -const TRANSPARENT_WRAPPERS: &[&str] = &["ManuallyDrop", "MaybeDangling"]; +const TRANSPARENT_WRAPPERS: &[&str] = &[ + "ManuallyDrop", + "MaybeDangling", + // Starting in Rust 1.96, `Atomic` types now are aliases and have an + // inner private `Align*` type. + // + // See https://github.com/rust-lang/rust/pull/153015 for details. + "Align1", + "Align2", + "Align4", + "Align8", + "Align16", +]; /// Returns true if `name` names one of the [`TRANSPARENT_WRAPPERS`]. /// diff --git a/humility-doppel/src/lib.rs b/humility-doppel/src/lib.rs index 0d9ef1684..c8e7a3a61 100644 --- a/humility-doppel/src/lib.rs +++ b/humility-doppel/src/lib.rs @@ -678,10 +678,22 @@ impl humility::reflect::Load for CounterVariant { return Ok(Self::Single(0)); } + // Starting in Rust 1.96, `Atomic` types now are aliases of + // `Atomic`. + // + // See https://github.com/rust-lang/rust/pull/153015 for details. + let is_atomic_u32 = |name| { + // Pre-hubris#2775 + let legacy = name.starts_with("AtomicU32"); + // Post-hubris#2775 + let generic = name == "Atomic"; + legacy || generic + }; + // If the value is directly an AtomicU32, we treat it as a single // counter value. if let Ok(counter) = value.as_struct() - && counter.name().starts_with("AtomicU32") + && is_atomic_u32(counter.name()) { let cell = UnsafeCell::::from_value(&counter["v"]) .context("ringbuf count must be a u32")?; From 0c76aa72d1022c7530799ef3f156a4c2914198f8 Mon Sep 17 00:00:00 2001 From: James Munns Date: Wed, 11 Nov 2026 18:28:27 +0100 Subject: [PATCH 2/4] Add type annotation --- humility-doppel/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/humility-doppel/src/lib.rs b/humility-doppel/src/lib.rs index c8e7a3a61..26ee65b13 100644 --- a/humility-doppel/src/lib.rs +++ b/humility-doppel/src/lib.rs @@ -682,7 +682,7 @@ impl humility::reflect::Load for CounterVariant { // `Atomic`. // // See https://github.com/rust-lang/rust/pull/153015 for details. - let is_atomic_u32 = |name| { + let is_atomic_u32 = |name: &str| { // Pre-hubris#2775 let legacy = name.starts_with("AtomicU32"); // Post-hubris#2775 From 2d353d87f2716d7b998863a8e17c97f64a335cbc Mon Sep 17 00:00:00 2001 From: James Munns Date: Wed, 11 Nov 2026 18:43:12 +0100 Subject: [PATCH 3/4] Add hint that `-v` will show the full context. --- cmd/ringbuf/src/lib.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/cmd/ringbuf/src/lib.rs b/cmd/ringbuf/src/lib.rs index fee80715e..d5364f430 100644 --- a/cmd/ringbuf/src/lib.rs +++ b/cmd/ringbuf/src/lib.rs @@ -324,6 +324,7 @@ fn ringbuf(subargs: RingbufArgs, context: &mut ExecutionContext) -> Result<()> { } let core = &mut *context.cli.attach_live_or_dump_match(hubris)?; + let mut show_hint = false; for (v, def) in ringbufs { // Try not to use `?` here, because it causes one bad ringbuf to make // them all unavailable. @@ -337,6 +338,7 @@ fn ringbuf(subargs: RingbufArgs, context: &mut ExecutionContext) -> Result<()> { if subargs.verbose { info!(log, "ringbuf dump failed: {e:?}"); } else { + show_hint = true; info!(log, "ringbuf dump failed: {e}"); } } @@ -345,6 +347,16 @@ fn ringbuf(subargs: RingbufArgs, context: &mut ExecutionContext) -> Result<()> { } } + // If any decoding failed, show a hint that rerunning with -v will show the + // whole error chain. + if show_hint { + info!( + log, + "one or more ringbufs failed to load. Rerun with `-v` to show more \ + details.", + ); + } + Ok(()) } From 8220d7cec81a6f892dfb0abe3d3bfd0ae8e30a65 Mon Sep 17 00:00:00 2001 From: James Munns Date: Wed, 11 Nov 2026 19:15:57 +0100 Subject: [PATCH 4/4] Bless new outputs --- .../tests/cmd/ringbuf-arg/ringbuf-arg.panic-on-boot.stderr | 1 + .../ringbuf-full-totals/ringbuf-full-totals.panic-on-boot.stderr | 1 + humility-bin/tests/cmd/ringbuf/ringbuf.panic-on-boot.stderr | 1 + 3 files changed, 3 insertions(+) diff --git a/humility-bin/tests/cmd/ringbuf-arg/ringbuf-arg.panic-on-boot.stderr b/humility-bin/tests/cmd/ringbuf-arg/ringbuf-arg.panic-on-boot.stderr index 3ee1271e4..d6aabea72 100644 --- a/humility-bin/tests/cmd/ringbuf-arg/ringbuf-arg.panic-on-boot.stderr +++ b/humility-bin/tests/cmd/ringbuf-arg/ringbuf-arg.panic-on-boot.stderr @@ -3,3 +3,4 @@ humility: ringbuf dump failed: loading value of type struct UnsafeCell> at address 0x0 humility: ringbuf dump failed: loading value of type struct UnsafeCell> at address 0x0 humility: ringbuf dump failed: loading value of type struct UnsafeCell> at address 0x0 +humility: one or more ringbufs failed to load. Rerun with `-v` to show more details. diff --git a/humility-bin/tests/cmd/ringbuf-full-totals/ringbuf-full-totals.panic-on-boot.stderr b/humility-bin/tests/cmd/ringbuf-full-totals/ringbuf-full-totals.panic-on-boot.stderr index df15ee5c0..d9888b97c 100644 --- a/humility-bin/tests/cmd/ringbuf-full-totals/ringbuf-full-totals.panic-on-boot.stderr +++ b/humility-bin/tests/cmd/ringbuf-full-totals/ringbuf-full-totals.panic-on-boot.stderr @@ -18,3 +18,4 @@ humility: ringbuf dump failed: loading value of type struct UnsafeCell> at address 0x0 humility: ringbuf dump failed: loading value of type struct UnsafeCell> at address 0x0 humility: ringbuf dump failed: loading value of type struct UnsafeCell> at address 0x0 +humility: one or more ringbufs failed to load. Rerun with `-v` to show more details. diff --git a/humility-bin/tests/cmd/ringbuf/ringbuf.panic-on-boot.stderr b/humility-bin/tests/cmd/ringbuf/ringbuf.panic-on-boot.stderr index df15ee5c0..d9888b97c 100644 --- a/humility-bin/tests/cmd/ringbuf/ringbuf.panic-on-boot.stderr +++ b/humility-bin/tests/cmd/ringbuf/ringbuf.panic-on-boot.stderr @@ -18,3 +18,4 @@ humility: ringbuf dump failed: loading value of type struct UnsafeCell> at address 0x0 humility: ringbuf dump failed: loading value of type struct UnsafeCell> at address 0x0 humility: ringbuf dump failed: loading value of type struct UnsafeCell> at address 0x0 +humility: one or more ringbufs failed to load. Rerun with `-v` to show more details.