Project

General

Profile

Bug #14260

test failure related with proc arity on 32-bit environment (e.g. Solaris 10 with 32-bit compile)

Added by ngoto (Naohisa Goto) over 1 year ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:84563]

Description

おそらく r59358 以降、sparc の Solaris 10 上の Oracle Developer Studio (Oracle Solaris Studio) 12.x にて 32ビットでコンパイルした際、
make test-all にて以下のような failure が出るようになりました。
(以下は r61515 での結果)

  1) Failure:
TestLazyEnumerator#test_each_cons_limit [/XXXXX/test/ruby/test_lazy_enumerator.rb:548]:
<[[1, 2], [2, 3]]> expected but was
<[[2, 3], [2, 3]]>.

  2) Failure:
TestProc#test_bound_parameters [/XXXXX/test/ruby/test_proc.rb:1184]:
<[[:rest]]> expected but was
<[]>.

  3) Failure:
TestEnumerator#test_cons [/XXXXX/test/ruby/test_enumerator.rb:118]:
<[[1, 2, 3],
 [2, 3, 4],
 [3, 4, 5],
 [4, 5, 6],
 [5, 6, 7],
 [6, 7, 8],
 [7, 8, 9],
 [8, 9, 10]]> expected but was
<[[8, 9, 10],
 [8, 9, 10],
 [8, 9, 10],
 [8, 9, 10],
 [8, 9, 10],
 [8, 9, 10],
 [8, 9, 10],
 [8, 9, 10]]>.

  4) Failure:
TestEnumerator#test_slice [/XXXXX/test/ruby/test_enumerator.rb:113]:
<[[1, 2, 3], [4, 5, 6], [7, 8, 9], [10]]> expected but was
<[[10], [10], [10], [10]]>.

(中略)

  6) Failure:
TestProc#test_arity2 [/XXXXX/test/ruby/test_proc.rb:439]:
<-1> expected but was
<0>.

  7) Failure:
TestLazyEnumerator#test_each_slice_limit [/XXXXX/test/ruby/test_lazy_enumerator.rb:554]:
<[[1, 2], [3, 4]]> expected but was
<[[3, 4], [3, 4]]>.

(後略)

結論から言うと以下のパッチで治りました。

ビットフィールドのデフォルトは unsigned int というのがCの規格にて定められていますが、
max に -1 (マクロ UNLIMITED_ARGUMENTS の値)が代入される場合があり、
これが unsigned int : 16 では 65535 が代入されたのと同じビット列になり、
それを再度 signed int に戻す際に、そのまま 65535 が戻され、つまり意図しない値に化けてしまい、
(ここらへんの処理は、Cの規格ではおそらく処理系依存とされていると思います)
arityを UNLIMITED_ARGUMENTS や 0 と比較する判定結果が異常になり、
上記のFailureが発生したようです。

なお、64ビットでコンパイルすると #else 以下の int min, max; が使われ、ビットフィールドではなくなるため、このFailreは発生しません。
また、gcc では 16 ビットのまま符号ありの -1 にしてから戻してくれる(場合もある?)ようで、(大部分の?)Failureは発生しないようです(が一部おかしい部分があったっぽい?)。

--- internal.h  (revision 61515)                                                
+++ internal.h  (working copy)                                                  
@@ -912,12 +912,12 @@

 /* IFUNC (Internal FUNCtion) */

 struct vm_ifunc_argc {
 #if SIZEOF_INT * 2 > SIZEOF_VALUE
-    int min: (SIZEOF_VALUE * CHAR_BIT) / 2;
-    int max: (SIZEOF_VALUE * CHAR_BIT) / 2;
+    signed int min: (SIZEOF_VALUE * CHAR_BIT) / 2;
+    signed int max: (SIZEOF_VALUE * CHAR_BIT) / 2;
 #else
     int min, max;
 #endif
 };

今回、Failureが発生したのは Solaris だけですが、全ての32ビット環境にて潜在的に問題が発生する可能性があるコードだったと言えます。

そして、ビットフィールドでメモリを節約している箇所は他にも複数あると思いますが、unsigned int 扱いで確かに正しいか、負の値を意図せず代入していないか、念のため総点検する必要があるかもしれないと思いました。

Associated revisions

Revision 2adf2815
Added by ngoto (Naohisa Goto) over 1 year ago

