Remainder of hashCode could be negative

[cushon]

Original issue created by eaftan@google.com on 2013-02-11 at 09:47 PM

---

Math.abs() does not always return a positive value. Math.abs(Integer.MIN_VALUE) is actually equal to Integer.MIN_VALUE. It would be an error to assume this when, for example, indexing into an array. A typical use might be bucket[Math.abs(y.hashCode()) % bucket.length].

[cushon]

Original comment posted by cpovirk@google.com on 2013-03-06 at 11:03 PM

---

I've also seen this with random numbers (why not use nextInt(n)? Maybe some of them aren't using java.util.Random? But IIRC most of them were) and with hashes generated from other APIs, e.g., <http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/hash/HashCode.html>.

I would go so far as to say that "abs(anything) % anything" is suspect, but I could believe that some uses are legitimate.

[cushon]

Original comment posted by kurt@kloover.com on 2013-03-06 at 11:43 PM

---

I wonder if it'd be worth adding something like: IntMath#checkedAbs(int) that would throw an IAE if passed MIN_VALUE (so it would always return a positive value). Similarly for the other primitives.

[cushon]

Original comment posted by kurt@kloover.com on 2013-03-06 at 11:43 PM

---

Erm, sorry, I thought I was on the Guava issue tracker...we'd want to add that API to Guava, not error-prone.

[cushon]

Original comment posted by eaftan@google.com on 2013-06-14 at 05:10 PM

---

Issue #136 has been merged into this issue.

[cushon]

Original comment posted by jdd@google.com on 2013-10-21 at 11:02 PM

---

We could make the check a little broader. I see hundreds and hundreds of lines in google3 that do something like

int modN = something.hashCode() % N; // for some positive int N

and then possibly fail (with a negative modN) if the hashCode is negative.

The slightly better

int modN = Math.abs(something.hashCode()) % N;

may also be broken, as noted above.

It will be tricky to get the analysis just right. We'd like to pass

if (something.hashCode() % N == 0) // do something

but fail

if (something.hashCode() % N < threshold) // do something

for obvious reasons.

I'm going to try Refaster to get a better idea of exactly what form these problems take.

[cushon]

Original comment posted by eaftan@google.com on 2014-04-07 at 10:15 PM

---

Issue #247 has been merged into this issue.