Skip to content

[Java] implement a better fix for an out of bounds exception#865

Merged
byroot merged 1 commit into
ruby:masterfrom
samyron:sm/swar-better-bounds-fix
Sep 20, 2025
Merged

[Java] implement a better fix for an out of bounds exception#865
byroot merged 1 commit into
ruby:masterfrom
samyron:sm/swar-better-bounds-fix

Conversation

@samyron

@samyron samyron commented Sep 19, 2025

Copy link
Copy Markdown
Contributor

After thinking about #859 and the fix #860 all day, something didn't feel quite right. After doing some debugging tonight, I realized the fix might defeat some of the SWAR optimizations.

When creating the ByteBuffer like so ByteBuffer.wrap(ptrBytes, 0, len), we are looking at a different portion of the ptrBytes array when ptr > 0. While we end up reading the correct chunk with bb.getLong(ptr + pos) or bb.getInt(ptr + pos), we may end up terminating the SWAR-optimized loop early.

Consider:

irb(main):001> require "json"
irb(main):002> s = "01234567890"
irb(main):003> JSON.dump(s[2..-1])
ptr: 2
ptrBytes.length: 15
ptrBytes: [48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 48, 0, 0, 0, 0]
len: 9

The backing array has a length of 15 bytes but the length of the string is 9 bytes.

The ByteBuffer in this case is [48, 49, 50, 51, 52, 53, 54, 55, 56] because we start it at index 0. When checking if ptr + pos + 8 <= len we get 2 + 0 + 8 <= 9 which is false.

We really should be looking at [50, 51, 52, 53, 54, 55, 56, 57, 48] assuming we do not offset the bounds check by ptr, as we did prior to the fix earlier today.

There are two possible fixes:

  1. Initialize the ByteBuffer as ByteBuffer.wrap(ptrBytes, ptr, len) and remove the ptr offset in the bounds checks.
  2. Initialize the ByteBuffer as ByteBuffer.wrap(ptrBytes, 0, ptrBytes.length) and leave the bounds checks offset by ptr.

CC: @headius

@headius headius left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either fix would be fine from a style perspective so it's just a matter of what fits the flow of the code best while un-breaking SWAR. I see you went with the first option and I think that's just fine.

@byroot byroot merged commit e809fab into ruby:master Sep 20, 2025
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants