/mruby

Permalink
Browse files

Simplify backtrace mechanism; fix #3633 #3634 #3644 

Instead of preserving a backtrace in `mrb_state`, `mrb_exc_set`
keeps packed backtrace in an exception object. `#backtrace` unpacks
it to an array of strings.
  • Loading branch information...
@matz
matz committed May 23, 2017
1 parent d2458e6 commit 9644ad51b4a63d8db46d7918ec5e89a547236c56
Unified Split
Showing with 91 additions and 297 deletions.
  1. +1 −13 include/mruby.h
  2. +0 −1 include/mruby/error.h
  3. +86 −235 src/backtrace.c
  4. +4 −40 src/error.c
  5. +0 −4 src/gc.c
  6. +0 −1 src/kernel.c
  7. +0 −1 src/state.c
  8. +0 −2 src/vm.c
View
14 include/mruby.h
@@ -152,12 +152,6 @@ struct mrb_context {
struct mrb_jmpbuf;
typedef struct {
const char *filename;
int lineno;
mrb_sym method_id;
} mrb_backtrace_entry;
typedef void (*mrb_atexit_func)(struct mrb_state*);
#define MRB_STATE_NO_REGEXP 1
@@ -172,15 +166,9 @@ typedef struct mrb_state {
struct mrb_context *c;
struct mrb_context *root_c;
struct iv_tbl *globals; /* global variable table */
struct RObject *exc; /* exception */
struct {
struct RObject *exc;
int n;
int n_allocated;
mrb_backtrace_entry *entries;
} backtrace;
struct iv_tbl *globals; /* global variable table */
struct RObject *top_self;
struct RClass *object_class; /* Object class */
View
1 include/mruby/error.h
@@ -25,7 +25,6 @@ MRB_API void mrb_sys_fail(mrb_state *mrb, const char *mesg);
MRB_API mrb_value mrb_exc_new_str(mrb_state *mrb, struct RClass* c, mrb_value str);
#define mrb_exc_new_str_lit(mrb, c, lit) mrb_exc_new_str(mrb, c, mrb_str_new_lit(mrb, lit))
MRB_API mrb_value mrb_make_exception(mrb_state *mrb, int argc, const mrb_value *argv);
MRB_API mrb_value mrb_exc_backtrace(mrb_state *mrb, mrb_value exc);
MRB_API mrb_value mrb_get_backtrace(mrb_state *mrb);
MRB_API mrb_noreturn void mrb_no_method_error(mrb_state *mrb, mrb_sym id, mrb_value args, const char *fmt, ...);
Oops, something went wrong.

6 comments on commit 9644ad5

@alexsnaps

This comment has been minimized.

Show comment
Hide comment
@alexsnaps

alexsnaps May 30, 2017

Not sure whether this fix is "correct" per se, but I think mrb_get_backtrace is broken (so is mrb_keep_backtrace, right?):

diff --git a/src/backtrace.c b/src/backtrace.c
index ea88aca9..d0693c65 100644
--- a/src/backtrace.c
+++ b/src/backtrace.c
@@ -230,5 +230,6 @@ mrb_unpack_backtrace(mrb_state *mrb, mrb_value backtrace)
 mrb_value
 mrb_get_backtrace(mrb_state *mrb)
 {
-  return mrb_unpack_backtrace(mrb, packed_backtrace(mrb));
+  mrb_value backtrace = mrb_obj_iv_get(mrb, mrb->exc, mrb_intern_lit(mrb, "backtrace"));
+  return mrb_unpack_backtrace(mrb, backtrace);
 }

mrb_get_backtrace doesn't return any backtraces otherwise (supposedly 1 stack frame, where the filename is empty...)

alexsnaps replied May 30, 2017

Not sure whether this fix is "correct" per se, but I think mrb_get_backtrace is broken (so is mrb_keep_backtrace, right?):

diff --git a/src/backtrace.c b/src/backtrace.c
index ea88aca9..d0693c65 100644
--- a/src/backtrace.c
+++ b/src/backtrace.c
@@ -230,5 +230,6 @@ mrb_unpack_backtrace(mrb_state *mrb, mrb_value backtrace)
 mrb_value
 mrb_get_backtrace(mrb_state *mrb)
 {
-  return mrb_unpack_backtrace(mrb, packed_backtrace(mrb));
+  mrb_value backtrace = mrb_obj_iv_get(mrb, mrb->exc, mrb_intern_lit(mrb, "backtrace"));
+  return mrb_unpack_backtrace(mrb, backtrace);
 }

mrb_get_backtrace doesn't return any backtraces otherwise (supposedly 1 stack frame, where the filename is empty...)
@matz

This comment has been minimized.

Show comment
Hide comment
@matz

matz May 30, 2017

Member

mrb_get_backtrace() is an implementation function of caller() so it's not a function to retrieve backtrace from an exception, but retrieve backtrace from call stack. Thus it's correct.
Member

matz replied May 30, 2017

mrb_get_backtrace() is an implementation function of caller() so it's not a function to retrieve backtrace from an exception, but retrieve backtrace from call stack. Thus it's correct.
@alexsnaps

This comment has been minimized.

Show comment
Hide comment
@alexsnaps

alexsnaps May 31, 2017

@matz cool (though I'm not really sure what the function does, but I guess that's the exact issue then).
I used to:

auto ruby_backtrace = mrb_exc_backtrace(mrb, exception);
// RARRAY_PTR(ruby_backtrace) and use the backtrace as needed

But mrb_exc_backtrace got yanked. So how am I to get to the backtrace now?
Based on the diff above, I guess I could now:

mrb_value backtrace = mrb_obj_iv_get(mrb, mrb->exc, mrb_intern_lit(mrb, "backtrace"));
auto ruby_backtrace = mrb_unpack_backtrace(mrb, backtrace);

But I don't think I should have access to mrb_unpack_backtrace... I must be missing the obvious, but how does one get to the backtrace now?

I was thinking mrb_get_backtrace would provide this for me. But surprisingly, while

mrb_print_backtrace(mrb);

prints out the backtrace just fine, while

auto ruby_backtrace = mrb_get_backtrace(mrb);

doesn't seem to return anything useful...

alexsnaps replied May 31, 2017 •
edited

@matz cool (though I'm not really sure what the function does, but I guess that's the exact issue then).
I used to:

auto ruby_backtrace = mrb_exc_backtrace(mrb, exception);
// RARRAY_PTR(ruby_backtrace) and use the backtrace as needed

But mrb_exc_backtrace got yanked. So how am I to get to the backtrace now?
Based on the diff above, I guess I could now:

mrb_value backtrace = mrb_obj_iv_get(mrb, mrb->exc, mrb_intern_lit(mrb, "backtrace"));
auto ruby_backtrace = mrb_unpack_backtrace(mrb, backtrace);

But I don't think I should have access to mrb_unpack_backtrace... I must be missing the obvious, but how does one get to the backtrace now?

I was thinking mrb_get_backtrace would provide this for me. But surprisingly, while

mrb_print_backtrace(mrb);

prints out the backtrace just fine, while

auto ruby_backtrace = mrb_get_backtrace(mrb);

doesn't seem to return anything useful...
@matz

This comment has been minimized.

Show comment
Hide comment
@matz

matz May 31, 2017

Member

Those functions work differently. mrb_print_backtrace prints the backtrace from the last exception (mrb->exc). But mrb_get_backtrace retrieves the backtrace from the current call frames (no relation to exceptions).

I think we should restore mrb_exc_backtrace.
Member

matz replied May 31, 2017

Those functions work differently. mrb_print_backtrace prints the backtrace from the last exception (mrb->exc). But mrb_get_backtrace retrieves the backtrace from the current call frames (no relation to exceptions).

I think we should restore mrb_exc_backtrace.
@matz

This comment has been minimized.

Show comment
Hide comment
@matz

matz May 31, 2017

Member

Restored.
Member

matz replied May 31, 2017

Restored.
@alexsnaps

This comment has been minimized.

Show comment
Hide comment
@alexsnaps

alexsnaps May 31, 2017

Awesome. Thanks!

alexsnaps replied May 31, 2017

Awesome. Thanks!
Please sign in to comment.