From 6308bb2e23182989b336c6e86fe398960cd21a3d Mon Sep 17 00:00:00 2001 From: Fenrir Date: Thu, 4 Sep 2025 20:39:32 -0700 Subject: [PATCH 1/7] Remove allocations from the ctru panic hook --- ctru-rs/Cargo.toml | 1 + ctru-rs/src/applets/error.rs | 75 ++++++++++++++++++++++++++++++++---- 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/ctru-rs/Cargo.toml b/ctru-rs/Cargo.toml index 9351af94..181fcbd4 100644 --- a/ctru-rs/Cargo.toml +++ b/ctru-rs/Cargo.toml @@ -25,6 +25,7 @@ libc = { workspace = true, default-features = true } bitflags = "2.6.0" macaddr = "1.0.1" widestring = "1.1.0" +itoa = "1.0.15" [build-dependencies] toml = "0.9.4" diff --git a/ctru-rs/src/applets/error.rs b/ctru-rs/src/applets/error.rs index 17f6acfa..7870cb4a 100644 --- a/ctru-rs/src/applets/error.rs +++ b/ctru-rs/src/applets/error.rs @@ -4,7 +4,9 @@ use crate::services::{apt::Apt, gfx::Gfx}; -use ctru_sys::errorConf; +use ctru_sys::{errorConf, errorDisp}; + +use std::cell::UnsafeCell; /// Configuration struct to set up the Error applet. #[doc(alias = "errorConf")] @@ -94,12 +96,32 @@ impl PopUp { } } +struct PanicHookConfig { + error_app: UnsafeCell, +} + +impl PanicHookConfig { + fn new() -> Self { + Self { + error_app: UnsafeCell::new(PopUp::new(WordWrap::Enabled)), + } + } + + unsafe fn get(&self) -> *mut errorConf { + unsafe { (*self.error_app.get()).state.as_mut() } + } +} + +unsafe impl Sync for PanicHookConfig {} + pub(crate) fn set_panic_hook(call_old_hook: bool) { use crate::services::gfx::GFX_ACTIVE; use std::sync::TryLockError; let old_hook = std::panic::take_hook(); + let config = PanicHookConfig::new(); + std::panic::set_hook(Box::new(move |panic_info| { // If we get a `WouldBlock` error, we know that the `Gfx` service has been initialized. // Otherwise fallback to using the old panic hook. @@ -108,18 +130,57 @@ pub(crate) fn set_panic_hook(call_old_hook: bool) { old_hook(panic_info); } - let thread = std::thread::current(); + let error_conf = unsafe { &mut *config.get() }; - let name = thread.name().unwrap_or(""); + let mut buf1 = itoa::Buffer::new(); - let message = format!("thread '{name}' {panic_info}"); + let mut buf2 = itoa::Buffer::new(); - let mut popup = PopUp::new(WordWrap::Enabled); + let thread = std::thread::current(); + + let name = thread.name().unwrap_or(""); - popup.set_text(&message); + let location = panic_info.location().unwrap(); + + let file = location.file(); + + let line = buf1.format(location.line()); + + let column = buf2.format(location.column()); + + let payload = if let Some(s) = panic_info.payload().downcast_ref::<&str>() { + s + } else if let Some(s) = panic_info.payload().downcast_ref::() { + s.as_str() + } else { + "" + }; + + let message = [ + "thread '", + name, + "' panicked at ", + file, + ":", + line, + ":", + column, + ":", + payload, + ]; + + for (idx, code_unit) in message + .into_iter() + .flat_map(str::encode_utf16) + .take(error_conf.Text.len() - 1) + .chain(std::iter::once(0)) + .enumerate() + { + error_conf.Text[idx] = code_unit; + } unsafe { - let _ = popup.launch_unchecked(); + errorDisp(error_conf); } } else { old_hook(panic_info); From 1cd9437f50e967fce974f0474c2c9c9d2fe31d4b Mon Sep 17 00:00:00 2001 From: Fenrir Date: Thu, 4 Sep 2025 22:02:39 -0700 Subject: [PATCH 2/7] Clarify our `UnsafeCell` shenanigans --- ctru-rs/src/applets/error.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ctru-rs/src/applets/error.rs b/ctru-rs/src/applets/error.rs index 7870cb4a..f6ebac91 100644 --- a/ctru-rs/src/applets/error.rs +++ b/ctru-rs/src/applets/error.rs @@ -107,6 +107,8 @@ impl PanicHookConfig { } } + // There can only be one invocation of an applet active at any given time, so our `UnsafeCell` + // crimes *should* be okay here. unsafe fn get(&self) -> *mut errorConf { unsafe { (*self.error_app.get()).state.as_mut() } } From b4ce468b64e17fff438762142dc0b0a3b1527c3c Mon Sep 17 00:00:00 2001 From: Fenrir Date: Thu, 4 Sep 2025 23:40:17 -0700 Subject: [PATCH 3/7] A rare case in which `static mut` is probably better --- ctru-rs/src/applets/error.rs | 34 +++++++--------------------------- 1 file changed, 7 insertions(+), 27 deletions(-) diff --git a/ctru-rs/src/applets/error.rs b/ctru-rs/src/applets/error.rs index f6ebac91..a7b426b0 100644 --- a/ctru-rs/src/applets/error.rs +++ b/ctru-rs/src/applets/error.rs @@ -4,9 +4,7 @@ use crate::services::{apt::Apt, gfx::Gfx}; -use ctru_sys::{errorConf, errorDisp}; - -use std::cell::UnsafeCell; +use ctru_sys::{errorConf, errorDisp, errorInit}; /// Configuration struct to set up the Error applet. #[doc(alias = "errorConf")] @@ -48,7 +46,7 @@ impl PopUp { pub fn new(word_wrap: WordWrap) -> Self { let mut state = Box::::default(); - unsafe { ctru_sys::errorInit(state.as_mut(), word_wrap as _, 0) }; + unsafe { errorInit(state.as_mut(), word_wrap as _, 0) }; Self { state } } @@ -96,33 +94,15 @@ impl PopUp { } } -struct PanicHookConfig { - error_app: UnsafeCell, -} - -impl PanicHookConfig { - fn new() -> Self { - Self { - error_app: UnsafeCell::new(PopUp::new(WordWrap::Enabled)), - } - } - - // There can only be one invocation of an applet active at any given time, so our `UnsafeCell` - // crimes *should* be okay here. - unsafe fn get(&self) -> *mut errorConf { - unsafe { (*self.error_app.get()).state.as_mut() } - } -} - -unsafe impl Sync for PanicHookConfig {} - pub(crate) fn set_panic_hook(call_old_hook: bool) { use crate::services::gfx::GFX_ACTIVE; use std::sync::TryLockError; - let old_hook = std::panic::take_hook(); + static mut ERROR_CONF: errorConf = unsafe { std::mem::zeroed() }; + + unsafe { errorInit(&raw mut ERROR_CONF, WordWrap::Enabled as _, 0) }; - let config = PanicHookConfig::new(); + let old_hook = std::panic::take_hook(); std::panic::set_hook(Box::new(move |panic_info| { // If we get a `WouldBlock` error, we know that the `Gfx` service has been initialized. @@ -132,7 +112,7 @@ pub(crate) fn set_panic_hook(call_old_hook: bool) { old_hook(panic_info); } - let error_conf = unsafe { &mut *config.get() }; + let error_conf = unsafe { (&raw mut ERROR_CONF).as_mut().unwrap() }; let mut buf1 = itoa::Buffer::new(); From 00ff94f951835dff76fa826f210d6bd34c9ec4dc Mon Sep 17 00:00:00 2001 From: Fenrir Date: Fri, 5 Sep 2025 00:54:16 -0700 Subject: [PATCH 4/7] Use a mutex to guard global `errorConf` access --- ctru-rs/src/applets/error.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ctru-rs/src/applets/error.rs b/ctru-rs/src/applets/error.rs index a7b426b0..c6abd4d1 100644 --- a/ctru-rs/src/applets/error.rs +++ b/ctru-rs/src/applets/error.rs @@ -96,11 +96,13 @@ impl PopUp { pub(crate) fn set_panic_hook(call_old_hook: bool) { use crate::services::gfx::GFX_ACTIVE; - use std::sync::TryLockError; + use std::sync::{Mutex, TryLockError}; - static mut ERROR_CONF: errorConf = unsafe { std::mem::zeroed() }; + static ERROR_CONF: Mutex = unsafe { Mutex::new(std::mem::zeroed()) }; - unsafe { errorInit(&raw mut ERROR_CONF, WordWrap::Enabled as _, 0) }; + let mut lock = ERROR_CONF.lock().unwrap(); + + unsafe { errorInit(&raw mut *lock, WordWrap::Enabled as _, 0) }; let old_hook = std::panic::take_hook(); @@ -112,7 +114,9 @@ pub(crate) fn set_panic_hook(call_old_hook: bool) { old_hook(panic_info); } - let error_conf = unsafe { (&raw mut ERROR_CONF).as_mut().unwrap() }; + let mut lock = ERROR_CONF.lock().unwrap(); + + let error_conf = unsafe { (&raw mut *lock).as_mut().unwrap() }; let mut buf1 = itoa::Buffer::new(); From 5f9e36aeb05d0f2a87b1618911c1626b1442f998 Mon Sep 17 00:00:00 2001 From: Fenrir Date: Fri, 5 Sep 2025 01:31:45 -0700 Subject: [PATCH 5/7] Use `std::fmt::write` again instead of `itoa` --- ctru-rs/Cargo.toml | 1 - ctru-rs/src/applets/error.rs | 68 ++++++++++++++---------------------- 2 files changed, 27 insertions(+), 42 deletions(-) diff --git a/ctru-rs/Cargo.toml b/ctru-rs/Cargo.toml index 181fcbd4..9351af94 100644 --- a/ctru-rs/Cargo.toml +++ b/ctru-rs/Cargo.toml @@ -25,7 +25,6 @@ libc = { workspace = true, default-features = true } bitflags = "2.6.0" macaddr = "1.0.1" widestring = "1.1.0" -itoa = "1.0.15" [build-dependencies] toml = "0.9.4" diff --git a/ctru-rs/src/applets/error.rs b/ctru-rs/src/applets/error.rs index c6abd4d1..be5507b1 100644 --- a/ctru-rs/src/applets/error.rs +++ b/ctru-rs/src/applets/error.rs @@ -94,8 +94,30 @@ impl PopUp { } } +struct ErrorConfWriter<'a> { + error_conf: &'a mut errorConf, + index: usize, +} + +impl std::fmt::Write for ErrorConfWriter<'_> { + fn write_str(&mut self, s: &str) -> Result<(), std::fmt::Error> { + for code_unit in s.encode_utf16() { + if self.index == self.error_conf.Text.len() - 1 { + self.error_conf.Text[self.index] = 0; + return Err(std::fmt::Error); + } else { + self.error_conf.Text[self.index] = code_unit; + self.index += 1; + } + } + + Ok(()) + } +} + pub(crate) fn set_panic_hook(call_old_hook: bool) { use crate::services::gfx::GFX_ACTIVE; + use std::fmt::Write; use std::sync::{Mutex, TryLockError}; static ERROR_CONF: Mutex = unsafe { Mutex::new(std::mem::zeroed()) }; @@ -118,52 +140,16 @@ pub(crate) fn set_panic_hook(call_old_hook: bool) { let error_conf = unsafe { (&raw mut *lock).as_mut().unwrap() }; - let mut buf1 = itoa::Buffer::new(); - - let mut buf2 = itoa::Buffer::new(); + let mut writer = ErrorConfWriter { + error_conf, + index: 0, + }; let thread = std::thread::current(); let name = thread.name().unwrap_or(""); - let location = panic_info.location().unwrap(); - - let file = location.file(); - - let line = buf1.format(location.line()); - - let column = buf2.format(location.column()); - - let payload = if let Some(s) = panic_info.payload().downcast_ref::<&str>() { - s - } else if let Some(s) = panic_info.payload().downcast_ref::() { - s.as_str() - } else { - "" - }; - - let message = [ - "thread '", - name, - "' panicked at ", - file, - ":", - line, - ":", - column, - ":", - payload, - ]; - - for (idx, code_unit) in message - .into_iter() - .flat_map(str::encode_utf16) - .take(error_conf.Text.len() - 1) - .chain(std::iter::once(0)) - .enumerate() - { - error_conf.Text[idx] = code_unit; - } + let _ = write!(writer, "thread '{name}' {panic_info}\0"); unsafe { errorDisp(error_conf); From 8d1bd8ef45084ca451bcd4e04ffa561ba19a12e9 Mon Sep 17 00:00:00 2001 From: Fenrir Date: Fri, 5 Sep 2025 13:22:18 -0700 Subject: [PATCH 6/7] Properly handle nul-termination in ErrorConfWriter --- ctru-rs/src/applets/error.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ctru-rs/src/applets/error.rs b/ctru-rs/src/applets/error.rs index be5507b1..524fd303 100644 --- a/ctru-rs/src/applets/error.rs +++ b/ctru-rs/src/applets/error.rs @@ -101,8 +101,10 @@ struct ErrorConfWriter<'a> { impl std::fmt::Write for ErrorConfWriter<'_> { fn write_str(&mut self, s: &str) -> Result<(), std::fmt::Error> { + let max = self.error_conf.Text.len() - 1; + for code_unit in s.encode_utf16() { - if self.index == self.error_conf.Text.len() - 1 { + if self.index == max { self.error_conf.Text[self.index] = 0; return Err(std::fmt::Error); } else { @@ -111,6 +113,8 @@ impl std::fmt::Write for ErrorConfWriter<'_> { } } + self.error_conf.Text[self.index] = 0; + Ok(()) } } @@ -149,7 +153,7 @@ pub(crate) fn set_panic_hook(call_old_hook: bool) { let name = thread.name().unwrap_or(""); - let _ = write!(writer, "thread '{name}' {panic_info}\0"); + let _ = write!(writer, "thread '{name}' {panic_info}"); unsafe { errorDisp(error_conf); From 394da2f66972821f8d0e063448d488d706b6e5e4 Mon Sep 17 00:00:00 2001 From: Fenrir Date: Sat, 11 Oct 2025 19:43:34 -0700 Subject: [PATCH 7/7] Remove some dumb unsafe code --- ctru-rs/src/applets/error.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ctru-rs/src/applets/error.rs b/ctru-rs/src/applets/error.rs index 524fd303..6add3444 100644 --- a/ctru-rs/src/applets/error.rs +++ b/ctru-rs/src/applets/error.rs @@ -128,7 +128,7 @@ pub(crate) fn set_panic_hook(call_old_hook: bool) { let mut lock = ERROR_CONF.lock().unwrap(); - unsafe { errorInit(&raw mut *lock, WordWrap::Enabled as _, 0) }; + unsafe { errorInit(&mut *lock, WordWrap::Enabled as _, 0) }; let old_hook = std::panic::take_hook(); @@ -142,7 +142,7 @@ pub(crate) fn set_panic_hook(call_old_hook: bool) { let mut lock = ERROR_CONF.lock().unwrap(); - let error_conf = unsafe { (&raw mut *lock).as_mut().unwrap() }; + let error_conf = &mut *lock; let mut writer = ErrorConfWriter { error_conf,