bit fields treating negative values should be declared as signed int

  • internal.h (struct vm_ifunc_argc): Bit fields are unsigned by default. For storing nagative values to bit fields, they must be declated as signed int. Fix multiple test failure observed by 32-bit binaries compiled with Oracle Developer Studio (Solaris Studio) 12.x on Solaris 10 on sparc architecture. [Bug #14260]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@61518 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 61518
Added by ngoto (Naohisa Goto) over 1 year ago

bit fields treating negative values should be declared as signed int

  • internal.h (struct vm_ifunc_argc): Bit fields are unsigned by default. For storing nagative values to bit fields, they must be declated as signed int. Fix multiple test failure observed by 32-bit binaries compiled with Oracle Developer Studio (Solaris Studio) 12.x on Solaris 10 on sparc architecture. [Bug #14260]

Revision 61518
Added by ngoto (Naohisa Goto) over 1 year ago

bit fields treating negative values should be declared as signed int

  • internal.h (struct vm_ifunc_argc): Bit fields are unsigned by default. For storing nagative values to bit fields, they must be declated as signed int. Fix multiple test failure observed by 32-bit binaries compiled with Oracle Developer Studio (Solaris Studio) 12.x on Solaris 10 on sparc architecture. [Bug #14260]

Revision 70c59ba5
Added by naruse (Yui NARUSE) over 1 year ago

merge revision(s) 61518: [Backport #14260]

    bit fields treating negative values should be declared as signed int

    * internal.h (struct vm_ifunc_argc): Bit fields are unsigned by default.
      For storing nagative values to bit fields, they must be declated as
      signed int. Fix multiple test failure observed by 32-bit binaries
      compiled with Oracle Developer Studio (Solaris Studio) 12.x on
      Solaris 10 on sparc architecture. [Bug #14260]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_5@61660 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 61660
Added by naruse (Yui NARUSE) over 1 year ago

merge revision(s) 61518: [Backport #14260]

bit fields treating negative values should be declared as signed int

* internal.h (struct vm_ifunc_argc): Bit fields are unsigned by default.
  For storing nagative values to bit fields, they must be declated as
  signed int. Fix multiple test failure observed by 32-bit binaries
  compiled with Oracle Developer Studio (Solaris Studio) 12.x on
  Solaris 10 on sparc architecture. [Bug #14260]

History

#1

Updated by ngoto (Naohisa Goto) over 1 year ago

  • Status changed from Open to Closed

Applied in changeset trunk|r61518.


bit fields treating negative values should be declared as signed int

  • internal.h (struct vm_ifunc_argc): Bit fields are unsigned by default. For storing nagative values to bit fields, they must be declated as signed int. Fix multiple test failure observed by 32-bit binaries compiled with Oracle Developer Studio (Solaris Studio) 12.x on Solaris 10 on sparc architecture. [Bug #14260]

Updated by nagachika (Tomoyuki Chikanaga) over 1 year ago

  • Backport changed from 2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: UNKNOWN to 2.3: DONTNEED, 2.4: DONTNEED, 2.5: REQUIRED

問題の bit field は r59358 で(2017-07-18)導入されていたものなので 2.5 のみ REQUIRED にしておきます。

Updated by shyouhei (Shyouhei Urabe) over 1 year ago

ビットフィールドのデフォルトは unsigned int というのがCの規格にて定められていますが、

ごめんなさい、いま家にいてCの規格にアクセスできないのですが、そのような規定があった記憶がないです。どこのセクションに書いてありましたでしょうか?

Updated by ngoto (Naohisa Goto) over 1 year ago

ビットフィールドのデフォルトは unsigned int というのがCの規格にて定められていますが、

ごめんなさい、いま家にいてCの規格にアクセスできないのですが、そのような規定があった記憶がないです。どこのセクションに書いてありましたでしょうか?

ご指摘のとおり、元の記述は間違いでした。
正しくは implementation-defined でしたので訂正します。ご指摘ありがとうございました。

原本は私も手元には無いのですが、C99 では 6.7.2.1 に書かれているとのことです。
参考にしたページ: http://en.cppreference.com/w/c/language/bit_field

Oracle Solaris Studio や IBM のコンパイラでは未指定のintは unsigned 扱いになるとの記述を見つけていたこともあり、勘違いしていました。

Oracle: https://docs.oracle.com/cd/E19205-01/820-1209/bjayo/index.html

IBM: https://www.ibm.com/support/knowledgecenter/en/SSQ2R2_8.5.1/com.ibm.tpf.toolkit.compilers.doc/ref/langref_zos/clrc03defbitf.htm

一方、GCCでは singed とのことです。
https://gcc.gnu.org/onlinedocs/gcc-5.2.0/gcc/Structures-unions-enumerations-and-bit-fields-implementation.html

なお、(少なくとも最近の) GCC や Oracle Developer Studio ではコンパイル時のオプションで変更可能とのことです。

Updated by shyouhei (Shyouhei Urabe) over 1 year ago

ngoto (Naohisa Goto) wrote:

ビットフィールドのデフォルトは unsigned int というのがCの規格にて定められていますが、

ごめんなさい、いま家にいてCの規格にアクセスできないのですが、そのような規定があった記憶がないです。どこのセクションに書いてありましたでしょうか?

ご指摘のとおり、元の記述は間違いでした。
正しくは implementation-defined でしたので訂正します。ご指摘ありがとうございました。

原本は私も手元には無いのですが、C99 では 6.7.2.1 に書かれているとのことです。
参考にしたページ: http://en.cppreference.com/w/c/language/bit_field

Oracle Solaris Studio や IBM のコンパイラでは未指定のintは unsigned 扱いになるとの記述を見つけていたこともあり、勘違いしていました。

Oracle: https://docs.oracle.com/cd/E19205-01/820-1209/bjayo/index.html

IBM: https://www.ibm.com/support/knowledgecenter/en/SSQ2R2_8.5.1/com.ibm.tpf.toolkit.compilers.doc/ref/langref_zos/clrc03defbitf.htm

一方、GCCでは singed とのことです。
https://gcc.gnu.org/onlinedocs/gcc-5.2.0/gcc/Structures-unions-enumerations-and-bit-fields-implementation.html

なお、(少なくとも最近の) GCC や Oracle Developer Studio ではコンパイル時のオプションで変更可能とのことです。

調査していただいてありがとうございました。そのような状況であれば裸のintの利用は控えてsignedかunsignedを明示したほうが良さそうですね。修正は妥当そうだと思いました。

Updated by naruse (Yui NARUSE) over 1 year ago

  • Backport changed from 2.3: DONTNEED, 2.4: DONTNEED, 2.5: REQUIRED to 2.3: DONTNEED, 2.4: DONTNEED, 2.5: DONE

ruby_2_5 r61660 merged revision(s) 61518.

Also available in: Atom PDF