Bug #6

sprintf() of %f on Windows(MSVCRT)

Added by usa (Usaku NAKAMURA) about 4 years ago. Updated about 1 year ago.

Status:Closed Start date:05/16/2008
Priority:High Due date:12/24/2008
Assignee:nobu (Nobuyoshi Nakada) % Done:

100%

Category:-
Target version:1.9.2
ruby -v:ruby 1.9.2dev (2010-02-26) [i386-mingw32]

Description

running test/ruby/test_sprintf.rb(test_float):
  <"36893488147419111424"> expected but was
  <"36893488147419111000">.

because sprintf() of MSVCRT is not precise.
should use our own dtoa().

sprintf.patch (4.2 kB) wanabe (_ wanabe), 10/26/2008 10:35 am

force_use_missing.patch (882 Bytes) wanabe (_ wanabe), 02/27/2010 10:40 pm

Associated revisions

Revision 20910
Added by nobu (Nobuyoshi Nakada) over 3 years ago

* ChangeLog: added ticket number [Bug#6].

History

Updated by usa (Usaku NAKAMURA) almost 4 years ago

忘れないように関係があると思われる問題を追加しておきます。

 test_attr(Complex_Test) [(snip)/test/ruby/test_complex.rb:186]:
 <"-0.0"> expected but was
 <"0.0">.

 test_fixed_bug(Complex_Test) [(snip)/test/ruby/test_complex.rb:1114]:
 <"-1.0-0.0i"> expected but was
 <"-1.0+0.0i">.

 test_to_s(TestFloat) [(snip)/test/ruby/test_float.rb:126]:
 <"1.0e+14"> expected but was
 <"1.0e+014">.

 test_format_float(TestSprintfComb)
    [(snip)/test/ruby/test_sprintf_comb.rb:539:in `block (2levels) in test_format_float'
     (snip...)
     (snip)/test/ruby/test_sprintf_comb.rb:526:in `test_format_float']:
 sprintf("%e", -123456789).
 <"-1.234568e+08"> expected but was
 <"-1.234568e+008">.

Updated by yugui (Yuki Sonoda) over 3 years ago

  • Target version set to 1.9.1 Release Candidate

Updated by wanabe (_ wanabe) over 3 years ago

missing/vsnprintf.c を利用するようなパッチを書きました。
vsnprintf.c 中の vsnprintf を外に出すやり方があまりうまくないように思うので
何かうまい方法をご存知の方は教えていただけるとありがたいです。

Updated by nobu (Nobuyoshi Nakada) over 3 years ago

なかだです。

At Sun, 26 Oct 2008 10:35:17 +0900,
_ wanabe wrote in [ruby-dev:36932]:
> vsnprintf.c 中の vsnprintf を外に出すやり方があまりうまくないように思うので
> 何かうまい方法をご存知の方は教えていただけるとありがたいです。

win32/Makefile.subのHAVE_VSNPRINTFを消せばmissingのほうを使うよ
うになるはずです。

ちなみに、missing/vsnprintf.cへの修正はどういうものでしょうか。

-- 
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
    中田 伸悦

Updated by wanabe (_ wanabe) over 3 years ago

ワナベです。

2008/10/26 15:45 Nobuyoshi Nakada <nobu@ruby-lang.org>:
> At Sun, 26 Oct 2008 10:35:17 +0900,
> _ wanabe wrote in [ruby-dev:36932]:
>> vsnprintf.c 中の vsnprintf を外に出すやり方があまりうまくないように思うので
>> 何かうまい方法をご存知の方は教えていただけるとありがたいです。
>
> win32/Makefile.subのHAVE_VSNPRINTFを消せばmissingのほうを使うよ
> うになるはずです。

mingw では configure で生成されたconfig.h を使うようですし
bcc では msvcrt を使わないようなので、それは最後の手段にしたいと思います。
ですがアドバイスのおかげでうまくいかない原因がわかりました。ありがとうございます。
vsnprintf を rb_win32_vsnprintf で上書きしなければいいだけだったようです。

> ちなみに、missing/vsnprintf.cへの修正はどういうものでしょうか。

説明不足でした。すみません。
test_sprintf_comb.rb が通るように修正しました。具体的には以下の通りです。

(1) sprintf("%e", -1.0000000000000000159e+100)
 ->修正前:"-1e+100" 修正後:"-1.000000e+100"
(2) sprintf("%.0f", 0.010000000000000000208)
 ->修正前:"0." 修正後:"0"
(3) sprintf("% #+-0.f", -0)
 ->修正前:"-0" 修正後:"-0."
(4) sprintf("%.0G", 1)
 ->修正前:"1E+00" 修正後:"1"

ですが今見直してみると、(1)の修正法が冗長かつ意味不明でしたので
先の rb_win32_vsnprintf の件と併せてパッチを書き直しました。

Index: include/ruby/win32.h
===================================================================
--- include/ruby/win32.h	(revision 19941)
+++ include/ruby/win32.h	(working copy)
@@ -243,7 +243,11 @@
 extern void   rb_w32_free_environ(char **);
 extern int    rb_w32_map_errno(DWORD);

+#if (defined(_MSC_VER) && defined(_DLL)) || defined(__MSVCRT__)
+#undef HAVE_VSNPRINTF
+#else
 #define vsnprintf(s,n,f,l) rb_w32_vsnprintf(s,n,f,l)
+#endif
 #define snprintf   rb_w32_snprintf
 extern int rb_w32_vsnprintf(char *, size_t, const char *, va_list);
 extern int rb_w32_snprintf(char *, size_t, const char *, ...);
Index: sprintf.c
===================================================================
--- sprintf.c	(revision 19941)
+++ sprintf.c	(working copy)
@@ -1018,7 +1018,7 @@
 		need += 20;

 		CHECK(need);
-		sprintf(&buf[blen], fbuf, fval);
+		snprintf(&buf[blen], need, fbuf, fval);
 		blen += strlen(&buf[blen]);
 	    }
 	    break;
Index: numeric.c
===================================================================
--- numeric.c	(revision 19941)
+++ numeric.c	(working copy)
@@ -530,12 +530,12 @@
     else if(isnan(value))
 	return rb_usascii_str_new2("NaN");

-    sprintf(buf, "%#.15g", value); /* ensure to print decimal point */
+    snprintf(buf, 32, "%#.15g", value); /* ensure to print decimal point */
     if (!(e = strchr(buf, 'e'))) {
 	e = buf + strlen(buf);
     }
     if (!ISDIGIT(e[-1])) { /* reformat if ended with decimal point
(ex 111111111111111.) */
-	sprintf(buf, "%#.14e", value);
+	snprintf(buf, 32, "%#.14e", value);
 	if (!(e = strchr(buf, 'e'))) {
 	    e = buf + strlen(buf);
 	}
@@ -1548,7 +1548,7 @@
 	    char buf[24];
 	    char *s;

-	    sprintf(buf, "%-.10g", RFLOAT_VALUE(val));
+	    snprintf(buf, 24, "%-.10g", RFLOAT_VALUE(val));
 	    if ((s = strchr(buf, ' ')) != 0) *s = '\0';
 	    rb_raise(rb_eRangeError, "float %s out of range of integer", buf);
 	}
@@ -1694,7 +1694,7 @@
 	    char buf[24];
 	    char *s;

-	    sprintf(buf, "%-.10g", RFLOAT_VALUE(val));
+	    snprintf(buf, 24, "%-.10g", RFLOAT_VALUE(val));
 	    if ((s = strchr(buf, ' ')) != 0) *s = '\0';
 	    rb_raise(rb_eRangeError, "float %s out of range of long long", buf);
 	}
