|
Description:
|
There seem to be long integer overflow problems in Fixnum#step illustrated by the (J)Ruby code just below. I've had a go at writing a patch (see below) and tried to put that as a "pull request" combined with my suggested patches for JRUBY-6777.
nn = 256
nn *= 256
nn *= nn * nn * (-nn / 2)
minv = nn
maxv = -(nn + 1)
p minv, maxv
def test_step( fromv, tov, stepv )
puts "#r #{fromv}.step( #{tov}, #{stepv} )"
kt = 0
v = fromv.step(tov, stepv) do |n|
kt += 1
puts "#r n= #{n} kt=#{kt}"
break if kt >= 9
end
puts "#r v= #{v.inspect}"
v
end
puts ; puts "this works properly"
test_step( maxv - 3, maxv - 1, 1 )
puts ; puts "this overflows"
test_step( maxv - 3, maxv - 0, 1 )
puts ; puts "this also overflows"
test_step( maxv - 7, maxv - 1, 5 )
This is a suggested patch:
private static void fixnumStep(ThreadContext context, Ruby runtime, long value, long end, long diff, Block block) {
if (diff == 0) throw runtime.newArgumentError("step cannot be 0");
// tempID: Patch by colinb2r on Github to deal with possible integer overflows;
// Now we must check for possible integer overflow in "i += diff".
// We can do that by checking for integer overflow in every "i += diff".
// We can do that by adapting code from RubyFixnum.java (or elsewhere),
// or by using "long iPrevious", and (for example) for "diff > 0"
// checking that if "iPrevious >= 0" then new value of i is also > 0.
// (Or possibly we could use a boolean iPreviousGTzero.)
// But we don't want to slow down the "for" statement.
// So instead run the "for" statement to an adjusted end value "endvv"
// which is sufficiently far from Long.MAX_VALUE or Long.MIN_VALUE
// that the last "i += diff" in the "for" statement won't overflow,
// and after the "for" statement check if we should yield another value,
// which will be ready and waiting in "i".
long endvv, i;
if (diff > 0) {
endvv = Long.MAX_VALUE - diff;
if (endvv > end) endvv = end;
for (i = value; i <= endvv; i += diff) {
block.yield(context, RubyFixnum.newFixnum(runtime, i));
}
} else {
endvv = Long.MIN_VALUE - diff;
if (endvv < end) endvv = end;
for (i = value; i >= endvv; i += diff) {
block.yield(context, RubyFixnum.newFixnum(runtime, i));
} }
}
if (endvv != end) {
block.yield(context, RubyFixnum.newFixnum(runtime, i));
}
}
|
