RFR: 8264976: Minor numeric bug in AbstractSplittableWithBrineGenerator.makeSplitsSpliterator
SonarCloud reports: Cast one of the operands of this subtraction operation to a "long". Here: Spliterator<SplittableGenerator> makeSplitsSpliterator(long index, long fence, SplittableGenerator source) { ... long multiplier = (1 << SALT_SHIFT) - 1; // <---- here The shift is integer, and the cast to long is too late. `SALT_SHIFT` is currently 4, so this is not the problem. But it would become a problem if `SALT_SHIFT` ever becomes 32 or larger. The shift operand should be `1L` for safety. Observe: jshell> Long.toHexString((1 << 31) - 1) $2 ==> "7fffffff" jshell> Long.toHexString((1 << 32) - 1) $3 ==> "0" jshell> Long.toHexString((1L << 32) - 1) $4 ==> "ffffffff" Additional testing: - [x] Linux x86_64 fastdebug, `jdk/utils/Random` ------------- Commit messages: - 8264976: Minor numeric bug in AbstractSplittableWithBrineGenerator.makeSplitsSpliterator Changes: https://git.openjdk.java.net/jdk/pull/3409/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3409&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264976 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3409.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3409/head:pull/3409 PR: https://git.openjdk.java.net/jdk/pull/3409
On Fri, 9 Apr 2021 09:41:19 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
SonarCloud reports: Cast one of the operands of this subtraction operation to a "long".
Here:
Spliterator<SplittableGenerator> makeSplitsSpliterator(long index, long fence, SplittableGenerator source) { ... long multiplier = (1 << SALT_SHIFT) - 1; // <---- here
The shift is integer, and the implicit cast to `long` happens too late. `SALT_SHIFT` is currently 4, so this is not the problem yet. But it would become a problem if `SALT_SHIFT` ever becomes 32 or larger. The shift operand should be `1L` for safety. Observe:
jshell> Long.toHexString((1 << 31) - 1) $2 ==> "7fffffff"
jshell> Long.toHexString((1 << 32) - 1) $3 ==> "0"
jshell> Long.toHexString((1L << 32) - 1) $4 ==> "ffffffff"
Additional testing: - [x] Linux x86_64 fastdebug, `jdk/utils/Random`
Anyone? :) ------------- PR: https://git.openjdk.java.net/jdk/pull/3409
On Fri, 9 Apr 2021 09:41:19 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
SonarCloud reports: Cast one of the operands of this subtraction operation to a "long".
Here:
Spliterator<SplittableGenerator> makeSplitsSpliterator(long index, long fence, SplittableGenerator source) { ... long multiplier = (1 << SALT_SHIFT) - 1; // <---- here
The shift is integer, and the implicit cast to `long` happens too late. `SALT_SHIFT` is currently 4, so this is not the problem yet. But it would become a problem if `SALT_SHIFT` ever becomes 32 or larger. The shift operand should be `1L` for safety. Observe:
jshell> Long.toHexString((1 << 31) - 1) $2 ==> "7fffffff"
jshell> Long.toHexString((1 << 32) - 1) $3 ==> "0"
jshell> Long.toHexString((1L << 32) - 1) $4 ==> "ffffffff"
Additional testing: - [x] Linux x86_64 fastdebug, `jdk/utils/Random`
Marked as reviewed by psandoz (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/3409
On Fri, 9 Apr 2021 09:41:19 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
SonarCloud reports: Cast one of the operands of this subtraction operation to a "long".
Here:
Spliterator<SplittableGenerator> makeSplitsSpliterator(long index, long fence, SplittableGenerator source) { ... long multiplier = (1 << SALT_SHIFT) - 1; // <---- here
The shift is integer, and the implicit cast to `long` happens too late. `SALT_SHIFT` is currently 4, so this is not the problem yet. But it would become a problem if `SALT_SHIFT` ever becomes 32 or larger. The shift operand should be `1L` for safety. Observe:
jshell> Long.toHexString((1 << 31) - 1) $2 ==> "7fffffff"
jshell> Long.toHexString((1 << 32) - 1) $3 ==> "0"
jshell> Long.toHexString((1L << 32) - 1) $4 ==> "ffffffff"
Additional testing: - [x] Linux x86_64 fastdebug, `jdk/utils/Random`
Marked as reviewed by jlaskey (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/3409
On Fri, 9 Apr 2021 09:41:19 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
SonarCloud reports: Cast one of the operands of this subtraction operation to a "long".
Here:
Spliterator<SplittableGenerator> makeSplitsSpliterator(long index, long fence, SplittableGenerator source) { ... long multiplier = (1 << SALT_SHIFT) - 1; // <---- here
The shift is integer, and the implicit cast to `long` happens too late. `SALT_SHIFT` is currently 4, so this is not the problem yet. But it would become a problem if `SALT_SHIFT` ever becomes 32 or larger. The shift operand should be `1L` for safety. Observe:
jshell> Long.toHexString((1 << 31) - 1) $2 ==> "7fffffff"
jshell> Long.toHexString((1 << 32) - 1) $3 ==> "0"
jshell> Long.toHexString((1L << 32) - 1) $4 ==> "ffffffff"
Additional testing: - [x] Linux x86_64 fastdebug, `jdk/utils/Random`
This pull request has now been integrated. Changeset: 94067446 Author: Aleksey Shipilev <shade@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/94067446 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8264976: Minor numeric bug in AbstractSplittableWithBrineGenerator.makeSplitsSpliterator Reviewed-by: psandoz, jlaskey ------------- PR: https://git.openjdk.java.net/jdk/pull/3409
participants (3)
-
Aleksey Shipilev
-
Jim Laskey
-
Paul Sandoz