Fix cext support for google-protobuf

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:
 

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}"

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)

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);

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

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
 

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));

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 {
 

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