From 72554b43ad16fd0a4782ad50c740d5d55fb13937 Mon Sep 17 00:00:00 2001 From: Seiya Nuta Date: Thu, 4 Nov 2021 22:20:34 +0900 Subject: [PATCH] Embed a backtrace into Error in debug build It helps where an error is constructed: [ 0.585] [7:curl] syscall: close(ffffffffffffffff, 0, 0, 0, 0, 0) [ 0.585] WARN: close: error: EBADF: This error originates from: #1: ffff800000184219 kerla::printk::capture_backtrace()+0x79 #2: ffff8000001dbb13 kerla::result::Error::new()+0x23 #3: ffff8000001dc0e7 >::from()+0x17 #4: ffff800000211177 >::into()+0x17 #5: ffff8000001fcb55 kerla::fs::opened_file::OpenedFileTable::close()+0xa5 #6: ffff80000010b124 kerla::syscalls::close::::sys_close()+0xa4 #7: ffff80000011bf62 kerla::syscalls::SyscallHandler::do_dispatch()+0x4e62 #8: ffff8000001168fc kerla::syscalls::SyscallHandler::dispatch()+0x1ac --- Cargo.lock | 1 + kernel/Cargo.toml | 1 + kernel/arch/x64/address.rs | 25 +++++++++++++---- kernel/arch/x64/backtrace.rs | 4 +-- kernel/printk.rs | 46 ++++++++++++++++++++++++++++++- kernel/result.rs | 53 ++++++++++++++++++++++++++++++++++-- 6 files changed, 119 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 05a81abd..3aedf3fb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -138,6 +138,7 @@ dependencies = [ "arrayvec", "bitflags", "buddy_system_allocator", + "cfg-if", "crossbeam", "goblin", "hashbrown", diff --git a/kernel/Cargo.toml b/kernel/Cargo.toml index 31eb7858..d9e95721 100644 --- a/kernel/Cargo.toml +++ b/kernel/Cargo.toml @@ -21,3 +21,4 @@ hashbrown = { version = "0.11.2", features = ["nightly"] } log = "0.4" crossbeam = { version = "0.8.1", default-features = false, features = ["alloc"] } smoltcp = { version = "0.7.5", default-features = false, features = ["alloc", "proto-ipv4", "socket", "socket-raw", "socket-udp", "socket-tcp", "proto-dhcpv4", "ethernet"] } +cfg-if = "1" diff --git a/kernel/arch/x64/address.rs b/kernel/arch/x64/address.rs index 20632333..b56442ac 100644 --- a/kernel/arch/x64/address.rs +++ b/kernel/arch/x64/address.rs @@ -153,7 +153,10 @@ pub struct UserVAddr(u64); impl UserVAddr { pub const fn new(addr: usize) -> Result> { if (addr as u64) >= KERNEL_BASE_ADDR { - return Err(Error::with_message(Errno::EFAULT, "invalid user pointer")); + return Err(Error::with_message_const( + Errno::EFAULT, + "invalid user pointer", + )); } if addr == 0 { @@ -165,11 +168,17 @@ impl UserVAddr { pub const fn new_nonnull(addr: usize) -> Result { if (addr as u64) >= KERNEL_BASE_ADDR { - return Err(Error::with_message(Errno::EFAULT, "invalid user pointer")); + return Err(Error::with_message_const( + Errno::EFAULT, + "invalid user pointer", + )); } if addr == 0 { - return Err(Error::with_message(Errno::EFAULT, "null user pointer")); + return Err(Error::with_message_const( + Errno::EFAULT, + "null user pointer", + )); } Ok(UserVAddr(addr as u64)) @@ -204,8 +213,14 @@ impl UserVAddr { pub fn access_ok(self, len: usize) -> Result<()> { match self.value().checked_add(len) { Some(end) if end <= KERNEL_BASE_ADDR as usize => Ok(()), - Some(_end) => Err(Error::with_message(Errno::EFAULT, "invalid user pointer")), - None => Err(Error::with_message(Errno::EFAULT, "overflow in access_ok")), + Some(_end) => Err(Error::with_message_const( + Errno::EFAULT, + "invalid user pointer", + )), + None => Err(Error::with_message_const( + Errno::EFAULT, + "overflow in access_ok", + )), } } diff --git a/kernel/arch/x64/backtrace.rs b/kernel/arch/x64/backtrace.rs index fcce436f..c5212419 100644 --- a/kernel/arch/x64/backtrace.rs +++ b/kernel/arch/x64/backtrace.rs @@ -19,9 +19,9 @@ impl Backtrace { } } - pub fn traverse(self, callback: F) + pub fn traverse(self, mut callback: F) where - F: Fn(usize, VAddr), + F: FnMut(usize, VAddr), { let mut frame = self.frame; for i in 0..BACKTRACE_MAX { diff --git a/kernel/printk.rs b/kernel/printk.rs index 2c9f79bc..59ae4013 100644 --- a/kernel/printk.rs +++ b/kernel/printk.rs @@ -1,12 +1,14 @@ +use alloc::boxed::Box; +use arrayvec::ArrayVec; use kerla_utils::ring_buffer::RingBuffer; use crate::arch::SpinLock; use crate::arch::{print_str, printchar, Backtrace, VAddr}; use crate::lang_items::PANICKED; use core::mem::size_of; -use core::slice; use core::str; use core::sync::atomic::Ordering; +use core::{fmt, slice}; pub struct Printer; pub const KERNEL_LOG_BUF_SIZE: usize = 8192; @@ -238,3 +240,45 @@ pub fn backtrace() { } }); } + +pub struct CapturedBacktraceFrame { + pub vaddr: VAddr, + pub offset: usize, + pub symbol_name: &'static str, +} + +pub struct CapturedBacktrace { + pub trace: Box>, +} + +/// Returns a saved backtrace. +pub fn capture_backtrace() -> CapturedBacktrace { + let mut trace = Box::new(ArrayVec::new()); + Backtrace::current_frame().traverse(|_, vaddr| { + if let Some(symbol) = resolve_symbol(vaddr) { + let _ = trace.try_push(CapturedBacktraceFrame { + vaddr, + symbol_name: symbol.name, + offset: vaddr.value() - symbol.addr.value(), + }); + } + }); + CapturedBacktrace { trace } +} + +impl fmt::Debug for CapturedBacktrace { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + for (i, frame) in self.trace.iter().enumerate() { + let _ = write!( + f, + " #{}: {} {}()+0x{:x}\n", + i + 1, + frame.vaddr, + frame.symbol_name, + frame.offset + ); + } + + Ok(()) + } +} diff --git a/kernel/result.rs b/kernel/result.rs index ac47c664..f5295c03 100644 --- a/kernel/result.rs +++ b/kernel/result.rs @@ -1,5 +1,9 @@ use core::fmt; +use cfg_if::cfg_if; + +use crate::printk::{capture_backtrace, CapturedBacktrace}; + #[derive(Debug, Copy, Clone, Eq, PartialEq)] #[repr(i32)] #[allow(unused)] @@ -63,6 +67,8 @@ enum ErrorMessage { pub struct Error { errno: Errno, message: Option, + #[cfg(debug_assertions)] + backtrace: Option, } impl Error { @@ -70,13 +76,26 @@ impl Error { Error { errno, message: None, + #[cfg(debug_assertions)] + backtrace: Some(capture_backtrace()), + } + } + + pub fn with_message(errno: Errno, message: &'static str) -> Error { + Error { + errno, + message: Some(ErrorMessage::StaticStr(message)), + #[cfg(debug_assertions)] + backtrace: Some(capture_backtrace()), } } - pub const fn with_message(errno: Errno, message: &'static str) -> Error { + pub const fn with_message_const(errno: Errno, message: &'static str) -> Error { Error { errno, message: Some(ErrorMessage::StaticStr(message)), + #[cfg(debug_assertions)] + backtrace: None, } } @@ -90,11 +109,39 @@ impl fmt::Debug for Error { if let Some(message) = self.message.as_ref() { match message { ErrorMessage::StaticStr(message) => { - write!(f, "[{:?}] {}", self.errno, message) + cfg_if! { + if #[cfg(debug_assertions)] { + if let Some(ref trace) = self.backtrace { + write!( + f, + "[{:?}] {}\n This error originates from:\n{:?}", + self.errno, message, trace + ) + } else { + write!(f, "[{:?}] {}", self.errno, message) + } + } else { + write!(f, "[{:?}] {}", self.errno, message) + } + } } } } else { - write!(f, "{:?}", self.errno) + cfg_if! { + if #[cfg(debug_assertions)] { + if let Some(ref trace) = self.backtrace { + write!( + f, + "{:?}: This error originates from:\n{:?}", + self.errno, trace + ) + } else { + write!(f, "{:?}", self.errno) + } + } else { + write!(f, "{:?}", self.errno) + } + } } } }