Bug #13232: Comparing BigDecimal to float or Rational fails sometimes

• Status: Closed
• Assignee: mrkn (Kenta Murata)
• Target version: -
• ruby -v: ruby 2.3.3p222 (2016-11-21 revision 56859) [x86_64-linux]
• Backport: 2.2: WONTFIX, 2.3: REQUIRED, 2.4: DONE

[romuloceccon (Rômulo Ceccon)]

Under very special cases trying to compare a BigDecimal to a float or Rational will give an unexpected result:

irb> BigDecimal('1') < 1e-10
=> true

I couldn't find a sequence of steps which reproduces the problem with 100% guarantee, but the following seem to get close:

-- install a fresh copy of Ruby 2.3.3 from the stable sources

$ ./configure --prefix=$HOME/ruby-2.3.3-test
$ make main && make install-nodoc
$ export PATH=$HOME/ruby-2.3.3-test/bin:$PATH

-- install active support in $HOME/.gem

$ mkdir $HOME/bug
$ cd $HOME/bug
$ gem install bundler
$ echo -e "source 'https://rubygems.org'\ngem 'activesupport'" > Gemfile
$ bundle install --path=$HOME/.gem

-- run the script:

$ while ruby -e "require 'tempfile'; require 'bigdecimal'; f = Tempfile.new('bug'); srand(1); (1..2000).each { |i| f.rewind && f.puts('test') if i % 1000 == 1; x = BigDecimal('1'); puts([i, x]) || exit(1) if x < 1e-10.to_r }" ; do true ; done

The last script should not return. If it does then the bug is present:

$ while ruby -e ...
962
0.1E1

I can reproduce the bug on Ubuntu 16.04 running with Intel Core i3-4130 (desktop) and on Ubuntu 12.04, 14.04 and 16.04 running with Intel Xeon E5-2651 v2 (AWS).

I could track the problem down to the method VpNewRbClass in bigdecimal.c. It looks like the garbage collector is being called at the wrong time and corrupting memory. Applying the following patch gives me the output below:

--- ext/bigdecimal/bigdecimal.c.orig	2017-02-19 00:30:06.885174089 -0300
+++ ext/bigdecimal/bigdecimal.c	2017-02-19 00:33:53.379501013 -0300
@@ -604,8 +604,14 @@
 VP_EXPORT Real *
 VpNewRbClass(size_t mx, const char *str, VALUE klass)
 {
-    VALUE obj = TypedData_Wrap_Struct(klass, &BigDecimal_data_type, 0);
-    Real *pv = VpAlloc(mx,str);
+    VALUE obj;
+    Real *pv;
+    if (mx == 36)
+        printf("--\nVpNewRbClass: %s\n", str);
+    obj = TypedData_Wrap_Struct(klass, &BigDecimal_data_type, 0);
+    if (mx == 36)
+        printf("VpNewRbClass: %s\n", str);
+    pv = VpAlloc(mx,str);
     RTYPEDDATA_DATA(obj) = pv;
     pv->obj = obj;
     return pv;

Output:

$ while ruby -e ...
....
--
VpNewRbClass: 77371252455336267181195264
VpNewRbClass: 77371252455336267181195264
--
VpNewRbClass: 77371252455336267181195264
VpNewRbClass: 77371252455336267181195264
--
VpNewRbClass: 77371252455336267181195264
VpNewRbClass: ��V
962
0.1E1

Here's a possible fix. Someone please review it, because I have no experience with CRuby and don't feel confident about its correctness:

--- ext/bigdecimal/bigdecimal.c.orig	2017-02-19 00:30:06.885174089 -0300
+++ ext/bigdecimal/bigdecimal.c	2017-02-19 00:39:32.884680313 -0300
@@ -227,6 +227,7 @@
 static Real*
 GetVpValueWithPrec(VALUE v, long prec, int must)
 {
+    ENTER(1);
     Real *pv;
     VALUE num, bg;
     char szD[128];
@@ -292,6 +293,7 @@
 
       case T_BIGNUM:
         bg = rb_big2str(v, 10);
+        PUSH(bg);
         return VpCreateRbObject(strlen(RSTRING_PTR(bg)) + VpBaseFig() + 1,
                                 RSTRING_PTR(bg));
       default:

[nobu (Nobuyoshi Nakada)]

• Description updated (diff)
• Status changed from Open to Assigned
• Assignee set to mrkn (Kenta Murata)

Seems correct to me.

[romuloceccon (Rômulo Ceccon)]

I didn't realize there was a Github repo for BigDecimal when opening this issue. May I repost it there as a pull request?

[romuloceccon (Rômulo Ceccon)]

Any update on this one?

Should I look for more easily reproducible steps? What about opening a Github issue/PR?

Thanks.

[mrkn (Kenta Murata)]

I'm sorry for my late response.
I'll import your patch by tomorrow.

Thanks a lot.

[mrkn (Kenta Murata)]

• Status changed from Assigned to Closed

Applied in changeset r57951.

---

bigdecimal: version 1.3.2

Import bigdecimal version 1.3.2. The full commit log is here:

https://github.com/ruby/bigdecimal/compare/v1.3.1...v1.3.2

This fixes [ruby-core:79603] [Bug #13232]

[hrsht (Harshit Chopra)]

Sorry if it is not the right place to ask this question, but is it possible to backport this fix in ruby 2.4 and 2.3 versions? This issue came to light for us when using ruby 2.4, as with 2.4 BigDecimal now throws an error when trying to initialize with an invalid string.

So for us, it would fail at times when trying to instantiate a BigDecimal object in ruby like:

BigDecimal.new(1.0e-12)

raising error of the form:

invalid value for BigDecimal(): ""

[usa (Usaku NAKAMURA)]

• Backport changed from 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN to 2.2: WONTFIX, 2.3: REQUIRED, 2.4: REQUIRED

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 2.2: WONTFIX, 2.3: REQUIRED, 2.4: REQUIRED to 2.2: WONTFIX, 2.3: REQUIRED, 2.4: DONE

ruby_2_4 r62638 merged revision(s) 57597,57951.