mruby-bigint: let mpz_get_int() answer the smallest Integer - #7212

Two spellings of the smallest Integer are not the same number. On today's master, ci/gcc-clang's full-debug (any build carrying mruby-bigint), x86-64:

$ build/full-debug/bin/mruby -e 'a = 0 - (2**63); b = -9223372036854775807 - 1; p a, b; p a == b'
-9223372036854775808
-9223372036854775808
false

An MRB_INT32 build says the same of -(2**31). CRuby answers true, on 3.2.3 and on 4.0.6.

The conversion that cannot deliver it

mpz_get_int() accumulates the absolute value of a bignum and refuses it once that exceeds MRB_INT_MAX, whatever the sign is:

  if (i > MRB_INT_MAX) {
    /* overflow */
    return FALSE;
  }
#endif

  if (y->sn < 0) {
    *v = -(mrb_int)i;
  }

An Integer's negative range is one wider than its positive one, so the smallest Integer is the one value this cannot answer, and both of its callers that take an mrb_int fail with it.

bint_norm() asks whether the result fits and keeps the bignum when the answer is no:

  bint_as_mpz(b, &a);
  if (mpz_get_int(&a, &i)) {
    return mrb_int_value(mrb, i);
  }
  return mrb_obj_value(b);

so 0 - (2**63) stays an MRB_TT_BIGINT where mrb_int is 64 bits wide, as does 0 - (2**31) where it is 32.

mrb_bint_cmp() takes the same route for a fixnum argument, and answers the sign of the receiver when the conversion fails:

    mrb_int i1, i2 = mrb_integer(y);
    if (mpz_get_int(&a, &i1)) {
      if (i1 == i2) return 0;
      if (i1 > i2) return 1;
      return -1;
    }
    if (a.sn > 0) return 1;
    return -1;

so the bignum compares less than a fixnum holding the very same value. That is the false above: one side never became a fixnum, and the comparison that should have caught up cannot see it either. eql? and hash disagree with each other on it in the process, hash reading the limb array for one and the value for the other.

The change

The limit widens by one when the sign is negative, and MRB_INT_MIN is spelled out where the accumulator reaches it, the way mrb_str_len_to_integer() names that value where it parses it: converting MRB_INT_MAX + 1 to mrb_int is implementation defined, and negating the result afterwards is the overflow the check exists to prevent.

Both limb widths take it. MRB_NO_MPZ64BIT needs the same widening in its pre-shift guard, which is the old limit shifted right.

Tests

assert 'Bigint normalizes the smallest Integer' asks ==, eql? in both directions, and hash, for exponent 63 and exponent 31, so whichever one is this build's width is covered and the other holds with two bignums on both sides. On master the exponent matching the build fails all four rows; the other passes.

Verification

rake -m test over ci/gcc-clang, every build green, 0 KO, 0 crash, and one test more than master everywhere, which is the new assert block.

build_config/asan.rb (clang, address,undefined) is green as well, 2313 tests and 3 skip plus 79 bintests, and reports nothing: the widened arithmetic stays on unsigned types, and UBSan agrees.

Two builds outside CI carry the halves of the change that CI cannot reach, both a default gembox, which brings in mruby-bigint through math.gembox.

MRB_INT32 meets the same defect one power lower. mrbgems/mruby-bigint/test/bigint.rb does not load on that build, which predates this commit, so the new assertions do not run there and the check is by hand:

$ build/bigint32/bin/mruby -e 'a = 0 - (2**31); b = -2147483647 - 1; p a, b; p a == b'
-2147483648
-2147483648
false   # master
true    # with this commit

MRB_NO_MPZ64BIT is the build the pre-shift guard is for. The new assertions fail there on master and pass with this commit, 2090 tests either way. That build also fails two tests of its own on master, Bigint Integer#pow(e, m) - Montgomery path and Bigint gcd, and it still fails exactly those two and no others afterwards. They are unrelated to this change.

The warnings in the ci/gcc-clang log come from mruby-compiler and are master's. bigint.c compiles warning free on every build above but the MRB_NO_MPZ64BIT one, which emits a -Woverflow for the mod 2^64 inverse of 3 at bigint.c:1208: 16 bit limbs take the #else arm written for 64 bit ones. That is master's too and is untouched here.

Environment

# ci/gcc-clang full-debug, -O0 because enable_debug appends -g3 -O0
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -g3 -O0 -DMRB_GC_STRESS -DMRB_USE_DEBUG_HOOK -DMRBGEM_MRUBY_BIGINT_VERSION=0.0.0 -DMRB_DEBUG -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER mrbgems/mruby-bigint/core/bigint.c

# ci/gcc-clang bintest
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_GC_FIXED_ARENA -DMRBGEM_MRUBY_BIGINT_VERSION=0.0.0 -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER -DMRB_USE_DEBUG_HOOK mrbgems/mruby-bigint/core/bigint.c

# ci/gcc-clang cxx_abi, gcc -x c++ rather than g++, which only links
gcc -g -O3 -Wall -Wundef -Wwrite-strings -x c++ -std=gnu++03 -DMRB_GC_FIXED_ARENA -DMRBGEM_MRUBY_BIGINT_VERSION=0.0.0 -DMRB_USE_CXX_EXCEPTION -DMRB_USE_CXX_ABI -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER mrbgems/mruby-bigint/core/bigint.c

# ci/gcc-clang byte-string
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRBGEM_MRUBY_BIGINT_VERSION=0.0.0 -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER mrbgems/mruby-bigint/core/bigint.c

# ci/gcc-clang ascii-case
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_USE_ASCII_CASE -DMRBGEM_MRUBY_BIGINT_VERSION=0.0.0 -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER mrbgems/mruby-bigint/core/bigint.c

# build_config/asan.rb, -O0 for the same reason as full-debug
clang -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -Wzero-length-array -fsanitize=address,undefined -g3 -O0 -DMRBGEM_MRUBY_BIGINT_VERSION=0.0.0 -DMRB_DEBUG -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER mrbgems/mruby-bigint/core/bigint.c

# a default gembox, with MRB_INT32
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_INT32 -DMRBGEM_MRUBY_BIGINT_VERSION=0.0.0 -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DMRB_USE_COMPLEX -DMRB_USE_BIGINT -DMRB_USE_DEBUG_HOOK mrbgems/mruby-bigint/core/bigint.c

# the same, with MRB_NO_MPZ64BIT
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_NO_MPZ64BIT -DMRBGEM_MRUBY_BIGINT_VERSION=0.0.0 -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DMRB_USE_COMPLEX -DMRB_USE_BIGINT -DMRB_USE_DEBUG_HOOK mrbgems/mruby-bigint/core/bigint.c

Summary by CodeRabbit

• Bug Fixes
  • Corrected handling of the smallest supported negative integer value.
  • Bigint and Fixnum-derived representations now consistently compare as equal and produce matching hashes at integer boundaries.