shyouhei	2018-11-07 16:16:50 +0900 (Wed, 07 Nov 2018)

  New Revision: 65595

  https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=65595

  Log:
    insns.def: avoid integer overflow
    
    In these expressions `1` is of type `signed int` (cf: ISO/IEC
    9899:1990 section 6.1.3.2). The variable (e.g. `num`) is of type
    `rb_num_t`, which is in fact `unsigned long`.  These two expressions
    then exercises the "usual arithmetic conversions" (cf: ISO/IEC
    9899:1990 section 6.2.1.5) and both eventually become `unsigned long`.
    The two unsigned expressions are then subtracted to generate another
    unsigned integer expression (cf: ISO/IEC 9899:1990 section 6.3.6).
    This is where integer overflows can occur.  OTOH the left hand side of
    the assignments are `rb_snum_t` which is `signed long`.  The
    assignments exercise the "implicit conversion" of "an unsigned integer
    is converted to its corresponding signed integer" case (cf: ISO/IEC
    9899:1990 section 6.2.1.2), which is "implementation-defined" (read:
    not portable).
    
    Casts are the proper way to avoid this problem.  Because all
    expressions are converted to some integer types before any binary
    operations are performed, the assignments now have fully defined
    behaviour.  These values can never exceed LONG_MAX so the casts must
    not lose any information.
    
    See also: https://travis-ci.org/ruby/ruby/jobs/451726874#L4357

  Modified files:
    trunk/insns.def
Index: insns.def
===================================================================
--- insns.def	(revision 65594)
+++ insns.def	(revision 65595)
@@ -380,7 +380,7 @@ concatstrings https://github.com/ruby/ruby/blob/trunk/insns.def#L380
 (rb_num_t num)
 (...)
 (VALUE val)
-// attr rb_snum_t sp_inc = 1 - num;
+// attr rb_snum_t sp_inc = 1 - (rb_snum_t)num;
 {
     val = rb_str_concat_literals(num, STACK_ADDR_FROM_TOP(num));
 }
@@ -416,7 +416,7 @@ toregexp https://github.com/ruby/ruby/blob/trunk/insns.def#L416
 /* This instruction has StringValue(), which is a method call.  But it
  * seems that path is never covered. */
 // attr bool leaf = true; /* yes it is */
-// attr rb_snum_t sp_inc = 1 - cnt;
+// attr rb_snum_t sp_inc = 1 - (rb_snum_t)cnt;
 {
     const VALUE ary = rb_ary_tmp_new_from_values(0, cnt, STACK_ADDR_FROM_TOP(cnt));
     val = rb_reg_new_ary(ary, (int)opt);
@@ -439,7 +439,7 @@ newarray https://github.com/ruby/ruby/blob/trunk/insns.def#L439
 (rb_num_t num)
 (...)
 (VALUE val)
-// attr rb_snum_t sp_inc = 1 - num;
+// attr rb_snum_t sp_inc = 1 - (rb_snum_t)num;
 {
     val = rb_ary_new4(num, STACK_ADDR_FROM_TOP(num));
 }
@@ -503,7 +503,7 @@ newhash https://github.com/ruby/ruby/blob/trunk/insns.def#L503
 (...)
 (VALUE val)
 // attr bool leaf = false; /* has rb_hash_key_str() */
-// attr rb_snum_t sp_inc = 1 - num;
+// attr rb_snum_t sp_inc = 1 - (rb_snum_t)num;
 {
     RUBY_DTRACE_CREATE_HOOK(HASH, num);
 
@@ -804,7 +804,7 @@ opt_newarray_max https://github.com/ruby/ruby/blob/trunk/insns.def#L804
  * necessary.  No way to detect such method calls beforehand.  We
  * cannot but mark it being not leaf. */
 // attr bool leaf = false; /* has rb_funcall() */
-// attr rb_snum_t sp_inc = 1 - num;
+// attr rb_snum_t sp_inc = 1 - (rb_snum_t)num;
 {
     val = vm_opt_newarray_max(num, STACK_ADDR_FROM_TOP(num));
 }
@@ -816,7 +816,7 @@ opt_newarray_min https://github.com/ruby/ruby/blob/trunk/insns.def#L816
 (VALUE val)
 /* Same discussion as opt_newarray_max. */
 // attr bool leaf = false; /* has rb_funcall() */
-// attr rb_snum_t sp_inc = 1 - num;
+// attr rb_snum_t sp_inc = 1 - (rb_snum_t)num;
 {
     val = vm_opt_newarray_min(num, STACK_ADDR_FROM_TOP(num));
 }

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/