From 43101ded7f90a803282223204b89a40ccb8894ae Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 30 Jul 2022 19:28:14 +0200 Subject: [PATCH] More consistent handling of exceptions and their display in the Ruby debugger - without triggering too many breakpoints on rethrow and showing the reason for exceptions arising from argument errors --- src/rba/rba/rba.cc | 126 +++++++++++++++++++++++++------------- src/rba/rba/rba.h | 10 +++ src/rba/rba/rbaUtils.cc | 12 ++++ src/rba/rba/rbaUtils.h | 8 +++ src/tl/tl/tlException.cc | 1 + src/tl/tl/tlException.h | 18 +++++- src/tl/tl/tlScriptError.h | 15 ++++- 7 files changed, 145 insertions(+), 45 deletions(-) diff --git a/src/rba/rba/rba.cc b/src/rba/rba/rba.cc index ab323b222..051ddac7a 100644 --- a/src/rba/rba/rba.cc +++ b/src/rba/rba/rba.cc @@ -675,49 +675,78 @@ struct RubyInterpreterPrivateData // ------------------------------------------------------------------- // Ruby API +static void +handle_exception (VALUE exc, bool first_chance) +{ + if (! first_chance) { + // Re-raise the exception without blocking in the debugger + block_exceptions (true); + } + + rb_exc_raise (exc); +} + +static void +handle_exception (const std::string &where, std::exception &ex) +{ + VALUE error_msg = rb_str_new2 ((std::string(ex.what ()) + tl::to_string (tr (" in ")) + where).c_str ()); + VALUE args [1]; + args [0] = error_msg; + VALUE exc = rb_class_new_instance(1, args, rb_eRuntimeError); + handle_exception (exc, true); +} + +static void +handle_exception (const std::string &where, tl::ExitException &ex) +{ + VALUE error_msg = rb_str_new2 ((ex.msg () + tl::to_string (tr (" in ")) + where).c_str ()); + VALUE args [2]; + args [0] = INT2NUM (ex.status ()); + args [1] = error_msg; + VALUE exc = rb_class_new_instance (2, args, rb_eSystemExit); + handle_exception (exc, ex.first_chance ()); +} + +static void +handle_exception (const std::string & /*where*/, rba::RubyError &ex) +{ + handle_exception (ex.exc (), ex.first_chance ()); +} + +static void +handle_exception (const std::string &where, tl::Exception &ex) +{ + VALUE error_msg = rb_str_new2 ((ex.msg () + tl::to_string (tr (" in ")) + where).c_str ()); \ + VALUE args [1]; + args [0] = error_msg; + VALUE exc = rb_class_new_instance(1, args, rb_eRuntimeError); + handle_exception (exc, ex.first_chance ()); +} + +static void +handle_exception (const std::string &where) +{ + VALUE error_msg = rb_str_new2 ((tl::to_string (tr ("Unspecific exception in ")) + where).c_str ()); \ + VALUE args [1]; + args [0] = error_msg; + VALUE exc = rb_class_new_instance(1, args, rb_eRuntimeError); + handle_exception (exc, true); +} + #define RBA_TRY \ - VALUE __error_msg = Qnil; \ - int __estatus = 0; \ - VALUE __exc = Qnil; \ - VALUE __eclass = Qnil; \ - { \ - try { + try { #define RBA_CATCH(where) \ - } catch (std::exception &ex) { \ - __eclass = rb_eRuntimeError; \ - __error_msg = rb_str_new2 ((std::string(ex.what ()) + tl::to_string (tr (" in ")) + (where)).c_str ()); \ - } catch (tl::ExitException &ex) { \ - __estatus = ex.status (); \ - __eclass = rb_eSystemExit; \ - __error_msg = rb_str_new2 ((ex.msg () + tl::to_string (tr (" in ")) + (where)).c_str ()); \ - } catch (rba::RubyError &ex) { \ - __eclass = rb_eRuntimeError; \ - __exc = ex.exc (); \ - } catch (tl::Exception &ex) { \ - __eclass = rb_eRuntimeError; \ - __error_msg = rb_str_new2 ((ex.msg () + tl::to_string (tr (" in ")) + (where)).c_str ()); \ - } catch (...) { \ - __eclass = rb_eRuntimeError; \ - __error_msg = rb_str_new2 ((tl::to_string (tr ("Unspecific exception in ")) + (where)).c_str ()); \ - } \ - } \ - if (__exc != Qnil) { \ - /* Re-raise the exception without blocking in the debugger */ \ - /* TODO: should not access private data */ \ - RubyInterpreter::instance ()->d->block_exceptions = true; \ - rb_exc_raise (__exc); \ - } else if (__eclass == rb_eSystemExit) { \ - /* HINT: we do the rb_raise outside any destructor code - sometimes this longjmp seems not to work properly */ \ - VALUE args [2]; \ - args [0] = INT2NUM (__estatus); \ - args [1] = __error_msg; \ - rb_exc_raise (rb_class_new_instance(2, args, __eclass)); \ - } else if (__eclass != Qnil) { \ - /* HINT: we do the rb_raise outside any destructor code - sometimes this longjmp seems not to work properly */ \ - VALUE args [1]; \ - args [0] = __error_msg; \ - rb_exc_raise (rb_class_new_instance(1, args, __eclass)); \ + } catch (std::exception &ex) { \ + handle_exception ((where), ex); \ + } catch (tl::ExitException &ex) { \ + handle_exception ((where), ex); \ + } catch (rba::RubyError &ex) { \ + handle_exception ((where), ex); \ + } catch (tl::Exception &ex) { \ + handle_exception ((where), ex); \ + } catch (...) { \ + handle_exception ((where)); \ } static VALUE @@ -943,8 +972,9 @@ push_args (gsi::SerialArgs &arglist, const gsi::MethodBase *meth, VALUE *argv, i msg = tl::sprintf (tl::to_string (tr ("%s for argument #%d")), ex.basic_msg (), i + 1); } - ex.set_basic_msg (msg); - throw ex; + tl::Exception new_ex (msg); + new_ex.set_first_chance (ex.first_chance ()); + throw new_ex; } catch (...) { @@ -2225,6 +2255,18 @@ RubyInterpreter::remove_console (gsi::Console *console) } } +void +RubyInterpreter::block_exceptions (bool f) +{ + d->block_exceptions = f; +} + +bool +RubyInterpreter::exceptions_blocked () +{ + return d->block_exceptions; +} + static size_t prepare_trace (RubyInterpreter *interp, const char *fn) { diff --git a/src/rba/rba/rba.h b/src/rba/rba/rba.h index 1b204e186..fa4e39d3b 100644 --- a/src/rba/rba/rba.h +++ b/src/rba/rba/rba.h @@ -178,6 +178,16 @@ public: */ void end_exec (); + /** + * @brief Gets a flag indicating whether exceptions are blocked from being seen in the debugger + */ + bool exceptions_blocked (); + + /** + * @brief Sets a flag indicating whether exceptions are blocked from being seen in the debugger + */ + void block_exceptions (bool f); + /** * @brief Fetch the version string * diff --git a/src/rba/rba/rbaUtils.cc b/src/rba/rba/rbaUtils.cc index ccc071d2a..439fe97bb 100644 --- a/src/rba/rba/rbaUtils.cc +++ b/src/rba/rba/rbaUtils.cc @@ -112,6 +112,18 @@ rba_get_backtrace_from_array (VALUE backtrace, std::vector } } +void +block_exceptions (bool f) +{ + RubyInterpreter::instance ()->block_exceptions (f); +} + +bool +exceptions_blocked () +{ + return RubyInterpreter::instance ()->exceptions_blocked (); +} + void rba_check_error () { diff --git a/src/rba/rba/rbaUtils.h b/src/rba/rba/rbaUtils.h index bcb514351..436841b50 100644 --- a/src/rba/rba/rbaUtils.h +++ b/src/rba/rba/rbaUtils.h @@ -224,6 +224,9 @@ VALUE rba_f_eval_checked (int argc, VALUE *argv, VALUE self); void rba_yield_checked (VALUE value); VALUE rba_eval_string_in_context (const char *expr, const char *file, int line, int context); +bool exceptions_blocked (); +void block_exceptions (bool f); + /** * @brief A struct encapsulating the call parameters for a function */ @@ -261,7 +264,12 @@ R rba_safe_func (R (*f) (A), A arg) int error = 0; RUBY_BEGIN_EXEC + // NOTE: we do not want exceptions to be seen in the debugger here - later they are rethrown after + // being annotated. This is when we want to see them. + bool eb = exceptions_blocked (); + block_exceptions (true); rb_protect (&rba_safe_func_caller, (VALUE) &cp, &error); + block_exceptions (eb); RUBY_END_EXEC if (error) { diff --git a/src/tl/tl/tlException.cc b/src/tl/tl/tlException.cc index 4c66acd1e..614836a03 100644 --- a/src/tl/tl/tlException.cc +++ b/src/tl/tl/tlException.cc @@ -31,6 +31,7 @@ namespace tl void Exception::init (const std::string &fmt, const std::vector &a) { + m_first_chance = true; m_msg = tl::sprintf (fmt, a); } diff --git a/src/tl/tl/tlException.h b/src/tl/tl/tlException.h index 0eb93543f..ecde47d9b 100644 --- a/src/tl/tl/tlException.h +++ b/src/tl/tl/tlException.h @@ -46,7 +46,7 @@ class TL_PUBLIC Exception { public: Exception (const std::string &msg) - : m_msg (msg) + : m_msg (msg), m_first_chance (true) { } Exception (const std::string &fmt, const std::vector &a) @@ -148,8 +148,24 @@ public: */ void set_basic_msg (const std::string &msg) { m_msg = msg; } + /** + * @brief Sets a flag indicating whether this exception is a first-chance one + * + * "first chance" means it has not been seen in the debugger. + * Set this flag to false to indicate that it already got seen. + * By default the flag is true, indicating it has not been handled + * by the debugger. + */ + void set_first_chance (bool f) { m_first_chance = f; } + + /** + * @brief Gets a flag indicating that is this a first-chance exception + */ + bool first_chance () const { return m_first_chance; } + private: std::string m_msg; + bool m_first_chance; void init (const std::string &fmt, const std::vector &a); }; diff --git a/src/tl/tl/tlScriptError.h b/src/tl/tl/tlScriptError.h index 312dd4a6b..3e00a4f60 100644 --- a/src/tl/tl/tlScriptError.h +++ b/src/tl/tl/tlScriptError.h @@ -148,8 +148,19 @@ class TL_PUBLIC ExitException : public tl::Exception { public: - ExitException () : tl::Exception ("exit"), m_status (1) { } - ExitException (int status) : tl::Exception ("exit"), m_status (status) { } + ExitException () + : tl::Exception ("exit"), m_status (1) + { + // do not catch in debugger + set_first_chance (false); + } + + ExitException (int status) + : tl::Exception ("exit"), m_status (status) + { + // do not catch in debugger + set_first_chance (false); + } int status() const { return m_status; }