fix: handle Integer.MIN_VALUE in shift method to prevent overflow#5300
fix: handle Integer.MIN_VALUE in shift method to prevent overflow#5300someshk1703 wants to merge 9 commits into
Conversation
🚀 Performance AnalysisAll benchmarks are within the acceptable range. No critical degradation detected (threshold is 100%). Please refer to the detailed report for more information. Click to see the detailed report
✅ Performance gain: |
|
@someshk1703 fix all CI builds |
gemshrine
left a comment
gemshrine
left a comment
There was a problem hiding this comment.
The bits == Integer.MIN_VALUE ? Integer.MAX_VALUE : Math.abs(bits) substitution avoids the overflow but silently changes the semantics of the shift by one bit. |Integer.MIN_VALUE| = 2^31 = 2147483648, which doesn't fit in an int — but Integer.MAX_VALUE is 2^31 - 1 = 2147483647, so mod and offset come out off-by-one:
PR fix: abs=2147483647 mod=7 offset=268435455
Truth: abs=2147483648 mod=0 offset=268435456
The added test uses a 64-bit input (0xFF000000L), where both shifts drop everything to zero regardless of that 1-bit gap, so the test can't distinguish the two behaviours. A larger BytesOf (say, a couple of MiB, so that ~2^31 bit shifts don't wipe the whole buffer) would surface the discrepancy.
A shape that stays exact and doesn't need the special case: take mod/div first, then abs — both stay in int range even for Integer.MIN_VALUE, because MIN_VALUE / 8 = -2^28 and MIN_VALUE % 8 = 0:
final int mod = Math.abs(bits % Byte.SIZE);
final int offset = Math.abs(bits / Byte.SIZE);Alternatively, promote once through long: final long abs = Math.abs((long) bits); and cast the derived mod/offset back to int. Either avoids losing the boundary bit.
|
@someshk1703 Thanks for your contribution, we appreciate it! Before we can merge this pull request, we need to see all CI workflows pass without errors. We can't merge the pull request into the master branch unless all workflows are green. Try to fix them, also paying attention to code style issues. |
… org.*, no blank lines)
… openValueGroup helpers; fix import order in EoSyntaxTest
|
|
@someshk1703 build is broken here |
3 participants
The PR fixes the issue, BytesRaw.shift results in ArrayIndexOutOfBoundsException for Integer.MIN_VALUE shift value due to Math.abs overflow.
Fix involves:
replaced the two raw Math.abs() calls with a single safe value:
final int abs = bits == Integer.MIN_VALUE ? Integer.MAX_VALUE : Math.abs(bits);
When bits == Integer.MIN_VALUE, Integer.MAX_VALUE is used instead. he resulting byte offset (268,435,455).
exceeds any realistic byte array length, so shiftLeft correctly zeroes all bytes — the same semantically correct result as shifting by 2³¹ positions.
A regression test shiftsIntegerMinValueWithoutOverflow was added to BytesOfTest test class to cover this path.