ruby-changes:23736
From: ko1 <ko1@a...>
Date: Fri, 25 May 2012 13:50:22 +0900 (JST)
Subject: [ruby-changes:23736] ko1:r35787 (trunk): * vm.c: refactoring backtrace related funcitons.
ko1 2012-05-25 13:50:10 +0900 (Fri, 25 May 2012) New Revision: 35787 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=35787 Log: * vm.c: refactoring backtrace related funcitons. (1) unify similar functions (rb_backtrace_each() and backtrace_object()). backtrace_each() is a unified function. variation: a) backtrace_object(): create backtrace object. b) vm_backtrace_str_ary(): create bt as an array of string. c) vm_backtrace_print(): print backtrace to specified file. d) rb_backtrace_print_as_bugreport(): print backtrace on bugreport style. (2) remove rb_backtrace_each(). Use backtrace_each() instead. (3) chang the type of lev parameter to size_t. a) lev == 0 means current frame (exception, etc use it). b) lev == 1 means upper frame (caller(0) use it). * vm_core.h, vm_dump.c, vm_eval.c: ditto. * vm.c (backtrace_object(), vm_backtrace_str_ary()): fix to return a correct size of caller(lev) array. Let n be a "caller(0).size" then ln as caller(lev).size should be (n - lev). However, the previous implementation returns a wrong size array (ln > n - lev). [ruby-dev:45673] * test/ruby/test_backtrace.rb: add tests for backtrace. Added files: trunk/test/ruby/test_backtrace.rb Modified files: trunk/ChangeLog trunk/vm.c trunk/vm_core.h trunk/vm_dump.c trunk/vm_eval.c Index: ChangeLog =================================================================== --- ChangeLog (revision 35786) +++ ChangeLog (revision 35787) @@ -1,3 +1,29 @@ +Fri May 25 10:52:52 2012 Koichi Sasada <ko1@a...> + + * vm.c: refactoring backtrace related funcitons. + (1) unify similar functions (rb_backtrace_each() and + backtrace_object()). backtrace_each() is a unified function. + variation: + a) backtrace_object(): create backtrace object. + b) vm_backtrace_str_ary(): create bt as an array of string. + c) vm_backtrace_print(): print backtrace to specified file. + d) rb_backtrace_print_as_bugreport(): print backtrace on + bugreport style. + (2) remove rb_backtrace_each(). Use backtrace_each() instead. + (3) chang the type of lev parameter to size_t. + a) lev == 0 means current frame (exception, etc use it). + b) lev == 1 means upper frame (caller(0) use it). + + * vm_core.h, vm_dump.c, vm_eval.c: ditto. + + * vm.c (backtrace_object(), vm_backtrace_str_ary()): fix to return a + correct size of caller(lev) array. + Let n be a "caller(0).size" then ln as caller(lev).size should be + (n - lev). However, the previous implementation returns a wrong + size array (ln > n - lev). [ruby-dev:45673] + + * test/ruby/test_backtrace.rb: add tests for backtrace. + Fri May 25 08:51:39 2012 Eric Hodel <drbrain@s...> * enum.c (enum_count): Enumerable#count no longer uses #size when Index: vm_core.h =================================================================== --- vm_core.h (revision 35786) +++ vm_core.h (revision 35787) @@ -680,7 +680,6 @@ int ruby_thread_has_gvl_p(void); VALUE rb_make_backtrace(void); typedef int rb_backtrace_iter_func(void *, VALUE, int, VALUE); -int rb_backtrace_each(rb_backtrace_iter_func *iter, void *arg); rb_control_frame_t *rb_vm_get_ruby_level_next_cfp(rb_thread_t *th, rb_control_frame_t *cfp); int rb_vm_get_sourceline(const rb_control_frame_t *); VALUE rb_name_err_mesg_new(VALUE obj, VALUE mesg, VALUE recv, VALUE method); Index: vm_eval.c =================================================================== --- vm_eval.c (revision 35786) +++ vm_eval.c (revision 35787) @@ -20,9 +20,8 @@ static void vm_set_eval_stack(rb_thread_t * th, VALUE iseqval, const NODE *cref); static int vm_collect_local_variables_in_heap(rb_thread_t *th, VALUE *dfp, VALUE ary); -static int vm_backtrace_each(rb_thread_t *th, int lev, ptrdiff_t n, void (*init)(void *), rb_backtrace_iter_func *iter, void *arg); -static VALUE backtrace_object(rb_thread_t *th, int lev, ptrdiff_t n); -static VALUE vm_backtrace_str_ary(rb_thread_t *th, int lev, int n); +static VALUE vm_backtrace_str_ary(rb_thread_t *th, size_t lev, size_t n); +static void vm_backtrace_print(FILE *fp); typedef enum call_type { CALL_PUBLIC, @@ -1087,7 +1086,7 @@ errat = rb_get_backtrace(errinfo); mesg = rb_attr_get(errinfo, id_mesg); if (!NIL_P(errat) && RB_TYPE_P(errat, T_ARRAY) && - (bt2 = vm_backtrace_str_ary(th, -2, 0), RARRAY_LEN(bt2) > 0)) { + (bt2 = vm_backtrace_str_ary(th, 0, 0), RARRAY_LEN(bt2) > 0)) { if (!NIL_P(mesg) && RB_TYPE_P(mesg, T_STRING) && !RSTRING_LEN(mesg)) { if (OBJ_FROZEN(mesg)) { VALUE m = rb_str_cat(rb_str_dup(RARRAY_PTR(errat)[0]), ": ", 2); @@ -1622,44 +1621,22 @@ if (lev < 0) rb_raise(rb_eArgError, "negative level (%d)", lev); - return vm_backtrace_str_ary(GET_THREAD(), lev, 0); + return vm_backtrace_str_ary(GET_THREAD(), lev+1, 0); } -static int -print_backtrace(void *arg, VALUE file, int line, VALUE method) -{ - FILE *fp = arg; - const char *filename = NIL_P(file) ? "ruby" : RSTRING_PTR(file); - if (NIL_P(method)) { - fprintf(fp, "\tfrom %s:%d:in unknown method\n", - filename, line); - } - else { - fprintf(fp, "\tfrom %s:%d:in `%s'\n", - filename, line, RSTRING_PTR(method)); - } - return FALSE; -} - void rb_backtrace(void) { - vm_backtrace_each(GET_THREAD(), -1, 0, NULL, print_backtrace, stderr); + vm_backtrace_print(stderr); } VALUE rb_make_backtrace(void) { - return vm_backtrace_str_ary(GET_THREAD(), -1, 0); + return vm_backtrace_str_ary(GET_THREAD(), 0, 0); } VALUE -rb_vm_backtrace_object() -{ - return backtrace_object(GET_THREAD(), -1, 0); -} - -VALUE rb_thread_backtrace(VALUE thval) { rb_thread_t *th; @@ -1678,12 +1655,6 @@ return vm_backtrace_str_ary(th, 0, 0); } -int -rb_backtrace_each(rb_backtrace_iter_func *iter, void *arg) -{ - return vm_backtrace_each(GET_THREAD(), -1, 0, NULL, iter, arg); -} - /* * call-seq: * local_variables -> array Index: vm.c =================================================================== --- vm.c (revision 35786) +++ vm.c (revision 35787) @@ -736,7 +736,7 @@ /* backtrace */ inline static int -calc_line_no(const rb_iseq_t *iseq, VALUE *pc) +calc_line_no(const rb_iseq_t *iseq, const VALUE *pc) { return rb_iseq_line_no(iseq, pc - iseq->iseq_encoded); } @@ -762,8 +762,8 @@ union { struct { - rb_iseq_t *iseq; - VALUE *pc; + const rb_iseq_t *iseq; + const VALUE *pc; } iseq; struct { ID mid; @@ -824,8 +824,9 @@ typedef struct rb_backtrace_struct { rb_frame_info_t *backtrace; + rb_frame_info_t *backtrace_base; size_t backtrace_size; - VALUE str; + VALUE strary; } rb_backtrace_t; static void @@ -837,7 +838,7 @@ for (i=0; i<s; i++) { frame_info_mark(&bt->backtrace[i]); - rb_gc_mark(bt->str); + rb_gc_mark(bt->strary); } } } @@ -847,7 +848,7 @@ { if (ptr) { rb_backtrace_t *bt = (rb_backtrace_t *)ptr; - if (bt->backtrace) ruby_xfree(bt->backtrace); + if (bt->backtrace) ruby_xfree(bt->backtrace_base); ruby_xfree(bt); } } @@ -875,73 +876,135 @@ { rb_backtrace_t *bt; VALUE obj = TypedData_Make_Struct(klass, rb_backtrace_t, &backtrace_data_type, bt); - bt->backtrace = 0; - bt->backtrace_size = 0; - bt->str = 0; return obj; } -static VALUE -backtrace_object(rb_thread_t *th, int lev, ptrdiff_t n) +static void +backtrace_each(rb_thread_t *th, + void (*init)(void *arg, int size), + void (*iter_iseq)(void *arg, const rb_iseq_t *iseq, const VALUE *pc), + void (*iter_cfunc)(void *arg, ID mid), + void *arg) { - VALUE btobj = backtrace_alloc(rb_cBacktrace); - rb_backtrace_t *bt; rb_control_frame_t *last_cfp = th->cfp; rb_control_frame_t *start_cfp = RUBY_VM_END_CONTROL_FRAME(th); rb_control_frame_t *cfp; - ptrdiff_t size, i, j; + ptrdiff_t size, i; - start_cfp = RUBY_VM_NEXT_CONTROL_FRAME( - RUBY_VM_NEXT_CONTROL_FRAME( - RUBY_VM_NEXT_CONTROL_FRAME(start_cfp))); /* skip top frames */ - size = (start_cfp - last_cfp) + 1; + /* <- start_cfp (end control frame) + * top frame (dummy) + * top frame (dummy) + * top frame <- start_cfp + * top frame + * ... + * 2nd frame <- lev:0 + * current frame <- th->cfp + */ - if (n <= 0) { - n = size + n; - if (n < 0) { - n = 0; - } + start_cfp = + RUBY_VM_NEXT_CONTROL_FRAME( + RUBY_VM_NEXT_CONTROL_FRAME( + RUBY_VM_NEXT_CONTROL_FRAME(start_cfp))); /* skip top frames */ + + if (start_cfp < last_cfp) { + size = 0; } - if (lev < 0) { - lev = 0; + else { + size = start_cfp - last_cfp + 1; } - GetCoreDataFromValue(btobj, rb_backtrace_t, bt); - bt->backtrace = ruby_xmalloc(sizeof(rb_frame_info_t) * n); - bt->backtrace_size = n; + init(arg, size); - for (i=lev, j=0, cfp = start_cfp; i<size && j<n; i++, cfp = RUBY_VM_NEXT_CONTROL_FRAME(cfp)) { - rb_frame_info_t *fi = &bt->backtrace[j]; - + /* SDR(); */ + for (i=0, cfp = start_cfp; i<size; i++, cfp = RUBY_VM_NEXT_CONTROL_FRAME(cfp)) { + /* fprintf(stderr, "cfp: %d\n", (rb_control_frame_t *)(th->stack + th->stack_size) - cfp); */ if (cfp->iseq) { if (cfp->pc) { - fi->type = FRAME_INFO_TYPE_ISEQ; - fi->body.iseq.iseq = cfp->iseq; - fi->body.iseq.pc = cfp->pc; - j++; + iter_iseq(arg, cfp->iseq, cfp->pc); } } else if (RUBYVM_CFUNC_FRAME_P(cfp)) { ID mid = cfp->me->def ? cfp->me->def->original_id : cfp->me->called_id; if (mid != ID_ALLOCATOR) { - fi->type = FRAME_INFO_TYPE_CFUNC; - fi->body.cfunc.mid = mid; - j++; + iter_cfunc(arg, mid); } } } +} - if (j > 0) { - bt->backtrace_size = j; /* TODO: realloc? */ - return btobj; +struct bt_iter_arg { + rb_backtrace_t *bt; + VALUE btobj; +}; + +static void +bt_init(void *ptr, int size) +{ + struct bt_iter_arg *arg = (struct bt_iter_arg *)ptr; + arg->btobj = backtrace_alloc(rb_cBacktrace); + GetCoreDataFromValue(arg->btobj, rb_backtrace_t, arg->bt); + arg->bt->backtrace_base = arg->bt->backtrace = ruby_xmalloc(sizeof(rb_frame_info_t) * size); + arg->bt->backtrace_size = 0; +} + +static void +bt_iter_iseq(void *ptr, const rb_iseq_t *iseq, const VALUE *pc) +{ + struct bt_iter_arg *arg = (struct bt_iter_arg *)ptr; + rb_frame_info_t *fi = &arg->bt->backtrace[arg->bt->backtrace_size++]; + fi->type = FRAME_INFO_TYPE_ISEQ; + fi->body.iseq.iseq = iseq; + fi->body.iseq.pc = pc; +} + +static void +bt_iter_cfunc(void *ptr, ID mid) +{ + struct bt_iter_arg *arg = (struct bt_iter_arg *)ptr; + rb_frame_info_t *fi = &arg->bt->backtrace[arg->bt->backtrace_size++]; + fi->type = FRAME_INFO_TYPE_CFUNC; + fi->body.cfunc.mid = mid; +} + +static VALUE +backtrace_object(rb_thread_t *th, size_t lev, size_t n) +{ + struct bt_iter_arg arg; + + backtrace_each(th, + bt_init, + bt_iter_iseq, + bt_iter_cfunc, + &arg); + + if (lev > 0) { + if (lev > arg.bt->backtrace_size) { + arg.bt->backtrace = 0; + arg.bt->backtrace_size = 0; + arg.btobj = Qnil; + } + else { + arg.bt->backtrace += lev; + arg.bt->backtrace_size -= lev; + } } - else { - /* gc will free object */ - return Qnil; + + if (n > 0) { + if (n < arg.bt->backtrace_size) { + arg.bt->backtrace_size = n; /* trim */ + } } + + return arg.btobj; } +VALUE +rb_vm_backtrace_object(void) +{ + return backtrace_object(GET_THREAD(), 0, 0); +} + static VALUE backtreace_collect(rb_backtrace_t *bt, VALUE (*func)(rb_frame_info_t *, VALUE *)) { @@ -971,12 +1034,12 @@ rb_backtrace_t *bt; GetCoreDataFromValue(self, rb_backtrace_t, bt); - if (bt->str) { - return bt->str; + if (bt->strary) { + return bt->strary; } else { - bt->str = backtreace_collect(bt, frame_info_to_str_override); - return bt->str; + bt->strary = backtreace_collect(bt, frame_info_to_str_override); + return bt->strary; } } @@ -988,9 +1051,9 @@ } static VALUE -vm_backtrace_frame_ary(rb_thread_t *th, int lev, int n) +backtrace_frame_ary(rb_thread_t *th, size_t lev, size_t n) { - return rb_backtrace_to_frame_ary(vm_backtrace_create(th, lev, n)); + return rb_backtrace_to_frame_ary(backtrace_create(th, lev, n)); } #endif @@ -1006,72 +1069,56 @@ { rb_backtrace_t *bt; GetCoreDataFromValue(self, rb_backtrace_t, bt); - bt->str = str; + bt->strary = str; return self; } -/* old style backtrace for compatibility */ +/* old style backtrace directly */ -static int -vm_backtrace_each(rb_thread_t *th, int lev, ptrdiff_t n, void (*init)(void *), rb_backtrace_iter_func *iter, void *arg) +struct oldbt_arg { + VALUE filename; + int line_no; + void (*func)(void *data, VALUE file, int line_no, VALUE name); + void *data; /* result */ +}; + +static void +oldbt_init(void *ptr, int dmy) { - const rb_control_frame_t *limit_cfp = th->cfp; - const rb_control_frame_t *cfp = (void *)(th->stack + th->stack_size); - VALUE file = Qnil; - int line_no = 0; + struct oldbt_arg *arg = (struct oldbt_arg *)ptr; + rb_thread_t *th = GET_THREAD(); - if (n <= 0) { - n = cfp - limit_cfp; - } + arg->filename = th->vm->progname ? th->vm->progname : ruby_engine_name;; + arg->line_no = 0; + arg->data = (void *)rb_ary_new(); +} - cfp -= 2; - while (lev-- >= 0 && n > 0) { - if (++limit_cfp > cfp) { - return FALSE; - } - } - if (init) (*init)(arg); - limit_cfp = RUBY_VM_NEXT_CONTROL_FRAME(limit_cfp); - if (th->vm->progname) file = th->vm->progname; - while (cfp > limit_cfp && n>0) { - if (cfp->iseq != 0) { - if (cfp->pc != 0) { - rb_iseq_t *iseq = cfp->iseq; +static void +oldbt_iter_iseq(void *ptr, const rb_iseq_t *iseq, const VALUE *pc) +{ + struct oldbt_arg *arg = (struct oldbt_arg *)ptr; + VALUE file = arg->filename = iseq->location.filename; + VALUE name = iseq->location.name; + int line_no = arg->line_no = calc_line_no(iseq, pc); - line_no = rb_vm_get_sourceline(cfp); - file = iseq->location.filename; - n--; - if ((*iter)(arg, file, line_no, iseq->location.name)) break; - } - } - else if (RUBYVM_CFUNC_FRAME_P(cfp)) { - ID id; - - n--; - if (NIL_P(file)) file = ruby_engine_name; - if (cfp->me->def) - id = cfp->me->def->original_id; - else - id = cfp->me->called_id; - if (id != ID_ALLOCATOR && (*iter)(arg, file, line_no, rb_id2str(id))) - break; - } - cfp = RUBY_VM_NEXT_CONTROL_FRAME(cfp); - } - return TRUE; + (arg->func)(arg->data, file, line_no, name); } static void -vm_backtrace_alloc(void *arg) +oldbt_iter_cfunc(void *ptr, ID mid) { - VALUE *aryp = arg; - *aryp = rb_ary_new(); + struct oldbt_arg *arg = (struct oldbt_arg *)ptr; + VALUE file = arg->filename; + VALUE name = rb_id2str(mid); + int line_no = arg->line_no; + + (arg->func)(arg->data, file, line_no, name); } -static int -vm_backtrace_push(void *arg, VALUE file, int line_no, VALUE name) +static void +oldbt_push(void *data, VALUE file, int line_no, VALUE name) { - VALUE *aryp = arg; + VALUE ary = (VALUE)data; VALUE bt; if (line_no) { @@ -1082,23 +1129,115 @@ bt = rb_enc_sprintf(rb_enc_compatible(file, name), "%s:in `%s'", RSTRING_PTR(file), RSTRING_PTR(name)); } - rb_ary_push(*aryp, bt); - return 0; + rb_ary_push(ary, bt); } static VALUE -vm_backtrace_str_ary(rb_thread_t *th, int lev, int n) +vm_backtrace_str_ary(rb_thread_t *th, size_t lev, size_t n) { - VALUE ary = 0; + struct oldbt_arg arg; + VALUE ary, result; + int i; + size_t size; + VALUE *ptr; - if (lev < 0) { - ary = rb_ary_new(); + arg.func = oldbt_push; + + backtrace_each(th, + oldbt_init, + oldbt_iter_iseq, + oldbt_iter_cfunc, + &arg); + + ary = (VALUE)arg.data; + size = RARRAY_LEN(ary); + + /* ["top", "2nd", ..........., "size-th"] */ + /* <-- n --> <-- lev --> */ + /* return: [.......] */ + + if (n == 0) { + n = size; } - vm_backtrace_each(th, lev, 0, vm_backtrace_alloc, vm_backtrace_push, &ary); - if (!ary) return Qnil; - return rb_ary_reverse(ary); + + if (size < lev) { + return Qnil; + } + + result = rb_ary_new(); + ptr = RARRAY_PTR(ary); + + for (i=0; i<(int)(size - lev) && i<(int)n; i++) { + rb_ary_push(result, ptr[(size - 1) - lev - i]); + } + + return result; } +static void +oldbt_print(void *data, VALUE file, int line_no, VALUE name) +{ + FILE *fp = (FILE *)data; + + if (NIL_P(name)) { + fprintf(fp, "\tfrom %s:%d:in unknown method\n", + RSTRING_PTR(file), line_no); + } + else { + fprintf(fp, "\tfrom %s:%d:in `%s'\n", + RSTRING_PTR(file), line_no, RSTRING_PTR(name)); + } +} + +static void +vm_backtrace_print(FILE *fp) +{ + struct oldbt_arg arg; + + arg.func = oldbt_print; + arg.data = (void *)fp; + backtrace_each(GET_THREAD(), + oldbt_init, + oldbt_iter_iseq, + oldbt_iter_cfunc, + &arg); +} + +static void +oldbt_bugreport(void *arg, VALUE file, int line, VALUE method) +{ + const char *filename = NIL_P(file) ? "ruby" : RSTRING_PTR(file); + if (!*(int *)arg) { + fprintf(stderr, "-- Ruby level backtrace information " + "----------------------------------------\n"); + *(int *)arg = 1; + } + if (NIL_P(method)) { + fprintf(stderr, "%s:%d:in unknown method\n", filename, line); + } + else { + fprintf(stderr, "%s:%d:in `%s'\n", filename, line, RSTRING_PTR(method)); + } +} + +void +rb_backtrace_print_as_bugreport(void) +{ + struct oldbt_arg arg; + int i; + + arg.func = oldbt_bugreport; + arg.data = (int *)&i; + + backtrace_each(GET_THREAD(), + oldbt_init, + oldbt_iter_iseq, + oldbt_iter_cfunc, + &arg); +} + +/* misc */ + VALUE rb_sourcefilename(void) { Index: vm_dump.c =================================================================== --- vm_dump.c (revision 35786) +++ vm_dump.c (revision 35787) @@ -572,24 +572,6 @@ return Qnil; } -static int -bugreport_backtrace(void *arg, VALUE file, int line, VALUE method) -{ - const char *filename = NIL_P(file) ? "ruby" : RSTRING_PTR(file); - if (!*(int *)arg) { - fprintf(stderr, "-- Ruby level backtrace information " - "----------------------------------------\n"); - *(int *)arg = 1; - } - if (NIL_P(method)) { - fprintf(stderr, "%s:%d:in unknown method\n", filename, line); - } - else { - fprintf(stderr, "%s:%d:in `%s'\n", filename, line, RSTRING_PTR(method)); - } - return 0; -} - #if defined(__FreeBSD__) && defined(__OPTIMIZE__) #undef HAVE_BACKTRACE #endif @@ -774,6 +756,8 @@ } #endif +void rb_backtrace_print_as_bugreport(void); + void rb_vm_bugreport(void) { @@ -787,12 +771,9 @@ #endif const rb_vm_t *const vm = GET_VM(); if (vm) { - int i = 0; SDR(); - - if (rb_backtrace_each(bugreport_backtrace, &i)) { - fputs("\n", stderr); - } + rb_backtrace_print_as_bugreport(); + fputs("\n", stderr); } #if HAVE_BACKTRACE || defined(_WIN32) Index: test/ruby/test_backtrace.rb =================================================================== --- test/ruby/test_backtrace.rb (revision 0) +++ test/ruby/test_backtrace.rb (revision 35787) @@ -0,0 +1,57 @@ + +require 'test/unit' + +class TestBacktrace < Test::Unit::TestCase + def test_exception + bt = Fiber.new{ + begin + raise + rescue => e + e.backtrace + end + }.resume + assert_equal(1, bt.size) + assert_match(/.+:\d+:.+/, bt[0]) + end + + def test_caller_lev + cs = [] + Fiber.new{ + Proc.new{ + cs << caller(0) + cs << caller(1) + cs << caller(2) + cs << caller(3) + cs << caller(4) + cs << caller(5) + }.call + }.resume + assert_equal(3, cs[0].size) + assert_equal(2, cs[1].size) + assert_equal(1, cs[2].size) + assert_equal(0, cs[3].size) + assert_equal(nil, cs[4]) + + # + max = 20 + rec = lambda{|n| + if n > 0 + 1.times{ + rec[n-1] + } + else + max.times{|i| + total_size = caller(0).size + c = caller(i) + if c + assert_equal(total_size - i, caller(i).size, "[ruby-dev:45673]") + end + } + end + } + bt = Fiber.new{ + rec[max] + }.resume + end +end + -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/