Bug #15105: `rb_debug_inspector_open` breaks lazy proc optimization

• Status: Closed
• Assignee: ko1 (Koichi Sasada)
• Target version: -
• ruby -v: ruby 2.6.0dev (2018-07-28 trunk 64081) [x86_64-darwin17]
• Backport: 2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: DONE

[tenderlovemaking (Aaron Patterson)]

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 <[e-mail]>
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.

[ko1 (Koichi Sasada)]

• 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.

[mame (Yusuke Endoh)]

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

[mame (Yusuke Endoh)]

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

[stanhu (Stan Hu)]

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.

[stanhu (Stan Hu)]

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.

[duerst (Martin Dürst)]

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

[nagachika (Tomoyuki Chikanaga)]

• 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.