Skip to content

Commit

Permalink
ruby: Fix object cache lookups on 32-bit platforms
Browse files Browse the repository at this point in the history
protocolbuffers#13204 refactored the
Ruby object cache to use a key of `LL2NUM(key_val)` instead of
`LL2NUM(key_val >> 2)`. On 32-bit systems, it appears that
`LL2NUM(key_val)` returns inconsistent results, possibly due to
overflow. 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.

Closes protocolbuffers#13481
  • Loading branch information
stanhu committed Aug 9, 2023
1 parent d99134f commit 84070be
Showing 1 changed file with 10 additions and 5 deletions.
15 changes: 10 additions & 5 deletions ruby/ext/google/protobuf_c/protobuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -276,17 +276,22 @@ 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);
// Avoid overflow on 32-bit systems by discarding the bottom zeros
return LL2NUM(key_val >> 2);
}

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

/*
Expand Down

0 comments on commit 84070be

Please sign in to comment.