Project

General

Profile

Bug #15270

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

Added by alanwu (Alan Wu) 10 months ago. Updated 4 months 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 master - Bug #15717: Backport #15270 to Ruby 2.5.xOpenActions

Associated revisions

Revision 65567
Added by ko1 (Koichi Sasada) 10 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) 9 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) 9 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) 10 months ago

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

Updated by alanwu (Alan Wu) 10 months ago

  • Description updated (diff)

Typos

Updated by ko1 (Koichi Sasada) 10 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) 10 months ago

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

#6

Updated by nagachika (Tomoyuki Chikanaga) 9 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) 9 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) 4 months ago

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

Updated by nagachika (Tomoyuki Chikanaga) 4 months 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