Project

General

Profile

Bug #15105

`rb_debug_inspector_open` breaks lazy proc optimization

Added by tenderlovemaking (Aaron Patterson) 8 months ago. Updated 5 months ago.

Status:
Closed
Priority:
Normal
Target version:
-
ruby -v:
ruby 2.6.0dev (2018-07-28 trunk 64081) [x86_64-darwin17]
[ruby-core:88951]

Description

Calling rb_debug_inspector_open inside a block that uses lazy proc optimizations breaks the implementation. r60397 introduced the bug.

I've attached a failing test, but I'll paste the diff here as well:

From 54e55b83f1a6365ded897ebbef2da758d5739eb0 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue, 11 Sep 2018 14:42:24 -0700
Subject: [PATCH] add a failing test for lazy block

---
 test/-ext-/debug/test_debug.rb | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/test/-ext-/debug/test_debug.rb b/test/-ext-/debug/test_debug.rb
index 3804714d0d..cc1e7b7997 100644
--- a/test/-ext-/debug/test_debug.rb
+++ b/test/-ext-/debug/test_debug.rb
@@ -56,4 +56,25 @@ def test_inspector_open_in_eval
     binds = inspector_in_eval
     binds_check binds, bug7635
   end
+
+  class MyRelation
+    include Enumerable
+
+    def each(&block)
+      records.each(&block)
+    end
+
+    def records
+      [1]
+    end
+  end
+
+  def test_lazy_block
+    x = MyRelation.new.any? do
+      Bug::Debug.inspector
+      true
+    end
+
+    assert x, "any should have returned true"
+  end
 end
-- 
2.17.0

I think this ep calculation might be wrong, but I'm not totally sure: https://github.com/ruby/ruby/blob/bddc28b2aeb6ca84c9eb2cdd59ccc9b76098e429/vm.c#L726

I don't understand r60397 well enough to figure out what the correct fix is.

Thanks.


Files

0001-add-a-failing-test-for-lazy-block.patch (949 Bytes) 0001-add-a-failing-test-for-lazy-block.patch tenderlovemaking (Aaron Patterson), 09/11/2018 09:55 PM

Related issues

Related to Ruby trunk - Bug #15234: Running redis on the #all? block returns nilClosedActions
Has duplicate Ruby trunk - Bug #15325: Ruby 2.5.3 seg fault after find block returnsClosedActions

Associated revisions

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

