diff --git a/COMPATIBILITY.md b/COMPATIBILITY.md index cdb6108f..a7c5f415 100644 --- a/COMPATIBILITY.md +++ b/COMPATIBILITY.md @@ -227,6 +227,6 @@ implemented nor are there plans to implement due to differences between the impl | ⚫️ | `setmetatable(value, table)` | Interesting thing to note is that this is _not_ the base library `setmetatable`, as `debug.setmetatable`'s first argument accepts any Lua value, while `setmetatable`'s first argument _must_ be a table. | | | ⚫️ | `setupvalue(f, up, value)` | | | | ⚫️ | `setuservalue(udata, value, n)` | | | -| ⚫️ | `traceback([thread,][message, level])` | | | +| 🔵 | `traceback([thread,][message, level])` | | | | ⚫️ | `upvalueid(f, n)` | | | | ⚫️ | `upvaluejoin(f1, n1, f2, n2)` | | | diff --git a/README.md b/README.md index 4703b8be..316af07f 100644 --- a/README.md +++ b/README.md @@ -72,7 +72,7 @@ that are usable from safe Rust. It achieves this by combining two things: that such pointers are isolated to a single root object, and to guarantee that, outside an active call to `mutate`, all such pointers are either reachable from the root object or are safe to collect. - + ## Stackless VM The `mutate` based GC API means that long running calls to `mutate` can be @@ -235,7 +235,6 @@ very much WIP, so ensuring this is done correctly is an ongoing effort. generate and the VM sorely needs optimization (very little effort has gone here so far). * Error messages that don't make you want to cry -* Stack traces * Debugger * Aggressive optimization and *real* effort towards matching or beating (or even just being within a respectable distance of) PUC-Rio Lua's performance diff --git a/src/closure.rs b/src/closure.rs index 311c9128..41287972 100644 --- a/src/closure.rs +++ b/src/closure.rs @@ -32,6 +32,7 @@ pub enum CompilerError { pub struct FunctionPrototype<'gc> { pub chunk_name: String<'gc>, pub reference: FunctionRef>, + pub parameters: boxed::Box<[String<'gc>], MetricsAlloc<'gc>>, pub fixed_params: u8, pub has_varargs: bool, pub stack_size: u16, @@ -65,6 +66,10 @@ impl<'gc> FunctionPrototype<'gc> { ) -> FunctionPrototype<'gc> { let alloc = MetricsAlloc::new(mc); + let mut parameters_vec = vec::Vec::new_in(alloc.clone()); + parameters_vec.extend(compiled_function.parameters.iter().map(map_string)); + let parameters = parameters_vec.into_boxed_slice(); + let mut constants = vec::Vec::new_in(alloc.clone()); constants.extend( compiled_function @@ -95,6 +100,7 @@ impl<'gc> FunctionPrototype<'gc> { .reference .as_string_ref() .map_strings(map_string), + parameters, fixed_params: compiled_function.fixed_params, has_varargs: compiled_function.has_varargs, stack_size: compiled_function.stack_size, diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index 29a2b79a..d807199d 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -111,6 +111,7 @@ impl FunctionRef { #[collect(no_drop)] pub struct CompiledPrototype { pub reference: FunctionRef, + pub parameters: Vec, pub fixed_params: u8, pub has_varargs: bool, pub stack_size: u16, @@ -132,6 +133,7 @@ impl CompiledPrototype { ) -> CompiledPrototype { CompiledPrototype { reference: this.reference.map_strings(f), + parameters: this.parameters.into_iter().map(f).collect(), fixed_params: this.fixed_params, has_varargs: this.has_varargs, stack_size: this.stack_size, @@ -183,6 +185,7 @@ struct Compiler { struct CompilerFunction { reference: FunctionRef, + parameters: Vec, constants: Vec>, constant_table: HashMap, ConstantIndex16>, @@ -804,14 +807,14 @@ impl Compiler { parameters.extend_from_slice(&function_statement.definition.parameters); self.new_prototype( - FunctionRef::Named(name.clone(), self.current_function.current_line_number), + FunctionRef::Named(name.clone(), function_statement.definition.line_number), ¶meters, function_statement.definition.has_varargs, &function_statement.definition.body, )? } else { self.new_prototype( - FunctionRef::Named(name.clone(), self.current_function.current_line_number), + FunctionRef::Named(name.clone(), function_statement.definition.line_number), &function_statement.definition.parameters, function_statement.definition.has_varargs, &function_statement.definition.body, @@ -1091,7 +1094,7 @@ impl Compiler { let proto = self.new_prototype( FunctionRef::Named( local_function.name.clone(), - self.current_function.current_line_number, + local_function.definition.line_number, ), &local_function.definition.parameters, local_function.definition.has_varargs, @@ -1185,7 +1188,7 @@ impl Compiler { function: &FunctionDefinition, ) -> Result, CompileErrorKind> { let proto = self.new_prototype( - FunctionRef::Expression(self.current_function.current_line_number), + FunctionRef::Expression(function.line_number), &function.parameters, function.has_varargs, &function.body, @@ -2477,14 +2480,9 @@ impl CompilerFunction { parameters: &[S], has_varargs: bool, ) -> Result, CompileErrorKind> { - let current_line_number = match reference { - FunctionRef::Named(_, ln) => ln, - FunctionRef::Expression(ln) => ln, - FunctionRef::Chunk => LineNumber(0), - }; - let mut function = CompilerFunction { reference, + parameters: parameters.to_vec(), constants: Vec::new(), constant_table: HashMap::default(), upvalues: Vec::new(), @@ -2499,8 +2497,13 @@ impl CompilerFunction { pending_jumps: Vec::new(), operations: Vec::new(), operation_lines: Vec::new(), - current_line_number, + current_line_number: LineNumber(0), }; + function.set_line_number(match &function.reference { + FunctionRef::Named(_, ln) => *ln, + FunctionRef::Expression(ln) => *ln, + FunctionRef::Chunk => LineNumber(0), + }); let fixed_params: u8 = parameters .len() @@ -2556,6 +2559,7 @@ impl CompilerFunction { Ok(CompiledPrototype { reference: self.reference, + parameters: self.parameters, fixed_params: self.fixed_params, has_varargs: self.has_varargs, stack_size: self.register_allocator.stack_size(), diff --git a/src/compiler/parser.rs b/src/compiler/parser.rs index 7ced0c28..cbc3e6ba 100644 --- a/src/compiler/parser.rs +++ b/src/compiler/parser.rs @@ -275,6 +275,7 @@ pub struct SuffixedExpression { #[derive(Debug, Clone)] pub struct FunctionDefinition { + pub line_number: LineNumber, pub parameters: Vec, pub has_varargs: bool, pub body: Block, @@ -584,7 +585,7 @@ impl Parser<'_, S> { } fn parse_function_statement(&mut self) -> Result, ParseError> { - self.expect_next(Token::Function)?; + let line_number = self.expect_next(Token::Function)?; let name = self.expect_name()?.inner; let mut fields = Vec::new(); @@ -604,7 +605,7 @@ impl Parser<'_, S> { } } - let definition = self.parse_function_definition()?; + let definition = self.parse_function_definition(line_number)?; Ok(FunctionStatement { name, @@ -617,10 +618,10 @@ impl Parser<'_, S> { fn parse_local_function_statement( &mut self, ) -> Result, ParseError> { - self.expect_next(Token::Function)?; + let line_number = self.expect_next(Token::Function)?; let name = self.expect_name()?.inner; - let definition = self.parse_function_definition()?; + let definition = self.parse_function_definition(line_number)?; Ok(LocalFunctionStatement { name, definition }) } @@ -828,8 +829,8 @@ impl Parser<'_, S> { } Token::LeftBrace => SimpleExpression::TableConstructor(self.parse_table_constructor()?), Token::Function => { - self.take_next()?; - SimpleExpression::Function(self.parse_function_definition()?) + let line_number = self.take_next()?.line_number; + SimpleExpression::Function(self.parse_function_definition(line_number)?) } _ => SimpleExpression::Suffixed(self.parse_suffixed_expression()?), }) @@ -965,7 +966,10 @@ impl Parser<'_, S> { Ok(SuffixedExpression { primary, suffixes }) } - fn parse_function_definition(&mut self) -> Result, ParseError> { + fn parse_function_definition( + &mut self, + line_number: LineNumber, + ) -> Result, ParseError> { self.expect_next(Token::LeftParen)?; let mut parameters = Vec::new(); @@ -1002,6 +1006,7 @@ impl Parser<'_, S> { self.expect_next(Token::End)?; Ok(FunctionDefinition { + line_number, parameters, has_varargs, body, diff --git a/src/error.rs b/src/error.rs index 462c9cd3..24737054 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,11 +1,14 @@ -use std::{error::Error as StdError, fmt, string::String as StdString, sync::Arc}; +use std::{ + collections::HashMap, error::Error as StdError, fmt, string::String as StdString, sync::Arc, + write, +}; use gc_arena::{Collect, Gc, Rootable}; use thiserror::Error; use crate::{ - Callback, CallbackReturn, Context, FromValue, Function, IntoValue, MetaMethod, Singleton, - Table, UserData, Value, + compiler::LineNumber, Callback, CallbackReturn, Context, FromValue, Function, IntoValue, + MetaMethod, Singleton, Table, UserData, Value, }; #[derive(Debug, Clone, Copy, Error)] @@ -18,25 +21,34 @@ pub struct TypeError { /// An error raised directly from Lua which contains a Lua value. /// /// Any [`Value`] can be raised as an error and it will be contained here. -#[derive(Debug, Copy, Clone, Collect)] +#[derive(Debug, Clone, Collect)] #[collect(no_drop)] -pub struct LuaError<'gc>(pub Value<'gc>); +pub struct LuaError<'gc> { + pub value: Value<'gc>, + pub backtrace: Option>, +} impl<'gc> fmt::Display for LuaError<'gc> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.0.display()) + if f.alternate() { + pretty_print_error_with_backtrace( + f, + &self.value.display(), + &self.backtrace.as_deref(), + None, + ) + } else { + write!(f, "{}", self.value.display()) + } } } impl<'gc> From> for LuaError<'gc> { - fn from(error: Value<'gc>) -> Self { - LuaError(error) - } -} - -impl<'gc> LuaError<'gc> { - pub fn to_extern(self) -> ExternLuaError { - self.into() + fn from(value: Value<'gc>) -> Self { + LuaError { + value, + backtrace: None, + } } } @@ -67,9 +79,9 @@ pub enum ExternLuaError { UserData(*const ()), } -impl<'gc> From> for ExternLuaError { - fn from(error: LuaError<'gc>) -> Self { - match error.0 { +impl<'gc> From> for ExternLuaError { + fn from(error: Value<'gc>) -> Self { + match error { Value::Nil => ExternLuaError::Nil, Value::Boolean(b) => ExternLuaError::Boolean(b), Value::Integer(i) => ExternLuaError::Integer(i), @@ -93,17 +105,40 @@ impl<'gc> From> for ExternLuaError { unsafe impl Send for ExternLuaError {} unsafe impl Sync for ExternLuaError {} +#[derive(Debug, Clone, Collect)] +#[collect(require_static)] +pub enum BacktraceFrame { + Lua { + chunk_name: StdString, + function_name: StdString, + line_number: LineNumber, + args: Vec<(StdString, StdString)>, + }, + Callback { + name: &'static str, + }, + Sequence, + Internal, +} + /// A shareable, dynamically typed wrapper around a normal Rust error. /// /// Rust errors can be caught and re-raised through Lua which allows for unrestricted sharing, so /// this type contains its error inside an `Arc` pointer to allow for this. #[derive(Debug, Clone, Collect)] #[collect(require_static)] -pub struct RuntimeError(pub Arc); +pub struct RuntimeError { + pub error: Arc, + pub backtrace: Option>, +} impl fmt::Display for RuntimeError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.0.fmt(f) + if f.alternate() { + pretty_print_error_with_backtrace(f, &self.error, &self.backtrace.as_deref(), None) + } else { + write!(f, "{}", self.error) + } } } @@ -115,31 +150,34 @@ impl> From for RuntimeError { impl RuntimeError { pub fn new(err: impl Into) -> Self { - Self(Arc::new(err.into())) + Self { + error: Arc::new(err.into()), + backtrace: None, + } } pub fn root_cause(&self) -> &(dyn StdError + 'static) { - self.0.root_cause() + self.error.root_cause() } pub fn is(&self) -> bool where E: fmt::Display + fmt::Debug + Send + Sync + 'static, { - self.0.is::() + self.error.is::() } pub fn downcast(&self) -> Option<&E> where E: fmt::Display + fmt::Debug + Send + Sync + 'static, { - self.0.downcast_ref::() + self.error.downcast_ref::() } } impl AsRef for RuntimeError { fn as_ref(&self) -> &(dyn StdError + 'static) { - (*self.0).as_ref() + (*self.error).as_ref() } } @@ -156,9 +194,16 @@ pub enum Error<'gc> { impl<'gc> fmt::Display for Error<'gc> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Error::Lua(err) => write!(f, "lua error: {err}"), - Error::Runtime(err) => write!(f, "runtime error: {err:#}"), + if f.alternate() { + match self { + Error::Lua(err) => write!(f, "lua error: {:#}", err), + Error::Runtime(err) => write!(f, "runtime error: {:#}", err), + } + } else { + match self { + Error::Lua(err) => write!(f, "lua error: {}", err), + Error::Runtime(err) => write!(f, "runtime error: {}", err), + } } } } @@ -217,7 +262,7 @@ impl<'gc> Error<'gc> { /// when printed from Lua. pub fn to_value(&self, ctx: Context<'gc>) -> Value<'gc> { match self { - Error::Lua(err) => err.0, + Error::Lua(err) => err.value, Error::Runtime(err) => { #[derive(Copy, Clone, Collect)] #[collect(no_drop)] @@ -233,7 +278,7 @@ impl<'gc> Error<'gc> { Callback::from_fn(&ctx, |ctx, _, mut stack| { let ud = stack.consume::(ctx)?; let error = ud.downcast_static::()?; - stack.replace(ctx, error.to_string()); + stack.replace(ctx, error.error.to_string()); Ok(CallbackReturn::Return) }), ) @@ -273,15 +318,30 @@ impl<'gc> FromValue<'gc> for Error<'gc> { /// An [`enum@Error`] that is not bound to the GC context. #[derive(Debug, Clone)] pub enum ExternError { - Lua(ExternLuaError), + Lua { + error: ExternLuaError, + backtrace: Option>, + }, Runtime(RuntimeError), } impl fmt::Display for ExternError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - ExternError::Lua(err) => write!(f, "lua error: {err}"), - ExternError::Runtime(err) => write!(f, "runtime error: {err:#}"), + if f.alternate() { + match self { + ExternError::Lua { error, backtrace } => { + write!(f, "lua error: ")?; + pretty_print_error_with_backtrace(f, error, &backtrace.as_deref(), None) + } + ExternError::Runtime(err) => { + write!(f, "runtime error: {:#}", err) + } + } + } else { + match self { + ExternError::Lua { error, .. } => write!(f, "lua error: {error}"), + ExternError::Runtime(err) => write!(f, "runtime error: {err}"), + } } } } @@ -289,7 +349,7 @@ impl fmt::Display for ExternError { impl StdError for ExternError { fn source(&self) -> Option<&(dyn StdError + 'static)> { match self { - ExternError::Lua(err) => Some(err), + ExternError::Lua { error, .. } => Some(error), ExternError::Runtime(err) => Some(err.as_ref()), } } @@ -298,15 +358,36 @@ impl StdError for ExternError { impl ExternError { pub fn root_cause(&self) -> &(dyn StdError + 'static) { match self { - ExternError::Lua(err) => err, + ExternError::Lua { error, .. } => error, ExternError::Runtime(err) => err.root_cause(), } } + + pub fn pretty_print( + &self, + writer: &mut impl fmt::Write, + source_map: Option<&SourceMap>, + ) -> Result<(), fmt::Error> { + match self { + ExternError::Lua { error, backtrace } => { + pretty_print_error_with_backtrace(writer, error, &backtrace.as_deref(), source_map) + } + ExternError::Runtime(err) => pretty_print_error_with_backtrace( + writer, + &err.error, + &err.backtrace.as_deref(), + source_map, + ), + } + } } impl From for ExternError { fn from(error: ExternLuaError) -> Self { - Self::Lua(error) + Self::Lua { + error, + backtrace: None, + } } } @@ -319,8 +400,68 @@ impl From for ExternError { impl<'gc> From> for ExternError { fn from(err: Error<'gc>) -> Self { match err { - Error::Lua(err) => err.to_extern().into(), + Error::Lua(err) => ExternError::Lua { + error: err.value.into(), + backtrace: err.backtrace, + }, Error::Runtime(e) => e.into(), } } } + +pub type SourceMap = HashMap; + +pub(crate) fn pretty_print_error_with_backtrace( + writer: &mut impl fmt::Write, + error: &dyn fmt::Display, + backtrace: &Option<&[BacktraceFrame]>, + source_map: Option<&SourceMap>, +) -> Result<(), fmt::Error> { + write!(writer, "{}", error)?; + if let Some(backtrace) = backtrace { + write!(writer, "\nstack traceback:")?; + + if backtrace.is_empty() { + write!(writer, " ")?; + } else { + for frame in backtrace.iter() { + match frame { + BacktraceFrame::Lua { + chunk_name, + function_name, + line_number, + args, + } => { + write!( + writer, + "\n {}:{} in {}", + chunk_name, line_number, function_name, + )?; + if let Some(source) = source_map.and_then(|sm| sm.get(chunk_name)) { + if let Some(line) = source.lines().nth(line_number.0 as usize) { + write!(writer, ": `{}`", line.trim())?; + } + } + if !args.is_empty() { + write!(writer, "\n arguments:")?; + for (name, value) in args { + write!(writer, "\n {}: {}", name, value)?; + } + } + } + BacktraceFrame::Callback { name } => { + write!(writer, "\n ", name)?; + } + BacktraceFrame::Sequence => { + write!(writer, "\n ")?; + } + BacktraceFrame::Internal => { + write!(writer, "\n ")?; + } + } + } + writeln!(writer)?; + } + } + Ok(()) +} diff --git a/src/lua.rs b/src/lua.rs index 67f03007..274ce080 100644 --- a/src/lua.rs +++ b/src/lua.rs @@ -9,7 +9,7 @@ use gc_arena::{ use crate::{ finalizers::Finalizers, stash::{Fetchable, Stashable}, - stdlib::{load_base, load_coroutine, load_io, load_math, load_string, load_table}, + stdlib::{load_base, load_coroutine, load_debug, load_io, load_math, load_string, load_table}, string::InternedStringSet, thread::BadThreadMode, Error, ExternError, FromMultiValue, FromValue, Fuel, IntoValue, Registry, RuntimeError, @@ -169,6 +169,7 @@ impl Lua { /// - `load_math` /// - `load_string` /// - `load_table` + /// - `load_debug` pub fn load_core(&mut self) { self.enter(|ctx| { load_base(ctx); @@ -176,6 +177,7 @@ impl Lua { load_math(ctx); load_string(ctx); load_table(ctx); + load_debug(ctx); }) } diff --git a/src/stack.rs b/src/stack.rs index f0f72e16..51f45880 100644 --- a/src/stack.rs +++ b/src/stack.rs @@ -154,6 +154,10 @@ impl<'gc, 'a> Stack<'gc, 'a> { pub fn consume>(&mut self, ctx: Context<'gc>) -> Result { V::from_multi_value(ctx, self.drain(..)) } + + pub fn borrow_thread_view(&self) -> &[Value<'gc>] { + self.values.as_slice() + } } impl<'gc: 'b, 'a, 'b> IntoIterator for &'b Stack<'gc, 'a> { diff --git a/src/stash.rs b/src/stash.rs index 784a2433..cd660b77 100644 --- a/src/stash.rs +++ b/src/stash.rs @@ -5,6 +5,7 @@ use gc_arena::{DynamicRoot, DynamicRootSet, Mutation, Rootable}; use crate::{ callback::CallbackInner, closure::ClosureInner, + error::LuaError, string::StringInner, table::TableInner, thread::{ExecutorInner, ThreadInner}, @@ -385,6 +386,7 @@ impl Fetchable for StashedValue { } } +#[derive(Debug, Clone)] pub enum StashedError { Lua(StashedValue), Runtime(RuntimeError), @@ -413,7 +415,7 @@ impl<'gc> Stashable<'gc> for Error<'gc> { fn stash(self, mc: &Mutation<'gc>, roots: DynamicRootSet<'gc>) -> Self::Stashed { match self { - Error::Lua(err) => StashedError::Lua(err.0.stash(mc, roots)), + Error::Lua(err) => StashedError::Lua(err.value.stash(mc, roots)), Error::Runtime(err) => StashedError::Runtime(err), } } @@ -424,7 +426,10 @@ impl Fetchable for StashedError { fn fetch<'gc>(&self, roots: DynamicRootSet<'gc>) -> Self::Fetched<'gc> { match self { - StashedError::Lua(err) => Error::from_value(err.fetch(roots)), + StashedError::Lua(err) => Error::Lua(LuaError { + value: err.fetch(roots), + backtrace: None, + }), StashedError::Runtime(err) => err.clone().into(), } } diff --git a/src/stdlib/base.rs b/src/stdlib/base.rs index 5a18ca80..47b4c10a 100644 --- a/src/stdlib/base.rs +++ b/src/stdlib/base.rs @@ -3,6 +3,7 @@ use std::pin::Pin; use gc_arena::Collect; use crate::{ + error::LuaError, meta_ops::{self, MetaResult}, table::NextValue, BoxSequence, Callback, CallbackReturn, Context, Error, Execution, IntoValue, MetaMethod, @@ -94,7 +95,7 @@ pub fn load_base<'gc>(ctx: Context<'gc>) { ctx.set_global( "error", - Callback::from_fn(&ctx, |_, _, stack| Err(stack.get(0).into())), + Callback::from_fn(&ctx, |_, _, stack| Err(LuaError::from(stack.get(0)).into())), ); ctx.set_global( @@ -103,9 +104,9 @@ pub fn load_base<'gc>(ctx: Context<'gc>) { if stack.get(0).to_bool() { Ok(CallbackReturn::Return) } else if stack.get(1).is_nil() { - Err("assertion failed!".into_value(ctx).into()) + Err(LuaError::from("assertion failed!".into_value(ctx)).into()) } else { - Err(stack.get(1).into()) + Err(LuaError::from(stack.get(1)).into()) } }), ); diff --git a/src/stdlib/debug.rs b/src/stdlib/debug.rs new file mode 100644 index 00000000..8a7851c1 --- /dev/null +++ b/src/stdlib/debug.rs @@ -0,0 +1,80 @@ +use crate::conversion::IntoValue; +use crate::error::BacktraceFrame; +use crate::{ + error::pretty_print_error_with_backtrace, Callback, CallbackReturn, Context, Thread, Value, +}; +use anyhow::anyhow; + +pub fn load_debug<'gc>(ctx: Context<'gc>) { + let debug = crate::Table::new(&ctx); + + debug.set_field( + ctx, + "traceback", + Callback::from_fn(&ctx, |ctx, exec, mut stack| { + let (thread, message, level): (Option, Option, Option) = + stack.consume(ctx)?; + + // Level defaults to 1 for user calls, skipping the `debug.traceback` frame itself. + let level = level.unwrap_or(1); + if level < 0 { + return Err(anyhow!("level must be non-negative").into()); + } + + let level0 = BacktraceFrame::Callback { + name: "debug.traceback", + }; + let backtrace_frames = + if let Some(t) = thread.filter(|t| t != &exec.current_thread().thread) { + // Try to borrow the target thread's state + // (Note: we filter out the current thread because we know it's already borrowed while running) + let thread_state_borrow = t.into_inner().try_borrow(); + let thread_state = thread_state_borrow + .as_ref() + .map_err(|_| anyhow!("cannot get traceback for running thread"))?; + crate::thread::backtrace( + thread_state.frames(), + thread_state.stack(), + &[], + Some(level0), + ) + } else { + // This is the current running thread, use its upper frames and the full stack + crate::thread::backtrace( + exec.upper_frames(), + stack.borrow_thread_view(), + &[], + Some(level0), + ) + }; + + let mut formatted_error_str = String::new(); + + if let Some(msg_value) = message { + formatted_error_str = msg_value + .into_string(ctx) + .map(|s| s.display_lossy().to_string()) + .unwrap_or_else(|| msg_value.display().to_string()); + } + + // Skip frames based on the level. + let skipped_frames = (level as usize).min(backtrace_frames.len()); + let remaining_frames = &backtrace_frames[skipped_frames..]; + + let dummy_error = formatted_error_str.into_value(ctx); + + let mut traceback_string = String::new(); + pretty_print_error_with_backtrace( + &mut traceback_string, + &dummy_error.display(), + &Some(remaining_frames), + None, // No source map + )?; + + stack.replace(ctx, traceback_string); + Ok(CallbackReturn::Return) + }), + ); + + ctx.set_global("debug", debug); +} diff --git a/src/stdlib/mod.rs b/src/stdlib/mod.rs index aa766153..ae6920e8 100644 --- a/src/stdlib/mod.rs +++ b/src/stdlib/mod.rs @@ -1,11 +1,12 @@ mod base; mod coroutine; +mod debug; mod io; mod math; mod string; mod table; pub use self::{ - base::load_base, coroutine::load_coroutine, io::load_io, math::load_math, string::load_string, - table::load_table, + base::load_base, coroutine::load_coroutine, debug::load_debug, io::load_io, math::load_math, + string::load_string, table::load_table, }; diff --git a/src/thread/executor.rs b/src/thread/executor.rs index 4f4630bd..382b39ca 100644 --- a/src/thread/executor.rs +++ b/src/thread/executor.rs @@ -465,7 +465,29 @@ impl<'gc> Executor<'gc> { } } } - Some(Frame::Error(err)) => { + Some(Frame::Error(mut err)) => { + let needs_backtrace = match &err { + Error::Lua(lua_err) => lua_err.backtrace.is_none(), + Error::Runtime(runtime_err) => runtime_err.backtrace.is_none(), + }; + + if needs_backtrace { + let backtrace = crate::thread::backtrace( + top_state.frames(), + top_state.stack(), + &state.thread_stack, + None, + ); + match &mut err { + Error::Lua(lua_err) => { + lua_err.backtrace = Some(backtrace); + } + Error::Runtime(runtime_err) => { + runtime_err.backtrace = Some(backtrace); + } + } + } + match top_state .frames .pop() @@ -631,6 +653,11 @@ impl<'gc, 'a> Execution<'gc, 'a> { self.executor } + /// Borrow the frames below the currently executing frame. + pub(crate) fn upper_frames(&self) -> &[Frame<'gc>] { + self.upper_frames + } + /// If the function we are returning to is Lua, returns information about the Lua frame we are /// returning to. pub fn upper_lua_frame(&self) -> Option> { diff --git a/src/thread/mod.rs b/src/thread/mod.rs index 8e6a0591..f4bba8a5 100644 --- a/src/thread/mod.rs +++ b/src/thread/mod.rs @@ -6,6 +6,7 @@ use thiserror::Error; use crate::meta_ops::{MetaCallError, MetaOperatorError}; +pub(crate) use self::thread::backtrace; pub use self::{ executor::{ BadExecutorMode, CurrentThread, Execution, Executor, ExecutorInner, ExecutorMode, @@ -24,9 +25,9 @@ pub enum VMError { ExpectedVariableStack(bool), #[error("Bad types for SetList op, expected table, integer, found {0}, {1}")] BadSetList(&'static str, &'static str), - #[error("bad call")] + #[error("bad call: {0}")] BadCall(#[from] MetaCallError), - #[error("operator error")] + #[error("operator error: {0}")] OperatorError(#[from] MetaOperatorError), #[error("_ENV upvalue is only allowed on top-level closure")] BadEnvUpValue, diff --git a/src/thread/thread.rs b/src/thread/thread.rs index d552bc4c..89c750bc 100644 --- a/src/thread/thread.rs +++ b/src/thread/thread.rs @@ -11,6 +11,7 @@ use thiserror::Error; use crate::{ closure::{UpValue, UpValueState}, + error::BacktraceFrame, fuel::count_fuel, meta_ops, types::{RegisterIndex, VarCount}, @@ -264,7 +265,7 @@ impl<'gc> OpenUpValue<'gc> { #[derive(Debug, Copy, Clone, Collect)] #[collect(require_static)] -pub(super) enum MetaReturn { +pub(crate) enum MetaReturn { /// No return value is expected. None, /// Place a single return value at an index relative to the returned to function's stack bottom. @@ -275,7 +276,7 @@ pub(super) enum MetaReturn { #[derive(Debug, Copy, Clone, Collect)] #[collect(require_static)] -pub(super) enum LuaReturn { +pub(crate) enum LuaReturn { /// Normal function call, place return values at the bottom of the returning function's stack, /// as normal. Normal(VarCount), @@ -285,7 +286,7 @@ pub(super) enum LuaReturn { #[derive(Debug, Collect)] #[collect(no_drop)] -pub(super) enum Frame<'gc> { +pub(crate) enum Frame<'gc> { /// A running Lua frame. Lua { bottom: usize, @@ -357,6 +358,16 @@ impl<'gc> ThreadState<'gc> { } } + /// Borrow the frames of this thread. + pub(crate) fn frames(&self) -> &[Frame<'gc>] { + &self.frames + } + + /// Borrow the stack of this thread. + pub(crate) fn stack(&self) -> &[Value<'gc>] { + &self.stack + } + /// Pushes a new function call frame. /// /// Arguments are taken from the top of the stack starting at `bottom`, which will become the @@ -529,6 +540,100 @@ impl<'gc> ThreadState<'gc> { } } +pub(crate) fn backtrace<'gc>( + frames: &[Frame<'gc>], + stack: &[Value<'gc>], + thread_stack: &[Thread<'gc>], + top_frame: Option, +) -> Vec { + let mut trace_frames = Vec::new(); + + if let Some(frame) = top_frame { + trace_frames.push(frame); + } + for i in (0..frames.len()).rev() { + if let Some(frame) = frames.get(i) { + match frame { + Frame::Lua { closure, pc, .. } => { + let proto = closure.prototype(); + let call_opcode = *pc - 1; + let current_line = match proto + .opcode_line_numbers + .binary_search_by_key(&call_opcode, |(opi, _)| *opi) + { + Ok(i) => proto.opcode_line_numbers[i].1, + Err(i) => proto.opcode_line_numbers[i - 1].1, + }; + + let mut args = Vec::new(); + if let Some(Frame::Lua { bottom, base, .. }) = frames.get(i) { + for (arg_index, param_name) in proto.parameters.iter().enumerate() { + if let Some(value) = stack.get(*base + arg_index) { + args.push(( + param_name.display_lossy().to_string(), + value.display().to_string(), + )); + } + } + + if proto.has_varargs { + if *base > *bottom { + for value in &stack[*bottom..*base] { + args.push(("...".to_string(), value.display().to_string())); + } + } + } + } + + trace_frames.push(BacktraceFrame::Lua { + chunk_name: proto.chunk_name.display_lossy().to_string(), + function_name: proto + .reference + .as_string_ref() + .map_strings(|s| s.display_lossy().to_string()) + .to_string(), + line_number: current_line, + args, + }); + } + Frame::Callback { .. } => { + trace_frames.push(BacktraceFrame::Callback { name: "anonymous" }); + } + Frame::Sequence { .. } => { + trace_frames.push(BacktraceFrame::Sequence); + } + Frame::Start(_) + | Frame::Yielded + | Frame::WaitThread + | Frame::Result { .. } + | Frame::Error(_) => { + // These frames do not have enough information to be represented in the backtrace. + // They are typically used for control flow and do not represent a Lua function call. + trace_frames.push(BacktraceFrame::Internal); + } + } + } + } + + if !thread_stack.is_empty() { + for thread in thread_stack[0..thread_stack.len() - 1].iter().rev() { + // When adding frames from other threads to the backtrace, + // we should only do so if we can *safely* borrow its state. + // If it's running, we can't (it would panic) so we skip it. + if let Ok(borrowed_state) = thread.0.try_borrow() { + trace_frames.extend(backtrace( + &borrowed_state.frames, + &borrowed_state.stack, + &[], // No nested thread_stack for recursive calls + None, + )); + } + } + } + + trace_frames +} + pub(super) struct LuaFrame<'gc, 'a> { pub(super) thread: Thread<'gc>, pub(super) state: &'a mut ThreadState<'gc>, @@ -953,8 +1058,8 @@ impl<'gc, 'a> LuaFrame<'gc, 'a> { } pub(super) struct LuaRegisters<'gc, 'a> { - pub pc: &'a mut usize, - pub stack_frame: &'a mut [Value<'gc>], + pub(super) pc: &'a mut usize, + pub(super) stack_frame: &'a mut [Value<'gc>], upper_stack: &'a mut [Value<'gc>], bottom: usize, base: usize, diff --git a/tests/backtrace.rs b/tests/backtrace.rs new file mode 100644 index 00000000..9550f519 --- /dev/null +++ b/tests/backtrace.rs @@ -0,0 +1,187 @@ +use piccolo::{error::BacktraceFrame, Closure, Executor, ExternError, Lua}; +use std::collections::HashMap; + +#[test] +fn test_backtrace() { + const SCRIPT: &str = include_str!("test_backtrace.lua"); + + let mut lua = Lua::full(); + + let executor = lua + .try_enter(|ctx| { + let closure = Closure::load(ctx, Some("test_backtrace.lua"), SCRIPT.as_bytes())?; + Ok(ctx.stash(Executor::start(ctx, closure.into(), ()))) + }) + .unwrap(); + + lua.finish(&executor).unwrap(); + let result = lua.try_enter(|ctx| ctx.fetch(&executor).take_result::<()>(ctx)?); + + let mut source_map = std::collections::HashMap::new(); + source_map.insert("test_backtrace.lua".to_string(), SCRIPT.to_string()); + + let Err( + ref err @ ExternError::Lua { + ref error, + ref backtrace, + .. + }, + ) = result + else { + panic!("expected an error, got: {:?}", result); + }; + + assert!(error.to_string().contains("test error from rust")); + + let backtrace = backtrace.as_ref().unwrap(); + let mut backtrace_string = String::new(); + eprintln!("Error: {err:#}"); + err.pretty_print(&mut backtrace_string, Some(&source_map)) + .unwrap(); + eprintln!("Backtrace: {backtrace_string}"); + assert_eq!(backtrace.len(), 5); + + if let BacktraceFrame::Lua { + chunk_name, + function_name, + line_number, + args, + } = &backtrace[0] + { + assert_eq!(chunk_name, "test_backtrace.lua"); + assert!(function_name.starts_with("")); + assert_eq!(line_number.0, 8); + assert_eq!( + args, + &vec![ + ("a".to_string(), "test".to_string()), + ("b".to_string(), " error from rust".to_string()) + ] + ); + } else { + panic!("Expected second frame to be Lua frame"); + } + + if let BacktraceFrame::Lua { + chunk_name, + function_name, + line_number, + args, + } = &backtrace[2] + { + assert_eq!(chunk_name, "test_backtrace.lua"); + assert!(function_name.starts_with("")); + assert_eq!(line_number.0, 12); + assert_eq!( + args, + &vec![ + ("x".to_string(), "test".to_string()), + ("...".to_string(), " error from rust".to_string()) + ] + ); + } else { + panic!("Expected third frame to be Lua frame"); + } + + if let BacktraceFrame::Lua { + chunk_name, + function_name, + line_number, + args, + } = &backtrace[3] + { + assert_eq!(chunk_name, "test_backtrace.lua"); + assert!(function_name.starts_with("")); + assert_eq!(line_number.0, 18); + assert_eq!(args, &vec![("arg1".to_string(), "test".to_string())]); + } else { + panic!("Expected fourth frame to be Lua frame"); + } + + if let BacktraceFrame::Lua { + chunk_name, + function_name, + line_number, + args, + } = &backtrace[4] + { + assert_eq!(chunk_name, "test_backtrace.lua"); + assert_eq!(function_name, ""); + assert_eq!(line_number.0, 21); + assert!(args.is_empty()); + } else { + panic!("Expected fifth frame to be Chunk frame"); + } + + let mut output = String::new(); + err.pretty_print(&mut output, Some(&source_map)).unwrap(); + + // Check we lookup the correct source map entries without being too pedantic about the format + assert!(output.contains("-- This is line 4")); + assert!(output.contains("-- This is line 9")); + assert!(output.contains("-- This is line 13")); + assert!(output.contains("-- This is line 19")); + assert!(output.contains("-- This is line 22")); +} + +#[test] +fn test_pretty_print_backtrace() -> Result<(), ExternError> { + let mut lua = Lua::core(); + let source = br#" +function f() g() end +function g() h() end +function h() error("an error") end +f() +"#; + + let executor = lua.try_enter(|ctx| { + let closure = Closure::load(ctx, Some("test.lua"), source)?; + Ok(ctx.stash(Executor::start(ctx, closure.into(), ()))) + })?; + + lua.finish(&executor).unwrap(); + let err = lua + .try_enter(|ctx| ctx.fetch(&executor).take_result::<()>(ctx)?) + .unwrap_err(); + + let mut out = String::new(); + let mut source_map = HashMap::new(); + source_map.insert( + "test.lua".to_string(), + String::from_utf8_lossy(source).into_owned(), + ); + err.pretty_print(&mut out, Some(&source_map)).unwrap(); + eprintln!("{}", out); + let expected_lines = [ + "an error", + "stack traceback:", + "test.lua:4 in : `function h() error(\"an error\") end`", + "test.lua:3 in : `function g() h() end`", + "test.lua:2 in : `function f() g() end`", + "test.lua:5 in : `f()`", + ]; + + for line in expected_lines { + assert!(out.contains(line), "output missing line: '{}'", line); + } + + Ok(()) +} diff --git a/tests/callback.rs b/tests/callback.rs index 6670fd5f..f0ae4271 100644 --- a/tests/callback.rs +++ b/tests/callback.rs @@ -298,7 +298,7 @@ fn resume_with_err() { lua.enter( |ctx| match ctx.fetch(&executor).take_result::<()>(ctx).unwrap() { Err(Error::Lua(val)) => { - assert!(matches!(val.0, Value::String(s) if s == "an error")) + assert!(matches!(val.value, Value::String(s) if s == "an error")) } _ => panic!("wrong error returned"), }, diff --git a/tests/error.rs b/tests/error.rs index 46360df7..cea10a70 100644 --- a/tests/error.rs +++ b/tests/error.rs @@ -1,4 +1,4 @@ -use piccolo::{error::LuaError, Callback, Closure, Error, Executor, ExternError, Lua, Value}; +use piccolo::{Callback, Closure, Error, Executor, ExternError, Lua, Value}; use thiserror::Error; #[test] @@ -23,7 +23,9 @@ fn error_unwind() -> Result<(), ExternError> { lua.finish(&executor).unwrap(); lua.try_enter(|ctx| { match ctx.fetch(&executor).take_result::<()>(ctx)? { - Err(Error::Lua(LuaError(Value::String(s)))) => assert!(s == "test error"), + Err(Error::Lua(lua_error)) => { + assert!(matches!(lua_error.value, Value::String(s) if s == "test error")) + } _ => panic!("wrong error returned"), } Ok(()) diff --git a/tests/goldenscripts.rs b/tests/goldenscripts.rs index 57029c45..c0609e80 100644 --- a/tests/goldenscripts.rs +++ b/tests/goldenscripts.rs @@ -214,7 +214,7 @@ fn test_goldenscripts() { continue; } if let Some(error) = run_error { - eprintln!("{path:?}: expected script to pass, but it threw and error at runtime\nerror: {error}"); + eprintln!("{path:?}: expected script to pass, but it threw an error at runtime\nerror: {error:#}"); failed_scripts.push(path); continue; } diff --git a/tests/goldenscripts/debug_traceback.lua b/tests/goldenscripts/debug_traceback.lua new file mode 100644 index 00000000..92e2ff4d --- /dev/null +++ b/tests/goldenscripts/debug_traceback.lua @@ -0,0 +1,80 @@ +--- pass +--- some message +--- stack traceback: +--- +--- ./tests/goldenscripts/debug_traceback.lua:62 in +--- arguments: +--- msg: some message +--- level: 0 +--- ./tests/goldenscripts/debug_traceback.lua:66 in +--- arguments: +--- msg: some message +--- level: 0 +--- ./tests/goldenscripts/debug_traceback.lua:73 in +--- +--- some other message +--- stack traceback: +--- ./tests/goldenscripts/debug_traceback.lua:62 in +--- arguments: +--- msg: some other message +--- level: 1 +--- ./tests/goldenscripts/debug_traceback.lua:66 in +--- arguments: +--- msg: some other message +--- level: 1 +--- ./tests/goldenscripts/debug_traceback.lua:74 in +--- +--- some other message +--- stack traceback: +--- ./tests/goldenscripts/debug_traceback.lua:62 in +--- arguments: +--- msg: some other message +--- level: nil +--- ./tests/goldenscripts/debug_traceback.lua:66 in +--- arguments: +--- msg: some other message +--- level: nil +--- ./tests/goldenscripts/debug_traceback.lua:75 in +--- +--- skip too much +--- stack traceback: +--- a pcall message +--- stack traceback: +--- ./tests/goldenscripts/debug_traceback.lua:62 in +--- arguments: +--- msg: a pcall message +--- level: nil +--- ./tests/goldenscripts/debug_traceback.lua:66 in +--- arguments: +--- msg: a pcall message +--- level: nil +--- +--- ./tests/goldenscripts/debug_traceback.lua:70 in +--- arguments: +--- msg: a pcall message +--- level: nil +--- ./tests/goldenscripts/debug_traceback.lua:77 in +--- + + +-- Line 60 +function test2(msg, level) + print(debug.traceback(nil, msg, level)) +end + +function test1(msg, level) + test2(msg, level) +end + +function sequence_test(msg, level) + pcall(test1, msg, level) +end + +test1("some message", 0) +test1("some other message", 1) +test1("some other message") +test1("skip too much", 100) +sequence_test("a pcall message") + +trace1 = debug.traceback(nil, nil, 1) trace2 = debug.traceback() +assert(trace1 == trace2, "traceback with level 1 should be the same as without level") \ No newline at end of file diff --git a/tests/test_backtrace.lua b/tests/test_backtrace.lua new file mode 100644 index 00000000..9002d9b6 --- /dev/null +++ b/tests/test_backtrace.lua @@ -0,0 +1,22 @@ +local function error_callback(err_msg) -- This is line 1 (base-1) + -- Padding comment on line 2 (base-1) + -- Padding comment on line 3 (base-1) + error(err_msg) -- This is line 4 (base-1) +end + +local function baz(a, b) -- This is line 7 (base-1) + -- Padding comment on line 8 (base-1) + error_callback(a .. b) -- This is line 9 (base-1) +end + +local function bar(x, ...) -- This is line 12 (base-1) + baz(x, ...) -- This is line 13 (base-1) +end + +local function foo(arg1) -- This is line 16 (base-1) + -- Padding comment on line 17 (base-1) + -- Padding comment on line 18 (base-1) + bar(arg1, " error from rust") -- This is line 19 (base-1) +end + +foo("test") -- This is line 22 (base-1) \ No newline at end of file