diff --git a/compile.c b/compile.c index de34b0f043..7ae6efb668 100644 --- a/compile.c +++ b/compile.c @@ -10217,12 +10217,6 @@ dump_disasm_list_with_cursor(const LINK_ELEMENT *link, const LINK_ELEMENT *curr, fflush(stdout); } -bool -rb_insns_leaf_p(int i) -{ - return insn_leaf_p(i); -} - int rb_insn_len(VALUE insn) { diff --git a/gc.c b/gc.c index 0d650dbe68..cc318b15e0 100644 --- a/gc.c +++ b/gc.c @@ -2423,42 +2423,7 @@ static void gc_event_hook_body(rb_execution_context_t *ec, rb_objspace_t *objspace, const rb_event_flag_t event, VALUE data) { if (UNLIKELY(!ec->cfp)) return; - const VALUE *pc = ec->cfp->pc; - if (pc && VM_FRAME_RUBYFRAME_P(ec->cfp)) { - int prev_opcode = rb_vm_insn_addr2opcode((void *)*ec->cfp->iseq->body->iseq_encoded); - for (const VALUE *insn = ec->cfp->iseq->body->iseq_encoded; insn < pc; insn += rb_insn_len(prev_opcode)) { - prev_opcode = rb_vm_insn_addr2opcode((void *)*insn); - } - - /* If the previous instruction is a leaf instruction, then the PC is - * the currently executing instruction. We should increment the PC - * because the source line is calculated with PC-1 in calc_pos. - * - * If the previous instruction is not a leaf instruction and the - * current instruction is not a leaf instruction, then the PC was - * incremented before the instruction was ran (meaning the currently - * executing instruction is actually the previous instruction), so we - * should not increment the PC otherwise we will calculate the source - * line for the next instruction. - * - * However, this implementation still has a bug. Consider the - * following situation: - * - * non-leaf - * leaf <- - * - * Where the PC currently points to a leaf instruction. We don't know - * which instruction we really are at since we could be at the non-leaf - * instruction (since it incremented the PC before executing the - * instruction). We could also be at the leaf instruction since the PC - * doesn't get incremented until the instruction finishes. - */ - if (rb_insns_leaf_p(prev_opcode)) { - ec->cfp->pc++; - } - } EXEC_EVENT_HOOK(ec, event, ec->cfp->self, 0, 0, 0, data); - ec->cfp->pc = pc; } #define gc_event_hook_available_p(objspace) ((objspace)->flags.has_hook) diff --git a/internal/compile.h b/internal/compile.h index eebb7605cd..2ece5396f6 100644 --- a/internal/compile.h +++ b/internal/compile.h @@ -17,7 +17,6 @@ struct rb_iseq_struct; /* in vm_core.h */ /* compile.c */ int rb_dvar_defined(ID, const struct rb_iseq_struct *); int rb_local_defined(ID, const struct rb_iseq_struct *); -bool rb_insns_leaf_p(int i); int rb_insn_len(VALUE insn); const char *rb_insns_name(int i); VALUE rb_insns_name_array(void); diff --git a/test/objspace/test_objspace.rb b/test/objspace/test_objspace.rb index 7edf6be930..43ccac7920 100644 --- a/test/objspace/test_objspace.rb +++ b/test/objspace/test_objspace.rb @@ -225,6 +225,13 @@ class TestObjSpace < Test::Unit::TestCase assert_equal(__FILE__, ObjectSpace.allocation_sourcefile(o4)) assert_equal(line4, ObjectSpace.allocation_sourceline(o4)) + # The line number should be based on newarray instead of getinstancevariable. + line5 = __LINE__; o5 = [ # newarray (leaf) + @ivar, # getinstancevariable (not leaf) + ] + assert_equal(__FILE__, ObjectSpace.allocation_sourcefile(o5)) + assert_equal(line5, ObjectSpace.allocation_sourceline(o5)) + # [Bug #19482] EnvUtil.under_gc_stress do 100.times do diff --git a/tool/ruby_vm/views/_insn_entry.erb b/tool/ruby_vm/views/_insn_entry.erb index 32070c5f34..6ec33461c4 100644 --- a/tool/ruby_vm/views/_insn_entry.erb +++ b/tool/ruby_vm/views/_insn_entry.erb @@ -24,7 +24,7 @@ INSN_ENTRY(<%= insn.name %>) <%= ope[:decl] %> = (<%= ope[:type] %>)GET_OPERAND(<%= i + 1 %>); % end # define INSN_ATTR(x) <%= insn.call_attribute(' ## x ## ') %> - const bool leaf = INSN_ATTR(leaf); + const bool MAYBE_UNUSED(leaf) = INSN_ATTR(leaf); % insn.pops.reverse_each.with_index.reverse_each do |pop, i| <%= pop[:decl] %> = <%= insn.cast_from_VALUE pop, "TOPN(#{i})"%>; % end @@ -35,7 +35,7 @@ INSN_ENTRY(<%= insn.name %>) % end /* ### Instruction preambles. ### */ - if (! leaf) ADD_PC(INSN_ATTR(width)); + ADD_PC(INSN_ATTR(width)); % if insn.handles_sp? POPN(INSN_ATTR(popn)); % end @@ -68,7 +68,6 @@ INSN_ENTRY(<%= insn.name %>) VM_ASSERT(!RB_TYPE_P(TOPN(<%= i %>), T_MOVED)); % end % end - if (leaf) ADD_PC(INSN_ATTR(width)); # undef INSN_ATTR /* ### Leave the instruction. ### */ diff --git a/tool/ruby_vm/views/_leaf_helpers.erb b/tool/ruby_vm/views/_leaf_helpers.erb index ac35df64f4..f740107a0a 100644 --- a/tool/ruby_vm/views/_leaf_helpers.erb +++ b/tool/ruby_vm/views/_leaf_helpers.erb @@ -10,31 +10,6 @@ #include "iseq.h" -extern const bool rb_vm_insn_leaf_p[]; - -#ifdef RUBY_VM_INSNS_INFO -const bool rb_vm_insn_leaf_p[] = { -% RubyVM::Instructions.each_slice(20) do |insns| - <%= insns.map do |insn| - if insn.is_a?(RubyVM::BareInstructions) - insn.always_leaf? ? '1' : '0' - else - '0' - end - end.join(', ') - %>, -% end -}; -#endif - -CONSTFUNC(MAYBE_UNUSED(static bool insn_leaf_p(VALUE insn))); - -bool -insn_leaf_p(VALUE insn) -{ - return rb_vm_insn_leaf_p[insn]; -} - // This is used to tell RJIT that this insn would be leaf if CHECK_INTS didn't exist. // It should be used only when RUBY_VM_CHECK_INTS is directly written in insns.def. static bool leafness_of_check_ints = false; diff --git a/vm_insnhelper.h b/vm_insnhelper.h index 66895cd142..90372853dd 100644 --- a/vm_insnhelper.h +++ b/vm_insnhelper.h @@ -181,10 +181,8 @@ CC_SET_FASTPATH(const struct rb_callcache *cc, vm_call_handler func, bool enable /**********************************************************/ #define CALL_SIMPLE_METHOD() do { \ - rb_snum_t x = leaf ? INSN_ATTR(width) : 0; \ - rb_snum_t y = attr_width_opt_send_without_block(0); \ - rb_snum_t z = x - y; \ - ADD_PC(z); \ + rb_snum_t insn_width = attr_width_opt_send_without_block(0); \ + ADD_PC(-insn_width); \ DISPATCH_ORIGINAL_INSN(opt_send_without_block); \ } while (0)