Bug #15105
closed`rb_debug_inspector_open` breaks lazy proc optimization
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
Updated by ko1 (Koichi Sasada) almost 6 years 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 andrb_iter_break()
will
fail. This is whyany?
fails. -
test/-ext-/debug/test_debug.rb: add a test.
Updated by mame (Yusuke Endoh) almost 6 years 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 mame (Yusuke Endoh) almost 6 years ago
- Related to Bug #15234: Running redis on the #all? block returns nil added
Updated by stanhu (Stan Hu) almost 6 years 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) almost 6 years 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.
Updated by duerst (Martin Dürst) almost 6 years ago
- Has duplicate Bug #15325: Ruby 2.5.3 seg fault after find block returns added
Updated by nagachika (Tomoyuki Chikanaga) almost 6 years 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.