merge revision(s) d19d683a354530a27b4cbb049223f8dc70c75849,de1a586ecc2ee7f465f0c0a69291054136a3a819: [Backport #20250] (#10308)

rb_obj_setup: do not copy RUBY_FL_SEEN_OBJ_ID

	[Bug #20250]

	We're seting up a new instance, so it never had an associated
	object_id.

	proc.c: get rid of `CLONESETUP`
	MIME-Version: 1.0
	Content-Type: text/plain; charset=UTF-8
	Content-Transfer-Encoding: 8bit

	[Bug #20253]

	All the way down to Ruby 1.9, `Proc`, `Method`, `UnboundMethod`
	and `Binding` always had their own specific clone and dup routine.

	This caused various discrepancies with how other objects behave
	on `dup` and `clone. [Bug #20250], [Bug #20253].

	This commit get rid of `CLONESETUP` and use the the same codepath
	as all other types, so ensure consistency.

	NB: It's still not accepting the `freeze` keyword argument on `clone`.

	Co-Authored-By: Étienne Barrié <etienne.barrie@gmail.com>

internal/object.h

@@ -16,6 +16,8 @@ VALUE rb_class_search_ancestor(VALUE klass, VALUE super);
 NORETURN(void rb_undefined_alloc(VALUE klass));
 double rb_num_to_dbl(VALUE val);
 VALUE rb_obj_dig(int argc, VALUE *argv, VALUE self, VALUE notfound);
+VALUE rb_obj_clone_setup(VALUE obj, VALUE clone, VALUE kwfreeze);
+VALUE rb_obj_dup_setup(VALUE obj, VALUE dup);
 VALUE rb_immutable_obj_clone(int, VALUE *, VALUE);
 VALUE rb_check_convert_type_with_id(VALUE,int,const char*,ID);
 int rb_bool_expected(VALUE, const char *, int raise);

object.c

@@ -119,7 +119,7 @@ rb_obj_reveal(VALUE obj, VALUE klass)
 VALUE
 rb_obj_setup(VALUE obj, VALUE klass, VALUE type)
 {
-    VALUE ignored_flags = RUBY_FL_PROMOTED;
+    VALUE ignored_flags = RUBY_FL_PROMOTED | RUBY_FL_SEEN_OBJ_ID;
     RBASIC(obj)->flags = (type & ~ignored_flags) | (RBASIC(obj)->flags & ignored_flags);
     RBASIC_SET_CLASS(obj, klass);
     return obj;
@@ -453,15 +453,12 @@ immutable_obj_clone(VALUE obj, VALUE kwfreeze)
     return obj;
 }
 
-static VALUE
-mutable_obj_clone(VALUE obj, VALUE kwfreeze)
+VALUE
+rb_obj_clone_setup(VALUE obj, VALUE clone, VALUE kwfreeze)
 {
-    VALUE clone, singleton;
     VALUE argv[2];
 
-    clone = rb_obj_alloc(rb_obj_class(obj));
-
-    singleton = rb_singleton_class_clone_and_attach(obj, clone);
+    VALUE singleton = rb_singleton_class_clone_and_attach(obj, clone);
     RBASIC_SET_CLASS(clone, singleton);
     if (FL_TEST(singleton, FL_SINGLETON)) {
         rb_singleton_class_attached(singleton, clone);
@@ -528,6 +525,13 @@ mutable_obj_clone(VALUE obj, VALUE kwfreeze)
     return clone;
 }
 
+static VALUE
+mutable_obj_clone(VALUE obj, VALUE kwfreeze)
+{
+    VALUE clone = rb_obj_alloc(rb_obj_class(obj));
+    return rb_obj_clone_setup(obj, clone, kwfreeze);
+}
+
 VALUE
 rb_obj_clone(VALUE obj)
 {
@@ -535,6 +539,15 @@ rb_obj_clone(VALUE obj)
     return mutable_obj_clone(obj, Qnil);
 }
 
+VALUE
+rb_obj_dup_setup(VALUE obj, VALUE dup)
+{
+    init_copy(dup, obj);
+    rb_funcall(dup, id_init_dup, 1, obj);
+
+    return dup;
+}
+
 /*
  *  call-seq:
  *     obj.dup -> an_object
@@ -583,10 +596,7 @@ rb_obj_dup(VALUE obj)
         return obj;
     }
     dup = rb_obj_alloc(rb_obj_class(obj));
-    init_copy(dup, obj);
-    rb_funcall(dup, id_init_dup, 1, obj);
-
-    return dup;
+    return rb_obj_dup_setup(obj, dup);
 }
 
 /*

proc.c

@@ -51,23 +51,6 @@ static VALUE proc_binding(VALUE self);
 
 #define IS_METHOD_PROC_IFUNC(ifunc) ((ifunc)->func == bmcall)
 
-/* :FIXME: The way procs are cloned has been historically different from the
- * way everything else are.  @shyouhei is not sure for the intention though.
- */
-#undef CLONESETUP
-static inline void
-CLONESETUP(VALUE clone, VALUE obj)
-{
-    RBIMPL_ASSERT_OR_ASSUME(! RB_SPECIAL_CONST_P(obj));
-    RBIMPL_ASSERT_OR_ASSUME(! RB_SPECIAL_CONST_P(clone));
-
-    const VALUE flags = RUBY_FL_PROMOTED | RUBY_FL_FINALIZE;
-    rb_obj_setup(clone, rb_singleton_class_clone(obj),
-                 RB_FL_TEST_RAW(obj, ~flags));
-    rb_singleton_class_attached(RBASIC_CLASS(clone), clone);
-    if (RB_FL_TEST(obj, RUBY_FL_EXIVAR)) rb_copy_generic_ivar(clone, obj);
-}
-
 static void
 block_mark_and_move(struct rb_block *block)
 {
@@ -142,9 +125,7 @@ static VALUE
 proc_clone(VALUE self)
 {
     VALUE procval = rb_proc_dup(self);
-    CLONESETUP(procval, self);
-    rb_check_funcall(procval, idInitialize_clone, 1, &self);
-    return procval;
+    return rb_obj_clone_setup(self, procval, Qnil);
 }
 
 /* :nodoc: */
@@ -152,8 +133,7 @@ static VALUE
 proc_dup(VALUE self)
 {
     VALUE procval = rb_proc_dup(self);
-    rb_check_funcall(procval, idInitialize_dup, 1, &self);
-    return procval;
+    return rb_obj_dup_setup(self, procval);
 }
 
 /*
@@ -328,7 +308,7 @@ binding_dup(VALUE self)
     rb_vm_block_copy(bindval, &dst->block, &src->block);
     RB_OBJ_WRITE(bindval, &dst->pathobj, src->pathobj);
     dst->first_lineno = src->first_lineno;
-    return bindval;
+    return rb_obj_dup_setup(self, bindval);
 }
 
 /* :nodoc: */
@@ -336,8 +316,7 @@ static VALUE
 binding_clone(VALUE self)
 {
     VALUE bindval = binding_dup(self);
-    CLONESETUP(bindval, self);
-    return bindval;
+    return rb_obj_clone_setup(self, bindval, Qnil);
 }
 
 VALUE
@@ -2385,7 +2364,25 @@ method_clone(VALUE self)
 
     TypedData_Get_Struct(self, struct METHOD, &method_data_type, orig);
     clone = TypedData_Make_Struct(CLASS_OF(self), struct METHOD, &method_data_type, data);
-    CLONESETUP(clone, self);
+    rb_obj_clone_setup(self, clone, Qnil);
+    RB_OBJ_WRITE(clone, &data->recv, orig->recv);
+    RB_OBJ_WRITE(clone, &data->klass, orig->klass);
+    RB_OBJ_WRITE(clone, &data->iclass, orig->iclass);
+    RB_OBJ_WRITE(clone, &data->owner, orig->owner);
+    RB_OBJ_WRITE(clone, &data->me, rb_method_entry_clone(orig->me));
+    return clone;
+}
+
+/* :nodoc: */
+static VALUE
+method_dup(VALUE self)
+{
+    VALUE clone;
+    struct METHOD *orig, *data;
+
+    TypedData_Get_Struct(self, struct METHOD, &method_data_type, orig);
+    clone = TypedData_Make_Struct(CLASS_OF(self), struct METHOD, &method_data_type, data);
+    rb_obj_dup_setup(self, clone);
     RB_OBJ_WRITE(clone, &data->recv, orig->recv);
     RB_OBJ_WRITE(clone, &data->klass, orig->klass);
     RB_OBJ_WRITE(clone, &data->iclass, orig->iclass);
@@ -4295,6 +4292,7 @@ Init_Proc(void)
     rb_define_method(rb_cMethod, "eql?", method_eq, 1);
     rb_define_method(rb_cMethod, "hash", method_hash, 0);
     rb_define_method(rb_cMethod, "clone", method_clone, 0);
+    rb_define_method(rb_cMethod, "dup", method_dup, 0);
     rb_define_method(rb_cMethod, "call", rb_method_call_pass_called_kw, -1);
     rb_define_method(rb_cMethod, "===", rb_method_call_pass_called_kw, -1);
     rb_define_method(rb_cMethod, "curry", rb_method_curry, -1);
@@ -4325,6 +4323,7 @@ Init_Proc(void)
     rb_define_method(rb_cUnboundMethod, "eql?", unbound_method_eq, 1);
     rb_define_method(rb_cUnboundMethod, "hash", method_hash, 0);
     rb_define_method(rb_cUnboundMethod, "clone", method_clone, 0);
+    rb_define_method(rb_cUnboundMethod, "dup", method_dup, 0);
     rb_define_method(rb_cUnboundMethod, "arity", method_arity_m, 0);
     rb_define_method(rb_cUnboundMethod, "inspect", method_inspect, 0);
     rb_define_method(rb_cUnboundMethod, "to_s", method_inspect, 0);

spec/ruby/core/binding/clone_spec.rb

@@ -4,4 +4,10 @@ require_relative 'shared/clone'
 
 describe "Binding#clone" do
   it_behaves_like :binding_clone, :clone
+
+  it "preserves frozen status" do
+    bind = binding.freeze
+    bind.frozen?.should == true
+    bind.clone.frozen?.should == true
+  end
 end

spec/ruby/core/binding/dup_spec.rb

@@ -4,4 +4,10 @@ require_relative 'shared/clone'
 
 describe "Binding#dup" do
   it_behaves_like :binding_clone, :dup
+
+  it "resets frozen status" do
+    bind = binding.freeze
+    bind.frozen?.should == true
+    bind.dup.frozen?.should == false
+  end
 end

spec/ruby/core/binding/shared/clone.rb

@@ -31,4 +31,26 @@ describe :binding_clone, shared: true do
       b2.local_variable_defined?(:x).should == false
     end
   end
+
+  ruby_version_is "3.4" do
+    it "copies instance variables" do
+      @b1.instance_variable_set(:@ivar, 1)
+      cl = @b1.send(@method)
+      cl.instance_variables.should == [:@ivar]
+    end
+
+    it "copies the finalizer" do
+      code = <<-RUBY
+        obj = binding
+
+        ObjectSpace.define_finalizer(obj, Proc.new { STDOUT.write "finalized\n" })
+
+        obj.clone
+
+        exit 0
+      RUBY
+
+      ruby_exe(code).lines.sort.should == ["finalized\n", "finalized\n"]
+    end
+  end
 end

spec/ruby/core/method/clone_spec.rb

@@ -1,14 +1,13 @@
 require_relative '../../spec_helper'
-require_relative 'fixtures/classes'
+require_relative 'shared/dup'
 
 describe "Method#clone" do
-  it "returns a copy of the method" do
-    m1 = MethodSpecs::Methods.new.method(:foo)
-    m2 = m1.clone
+  it_behaves_like :method_dup, :clone
 
-    m1.should == m2
-    m1.should_not equal(m2)
-
-    m1.call.should == m2.call
+  it "preserves frozen status" do
+    method = Object.new.method(:method)
+    method.freeze
+    method.frozen?.should == true
+    method.clone.frozen?.should == true
   end
 end

spec/ruby/core/method/dup_spec.rb

@@ -0,0 +1,15 @@
+require_relative '../../spec_helper'
+require_relative 'shared/dup'
+
+describe "Method#dup" do
+  ruby_version_is "3.4" do
+    it_behaves_like :method_dup, :dup
+
+    it "resets frozen status" do
+      method = Object.new.method(:method)
+      method.freeze
+      method.frozen?.should == true
+      method.dup.frozen?.should == false
+    end
+  end
+end

spec/ruby/core/method/shared/dup.rb

@@ -0,0 +1,32 @@
+describe :method_dup, shared: true do
+  it "returns a copy of self" do
+    a = Object.new.method(:method)
+    b = a.send(@method)
+
+    a.should == b
+    a.should_not equal(b)
+  end
+
+  ruby_version_is "3.4" do
+    it "copies instance variables" do
+      method = Object.new.method(:method)
+      method.instance_variable_set(:@ivar, 1)
+      cl = method.send(@method)
+      cl.instance_variables.should == [:@ivar]
+    end
+
+    it "copies the finalizer" do
+      code = <<-RUBY
+        obj = Object.new.method(:method)
+
+        ObjectSpace.define_finalizer(obj, Proc.new { STDOUT.write "finalized\n" })
+
+        obj.clone
+
+        exit 0
+      RUBY
+
+      ruby_exe(code).lines.sort.should == ["finalized\n", "finalized\n"]
+    end
+  end
+end

spec/ruby/core/proc/clone_spec.rb

@@ -3,4 +3,13 @@ require_relative 'shared/dup'
 
 describe "Proc#clone" do
   it_behaves_like :proc_dup, :clone
+
+  ruby_bug "cloning a frozen proc is broken on Ruby 3.3", "3.3"..."3.4" do
+    it "preserves frozen status" do
+      proc = Proc.new { }
+      proc.freeze
+      proc.frozen?.should == true
+      proc.clone.frozen?.should == true
+    end
+  end
 end

spec/ruby/core/proc/dup_spec.rb

@@ -3,4 +3,11 @@ require_relative 'shared/dup'
 
 describe "Proc#dup" do
   it_behaves_like :proc_dup, :dup
+
+  it "resets frozen status" do
+    proc = Proc.new { }
+    proc.freeze
+    proc.frozen?.should == true
+    proc.dup.frozen?.should == false
+  end
 end

spec/ruby/core/proc/shared/dup.rb

@@ -15,4 +15,27 @@ describe :proc_dup, shared: true do
       cl.new{}.send(@method).class.should == cl
     end
   end
+
+  ruby_version_is "3.4" do
+    it "copies instance variables" do
+      proc = -> { "hello" }
+      proc.instance_variable_set(:@ivar, 1)
+      cl = proc.send(@method)
+      cl.instance_variables.should == [:@ivar]
+    end
+
+    it "copies the finalizer" do
+      code = <<-RUBY
+        obj = Proc.new { }
+
+        ObjectSpace.define_finalizer(obj, Proc.new { STDOUT.write "finalized\n" })
+
+        obj.clone
+
+        exit 0
+      RUBY
+
+      ruby_exe(code).lines.sort.should == ["finalized\n", "finalized\n"]
+    end
+  end
 end

spec/ruby/core/unboundmethod/clone_spec.rb

@@ -1,12 +1,13 @@
 require_relative '../../spec_helper'
-require_relative 'fixtures/classes'
+require_relative 'shared/dup'
 
 describe "UnboundMethod#clone" do
-  it "returns a copy of the UnboundMethod" do
-    um1 = UnboundMethodSpecs::Methods.instance_method(:foo)
-    um2 = um1.clone
+  it_behaves_like :unboundmethod_dup, :clone
 
-    (um1 == um2).should == true
-    um1.bind(UnboundMethodSpecs::Methods.new).call.should == um2.bind(UnboundMethodSpecs::Methods.new).call
+  it "preserves frozen status" do
+    method = Class.instance_method(:instance_method)
+    method.freeze
+    method.frozen?.should == true
+    method.clone.frozen?.should == true
   end
 end

spec/ruby/core/unboundmethod/dup_spec.rb

@@ -0,0 +1,15 @@
+require_relative '../../spec_helper'
+require_relative 'shared/dup'
+
+describe "UnboundMethod#dup" do
+  ruby_version_is "3.4" do
+    it_behaves_like :unboundmethod_dup, :dup
+
+    it "resets frozen status" do
+      method = Class.instance_method(:instance_method)
+      method.freeze
+      method.frozen?.should == true
+      method.dup.frozen?.should == false
+    end
+  end
+end

spec/ruby/core/unboundmethod/shared/dup.rb

@@ -0,0 +1,32 @@
+describe :unboundmethod_dup, shared: true do
+  it "returns a copy of self" do
+    a = Class.instance_method(:instance_method)
+    b = a.send(@method)
+
+    a.should == b
+    a.should_not equal(b)
+  end
+
+  ruby_version_is "3.4" do
+    it "copies instance variables" do
+      method = Class.instance_method(:instance_method)
+      method.instance_variable_set(:@ivar, 1)
+      cl = method.send(@method)
+      cl.instance_variables.should == [:@ivar]
+    end
+
+    it "copies the finalizer" do
+      code = <<-RUBY
+        obj = Class.instance_method(:instance_method)
+
+        ObjectSpace.define_finalizer(obj, Proc.new { STDOUT.write "finalized\n" })
+
+        obj.clone
+
+        exit 0
+      RUBY
+
+      ruby_exe(code).lines.sort.should == ["finalized\n", "finalized\n"]
+    end
+  end
+end

test/ruby/test_clone.rb

@@ -73,6 +73,13 @@ class TestClone < Test::Unit::TestCase
     assert_equal(cloned_obj.instance_variable_get(:@a), 1)
   end
 
+  def test_proc_obj_id_flag_reset
+    # [Bug #20250]
+    proc = Proc.new { }
+    proc.object_id
+    proc.clone.object_id # Would crash with RUBY_DEBUG=1
+  end
+
   def test_user_flags
     assert_separately([], <<-EOS)
       #