Project

General

Profile

Bug #15270

[PATCH] Fix TracePoint for code loaded using ISeq.load_from_binary

Added by alanwu (Alan Wu) 7 months ago. Updated 29 days ago.

Status:
Closed
Priority:
Normal
Target version:
-
[ruby-core:89641]

Description

Hello. This is a patch for Bug #14702, which I don't think is fully addressed.
The tests in the patch should fail on both trunk and ruby_2_5.

    Fix TracePoint for nested iseq loaded from binary [Bug #14702]

    When loading iseq from binary while a TracePoint is on, we need to
    recompile instructions to their "trace_" variant. Before this commit
    we only recompiled instructions in the top level iseq, which meant
    that TracePoint was malfunctioning for code inside module/class/method
    definitions.

    * compile.c: Move rb_iseq_init_trace to rb_ibf_load_iseq_complete.
      It is called on all iseqs during loading.

    * test_iseq.rb: Test that tracepoints fire within children iseq when
      using load_from_binary.

Files


Related issues

Related to Ruby trunk - Bug #15717: Backport #15270 to Ruby 2.5.xOpenActions

Associated revisions

Revision 65567
Added by ko1 (Koichi Sasada) 7 months ago

Fix TracePoint for nested iseq loaded from binary [Bug#14702]

When loading iseq from binary while a TracePoint is on, we need to
recompile instructions to their "trace_" variant. Before this commit
we only recompiled instructions in the top level iseq, which meant
that TracePoint was malfunctioning for code inside module/class/method
definitions.

  • compile.c: Move rb_iseq_init_trace to rb_ibf_load_iseq_complete.
    It is called on all iseqs during loading.

  • test_iseq.rb: Test that tracepoints fire within children iseq when
    using load_from_binary.

This patch is from: Alan Wu XrXr@users.noreply.github.com

Revision aba207b7
Added by nagachika (Tomoyuki Chikanaga) 6 months ago

merge revision(s) 64736,65567: [Backport #15270]

    iseq.c: prefix rb_ to non-static iseq functions

    I assume we always prefix rb_ to non-static functions to avoid conflict.
    These functions are not exported and safe to be renamed.

    iseq.h: ditto
    compile.c: ditto

    Fix TracePoint for nested iseq loaded from binary [Bug#14702]

    When loading iseq from binary while a TracePoint is on, we need to
    recompile instructions to their "trace_" variant. Before this commit
    we only recompiled instructions in the top level iseq, which meant
    that TracePoint was malfunctioning for code inside module/class/method
    definitions.

    * compile.c: Move rb_iseq_init_trace to rb_ibf_load_iseq_complete.
      It is called on all iseqs during loading.

    * test_iseq.rb: Test that tracepoints fire within children iseq when
      using load_from_binary.

    This patch is from: Alan Wu <XrXr@users.noreply.github.com>

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_5@66225 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 66225
Added by nagachika (Tomoyuki Chikanaga) 6 months ago

merge revision(s) 64736,65567: [Backport #15270]

iseq.c: prefix rb_ to non-static iseq functions

I assume we always prefix rb_ to non-static functions to avoid conflict.
These functions are not exported and safe to be renamed.

iseq.h: ditto
compile.c: ditto

Fix TracePoint for nested iseq loaded from binary [Bug#14702]

When loading iseq from binary while a TracePoint is on, we need to
recompile instructions to their "trace_" variant. Before this commit
we only recompiled instructions in the top level iseq, which meant
that TracePoint was malfunctioning for code inside module/class/method
definitions.

* compile.c: Move rb_iseq_init_trace to rb_ibf_load_iseq_complete.
  It is called on all iseqs during loading.

* test_iseq.rb: Test that tracepoints fire within children iseq when
  using load_from_binary.

This patch is from: Alan Wu <XrXr@users.noreply.github.com>

History

#2

Updated by alanwu (Alan Wu) 7 months ago

  • File deleted (0001-Fix-TracePoint-for-nested-iseq-loaded-from-binary.patch)

Updated by alanwu (Alan Wu) 7 months ago

  • Description updated (diff)

Typos

Updated by ko1 (Koichi Sasada) 7 months ago

  • Assignee set to ko1 (Koichi Sasada)
  • Status changed from Open to Closed

Thank you! You are completely right.

applied at r65567 (I forget to add ticket reference on commit log).

Updated by rafaelfranca (Rafael Fran├ža) 7 months ago

ko1 (Koichi Sasada) can you mark this issue to be backported to 2.5.x please?

#6

Updated by nagachika (Tomoyuki Chikanaga) 7 months ago

  • Backport changed from 2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: UNKNOWN to 2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: REQUIRED

Updated by nagachika (Tomoyuki Chikanaga) 6 months ago

  • Backport changed from 2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: REQUIRED to 2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: DONE

ruby_2_5 r66225 merged revision(s) 64736,65567.

#8

Updated by nagachika (Tomoyuki Chikanaga) 29 days ago

  • Related to Bug #15717: Backport #15270 to Ruby 2.5.x added

Updated by nagachika (Tomoyuki Chikanaga) 29 days ago

  • Backport changed from 2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: DONE to 2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: REQUIRED

I'll set Backport field for 2.5 as REQUIRED since https://bugs.ruby-lang.org/issues/15717 claims the issue was not fixed on ruby_2_5.

Also available in: Atom PDF