From 59f7b6fffd9f96d0b5b5bd463df7c1e985253538 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Fri, 16 Nov 2018 15:00:31 -0500 Subject: [PATCH] Add a tail call predicate for TracePoint This commit allows TracePoint users to check whether a "call" tracepoint event is a tail call. This feature is useful for applications that want to keep track of execution progress of stack frames. This ability should be useful for debuggers. Before this feature, it was not possible to get a notification for a stack frame finishing due to tail call; the "return" trace point event does not fire for a stack frame that is replace. See Bug #15303. * vm_core.h: Add a new frame flag: VM_FRAME_FLAG_TAILCALLED. It Should be set on a frame that is put on the stack due to a tail call. * vm_insnhelper.c: Set this flag when a tail call happens and check for this flag and pass it as a Ruby boolean to RUBY_EVENT_CALL trace points. * vm.c: Pass Qfalse instead of Qnil to RUBY_EVENT_CALL to document the type of the data field on rb_trace_arg_t when it is a RUBY_EVENT_CALL. * vm_trace.c, debug.h: Add rb_tracearg_is_tailcall for native extensions and #tailcall? for Ruby. A bit defensive here to guard against future changes. --- include/ruby/debug.h | 1 + spec/ruby/core/tracepoint/tailcall_spec.rb | 37 ++++++++ test/ruby/test_settracefunc.rb | 102 ++++++++++----------- vm.c | 2 +- vm_core.h | 5 +- vm_insnhelper.c | 15 ++- vm_trace.c | 20 ++++ 7 files changed, 123 insertions(+), 59 deletions(-) create mode 100644 spec/ruby/core/tracepoint/tailcall_spec.rb diff --git a/include/ruby/debug.h b/include/ruby/debug.h index 8a831e61ab..9a3b20829e 100644 --- a/include/ruby/debug.h +++ b/include/ruby/debug.h @@ -82,6 +82,7 @@ VALUE rb_tracearg_self(rb_trace_arg_t *trace_arg); VALUE rb_tracearg_return_value(rb_trace_arg_t *trace_arg); VALUE rb_tracearg_raised_exception(rb_trace_arg_t *trace_arg); VALUE rb_tracearg_object(rb_trace_arg_t *trace_arg); +VALUE rb_tracearg_is_tailcall(rb_trace_arg_t *trace_arg); /* * Postponed Job API diff --git a/spec/ruby/core/tracepoint/tailcall_spec.rb b/spec/ruby/core/tracepoint/tailcall_spec.rb new file mode 100644 index 0000000000..25376937bc --- /dev/null +++ b/spec/ruby/core/tracepoint/tailcall_spec.rb @@ -0,0 +1,37 @@ +require_relative '../../spec_helper' + +describe 'TracePoint#tailcall?' do + it 'returns true when a tailcall happens' do + iseq = RubyVM::InstructionSequence.compile(<<-RUBY, "#{__FILE__}:#{__LINE__}", tailcall_optimization: true) + Module.new do + def self.leaf(foo, bar) + foo * bar + end + + def self.middle + leaf(2, 3) + end + + def self.tco + leaf(1, 9) + middle + end + end + RUBY + + mod = iseq.eval + events = [] + TracePoint.new(:call) { |tp| events << [tp.method_id, tp.tailcall?] }.enable do + mod.tco + mod.leaf(2, 3) + end + + events.should == [ + [:tco, false], + [:leaf, false], + [:middle, true], + [:leaf, true], + [:leaf, false] + ] + end +end diff --git a/test/ruby/test_settracefunc.rb b/test/ruby/test_settracefunc.rb index edb356d3dd..0f20190197 100644 --- a/test/ruby/test_settracefunc.rb +++ b/test/ruby/test_settracefunc.rb @@ -440,7 +440,7 @@ def trace_by_tracepoint *trace_events begin eval <<-EOF.gsub(/^.*?: /, ""), nil, 'xyzzy' 1: trace = TracePoint.trace(*trace_events){|tp| next if !target_thread? - 2: events << [tp.event, tp.lineno, tp.path, _defined_class.(tp), tp.method_id, tp.self, tp.binding.eval("_local_var"), _get_data.(tp)] if tp.path == 'xyzzy' + 2: events << [tp.event, tp.lineno, tp.path, _defined_class.(tp), tp.method_id, tp.self, tp.binding.eval("_local_var"), _get_data.(tp), tp.tailcall?] if tp.path == 'xyzzy' 3: } 4: 1.times{|;_local_var| _local_var = :inner 5: tap{} @@ -468,56 +468,56 @@ def trace_by_tracepoint *trace_events answer_events = [ # - [:c_return, 1, "xyzzy", TracePoint, :trace, TracePoint, :outer, trace], - [:line, 4, 'xyzzy', self.class, method, self, :outer, :nothing], - [:c_call, 4, 'xyzzy', Integer, :times, 1, :outer, :nothing], - [:line, 4, 'xyzzy', self.class, method, self, nil, :nothing], - [:line, 5, 'xyzzy', self.class, method, self, :inner, :nothing], - [:c_call, 5, 'xyzzy', Kernel, :tap, self, :inner, :nothing], - [:c_return, 5, "xyzzy", Kernel, :tap, self, :inner, self], - [:c_return, 4, "xyzzy", Integer, :times, 1, :outer, 1], - [:line, 7, 'xyzzy', self.class, method, self, :outer, :nothing], - [:c_call, 7, "xyzzy", Class, :inherited, Object, :outer, :nothing], - [:c_return, 7, "xyzzy", Class, :inherited, Object, :outer, nil], - [:class, 7, "xyzzy", nil, nil, xyzzy.class, nil, :nothing], - [:line, 8, "xyzzy", nil, nil, xyzzy.class, nil, :nothing], - [:line, 9, "xyzzy", nil, nil, xyzzy.class, :XYZZY_outer, :nothing], - [:c_call, 9, "xyzzy", Module, :method_added, xyzzy.class, :XYZZY_outer, :nothing], - [:c_return, 9, "xyzzy", Module, :method_added, xyzzy.class, :XYZZY_outer, nil], - [:line, 13, "xyzzy", nil, nil, xyzzy.class, :XYZZY_outer, :nothing], - [:c_call, 13, "xyzzy", Module, :method_added, xyzzy.class, :XYZZY_outer, :nothing], - [:c_return,13, "xyzzy", Module, :method_added, xyzzy.class, :XYZZY_outer, nil], - [:end, 17, "xyzzy", nil, nil, xyzzy.class, :XYZZY_outer, :nothing], - [:line, 18, "xyzzy", TestSetTraceFunc, method, self, :outer, :nothing], - [:c_call, 18, "xyzzy", Class, :new, xyzzy.class, :outer, :nothing], - [:c_call, 18, "xyzzy", BasicObject, :initialize, xyzzy, :outer, :nothing], - [:c_return,18, "xyzzy", BasicObject, :initialize, xyzzy, :outer, nil], - [:c_return,18, "xyzzy", Class, :new, xyzzy.class, :outer, xyzzy], - [:line, 19, "xyzzy", TestSetTraceFunc, method, self, :outer, :nothing], - [:call, 9, "xyzzy", xyzzy.class, :foo, xyzzy, nil, :nothing], - [:line, 10, "xyzzy", xyzzy.class, :foo, xyzzy, nil, :nothing], - [:line, 11, "xyzzy", xyzzy.class, :foo, xyzzy, :XYZZY_foo, :nothing], - [:call, 13, "xyzzy", xyzzy.class, :bar, xyzzy, nil, :nothing], - [:line, 14, "xyzzy", xyzzy.class, :bar, xyzzy, nil, :nothing], - [:line, 15, "xyzzy", xyzzy.class, :bar, xyzzy, :XYZZY_bar, :nothing], - [:c_call, 15, "xyzzy", Kernel, :tap, xyzzy, :XYZZY_bar, :nothing], - [:c_return,15, "xyzzy", Kernel, :tap, xyzzy, :XYZZY_bar, xyzzy], - [:return, 16, "xyzzy", xyzzy.class, :bar, xyzzy, :XYZZY_bar, xyzzy], - [:return, 12, "xyzzy", xyzzy.class, :foo, xyzzy, :XYZZY_foo, xyzzy], - [:line, 20, "xyzzy", TestSetTraceFunc, method, self, :outer, :nothing], - [:c_call, 20, "xyzzy", Kernel, :raise, self, :outer, :nothing], - [:c_call, 20, "xyzzy", Exception, :exception, RuntimeError, :outer, :nothing], - [:c_call, 20, "xyzzy", Exception, :initialize, raised_exc, :outer, :nothing], - [:c_return,20, "xyzzy", Exception, :initialize, raised_exc, :outer, raised_exc], - [:c_return,20, "xyzzy", Exception, :exception, RuntimeError, :outer, raised_exc], - [:c_return,20, "xyzzy", Kernel, :raise, self, :outer, nil], - [:c_call, 20, "xyzzy", Exception, :backtrace, raised_exc, :outer, :nothing], - [:c_return,20, "xyzzy", Exception, :backtrace, raised_exc, :outer, nil], - [:raise, 20, "xyzzy", TestSetTraceFunc, :trace_by_tracepoint, self, :outer, raised_exc], - [:c_call, 20, "xyzzy", Module, :===, RuntimeError,:outer, :nothing], - [:c_return,20, "xyzzy", Module, :===, RuntimeError,:outer, true], - [:line, 21, "xyzzy", TestSetTraceFunc, method, self, :outer, :nothing], - [:c_call, 21, "xyzzy", TracePoint, :disable, trace, :outer, :nothing], + [:c_return, 1, "xyzzy", TracePoint, :trace, TracePoint, :outer, trace, false], + [:line, 4, 'xyzzy', self.class, method, self, :outer, :nothing, false], + [:c_call, 4, 'xyzzy', Integer, :times, 1, :outer, :nothing, false], + [:line, 4, 'xyzzy', self.class, method, self, nil, :nothing, false], + [:line, 5, 'xyzzy', self.class, method, self, :inner, :nothing, false], + [:c_call, 5, 'xyzzy', Kernel, :tap, self, :inner, :nothing, false], + [:c_return, 5, "xyzzy", Kernel, :tap, self, :inner, self, false], + [:c_return, 4, "xyzzy", Integer, :times, 1, :outer, 1, false], + [:line, 7, 'xyzzy', self.class, method, self, :outer, :nothing, false], + [:c_call, 7, "xyzzy", Class, :inherited, Object, :outer, :nothing, false], + [:c_return, 7, "xyzzy", Class, :inherited, Object, :outer, nil, false], + [:class, 7, "xyzzy", nil, nil, xyzzy.class, nil, :nothing, false], + [:line, 8, "xyzzy", nil, nil, xyzzy.class, nil, :nothing, false], + [:line, 9, "xyzzy", nil, nil, xyzzy.class, :XYZZY_outer, :nothing, false], + [:c_call, 9, "xyzzy", Module, :method_added, xyzzy.class, :XYZZY_outer, :nothing, false], + [:c_return, 9, "xyzzy", Module, :method_added, xyzzy.class, :XYZZY_outer, nil, false], + [:line, 13, "xyzzy", nil, nil, xyzzy.class, :XYZZY_outer, :nothing, false], + [:c_call, 13, "xyzzy", Module, :method_added, xyzzy.class, :XYZZY_outer, :nothing, false], + [:c_return,13, "xyzzy", Module, :method_added, xyzzy.class, :XYZZY_outer, nil, false], + [:end, 17, "xyzzy", nil, nil, xyzzy.class, :XYZZY_outer, :nothing, false], + [:line, 18, "xyzzy", TestSetTraceFunc, method, self, :outer, :nothing, false], + [:c_call, 18, "xyzzy", Class, :new, xyzzy.class, :outer, :nothing, false], + [:c_call, 18, "xyzzy", BasicObject, :initialize, xyzzy, :outer, :nothing, false], + [:c_return,18, "xyzzy", BasicObject, :initialize, xyzzy, :outer, nil, false], + [:c_return,18, "xyzzy", Class, :new, xyzzy.class, :outer, xyzzy, false], + [:line, 19, "xyzzy", TestSetTraceFunc, method, self, :outer, :nothing, false], + [:call, 9, "xyzzy", xyzzy.class, :foo, xyzzy, nil, :nothing, false], + [:line, 10, "xyzzy", xyzzy.class, :foo, xyzzy, nil, :nothing, false], + [:line, 11, "xyzzy", xyzzy.class, :foo, xyzzy, :XYZZY_foo, :nothing, false], + [:call, 13, "xyzzy", xyzzy.class, :bar, xyzzy, nil, :nothing, false], + [:line, 14, "xyzzy", xyzzy.class, :bar, xyzzy, nil, :nothing, false], + [:line, 15, "xyzzy", xyzzy.class, :bar, xyzzy, :XYZZY_bar, :nothing, false], + [:c_call, 15, "xyzzy", Kernel, :tap, xyzzy, :XYZZY_bar, :nothing, false], + [:c_return,15, "xyzzy", Kernel, :tap, xyzzy, :XYZZY_bar, xyzzy, false], + [:return, 16, "xyzzy", xyzzy.class, :bar, xyzzy, :XYZZY_bar, xyzzy, false], + [:return, 12, "xyzzy", xyzzy.class, :foo, xyzzy, :XYZZY_foo, xyzzy, false], + [:line, 20, "xyzzy", TestSetTraceFunc, method, self, :outer, :nothing, false], + [:c_call, 20, "xyzzy", Kernel, :raise, self, :outer, :nothing, false], + [:c_call, 20, "xyzzy", Exception, :exception, RuntimeError, :outer, :nothing, false], + [:c_call, 20, "xyzzy", Exception, :initialize, raised_exc, :outer, :nothing, false], + [:c_return,20, "xyzzy", Exception, :initialize, raised_exc, :outer, raised_exc, false], + [:c_return,20, "xyzzy", Exception, :exception, RuntimeError, :outer, raised_exc, false], + [:c_return,20, "xyzzy", Kernel, :raise, self, :outer, nil, false], + [:c_call, 20, "xyzzy", Exception, :backtrace, raised_exc, :outer, :nothing, false], + [:c_return,20, "xyzzy", Exception, :backtrace, raised_exc, :outer, nil, false], + [:raise, 20, "xyzzy", TestSetTraceFunc, :trace_by_tracepoint, self, :outer, raised_exc, false], + [:c_call, 20, "xyzzy", Module, :===, RuntimeError,:outer, :nothing, false], + [:c_return,20, "xyzzy", Module, :===, RuntimeError,:outer, true, false], + [:line, 21, "xyzzy", TestSetTraceFunc, method, self, :outer, :nothing, false], + [:c_call, 21, "xyzzy", TracePoint, :disable, trace, :outer, :nothing, false], ] return events, answer_events diff --git a/vm.c b/vm.c index a10fff640b..9734057004 100644 --- a/vm.c +++ b/vm.c @@ -1025,7 +1025,7 @@ invoke_bmethod(rb_execution_context_t *ec, const rb_iseq_t *iseq, VALUE self, co iseq->body->stack_max); RUBY_DTRACE_METHOD_ENTRY_HOOK(ec, me->owner, me->def->original_id); - EXEC_EVENT_HOOK(ec, RUBY_EVENT_CALL, self, me->def->original_id, me->called_id, me->owner, Qnil); + EXEC_EVENT_HOOK(ec, RUBY_EVENT_CALL, self, me->def->original_id, me->called_id, me->owner, Qfalse); VM_ENV_FLAGS_SET(ec->cfp->ep, VM_FRAME_FLAG_FINISH); ret = vm_exec(ec, TRUE); EXEC_EVENT_HOOK(ec, RUBY_EVENT_RETURN, self, me->def->original_id, me->called_id, me->owner, ret); diff --git a/vm_core.h b/vm_core.h index d578b5c6e5..0f9a3d1ed9 100644 --- a/vm_core.h +++ b/vm_core.h @@ -1115,11 +1115,11 @@ typedef rb_control_frame_t * enum { /* Frame/Environment flag bits: - * MMMM MMMM MMMM MMMM ____ __FF FFFF EEEX (LSB) + * MMMM MMMM MMMM MMMM ____ _FFF FFFF EEEX (LSB) * * X : tag for GC marking (It seems as Fixnum) * EEE : 3 bits Env flags - * FF..: 6 bits Frame flags + * FF..: 7 bits Frame flags * MM..: 15 bits frame magic (to check frame corruption) */ @@ -1143,6 +1143,7 @@ enum { VM_FRAME_FLAG_CFRAME = 0x0080, VM_FRAME_FLAG_LAMBDA = 0x0100, VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM = 0x0200, + VM_FRAME_FLAG_TAILCALLED = 0x0400, /* env flag */ VM_ENV_FLAG_LOCAL = 0x0002, diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 39f8ad1cd8..1530ab2884 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1719,7 +1719,7 @@ vm_call_iseq_setup_tailcall(rb_execution_context_t *ec, rb_control_frame_t *cfp, *sp++ = src_argv[i]; } - vm_push_frame(ec, iseq, VM_FRAME_MAGIC_METHOD | VM_ENV_FLAG_LOCAL | finish_flag, + vm_push_frame(ec, iseq, VM_FRAME_MAGIC_METHOD | VM_FRAME_FLAG_TAILCALLED | VM_ENV_FLAG_LOCAL | finish_flag, calling->recv, calling->block_handler, (VALUE)me, iseq->body->iseq_encoded + opt_pc, sp, iseq->body->local_table_size - iseq->body->param.size, @@ -3889,15 +3889,20 @@ vm_trace(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, const VALUE *p VM_ASSERT(vm_event_flags & events); /* increment PC because source line is calculated with PC-1 */ - if ((event = (events & (RUBY_EVENT_CLASS | RUBY_EVENT_CALL | RUBY_EVENT_B_CALL))) != 0) { - VM_ASSERT(event == RUBY_EVENT_CLASS || - event == RUBY_EVENT_CALL || - event == RUBY_EVENT_B_CALL); + if ((event = (events & (RUBY_EVENT_CLASS | RUBY_EVENT_B_CALL))) != 0) { + VM_ASSERT(event == RUBY_EVENT_CLASS || event == RUBY_EVENT_B_CALL); reg_cfp->pc++; vm_dtrace(event, ec); EXEC_EVENT_HOOK(ec, event, GET_SELF(), 0, 0, 0, Qundef); reg_cfp->pc--; } + if (events & RUBY_EVENT_CALL) { + VALUE is_tailcall = (VM_ENV_FLAGS(reg_cfp->ep, VM_FRAME_FLAG_TAILCALLED)) ? Qtrue : Qfalse; + reg_cfp->pc++; + vm_dtrace(RUBY_EVENT_CALL, ec); + EXEC_EVENT_HOOK(ec, RUBY_EVENT_CALL, GET_SELF(), 0, 0, 0, is_tailcall); + reg_cfp->pc--; + } if (events & RUBY_EVENT_LINE) { reg_cfp->pc++; vm_dtrace(RUBY_EVENT_LINE, ec); diff --git a/vm_trace.c b/vm_trace.c index 3fc4da9f9a..d9f6481f9f 100644 --- a/vm_trace.c +++ b/vm_trace.c @@ -935,6 +935,15 @@ rb_tracearg_object(rb_trace_arg_t *trace_arg) return trace_arg->data; } +VALUE +rb_tracearg_is_tailcall(rb_trace_arg_t *trace_arg) +{ + if (trace_arg->event == RUBY_EVENT_CALL) { + return (trace_arg->data == Qtrue) ? Qtrue : Qfalse; + } + return Qfalse; +} + /* * Type of event * @@ -1070,6 +1079,16 @@ tracepoint_attr_raised_exception(VALUE tpval) return rb_tracearg_raised_exception(get_trace_arg()); } +/* + * Whether the interpreter performed a tail call. Can only + * be true for +:call+ events. + */ +static VALUE +tracepoint_attr_is_tailcall(VALUE tpval) +{ + return rb_tracearg_is_tailcall(get_trace_arg()); +} + static void tp_call_trace(VALUE tpval, rb_trace_arg_t *trace_arg) { @@ -1562,6 +1581,7 @@ Init_vm_trace(void) rb_define_method(rb_cTracePoint, "self", tracepoint_attr_self, 0); rb_define_method(rb_cTracePoint, "return_value", tracepoint_attr_return_value, 0); rb_define_method(rb_cTracePoint, "raised_exception", tracepoint_attr_raised_exception, 0); + rb_define_method(rb_cTracePoint, "tailcall?", tracepoint_attr_is_tailcall, 0); rb_define_singleton_method(rb_cTracePoint, "stat", tracepoint_stat_s, 0); } -- 2.19.1