Index: missing/vsnprintf.c
===================================================================
--- missing/vsnprintf.c	(revision 19941)
+++ missing/vsnprintf.c	(working copy)
@@ -753,6 +753,8 @@
 #ifdef FLOATING_POINT
 		case 'e':		/* anomalous precision */
 		case 'E':
+			if (prec != 0)
+				flags |= ALT;
 			prec = (prec == -1) ?
 				DEFPREC + 1 : prec + 1;
 			/* FALLTHROUGH */
@@ -782,7 +784,7 @@
 			cp = cvt(_double, prec, flags, &softsign,
 				&expt, ch, &ndig);
 			if (ch == 'g' || ch == 'G') {
-				if (expt <= -4 || expt > prec)
+				if (expt <= -4 || (expt > prec && expt > 1))
 					ch = (ch == 'g') ? 'e' : 'E';
 				else
 					ch = 'g';
@@ -798,6 +800,8 @@
 					size = expt;
 					if (prec || flags & ALT)
 						size += prec + 1;
+				} else if (!prec) { /* "0" */
+					size = 1;
 				} else	/* "0.X" */
 					size = prec + 2;
 			} else if (expt >= ndig) {	/* fixed g fmt */
@@ -1008,13 +1012,15 @@
 			if (ch >= 'f') {	/* 'f' or 'g' */
 				if (_double == 0) {
 				/* kludge for __dtoa irregularity */
-					if (prec == 0 ||
+					if (ndig <= 1 &&
 					    (flags & ALT) == 0) {
 						PRINT("0", 1);
 					} else {
 						PRINT("0.", 2);
 						PAD(ndig - 1, zeroes);
 					}
+				} else if (expt == 0 && ndig == 0 && (flags & ALT) == 0) {
+					PRINT("0", 1);
 				} else if (expt <= 0) {
 					PRINT("0.", 2);
 					PAD(-expt, zeroes);


-- 
ワナベ

Updated by nobu (Nobuyoshi Nakada) over 3 years ago

なかだです。

At Sun, 26 Oct 2008 20:42:54 +0900,
wanabe wrote in [ruby-dev:36935]:
> > win32/Makefile.subのHAVE_VSNPRINTFを消せばmissingのほうを使うよ
> > うになるはずです。
> 
> mingw では configure で生成されたconfig.h を使うようですし
> bcc では msvcrt を使わないようなので、それは最後の手段にしたいと思います。

mingwではconfiugre.inの ac_cv_func_vsnprintf=yes という行を消せ
ばできるはずです。bccはbcc32/Makefile.subにある定義を使うので、
win32/Makefile.subを変更しても影響は受けません。


> ですがアドバイスのおかげでうまくいかない原因がわかりました。ありがとうございます。
> vsnprintf を rb_win32_vsnprintf で上書きしなければいいだけだったようです。

rb_w32_vsnprintf()はmsvcrtのvsnprintf()のバグを回避するためのも
のなので、missingのものを使うのであればsnprintf()も不要になりま
す。バイナリ互換についてはwin32/mkexports.rbでaliasしておけば充
分でしょう。

> > ちなみに、missing/vsnprintf.cへの修正はどういうものでしょうか。
> 
> 説明不足でした。すみません。
> test_sprintf_comb.rb が通るように修正しました。具体的には以下の通りです。

なるほど。

> -    sprintf(buf, "%#.15g", value); /* ensure to print decimal point */
> +    snprintf(buf, 32, "%#.15g", value); /* ensure to print decimal point */

sizeof(buf)のほうがいいと思います。

-- 
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
    中田 伸悦

Updated by yugui (Yuki Sonoda) over 3 years ago

  • Due date set to 12/24/2008

Updated by yugui (Yuki Sonoda) over 3 years ago

  • Assignee changed from usa (Usaku NAKAMURA) to nobu (Nobuyoshi Nakada)
  • Priority changed from Low to High
「分かってるならコミットして」(うささん)とのことなのでなかださんに任せます。

Updated by nobu (Nobuyoshi Nakada) over 3 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100
Applied in changeset r20910.

Updated by mame (Yusuke Endoh) about 2 years ago

  • Target version changed from 1.9.1 Release Candidate to 1.9.2
  • ruby -v set to ruby 1.9.2dev (2010-02-26) [i386-mingw32]
遠藤です。

古いチケットですが reopen します。


> test_to_s(TestFloat) [(snip)/test/ruby/test_float.rb:126]:
> <"1.0e+14"> expected but was
> <"1.0e+014">.


mingw32 で今でも再現するようです。
mingw では直ってなかったのか regression なのかは知りません。


$ ./ruby test/ruby/test_float.rb
Loaded suite test/ruby/test_float
Started
............................F..
Finished in 1.019000 seconds.

  1) Failure:
test_to_s(TestFloat) [test/ruby/test_float.rb:126]:
<"1.0e+18"> expected but was
<"1.0e+018">.

31 tests, 1120 assertions, 1 failures, 0 errors, 0 skips

-- 
Yusuke Endoh <mame@tsg.ne.jp>

Updated by wanabe (_ wanabe) about 2 years ago

こんな感じでどうでしょうか。
configure.in は触ったことがないのでこれで良いのかよくわからないのですが
問題なさそうでしたらこれでコミットしたいと思います。

Updated by mame (Yusuke Endoh) about 2 years ago

遠藤です。

2010年2月27日22:40 _ wanabe <redmine@ruby-lang.org>:
> こんな感じでどうでしょうか。
> configure.in は触ったことがないのでこれで良いのかよくわからないのですが
> 問題なさそうでしたらこれでコミットしたいと思います。

mingw で直ることを確認しました。

$ ./ruby test/ruby/test_float.rb
Loaded suite test/ruby/test_float
Started
...............................
Finished in 1.019000 seconds.

31 tests, 1120 assertions, 0 failures, 0 errors, 0 skips

ありがとうございます。

-- 
Yusuke ENDOH <mame@tsg.ne.jp>

Also available in: Atom PDF