From edec690e0331b5aa851b3a7c9872b2bf92f3cb21 Mon Sep 17 00:00:00 2001
From: Peter Zhu <peter@peterzhu.ca>
Date: Thu, 11 Apr 2024 14:30:30 -0400
Subject: [PATCH] Refactor how object IDs work for special consts

We don't need to treat static symbols in any special way since they
can't be confused with other special consts or GC managed objects.
---
 gc.c                          | 42 +++++++++++++++++------------------
 test/ruby/test_objectspace.rb |  5 ++++-
 2 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/gc.c b/gc.c
index df3a3b8a77..f67a5d8326 100644
--- a/gc.c
+++ b/gc.c
@@ -3460,8 +3460,8 @@ obj_free(rb_objspace_t *objspace, VALUE obj)
 }
 
 
-#define OBJ_ID_INCREMENT (sizeof(RVALUE) / 2)
-#define OBJ_ID_INITIAL (OBJ_ID_INCREMENT * 2)
+#define OBJ_ID_INCREMENT (sizeof(RVALUE))
+#define OBJ_ID_INITIAL (OBJ_ID_INCREMENT)
 
 static int
 object_id_cmp(st_data_t x, st_data_t y)
@@ -4468,25 +4468,28 @@ id2ref(VALUE objid)
 #define NUM2PTR(x) NUM2ULL(x)
 #endif
     rb_objspace_t *objspace = &rb_objspace;
-    VALUE ptr;
-    void *p0;
 
     objid = rb_to_int(objid);
     if (FIXNUM_P(objid) || rb_big_size(objid) <= SIZEOF_VOIDP) {
-        ptr = NUM2PTR(objid);
-        if (ptr == Qtrue) return Qtrue;
-        if (ptr == Qfalse) return Qfalse;
-        if (NIL_P(ptr)) return Qnil;
-        if (FIXNUM_P(ptr)) return ptr;
-        if (FLONUM_P(ptr)) return ptr;
+        VALUE ptr = NUM2PTR(objid);
+        if (SPECIAL_CONST_P(ptr)) {
+            if (ptr == Qtrue) return Qtrue;
+            if (ptr == Qfalse) return Qfalse;
+            if (NIL_P(ptr)) return Qnil;
+            if (FIXNUM_P(ptr)) return ptr;
+            if (FLONUM_P(ptr)) return ptr;
 
-        ptr = obj_id_to_ref(objid);
-        if ((ptr % sizeof(RVALUE)) == (4 << 2)) {
-            ID symid = ptr / sizeof(RVALUE);
-            p0 = (void *)ptr;
-            if (!rb_static_id_valid_p(symid))
-                rb_raise(rb_eRangeError, "%p is not symbol id value", p0);
-            return ID2SYM(symid);
+            if (SYMBOL_P(ptr)) {
+                // Check that the symbol is valid
+                if (rb_static_id_valid_p(SYM2ID(ptr))) {
+                    return ptr;
+                }
+                else {
+                    rb_raise(rb_eRangeError, "%p is not symbol id value", (void *)ptr);
+                }
+            }
+
+            rb_raise(rb_eRangeError, "%+"PRIsVALUE" is not id value", rb_int2str(objid, 10));
         }
     }
 
@@ -4519,10 +4522,7 @@ os_id2ref(VALUE os, VALUE objid)
 static VALUE
 rb_find_object_id(VALUE obj, VALUE (*get_heap_object_id)(VALUE))
 {
-    if (STATIC_SYM_P(obj)) {
-        return (SYM2ID(obj) * sizeof(RVALUE) + (4 << 2)) | FIXNUM_FLAG;
-    }
-    else if (FLONUM_P(obj)) {
+    if (FLONUM_P(obj)) {
 #if SIZEOF_LONG == SIZEOF_VOIDP
         return LONG2NUM((SIGNED_VALUE)obj);
 #else
diff --git a/test/ruby/test_objectspace.rb b/test/ruby/test_objectspace.rb
index a7cfb064a8..1c97bd517e 100644
--- a/test/ruby/test_objectspace.rb
+++ b/test/ruby/test_objectspace.rb
@@ -66,8 +66,11 @@ End
   end
 
   def test_id2ref_invalid_symbol_id
+    # RB_STATIC_SYM_P checks for static symbols by checking that the bottom
+    # 8 bits of the object is equal to RUBY_SYMBOL_FLAG, so we need to make
+    # sure that the bottom 8 bits remain unchanged.
     msg = /is not symbol id value/
-    assert_raise_with_message(RangeError, msg) { ObjectSpace._id2ref(:a.object_id + GC::INTERNAL_CONSTANTS[:RVALUE_SIZE]) }
+    assert_raise_with_message(RangeError, msg) { ObjectSpace._id2ref(:a.object_id + 256) }
   end
 
   def test_count_objects