Skip to content

Commit

Permalink
Use Java 8 Math.*Exact methods for intrinsic overflow checks.
Browse files Browse the repository at this point in the history
  • Loading branch information
@headius
headius committed Jun 28, 2018
1 parent 459262a commit 14f00d7
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 58 deletions.
8 changes: 3 additions & 5 deletions core/src/main/java/org/jruby/RubyArray.java
View file
Expand Up @@ -4521,13 +4521,11 @@ public IRubyObject sumCommon(final ThreadContext context, IRubyObject init, fina

if (value instanceof RubyFixnum) {
/* should not overflow long type */
long other = ((RubyFixnum) value).getLongValue();
long sum2 = sum + other;
if (Helpers.additionOverflowed(sum, other, sum2)) {
try {
sum = Math.addExact(sum, ((RubyFixnum) value).getLongValue());
} catch (ArithmeticException ae) {
is_bignum = true;
break fixnum_loop;
} else {
sum = sum2;
}
} else if (value instanceof RubyBignum) {
is_bignum = true;
Expand Down
79 changes: 26 additions & 53 deletions core/src/main/java/org/jruby/RubyFixnum.java
View file
Expand Up @@ -454,19 +454,19 @@ public IRubyObject op_plus(ThreadContext context, IRubyObject other) {

@Override
public IRubyObject op_plus(ThreadContext context, long otherValue) {
long result = value + otherValue;
if (Helpers.additionOverflowed(value, otherValue, result)) {
try {
return newFixnum(Math.addExact(value, otherValue));
} catch (ArithmeticException ae) {
return addAsBignum(context, otherValue);
}
return newFixnum(context.runtime, result);
}

public IRubyObject op_plus_one(ThreadContext context) {
long result = value + 1;
if (result == Long.MIN_VALUE) {
try {
return newFixnum(context.runtime, Math.addExact(value, 1));
} catch (ArithmeticException ae) {
return addAsBignum(context, 1);
}
return newFixnum(context.runtime, result);
}

public IRubyObject op_plus_two(ThreadContext context) {
Expand All @@ -479,12 +479,11 @@ public IRubyObject op_plus_two(ThreadContext context) {
}

private RubyInteger addFixnum(ThreadContext context, RubyFixnum other) {
long otherValue = other.value;
long result = value + otherValue;
if (Helpers.additionOverflowed(value, otherValue, result)) {
try {
return newFixnum(context.runtime, Math.addExact(value, other.value));
} catch (ArithmeticException ae) {
return (RubyInteger) addAsBignum(context, other);
}
return newFixnum(context.runtime, result);
}

private IRubyObject addAsBignum(ThreadContext context, RubyFixnum other) {
Expand Down Expand Up @@ -518,37 +517,35 @@ public IRubyObject op_minus(ThreadContext context, IRubyObject other) {

@Override
public IRubyObject op_minus(ThreadContext context, long otherValue) {
long result = value - otherValue;
if (Helpers.subtractionOverflowed(value, otherValue, result)) {
try {
return newFixnum(context.runtime, Math.subtractExact(value, otherValue));
} catch (ArithmeticException ae) {
return subtractAsBignum(context, otherValue);
}
return newFixnum(context.runtime, result);
}

public IRubyObject op_minus_one(ThreadContext context) {
long result = value - 1;
if (result == Long.MAX_VALUE) {
try {
return newFixnum(context.runtime, Math.subtractExact(value, 1));
} catch (ArithmeticException ae) {
return subtractAsBignum(context, 1);
}
return newFixnum(context.runtime, result);
}

public IRubyObject op_minus_two(ThreadContext context) {
long result = value - 2;
//- if (result == Long.MAX_VALUE - 1) { //-code
if (value < result) { //+code+patch; maybe use if (value <= result) {
try {
return newFixnum(context.runtime, Math.subtractExact(value, 2));
} catch (ArithmeticException ae) {
return subtractAsBignum(context, 2);
}
return newFixnum(context.runtime, result);
}

private IRubyObject subtractFixnum(ThreadContext context, RubyFixnum other) {
long otherValue = other.value;
long result = value - otherValue;
if (Helpers.subtractionOverflowed(value, otherValue, result)) {
try {
return newFixnum(context.runtime, Math.subtractExact(value, other.value));
} catch (ArithmeticException ae) {
return subtractAsBignum(context, other);
}
return newFixnum(context.runtime, result);
}

private IRubyObject subtractAsBignum(ThreadContext context, RubyFixnum other) {
Expand Down Expand Up @@ -591,36 +588,12 @@ private IRubyObject multiplyOther(ThreadContext context, IRubyObject other) {

@Override
public IRubyObject op_mul(ThreadContext context, long otherValue) {
// See JRUBY-6612 for reasons for these different cases.
// The problem is that these Java long calculations overflow:
// value == -1; otherValue == Long.MIN_VALUE;
// result = value * othervalue; #=> Long.MIN_VALUE (overflow)
// result / value #=> Long.MIN_VALUE (overflow) == otherValue

final Ruby runtime = context.runtime;
final long value = this.value;

// fast check for known ranges that won't overflow
if (value <= 3037000499L && otherValue <= 3037000499L &&
value >= -3037000499L && otherValue >= -3037000499L) {
return newFixnum(runtime, value * otherValue);
}

if (value == 0 || otherValue == 0) {
return RubyFixnum.zero(runtime);
}
if (value == -1) {
if (otherValue != Long.MIN_VALUE) {
return newFixnum(runtime, -otherValue);
}
} else {
long result = value * otherValue;
if (result / value == otherValue) {
return newFixnum(runtime, result);
}
Ruby runtime = context.runtime;
try {
return newFixnum(runtime, Math.multiplyExact(value, otherValue));
} catch (ArithmeticException ae) {
return RubyBignum.newBignum(runtime, value).op_mul(context, otherValue);
}
// if here (value * otherValue) overflows long, so must return Bignum
return RubyBignum.newBignum(runtime, value).op_mul(context, otherValue);
}

/** fix_div
Expand Down

0 comments on commit 14f00d7

Please sign in to comment.