Skip to content

8346657: Improve out of bounds exception messages for MemorySegments #28124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
+93 −30
Open

8346657: Improve out of bounds exception messages for MemorySegments #28124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a106ac8
JDK-8346657: Improve out of bounds exception messages for MemorySegments
hextriclosan Nov 4, 2025
adc7a06
Fixes according to reviewer's comments
hextriclosan Nov 8, 2025
49c35fa
Merge branch 'openjdk:master' into JDK-8346657
hextriclosan Nov 10, 2025
53f68ba
Remove the length > 0 condition, as it brakes multiple unit tests
hextriclosan Nov 10, 2025
fb7b28c
missed condition fix
hextriclosan Nov 11, 2025
b96a015
rework
hextriclosan Nov 14, 2025
9a2cc13
rework: eliminate lambda binding; introduce lightweight wrapper which…
hextriclosan Nov 15, 2025
df86a85
Adjust to TestMergeStoresMemorySegment.java requirements
hextriclosan Nov 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 67 additions & 22 deletions src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
* {@link MappedMemorySegmentImpl}.
*/
public abstract sealed class AbstractMemorySegmentImpl
implements MemorySegment, SegmentAllocator, BiFunction<String, List<Number>, RuntimeException>
implements MemorySegment, SegmentAllocator
permits HeapMemorySegmentImpl, NativeMemorySegmentImpl {

static final JavaNioAccess NIO_ACCESS = SharedSecrets.getJavaNioAccess();
Expand Down Expand Up @@ -100,19 +100,19 @@ public boolean isReadOnly() {

@Override
public AbstractMemorySegmentImpl asSlice(long offset, long newSize) {
checkBounds(offset, newSize);
checkSliceBounds(offset, newSize);
return asSliceNoCheck(offset, newSize);
}

@Override
public AbstractMemorySegmentImpl asSlice(long offset) {
checkBounds(offset, 0);
checkSliceBounds(offset, 0);
return asSliceNoCheck(offset, length - offset);
}

@Override
public MemorySegment asSlice(long offset, long newSize, long byteAlignment) {
checkBounds(offset, newSize);
checkSliceBounds(offset, newSize);
Utils.checkAlign(byteAlignment);

if (!isAlignedForElement(offset, byteAlignment)) {
Expand Down Expand Up @@ -354,7 +354,7 @@ public void checkReadOnly(boolean readOnly) {
@ForceInline
public void checkAccess(long offset, long length, boolean readOnly) {
checkReadOnly(readOnly);
checkBounds(offset, length);
checkAccessBounds(offset, length);
}

@ForceInline
Expand Down Expand Up @@ -398,20 +398,22 @@ private int checkArraySize(String typeName, int elemSize) {
}

@ForceInline
void checkBounds(long offset, long length) {
if (length > 0) {
Preconditions.checkIndex(offset, this.length - length + 1, this);
} else if (length < 0 || offset < 0 ||
offset > this.length - length) {
throw outOfBoundException(offset, length);
}
void checkSliceBounds(long offset, long length) {
checkBounds(offset, length, BoundPolicy.SLICE);
}

@Override
public RuntimeException apply(String s, List<Number> numbers) {
long offset = numbers.get(0).longValue();
long length = byteSize() - numbers.get(1).longValue() + 1;
return outOfBoundException(offset, length);
@ForceInline
void checkAccessBounds(long offset, long length) {
checkBounds(offset, length, BoundPolicy.ACCESS);
}

@ForceInline
private void checkBounds(long offset, long length, BoundPolicy policy) {
if (length > 0) {
Preconditions.checkIndex(offset, this.length - length + 1, new BoundsCheckHandler(this, policy));
} else if (length < 0 || offset < 0 || offset > this.length - length) {
throw policy.outOfBoundException(this, offset, length, this.length);
}
}

@Override
Expand All @@ -429,11 +431,6 @@ public final MemorySessionImpl sessionImpl() {
return scope;
}

private IndexOutOfBoundsException outOfBoundException(long offset, long length) {
return new IndexOutOfBoundsException(String.format("Out of bound access on segment %s; new offset = %d; new length = %d",
this, offset, length));
}

static class SegmentSplitter implements Spliterator<MemorySegment> {
AbstractMemorySegmentImpl segment;
long elemCount;
Expand Down Expand Up @@ -508,6 +505,54 @@ public int characteristics() {
}
}

private enum BoundPolicy {
SLICE {
@Override
String format(AbstractMemorySegmentImpl segment, long offset, long size, long length) {
return String.format(
"Out of bound access on segment %s; attempting to get slice of length %d at offset %d " +
"which is outside the valid range 0 <= offset+length < byteSize (=%d)",
segment, size, offset, length
);
}
},
ACCESS {
@Override
String format(AbstractMemorySegmentImpl segment, long offset, long size, long length) {
return String.format(
"Out of bound access on segment %s; attempting to access an element of length %d at offset %d " +
"which is outside the valid range 0 <= offset+length < byteSize (=%d)",
segment, size, offset, length
);
}
};

private IndexOutOfBoundsException outOfBoundException(AbstractMemorySegmentImpl segment, long offset, long size,
long length) {
return new IndexOutOfBoundsException(format(segment, offset, size, length));
}

abstract String format(AbstractMemorySegmentImpl segment, long offset, long size, long length);
Copy link
Member

@liach liach Nov 16, 2025

Choose a reason for hiding this comment

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

Even though this does look better, but abstract method in enum means extra classes, which become some extra burden for startup. If you can write this method to have a body like:

String formatString = switch (this) {
    case SLICE -> ...
    case ACCESS -> ...
};
return formatString.formatted(segment, size, offset, length);

This would be less costly.

}

private static final class BoundsCheckHandler implements BiFunction<String, List<Number>, IndexOutOfBoundsException> {
final AbstractMemorySegmentImpl segment;
final BoundPolicy policy;
Copy link
Member

@liach liach Nov 16, 2025

Choose a reason for hiding this comment

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

I recommend turning this into a private record. In addition, we should add a comment that we anticipate this record instance creation to be eliminated by escape analysis when segment access is compiled. I wonder if you have run benchmarks to verify this.


BoundsCheckHandler(AbstractMemorySegmentImpl segment, BoundPolicy policy) {
this.segment = segment;
this.policy = policy;
}

@Override
public IndexOutOfBoundsException apply(String s, List<Number> numbers) {
long offset = numbers.get(0).longValue();
long length = segment.byteSize() - numbers.get(1).longValue() + 1;

return policy.outOfBoundException(segment, offset, length, segment.byteSize());
}
}

// Object methods

@Override
Expand Down
2 changes: 1 addition & 1 deletion src/java.base/share/classes/jdk/internal/foreign/SegmentBulkOperations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public static void copy(AbstractMemorySegmentImpl src, long srcOffset,
@ForceInline
public static int contentHash(AbstractMemorySegmentImpl segment, long fromOffset, long toOffset) {
final long length = toOffset - fromOffset;
segment.checkBounds(fromOffset, length);
segment.checkSliceBounds(fromOffset, length);
if (length == 0) {
// The state has to be checked explicitly for zero-length segments
segment.scope.checkValidState();
Expand Down
6 changes: 3 additions & 3 deletions src/java.base/share/classes/jdk/internal/foreign/StringSupport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public static int strlenByte(final AbstractMemorySegmentImpl segment,
final long fromOffset,
final long toOffset) {
final long length = toOffset - fromOffset;
segment.checkBounds(fromOffset, length);
segment.checkSliceBounds(fromOffset, length);
if (length < Byte.BYTES) {
// There can be no null terminator present
segment.scope.checkValidState();
Expand Down Expand Up @@ -179,7 +179,7 @@ public static int strlenShort(final AbstractMemorySegmentImpl segment,
final long fromOffset,
final long toOffset) {
final long length = toOffset - fromOffset;
segment.checkBounds(fromOffset, length);
segment.checkSliceBounds(fromOffset, length);
if (length < Short.BYTES) {
// There can be no null terminator present
segment.scope.checkValidState();
Expand Down Expand Up @@ -215,7 +215,7 @@ public static int strlenInt(final AbstractMemorySegmentImpl segment,
final long fromOffset,
final long toOffset) {
final long length = toOffset - fromOffset;
segment.checkBounds(fromOffset, length);
segment.checkSliceBounds(fromOffset, length);
if (length < Integer.BYTES) {
// There can be no null terminator present
segment.scope.checkValidState();
Expand Down
26 changes: 22 additions & 4 deletions test/jdk/java/foreign/TestSegments.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,14 +212,32 @@ public void testSmallSegmentMin() {
}

@Test
public void testSegmentOOBMessage() {
public void testSegmentAccessOOBMessage() {
try {
var segment = Arena.global().allocate(10, 1);
segment.getAtIndex(ValueLayout.JAVA_INT, 2);
fail("Expected IndexOutOfBoundsException was not thrown");
} catch (IndexOutOfBoundsException ex) {
assertTrue(ex.getMessage().contains("Out of bound access"));
assertTrue(ex.getMessage().contains("offset = 8"));
assertTrue(ex.getMessage().contains("length = 4"));
assertTrue(ex.getMessage().startsWith("Out of bound access"));
assertTrue(ex.getMessage().endsWith("attempting to access an element of length 4 at offset 8 " +
"which is outside the valid range 0 <= offset+length < byteSize (=10)"));
} catch (Exception ex) {
fail("Unexpected exception type thrown: " + ex);
}
}

@Test
public void testSegmentSliceOOBMessage() {
try {
var segment = Arena.global().allocate(10, 1);
var slice = segment.asSlice(8, 4);
fail("Expected IndexOutOfBoundsException was not thrown");
} catch (IndexOutOfBoundsException ex) {
assertTrue(ex.getMessage().startsWith("Out of bound access"));
assertTrue(ex.getMessage().endsWith("attempting to get slice of length 4 at offset 8 " +
"which is outside the valid range 0 <= offset+length < byteSize (=10)"));
} catch (Exception ex) {
fail("Unexpected exception type thrown: " + ex);
}
}

Expand Down