Bug #12427
closedDefining methods with the same name to both Fixnum and Bignum classes could cause SEGV in C extensions since Feature #12005
Description
My gem (msgpack.gem) includes C extension with following pseudo code.
This code is working well with released ruby versions. But it caused SEGV with ruby 2.4.0-dev trunk 55091.
static VALUE Fixnum_to_msgpack(int argc, VALUE* argv, VALUE self)
{
long v = FIX2LONG(self);
printf("%ld", v); // work with v
}
static VALUE Bignum_to_msgpack(int argc, VALUE* argv, VALUE self)
{
if (RBIGNUM_POSITIVE_P(self)) {
uint64_t positive = rb_big2ull(self);
printf("%llu", positive); // work with positive
} else {
int64_t negative = rb_big2ll(self);
printf("%lld", negative); // work with negative
}
}
void Init_msgpack(void)
{
rb_define_method(rb_cFixnum, "to_msgpack", Fixnum_to_msgpack, -1);
rb_define_method(rb_cBignum, "to_msgpack", Bignum_to_msgpack, -1);
}
Apparently, since Feature #12005 (https://bugs.ruby-lang.org/issues/12005), both rb_cFixnum
and rb_cBignum
are reference to rb_cInteger
(rb_cFixnum == rb_cBignum
). Therefore, the later rb_define_method
call on rb_cBignum
overwrites the method with same name on rb_cFixnum
. So if to_msgpack method is called against an integer in the range of fixnum, Bignum_to_msgpack
function is called against a fixnum object. Then, RBIGNUM_POSITIVE_P
is called against a fixnum object. However, because RBIGNUM_POSITIVE_P
expects bignum object strictly despite of underlaying unified class definition with fixnum, it causes SEGV. This issue happens differently if the order of rb_define_method
against rb_cFixnum
and rb_cBignum
is opposite.
This test code reproduces the problem:
https://github.com/msgpack/msgpack-ruby/pull/115/commits/26183b718963f882309d52c167dc866ba5260272
I added following fix to the C extension. It succeeded to avoid the problem:
https://github.com/msgpack/msgpack-ruby/pull/115/files
Concern 1 is that this issue seems a common problem with C extensions and hard to debug unless the author is aware of changes of Feature #12005 including its influence on C API.
Concern 2 is that C extensions want to use a macro instead of runtime if (rb_cFixnum == rb_cBignum)
check to branch code at compile time.
I hope that documents or release note of ruby 2.4 includes mention for those concerns and guidelines for their solution.
Updated by nobu (Nobuyoshi Nakada) over 8 years ago
- Description updated (diff)
- Status changed from Open to Feedback
https://github.com/ruby/ruby/compare/trunk...nobu:bug/12427-integer-integration
This patch does:
Concern 1 is that this issue seems a common problem with C extensions and hard to debug unless the author is aware of changes of Feature #12005 including its influence on C API.
Show warnings if rb_cFixnum
or rb_cBignum
is used,
Concern 2 is that C extensions want to use a macro instead of runtime
if (rb_cFixnum == rb_cBignum)
check to branch code at compile time.
Add a macro RUBY_INTEGER_INTEGRATION
.
Updated by nobu (Nobuyoshi Nakada) over 8 years ago
- Status changed from Feedback to Closed
Applied in changeset r55394.
Integer unification macro
- include/ruby/ruby.h (RUBY_INTEGER_UNIFICATION): macro to tell if
Integer is integrated. [ruby-core:75718][Bug #12427] - include/ruby/backward.h, internal.h (rb_cFixnum, rb_cBignum):
fallback to rb_cInteger. - bignum.c, numeric.c, ext/json/generator/generator.{c,h}: use the
macro.
Updated by naruse (Yui NARUSE) over 8 years ago
- Related to Feature #12005: Unify Fixnum and Bignum into Integer added
Updated by usa (Usaku NAKAMURA) over 8 years ago
- Backport changed from 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN to 2.1: DONTNEED, 2.2: DONTNEED, 2.3: DONTNEED