From 3e92117c22f5d2d53e60b073e397d80583afc545 Mon Sep 17 00:00:00 2001 From: Daniel Azuma Date: Wed, 5 May 2021 05:23:20 +0000 Subject: [PATCH 1/5] fix(ruby): Fix crash when calculating Message hash values on 64-bit Windows --- ruby/ext/google/protobuf_c/message.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ruby/ext/google/protobuf_c/message.c b/ruby/ext/google/protobuf_c/message.c index 1eb0cb76dc84..a4e7d441ea23 100644 --- a/ruby/ext/google/protobuf_c/message.c +++ b/ruby/ext/google/protobuf_c/message.c @@ -734,7 +734,12 @@ uint64_t Message_Hash(const upb_msg* msg, const upb_msgdef* m, uint64_t seed) { */ static VALUE Message_hash(VALUE _self) { Message* self = ruby_to_Message(_self); - return INT2FIX(Message_Hash(self->msg, self->msgdef, 0)); + uint64_t hash_value = Message_Hash(self->msg, self->msgdef, 0); + long long_value = (long)hash_value; + if (!RB_FIXABLE(long_value)) { + long_value = long_value >> 1; + } + return INT2FIX(long_value); } /* From 9f728242335173d56c6b5b84a4ef2a7f6ff69b3a Mon Sep 17 00:00:00 2001 From: Daniel Azuma Date: Wed, 5 May 2021 19:22:53 +0000 Subject: [PATCH 2/5] Better mapping for values outside the fixnum range --- ruby/ext/google/protobuf_c/message.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ruby/ext/google/protobuf_c/message.c b/ruby/ext/google/protobuf_c/message.c index a4e7d441ea23..567af515d4bc 100644 --- a/ruby/ext/google/protobuf_c/message.c +++ b/ruby/ext/google/protobuf_c/message.c @@ -735,9 +735,13 @@ uint64_t Message_Hash(const upb_msg* msg, const upb_msgdef* m, uint64_t seed) { static VALUE Message_hash(VALUE _self) { Message* self = ruby_to_Message(_self); uint64_t hash_value = Message_Hash(self->msg, self->msgdef, 0); + // Fixnum is restricted to signed long, so downcast if necessary. long long_value = (long)hash_value; - if (!RB_FIXABLE(long_value)) { - long_value = long_value >> 1; + // Ensure we stay within the range allowed by Fixnum. + if (long_value < RUBY_FIXNUM_MIN) { + long_value = long_value + RUBY_FIXNUM_MAX + 1; + } else if (long_value > RUBY_FIXNUM_MAX) { + long_value = long_value - RUBY_FIXNUM_MAX - 1; } return INT2FIX(long_value); } From 4257f55ec08890702d714d5b02e6bb7bc9b90b8c Mon Sep 17 00:00:00 2001 From: Daniel Azuma Date: Wed, 5 May 2021 23:05:35 +0000 Subject: [PATCH 3/5] Simpler downcasting of hash values --- ruby/ext/google/protobuf_c/message.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/ruby/ext/google/protobuf_c/message.c b/ruby/ext/google/protobuf_c/message.c index 567af515d4bc..1cadebdcce85 100644 --- a/ruby/ext/google/protobuf_c/message.c +++ b/ruby/ext/google/protobuf_c/message.c @@ -735,15 +735,9 @@ uint64_t Message_Hash(const upb_msg* msg, const upb_msgdef* m, uint64_t seed) { static VALUE Message_hash(VALUE _self) { Message* self = ruby_to_Message(_self); uint64_t hash_value = Message_Hash(self->msg, self->msgdef, 0); - // Fixnum is restricted to signed long, so downcast if necessary. - long long_value = (long)hash_value; - // Ensure we stay within the range allowed by Fixnum. - if (long_value < RUBY_FIXNUM_MIN) { - long_value = long_value + RUBY_FIXNUM_MAX + 1; - } else if (long_value > RUBY_FIXNUM_MAX) { - long_value = long_value - RUBY_FIXNUM_MAX - 1; - } - return INT2FIX(long_value); + // RUBY_FIXNUM_MAX should be one less than a power of 2. + assert(RUBY_FIXNUM_MAX & (RUBY_FIXNUM_MAX + 1) == 0); + return INT2FIX(hash_value & RUBY_FIXNUM_MAX); } /* From ee53fe84c8a309298eca7be0298e415dfdf839ad Mon Sep 17 00:00:00 2001 From: Daniel Azuma Date: Thu, 6 May 2021 15:00:15 +0000 Subject: [PATCH 4/5] Fix precedence --- ruby/ext/google/protobuf_c/message.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ruby/ext/google/protobuf_c/message.c b/ruby/ext/google/protobuf_c/message.c index 1cadebdcce85..ffdae6a4017f 100644 --- a/ruby/ext/google/protobuf_c/message.c +++ b/ruby/ext/google/protobuf_c/message.c @@ -736,7 +736,7 @@ static VALUE Message_hash(VALUE _self) { Message* self = ruby_to_Message(_self); uint64_t hash_value = Message_Hash(self->msg, self->msgdef, 0); // RUBY_FIXNUM_MAX should be one less than a power of 2. - assert(RUBY_FIXNUM_MAX & (RUBY_FIXNUM_MAX + 1) == 0); + assert((RUBY_FIXNUM_MAX & (RUBY_FIXNUM_MAX + 1)) == 0); return INT2FIX(hash_value & RUBY_FIXNUM_MAX); } From c9fb5e0ddcc7cbe874c7eedb6d9c5965739561d7 Mon Sep 17 00:00:00 2001 From: Daniel Azuma Date: Thu, 6 May 2021 15:23:24 +0000 Subject: [PATCH 5/5] Fix bundle on Ruby 2.4 --- ruby/Gemfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ruby/Gemfile b/ruby/Gemfile index fa75df156323..76b23ad91d1e 100644 --- a/ruby/Gemfile +++ b/ruby/Gemfile @@ -1,3 +1,5 @@ source 'https://rubygems.org' gemspec + +gem "irb", "~> 1.1", "< 1.2.0" if RUBY_VERSION < "2.5"