Clojure

Multiplication overflow issues around Long/MIN_VALUE

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: Release 1.6
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test
  • Approval:
    Ok

Description

Long story short (ha!), there are several related issues where overflow checking doesn't work correctly with Long/MIN_VALUE. The cause of this missed check is that in Java, these both pass:

assertTrue(Long.MIN_VALUE * -1 == Long.MIN_VALUE);
assertTrue(Long.MIN_VALUE / -1 == Long.MIN_VALUE);

This causes the following results in Clojure that do not match their siblings where the order of operands is switched:

user=> (* Long/MIN_VALUE -1)
-922337203685477580 ;; should throw
user=> (*' Long/MIN_VALUE -1)
-9223372036854775808 ;; should be positive
user=> (* Long/MIN_VALUE -1N)
-9223372036854775808N ;; should be positive
user=> (* (bigint Long/MIN_VALUE) -1N)
-9223372036854775808N ;; should be positive

Mailing list discussion here: https://groups.google.com/d/msg/clojure-dev/o1QGR1QK70M/A1KykR9OQqwJ

Patch: min_value_multiplication.diff

Approach: Modifies BigInt.multiply() and multiply/multiplyP in Numbers to take into account Long.MIN_VALUE.

After:

user=> (* Long/MIN_VALUE -1)
ArithmeticException integer overflow  clojure.lang.Numbers.throwIntOverflow (Numbers.java:1390)
user=> (*' Long/MIN_VALUE -1)
9223372036854775808N
user=> (* Long/MIN_VALUE -1N)
9223372036854775808N
user=> (* (bigint Long/MIN_VALUE) -1N)
9223372036854775808N

Screened by: Alex Miller (should also include CLJ-1225, CLJ-1253, CLJ-1254)

Activity

Colin Jones made changes -
Field Original Value New Value
Attachment min_value_multiplication.diff [ 12037 ]
Colin Jones made changes -
Patch Code and Test [ 10002 ]
Labels bug
Alex Miller made changes -
Approval Triaged [ 10120 ]
Alex Miller made changes -
Labels bug math
Rich Hickey made changes -
Approval Triaged [ 10120 ] Vetted [ 10003 ]
Fix Version/s Release 1.6 [ 10157 ]
Alex Miller made changes -
Description Long story short (ha!), there are several related issues where overflow checking doesn't work correctly with Long/MIN_VALUE. The cause of this missed check is that in Java, these both pass:

{code:java}
assertTrue(Long.MIN_VALUE * -1 == Long.MIN_VALUE);
assertTrue(Long.MIN_VALUE / -1 == Long.MIN_VALUE);
{code}

This causes the following results in Clojure that do not match their siblings where the order of operands is switched:

{code}
user=> (* Long/MIN_VALUE -1)
-922337203685477580 ;; should throw
user=> (*' Long/MIN_VALUE -1)
-9223372036854775808 ;; should be positive
user=> (* Long/MIN_VALUE -1N)
-9223372036854775808N ;; should be positive
user=> (* (bigint Long/MIN_VALUE) -1N)
-9223372036854775808N ;; should be positive
{code}

Mailing list discussion here: https://groups.google.com/d/msg/clojure-dev/o1QGR1QK70M/A1KykR9OQqwJ
Long story short (ha!), there are several related issues where overflow checking doesn't work correctly with Long/MIN_VALUE. The cause of this missed check is that in Java, these both pass:

{code:java}
assertTrue(Long.MIN_VALUE * -1 == Long.MIN_VALUE);
assertTrue(Long.MIN_VALUE / -1 == Long.MIN_VALUE);
{code}

This causes the following results in Clojure that do not match their siblings where the order of operands is switched:

{code}
user=> (* Long/MIN_VALUE -1)
-922337203685477580 ;; should throw
user=> (*' Long/MIN_VALUE -1)
-9223372036854775808 ;; should be positive
user=> (* Long/MIN_VALUE -1N)
-9223372036854775808N ;; should be positive
user=> (* (bigint Long/MIN_VALUE) -1N)
-9223372036854775808N ;; should be positive
{code}

Mailing list discussion here: https://groups.google.com/d/msg/clojure-dev/o1QGR1QK70M/A1KykR9OQqwJ

*Patch:* min_value_multiplication.diff

*Approach:*

*Screened by:*
Assignee Alex Miller [ alexmiller ]
Alex Miller made changes -
Priority Major [ 3 ] Minor [ 4 ]
Alex Miller made changes -
Approval Vetted [ 10003 ] Screened [ 10004 ]
Description Long story short (ha!), there are several related issues where overflow checking doesn't work correctly with Long/MIN_VALUE. The cause of this missed check is that in Java, these both pass:

{code:java}
assertTrue(Long.MIN_VALUE * -1 == Long.MIN_VALUE);
assertTrue(Long.MIN_VALUE / -1 == Long.MIN_VALUE);
{code}

This causes the following results in Clojure that do not match their siblings where the order of operands is switched:

{code}
user=> (* Long/MIN_VALUE -1)
-922337203685477580 ;; should throw
user=> (*' Long/MIN_VALUE -1)
-9223372036854775808 ;; should be positive
user=> (* Long/MIN_VALUE -1N)
-9223372036854775808N ;; should be positive
user=> (* (bigint Long/MIN_VALUE) -1N)
-9223372036854775808N ;; should be positive
{code}

Mailing list discussion here: https://groups.google.com/d/msg/clojure-dev/o1QGR1QK70M/A1KykR9OQqwJ

*Patch:* min_value_multiplication.diff

*Approach:*

*Screened by:*
Long story short (ha!), there are several related issues where overflow checking doesn't work correctly with Long/MIN_VALUE. The cause of this missed check is that in Java, these both pass:

{code:java}
assertTrue(Long.MIN_VALUE * -1 == Long.MIN_VALUE);
assertTrue(Long.MIN_VALUE / -1 == Long.MIN_VALUE);
{code}

This causes the following results in Clojure that do not match their siblings where the order of operands is switched:

{code}
user=> (* Long/MIN_VALUE -1)
-922337203685477580 ;; should throw
user=> (*' Long/MIN_VALUE -1)
-9223372036854775808 ;; should be positive
user=> (* Long/MIN_VALUE -1N)
-9223372036854775808N ;; should be positive
user=> (* (bigint Long/MIN_VALUE) -1N)
-9223372036854775808N ;; should be positive
{code}

Mailing list discussion here: https://groups.google.com/d/msg/clojure-dev/o1QGR1QK70M/A1KykR9OQqwJ

*Patch:* min_value_multiplication.diff

*Approach:* Modifies BigInt.multiply() and multiply/multiplyP in Numbers to take into account Long.MIN_VALUE.

After:
{code}
user=> (* Long/MIN_VALUE -1)
ArithmeticException integer overflow clojure.lang.Numbers.throwIntOverflow (Numbers.java:1390)
user=> (*' Long/MIN_VALUE -1)
9223372036854775808N
user=> (* Long/MIN_VALUE -1N)
9223372036854775808N
user=> (* (bigint Long/MIN_VALUE) -1N)
9223372036854775808N
{code}

*Screened by:* Alex Miller (should also include CLJ-1225, CLJ-1253, CLJ-1254)
Alex Miller made changes -
Assignee Alex Miller [ alexmiller ]
Rich Hickey made changes -
Approval Screened [ 10004 ] Incomplete [ 10006 ]
Rich Hickey made changes -
Approval Incomplete [ 10006 ] Ok [ 10007 ]
Stuart Halloway made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Closed [ 6 ]

People

Vote (1)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: