From 2210fd880b83f6e686e7c96310e478f05f2aee5f Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 9 Aug 2023 13:32:15 -0700 Subject: [PATCH] ruby: Fix object cache lookups on 32-bit platforms https://github.com/protocolbuffers/protobuf/pull/13204 refactored the Ruby object cache to use a key of `LL2NUM(key_val)` instead of `LL2NUM(key_val >> 2)`. On 32-bit systems, `LL2NUM(key_val)` returns inconsistent results because a large value has to be stored as a Bignum on the heap. This causes cache lookups to fail. This commit restores the previous behavior of using `ObjectCache_GetKey`, which discards the lower 2 bits, which are zero. This enables a key to be stored as a Fixnum on both 32 and 64-bit platforms. As https://patshaughnessy.net/2014/1/9/how-big-is-a-bignum describes, a Fixnum uses: * 1 bit for the `FIXNUM_FLAG`. * 1 bit for the sign bit. Therefore the largest possible Fixnum value on a 64-bit value is 4611686018427387903 (2^62 - 1). On a 32-bit system, the largest value is 1073741823 (2^30 - 1). For example, a possible VALUE pointer address on a 32-bit system: 0xff5b4af8 => 4284173048 Dropping the lower 2 bits makes up for the loss of range to these flags. In the example above, we see that shifting by 2 turns the value into a 30-bit number, which can be represented as a Fixnum: (0xff5b4af8 >> 2) => 1071043262 This bug can also manifest on a 64-bit system if the upper bits are 0xff. Closes #13481 --- ruby/ext/google/protobuf_c/protobuf.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/ruby/ext/google/protobuf_c/protobuf.c b/ruby/ext/google/protobuf_c/protobuf.c index c9354db4e340c..39062cfca65a6 100644 --- a/ruby/ext/google/protobuf_c/protobuf.c +++ b/ruby/ext/google/protobuf_c/protobuf.c @@ -276,17 +276,25 @@ static void ObjectCache_Init(VALUE protobuf) { rb_const_set(protobuf, rb_intern("SIZEOF_VALUE"), INT2NUM(SIZEOF_VALUE)); } -VALUE ObjectCache_TryAdd(const void *key, VALUE val) { +static VALUE ObjectCache_GetKey(const void *key) { VALUE key_val = (VALUE)key; PBRUBY_ASSERT((key_val & 3) == 0); - return rb_funcall(weak_obj_cache, item_try_add, 2, LL2NUM(key_val), val); + // Ensure the key can be stored as a Fixnum since 1 bit is needed for + // FIXNUM_FLAG and 1 bit is needed for the sign bit. + VALUE new_key = LL2NUM(key_val >> 2); + PBRUBY_ASSERT(FIXNUM_P(new_key)); + return new_key; +} + +VALUE ObjectCache_TryAdd(const void *key, VALUE val) { + VALUE key_val = ObjectCache_GetKey(key); + return rb_funcall(weak_obj_cache, item_try_add, 2, key_val, val); } // Returns the cached object for this key, if any. Otherwise returns Qnil. VALUE ObjectCache_Get(const void *key) { - VALUE key_val = (VALUE)key; - PBRUBY_ASSERT((key_val & 3) == 0); - return rb_funcall(weak_obj_cache, item_get, 1, LL2NUM(key_val)); + VALUE key_val = ObjectCache_GetKey(key); + return rb_funcall(weak_obj_cache, item_get, 1, key_val); } /*