From 2b54c135ff3ae2fb362a5efaa542ec9236116add Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Tue, 6 Jun 2023 10:21:29 -0400 Subject: [PATCH] YJIT: Avoid identity-based known-class guards for IO objects (#7911) `IO#reopen` is very special in that it is able to change the class and singleton class of IO instances. In its presence, it is not correct to assume that IO instances has a stable class/singleton class and guard by comparing identity. --- test/ruby/test_yjit.rb | 12 ++++++++++++ yjit/bindgen/src/main.rs | 1 + yjit/src/codegen.rs | 3 +++ yjit/src/cruby_bindings.inc.rs | 1 + 4 files changed, 17 insertions(+) diff --git a/test/ruby/test_yjit.rb b/test/ruby/test_yjit.rb index 6700e58176..115c1902f0 100644 --- a/test/ruby/test_yjit.rb +++ b/test/ruby/test_yjit.rb @@ -1277,6 +1277,18 @@ class TestYJIT < Test::Unit::TestCase RUBY end + def test_io_reopen_clobbering_singleton_class + assert_compiles(<<~RUBY, result: [:ok, :ok]) + def $stderr.to_i = :i + + def test = $stderr.to_i + + [test, test] + $stderr.reopen($stderr.dup) + [test, test].map { :ok unless _1 == :i } + RUBY + end + private def code_gc_helpers diff --git a/yjit/bindgen/src/main.rs b/yjit/bindgen/src/main.rs index 4fb02cfdf0..b9bef8e176 100644 --- a/yjit/bindgen/src/main.rs +++ b/yjit/bindgen/src/main.rs @@ -165,6 +165,7 @@ fn main() { .allowlist_var("rb_cTrueClass") .allowlist_var("rb_cFalseClass") .allowlist_var("rb_cInteger") + .allowlist_var("rb_cIO") .allowlist_var("rb_cSymbol") .allowlist_var("rb_cFloat") .allowlist_var("rb_cString") diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index b64bdc0258..18d53fe65e 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -3871,6 +3871,7 @@ fn jit_guard_known_klass( } else if unsafe { FL_TEST(known_klass, VALUE(RUBY_FL_SINGLETON as usize)) != VALUE(0) && sample_instance == rb_class_attached_object(known_klass) + && !rb_obj_is_kind_of(sample_instance, rb_cIO).test() } { // Singleton classes are attached to one specific object, so we can // avoid one memory access (and potentially the is_heap check) by @@ -3882,6 +3883,8 @@ fn jit_guard_known_klass( // that its singleton class is empty, so we can't avoid the memory // access. As an example, `Object.new.singleton_class` is an object in // this situation. + // Also, guarding by identity is incorrect for IO objects because + // IO#reopen can be used to change the class and singleton class of IO objects! asm.comment("guard known object with singleton class"); asm.cmp(obj_opnd, sample_instance.into()); jit_chain_guard(JCC_JNE, jit, asm, ocb, max_chain_depth, counter); diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index 8415b444ba..1f549161ca 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -1083,6 +1083,7 @@ extern "C" { pub static mut rb_cFalseClass: VALUE; pub static mut rb_cFloat: VALUE; pub static mut rb_cHash: VALUE; + pub static mut rb_cIO: VALUE; pub static mut rb_cInteger: VALUE; pub static mut rb_cModule: VALUE; pub static mut rb_cNilClass: VALUE;