diff --git a/CHANGELOG.md b/CHANGELOG.md index 15aff6ebce4..03c7a467c90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Bug fixes: * Fix `Range#cover?` on begin-less ranges and non-integer values (@nirvdrum, @rwstauner). * Fix `Time.new` with String argument and handle nanoseconds correctly (#3836, @andrykonchin). +* Fix a possible case of infinite recursion when implementing `frozen?` in a native extension (@nirvdrum). Compatibility: @@ -25,6 +26,7 @@ Compatibility: * Fix `rb_str_locktmp()` and `rb_str_unlocktmp()` to raise `FrozenError` when string argument is frozen (#3752, @andrykonchin). * Fix Struct setters to raise `FrozenError` when a struct is frozen (#3850, @andrykonchin). * Fix `Struct#initialize` when mixed positional and keyword arguments (#3855, @andrykonchin). +* Implement `rb_error_frozen_object` for the google-protobuf gem (@nirvdrum). Performance: diff --git a/lib/truffle/truffle/cext.rb b/lib/truffle/truffle/cext.rb index aeb84e4613f..edbad62e0af 100644 --- a/lib/truffle/truffle/cext.rb +++ b/lib/truffle/truffle/cext.rb @@ -477,7 +477,7 @@ def rb_obj_freeze(obj) end def rb_obj_frozen_p(object) - object.frozen? + Primitive.frozen?(object) end def rb_obj_id(object) @@ -2374,6 +2374,10 @@ def rb_eval_cmd_kw(cmd, args, kw_splat) end end + def rb_error_frozen_object(object) + raise FrozenError.new("can't modify frozen #{Primitive.class(object)}", receiver: object) + end + def rb_tr_warn(message) location = caller_locations(1, 1)[0] message_with_prefix = "#{location.label}: warning: #{message}" diff --git a/spec/ruby/optional/capi/exception_spec.rb b/spec/ruby/optional/capi/exception_spec.rb index 5bb60608b22..8ce6cde7fe6 100644 --- a/spec/ruby/optional/capi/exception_spec.rb +++ b/spec/ruby/optional/capi/exception_spec.rb @@ -100,6 +100,15 @@ end end + describe "rb_error_frozen_object" do + it "raises a FrozenError regardless of the object's frozen state" do + # The type of the argument we supply doesn't matter. The choice here is arbitrary and we only change the type + # of the argument to ensure the exception messages are set correctly. + -> { @s.rb_error_frozen_object(Hash.new) }.should raise_error(FrozenError, "can't modify frozen Hash") + -> { @s.rb_error_frozen_object(Array.new.freeze) }.should raise_error(FrozenError, "can't modify frozen Array") + end + end + describe "rb_syserr_new" do it "returns system error with default message when passed message is NULL" do exception = @s.rb_syserr_new(Errno::ENOENT::Errno, nil) diff --git a/spec/ruby/optional/capi/ext/exception_spec.c b/spec/ruby/optional/capi/ext/exception_spec.c index 0e8347ab0d6..e0d96b55e9c 100644 --- a/spec/ruby/optional/capi/ext/exception_spec.c +++ b/spec/ruby/optional/capi/ext/exception_spec.c @@ -36,6 +36,11 @@ VALUE exception_spec_rb_set_errinfo(VALUE self, VALUE exc) { return Qnil; } +VALUE exception_spec_rb_error_frozen_object(VALUE self, VALUE object) { + rb_error_frozen_object(object); + return Qnil; +} + VALUE exception_spec_rb_syserr_new(VALUE self, VALUE num, VALUE msg) { int n = NUM2INT(num); char *cstr = NULL; @@ -66,6 +71,7 @@ void Init_exception_spec(void) { rb_define_method(cls, "rb_exc_new3", exception_spec_rb_exc_new3, 1); rb_define_method(cls, "rb_exc_raise", exception_spec_rb_exc_raise, 1); rb_define_method(cls, "rb_set_errinfo", exception_spec_rb_set_errinfo, 1); + rb_define_method(cls, "rb_error_frozen_object", exception_spec_rb_error_frozen_object, 1); rb_define_method(cls, "rb_syserr_new", exception_spec_rb_syserr_new, 2); rb_define_method(cls, "rb_syserr_new_str", exception_spec_rb_syserr_new_str, 2); rb_define_method(cls, "rb_make_exception", exception_spec_rb_make_exception, 1); diff --git a/spec/ruby/optional/capi/ext/object_spec.c b/spec/ruby/optional/capi/ext/object_spec.c index eab0eb75342..995bc38fcf8 100644 --- a/spec/ruby/optional/capi/ext/object_spec.c +++ b/spec/ruby/optional/capi/ext/object_spec.c @@ -383,6 +383,16 @@ static VALUE object_spec_custom_alloc_func_p(VALUE self, VALUE klass) { return allocator ? Qtrue : Qfalse; } +static VALUE object_spec_redefine_frozen(VALUE self) { + // The purpose of this spec is to verify that `frozen?` + // and `RB_OBJ_FROZEN` do not mutually recurse infinitely. + if (RB_OBJ_FROZEN(self)) { + return Qtrue; + } + + return Qfalse; +} + void Init_object_spec(void) { VALUE cls = rb_define_class("CApiObjectSpecs", rb_cObject); rb_define_method(cls, "FL_ABLE", object_spec_FL_ABLE, 1); @@ -455,6 +465,9 @@ void Init_object_spec(void) { rb_define_method(cls, "custom_alloc_func?", object_spec_custom_alloc_func_p, 1); rb_define_method(cls, "not_implemented_method", rb_f_notimplement, -1); rb_define_method(cls, "rb_ivar_foreach", object_spec_rb_ivar_foreach, 1); + + cls = rb_define_class("CApiObjectRedefinitionSpecs", rb_cObject); + rb_define_method(cls, "frozen?", object_spec_redefine_frozen, 0); } #ifdef __cplusplus diff --git a/spec/ruby/optional/capi/object_spec.rb b/spec/ruby/optional/capi/object_spec.rb index 27faecbb49f..8b4d8a9bba0 100644 --- a/spec/ruby/optional/capi/object_spec.rb +++ b/spec/ruby/optional/capi/object_spec.rb @@ -691,6 +691,16 @@ def reach end end + describe "redefining frozen? works" do + it "allows an object to override frozen?" do + obj = CApiObjectRedefinitionSpecs.new + + obj.frozen?.should == false + obj.freeze + obj.frozen?.should == true + end + end + describe "rb_obj_taint" do end diff --git a/src/main/c/cext/exception.c b/src/main/c/cext/exception.c index 0d8a0705165..6c5352cc891 100644 --- a/src/main/c/cext/exception.c +++ b/src/main/c/cext/exception.c @@ -61,6 +61,11 @@ VALUE rb_errinfo(void) { return RUBY_CEXT_INVOKE("rb_errinfo"); } +void rb_error_frozen_object(VALUE frozen_obj) { + RUBY_CEXT_INVOKE_NO_WRAP("rb_error_frozen_object", frozen_obj); + UNREACHABLE; +} + void rb_syserr_fail(int eno, const char *message) { VALUE messageValue = (message == NULL) ? Qnil : rb_str_new_cstr(message); polyglot_invoke(RUBY_CEXT, "rb_syserr_fail", eno, rb_tr_unwrap(messageValue)); diff --git a/src/main/java/org/truffleruby/core/support/TypeNodes.java b/src/main/java/org/truffleruby/core/support/TypeNodes.java index 7f987e73168..44653a574cd 100644 --- a/src/main/java/org/truffleruby/core/support/TypeNodes.java +++ b/src/main/java/org/truffleruby/core/support/TypeNodes.java @@ -161,6 +161,15 @@ Object freeze(Object self, } } + @Primitive(name = "frozen?") + public abstract static class IsFrozenPrimitive extends PrimitiveArrayArgumentsNode { + @Specialization + boolean isFrozen(Object self, + @Cached IsFrozenNode isFrozenNode) { + return isFrozenNode.execute(self); + } + } + @Primitive(name = "immediate_value?") public abstract static class IsImmediateValueNode extends PrimitiveArrayArgumentsNode { diff --git a/tool/generate-cext-trampoline.rb b/tool/generate-cext-trampoline.rb index 4923a680be3..0bb7295027d 100755 --- a/tool/generate-cext-trampoline.rb +++ b/tool/generate-cext-trampoline.rb @@ -13,6 +13,7 @@ NO_RETURN_FUNCTIONS = %w[ ruby_malloc_size_overflow rb_error_arity + rb_error_frozen_object rb_iter_break rb_iter_break_value rb_f_notimplement