[ruby-core:75718] [Ruby trunk Bug#12427] Defining methods with the same name to both Fixnum and Bignum classes could cause SEGV in C extensions since Feature #12005

[ruby-core:75718] [Ruby trunk Bug#12427] Defining methods with the same name to both Fixnum and Bignum classes could cause SEGV in C extensions since Feature #12005

[frsyuki]

Issue #12427 has been reported by Sadayuki Furuhashi.

----------------------------------------
Bug #12427: Defining methods with the same name to both Fixnum and Bignum classes could cause SEGV in C extensions since Feature #12005
https://bugs.ruby-lang.org/issues/12427

* Author: Sadayuki Furuhashi
* Status: Open
* Priority: Normal
* Assignee: 
* ruby -v: ruby 2.4.0dev (2016-05-21 trunk 55091) [x86_64-darwin14]
* Backport: 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN
----------------------------------------
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:75719] [Ruby trunk Bug#12427][Feedback] Defining methods with the same name to both Fixnum and Bignum classes could cause SEGV in C extensions since Feature #12005

[nobu]

Issue #12427 has been updated by Nobuyoshi Nakada.

Description updated
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.

Add warnings against 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`.

----------------------------------------
Bug #12427: Defining methods with the same name to both Fixnum and Bignum classes could cause SEGV in C extensions since Feature #12005
https://bugs.ruby-lang.org/issues/12427#change-58843

* Author: Sadayuki Furuhashi
* Status: Feedback
* Priority: Normal
* Assignee: 
* ruby -v: ruby 2.4.0dev (2016-05-21 trunk 55091) [x86_64-darwin14]
* Backport: 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN
----------------------------------------
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.

```C
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.




-- 
https://bugs.ruby-lang.org/

[ruby-core:76700] [Ruby trunk Bug#12427] Defining methods with the same name to both Fixnum and Bignum classes could cause SEGV in C extensions since Feature #12005

[usa]

Issue #12427 has been updated by Usaku NAKAMURA.

Backport changed from 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN to 2.1: DONTNEED, 2.2: DONTNEED, 2.3: DONTNEED

----------------------------------------
Bug #12427: Defining methods with the same name to both Fixnum and Bignum classes could cause SEGV in C extensions since Feature #12005
https://bugs.ruby-lang.org/issues/12427#change-59918

* Author: Sadayuki Furuhashi
* Status: Closed
* Priority: Normal
* Assignee: 
* ruby -v: ruby 2.4.0dev (2016-05-21 trunk 55091) [x86_64-darwin14]
* Backport: 2.1: DONTNEED, 2.2: DONTNEED, 2.3: DONTNEED
----------------------------------------
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.

```C
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.




-- 
https://bugs.ruby-lang.org/