Project

General

Profile

Actions

Bug #13369

closed

TracePoint gives incorrect `return_value` after rescuing error when using `return`

Added by backus (John Backus) over 5 years ago. Updated almost 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-darwin16]
[ruby-core:80369]

Description

In the following example

class Foo
  MyError = Class.new(StandardError)

  def example1
    norf
  rescue MyError => error
    2
  end

  def example2
    norf
  rescue MyError => error
    return 2
  end

  def norf
    raise MyError
  end
end

trace = TracePoint.new(:return) do |tp|
  puts "#{tp.defined_class}##{tp.method_id} RETURNS #{tp.return_value.inspect}"
end

trace.enable

Foo.new.example1 # => 2
puts
Foo.new.example2 # => 2

# >> Foo#norf RETURNS nil
# >> Foo#example1 RETURNS 2
# >> 
# >> Foo#norf RETURNS nil
# >> Foo#example2 RETURNS nil

Both example1 AND example2 return 2. For some reason though, the tracepoint event for example2 says that it returns nil.


Files

tracepoint-bug-example.rb (529 Bytes) tracepoint-bug-example.rb backus (John Backus), 03/27/2017 05:49 AM
diff.patch (1.29 KB) diff.patch ayemos (Yuichiro Someya), 03/29/2017 09:48 AM

Updated by ayemos (Yuichiro Someya) over 5 years ago

By some investigations, it turned out to be caused by the result value passed in hook_before_rewind method .

The return value would be as expected by the batch, but it would fail in some tests.

Actions #2

Updated by ko1 (Koichi Sasada) about 5 years ago

  • Status changed from Open to Closed

Applied in changeset trunk|r58262.


fix TracePoint#return_value with non-local exits

  • vm.c: get return_value from imemo_throw_data object (THROW_DATA_VAL()).
    imemo_throw_data (TAG_BREAK) contains returned value.
    However, imemo_throw_data (TAG_BREAK) can skip several frames so that
    we need to use it only once (at most internal frame). To record it,
    we introduced THROW_DATA_CONSUMED and check it.

  • internal.h: define THROW_DATA_CONSUMED flag.

  • test/ruby/test_settracefunc.rb: add tests for [Bug #13369]

  • vm_insnhelper.h: add THROW_DATA_CONSUMED_P() and
    THROW_DATA_CONSUMED_SET().

Actions #3

Updated by nagachika (Tomoyuki Chikanaga) almost 5 years ago

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

Updated by nagachika (Tomoyuki Chikanaga) almost 5 years ago

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

ruby_2_4 r59296 merged revision(s) 58262,5826.

Actions #5

Updated by usa (Usaku NAKAMURA) almost 5 years ago

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

Updated by usa (Usaku NAKAMURA) almost 5 years ago

  • Backport changed from 2.2: WONTFIX, 2.3: REQUIRED, 2.4: DONE to 2.2: WONTFIX, 2.3: DONE, 2.4: DONE

ruby_2_3 r59547 merged revision(s) 58262,58263.

Actions

Also available in: Atom PDF