escape all env properly.

  • vm_backtrace.c (rb_debug_inspector_open): escape all env using rb_vm_stack_to_heap() before making bindings. [Bug #15105]

There is a complicated story of this issue:
Without this patch, IFUNC frame does not escaped. A IFUNC frame
points to CFUNC ep as previous ep. However, CFUNC ep can be escaped
because of making bindings of Ruby level frames.
IFUNC's ep can points to invalidated ep and rb_iter_break() will
fail. This is why any? fails.

  • test/-ext-/debug/test_debug.rb: add a test.

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

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

escape all env properly.

  • vm_backtrace.c (rb_debug_inspector_open): escape all env using rb_vm_stack_to_heap() before making bindings. [Bug #15105]

There is a complicated story of this issue:
Without this patch, IFUNC frame does not escaped. A IFUNC frame
points to CFUNC ep as previous ep. However, CFUNC ep can be escaped
because of making bindings of Ruby level frames.
IFUNC's ep can points to invalidated ep and rb_iter_break() will
fail. This is why any? fails.

  • test/-ext-/debug/test_debug.rb: add a test.

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

escape all env properly.

  • vm_backtrace.c (rb_debug_inspector_open): escape all env using rb_vm_stack_to_heap() before making bindings. [Bug #15105]

There is a complicated story of this issue:
Without this patch, IFUNC frame does not escaped. A IFUNC frame
points to CFUNC ep as previous ep. However, CFUNC ep can be escaped
because of making bindings of Ruby level frames.
IFUNC's ep can points to invalidated ep and rb_iter_break() will
fail. This is why any? fails.

  • test/-ext-/debug/test_debug.rb: add a test.

Revision e84f8076
Added by nagachika (Tomoyuki Chikanaga) 5 months ago

merge revision(s) 64799,64800,64801: [Backport #15105]

    fix typo.

    * vm_exec.h (DEBUG_END_INSN()): use `ec` instead of `th`.
      This macro is used when `VMDEBUG > 0`.


    escape all env properly.

    * vm_backtrace.c (rb_debug_inspector_open): escape all env using
      `rb_vm_stack_to_heap()` before making bindings.
      [Bug #15105]

      There is a complicated story of this issue:
      Without this patch, IFUNC frame does not escaped. A IFUNC frame
      points to CFUNC ep as previous ep. However, CFUNC ep can be escaped
      because of making bindings of Ruby level frames.
      IFUNC's ep can points to invalidated ep and `rb_iter_break()` will
      fail. This is why `any?` fails.

    * test/-ext-/debug/test_debug.rb: add a test.


    * remove trailing spaces.

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

Revision 66074
Added by nagachika (Tomoyuki Chikanaga) 5 months ago

merge revision(s) 64799,64800,64801: [Backport #15105]

fix typo.

* vm_exec.h (DEBUG_END_INSN()): use `ec` instead of `th`.
  This macro is used when `VMDEBUG > 0`.


escape all env properly.

* vm_backtrace.c (rb_debug_inspector_open): escape all env using
  `rb_vm_stack_to_heap()` before making bindings.
  [Bug #15105]

  There is a complicated story of this issue:
  Without this patch, IFUNC frame does not escaped. A IFUNC frame
  points to CFUNC ep as previous ep. However, CFUNC ep can be escaped
  because of making bindings of Ruby level frames.
  IFUNC's ep can points to invalidated ep and `rb_iter_break()` will
  fail. This is why `any?` fails.

* test/-ext-/debug/test_debug.rb: add a test.


* remove trailing spaces.

History

#1

Updated by ko1 (Koichi Sasada) 7 months ago

  • Status changed from Open to Closed

Applied in changeset trunk|r64800.


escape all env properly.

  • vm_backtrace.c (rb_debug_inspector_open): escape all env using rb_vm_stack_to_heap() before making bindings. [Bug #15105]

There is a complicated story of this issue:
Without this patch, IFUNC frame does not escaped. A IFUNC frame
points to CFUNC ep as previous ep. However, CFUNC ep can be escaped
because of making bindings of Ruby level frames.
IFUNC's ep can points to invalidated ep and rb_iter_break() will
fail. This is why any? fails.

  • test/-ext-/debug/test_debug.rb: add a test.
#2

Updated by mame (Yusuke Endoh) 6 months ago

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

Updated by mame (Yusuke Endoh) 6 months ago

  • Related to Bug #15234: Running redis on the #all? block returns nil added

Updated by stanhu (Stan Hu) 5 months ago

This bug also appears to cause a seg fault in certain situations with Ruby 2.5.3 but not with 2.4.5. We were using the binding_of_caller gem, which calls https://github.com/banister/debug_inspector/blob/7160c09906b383846a6e55671913c1ad551f1bf9/ext/rubyvm/debug_inspector/debug_inspector.c#L87.

We have a fairly reproducible test case in GitLab (https://gitlab.com/gitlab-org/gitlab-ce/issues/54281#note_119245423):

build = Ci::Build.last; nil
build.status = 'pending'
build.save

def test
  Ci::Build.all.find do |build|
    next unless build.status == 'pending'

    build.run!
    return build
  end
end

test.try(:id)

We've worked around it by using each instead of find for the ActiveRecord relation.

Updated by stanhu (Stan Hu) 5 months ago

I've confirmed that r64800 makes the seg fault go away. I cherry-picked that commit on top of the 2.5.3 tag. Would love to see a backport to 2.5.4 for this.

#6

Updated by duerst (Martin Dürst) 5 months ago

  • Has duplicate Bug #15325: Ruby 2.5.3 seg fault after find block returns added

Updated by nagachika (Tomoyuki Chikanaga) 5 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 r66074 merged revision(s) 64799,64800,64801.

Also available in: Atom PDF