Skip to content

8345120: A likely bug in StringSupport::chunkedStrlenShort #22451

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

Closed
wants to merge 12 commits into from
Closed

8345120: A likely bug in StringSupport::chunkedStrlenShort #22451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c5d764d
Move and rewrite strlen methods
minborg Nov 29, 2024
58e3d35
Fix benchmark
minborg Nov 29, 2024
50b60b4
Add int method
minborg Nov 29, 2024
b6b4644
Clean up
minborg Nov 29, 2024
777458a
Move back methods to StringSupport
minborg Nov 29, 2024
5426849
Fix imports
minborg Nov 29, 2024
641b463
Add test for segments with odd byte at the end
minborg Dec 2, 2024
2a78886
Rename method
minborg Dec 2, 2024
7ab520d
Add zero detection method for quad words
minborg Dec 2, 2024
72b66bb
Improve docs
minborg Dec 2, 2024
8de8429
Merge branch 'master' into simplify-string-support
minborg Dec 2, 2024
ae1acf5
Add test and change exception type for empty segments
minborg Dec 2, 2024
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
4 changes: 4 additions & 0 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 @@ -889,23 +889,27 @@ public void setAtIndex(AddressLayout layout, long index, MemorySegment value) {
layout.varHandle().set((MemorySegment)this, index * layout.byteSize(), value);
}

@ForceInline
@Override
public String getString(long offset) {
return getString(offset, sun.nio.cs.UTF_8.INSTANCE);
}

@ForceInline
@Override
public String getString(long offset, Charset charset) {
Objects.requireNonNull(charset);
return StringSupport.read(this, offset, charset);
}

@ForceInline
@Override
public void setString(long offset, String str) {
Objects.requireNonNull(str);
setString(offset, str, sun.nio.cs.UTF_8.INSTANCE);
}

@ForceInline
@Override
public void setString(long offset, String str, Charset charset) {
Objects.requireNonNull(charset);
Expand Down
62 changes: 31 additions & 31 deletions 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 @@ -28,7 +28,6 @@
import jdk.internal.misc.ScopedMemoryAccess;
import jdk.internal.util.Architecture;
import jdk.internal.util.ArraysSupport;
import jdk.internal.util.ByteArrayLittleEndian;
import jdk.internal.vm.annotation.ForceInline;
import jdk.internal.vm.annotation.Stable;

Expand All @@ -50,6 +49,7 @@ public final class SegmentBulkOperations {
private SegmentBulkOperations() {}

private static final ScopedMemoryAccess SCOPED_MEMORY_ACCESS = ScopedMemoryAccess.getScopedMemoryAccess();
private static final long LONG_MASK = ~7L; // The last three bits are zero

// All the threshold values below MUST be a power of two and should preferably be
// greater or equal to 2^3.
Expand All @@ -75,21 +75,21 @@ public static MemorySegment fill(AbstractMemorySegmentImpl dst, byte value) {
int offset = 0;
// 0...0X...X000
final int limit = (int) (dst.length & (NATIVE_THRESHOLD_FILL - 8));
for (; offset < limit; offset += 8) {
for (; offset < limit; offset += Long.BYTES) {
SCOPED_MEMORY_ACCESS.putLongUnaligned(dst.sessionImpl(), dst.unsafeGetBase(), dst.unsafeGetOffset() + offset, longValue, !Architecture.isLittleEndian());
}
int remaining = (int) dst.length - limit;
// 0...0X00
if (remaining >= 4) {
if (remaining >= Integer.BYTES) {
SCOPED_MEMORY_ACCESS.putIntUnaligned(dst.sessionImpl(), dst.unsafeGetBase(), dst.unsafeGetOffset() + offset, (int) longValue, !Architecture.isLittleEndian());
offset += 4;
remaining -= 4;
offset += Integer.BYTES;
remaining -= Integer.BYTES;
}
// 0...00X0
if (remaining >= 2) {
if (remaining >= Short.BYTES) {
SCOPED_MEMORY_ACCESS.putShortUnaligned(dst.sessionImpl(), dst.unsafeGetBase(), dst.unsafeGetOffset() + offset, (short) longValue, !Architecture.isLittleEndian());
offset += 2;
remaining -= 2;
offset += Short.BYTES;
remaining -= Short.BYTES;
}
// 0...000X
if (remaining == 1) {
Expand Down Expand Up @@ -123,26 +123,26 @@ public static void copy(AbstractMemorySegmentImpl src, long srcOffset,
// is an overlap, we could tolerate one particular direction of overlap (but not the other).

// 0...0X...X000
final int limit = (int) (size & (NATIVE_THRESHOLD_COPY - 8));
final int limit = (int) (size & (NATIVE_THRESHOLD_COPY - Long.BYTES));
int offset = 0;
for (; offset < limit; offset += 8) {
for (; offset < limit; offset += Long.BYTES) {
final long v = SCOPED_MEMORY_ACCESS.getLongUnaligned(src.sessionImpl(), src.unsafeGetBase(), src.unsafeGetOffset() + srcOffset + offset, !Architecture.isLittleEndian());
SCOPED_MEMORY_ACCESS.putLongUnaligned(dst.sessionImpl(), dst.unsafeGetBase(), dst.unsafeGetOffset() + dstOffset + offset, v, !Architecture.isLittleEndian());
}
int remaining = (int) size - offset;
// 0...0X00
if (remaining >= 4) {
if (remaining >= Integer.BYTES) {
final int v = SCOPED_MEMORY_ACCESS.getIntUnaligned(src.sessionImpl(), src.unsafeGetBase(),src.unsafeGetOffset() + srcOffset + offset, !Architecture.isLittleEndian());
SCOPED_MEMORY_ACCESS.putIntUnaligned(dst.sessionImpl(), dst.unsafeGetBase(), dst.unsafeGetOffset() + dstOffset + offset, v, !Architecture.isLittleEndian());
offset += 4;
remaining -= 4;
offset += Integer.BYTES;
remaining -= Integer.BYTES;
}
// 0...00X0
if (remaining >= 2) {
if (remaining >= Short.BYTES) {
final short v = SCOPED_MEMORY_ACCESS.getShortUnaligned(src.sessionImpl(), src.unsafeGetBase(), src.unsafeGetOffset() + srcOffset + offset, !Architecture.isLittleEndian());
SCOPED_MEMORY_ACCESS.putShortUnaligned(dst.sessionImpl(), dst.unsafeGetBase(), dst.unsafeGetOffset() + dstOffset + offset, v, !Architecture.isLittleEndian());
offset += 2;
remaining -=2;
offset += Short.BYTES;
remaining -= Short.BYTES;
}
// 0...000X
if (remaining == 1) {
Expand Down Expand Up @@ -202,9 +202,9 @@ public static int contentHash(AbstractMemorySegmentImpl segment, long fromOffset
return 1;
}
int result = 1;
final long longBytes = length & ((1L << 62) - 8);
final long longBytes = length & LONG_MASK;
final long limit = fromOffset + longBytes;
for (; fromOffset < limit; fromOffset += 8) {
for (; fromOffset < limit; fromOffset += Long.BYTES) {
long val = SCOPED_MEMORY_ACCESS.getLongUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + fromOffset, !Architecture.isLittleEndian());
result = result * POWERS_OF_31[7]
+ ((byte) (val >>> 56)) * POWERS_OF_31[6]
Expand All @@ -218,24 +218,24 @@ public static int contentHash(AbstractMemorySegmentImpl segment, long fromOffset
}
int remaining = (int) (length - longBytes);
// 0...0X00
if (remaining >= 4) {
if (remaining >= Integer.BYTES) {
int val = SCOPED_MEMORY_ACCESS.getIntUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + fromOffset, !Architecture.isLittleEndian());
result = result * POWERS_OF_31[3]
+ ((byte) (val >>> 24)) * POWERS_OF_31[2]
+ ((byte) (val >>> 16)) * POWERS_OF_31[1]
+ ((byte) (val >>> 8)) * POWERS_OF_31[0]
+ ((byte) val);
fromOffset += 4;
remaining -= 4;
fromOffset += Integer.BYTES;
remaining -= Integer.BYTES;
}
// 0...00X0
if (remaining >= 2) {
if (remaining >= Short.BYTES) {
short val = SCOPED_MEMORY_ACCESS.getShortUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + fromOffset, !Architecture.isLittleEndian());
result = result * POWERS_OF_31[1]
+ ((byte) (val >>> 8)) * POWERS_OF_31[0]
+ ((byte) val);
fromOffset += 2;
remaining -= 2;
fromOffset += Short.BYTES;
remaining -= Short.BYTES;
}
// 0...000X
if (remaining == 1) {
Expand Down Expand Up @@ -288,7 +288,7 @@ private static long mismatch(AbstractMemorySegmentImpl src, long srcFromOffset,
long start, int length, boolean srcAndDstBytesDiffer) {
int offset = 0;
final int limit = length & (NATIVE_THRESHOLD_MISMATCH - 8);
for (; offset < limit; offset += 8) {
for (; offset < limit; offset += Long.BYTES) {
final long s = SCOPED_MEMORY_ACCESS.getLongUnaligned(src.sessionImpl(), src.unsafeGetBase(), src.unsafeGetOffset() + srcFromOffset + offset, false);
final long d = SCOPED_MEMORY_ACCESS.getLongUnaligned(dst.sessionImpl(), dst.unsafeGetBase(), dst.unsafeGetOffset() + dstFromOffset + offset, false);
if (s != d) {
Expand All @@ -298,24 +298,24 @@ private static long mismatch(AbstractMemorySegmentImpl src, long srcFromOffset,
int remaining = length - offset;

// 0...0X00
if (remaining >= 4) {
if (remaining >= Integer.BYTES) {
final int s = SCOPED_MEMORY_ACCESS.getIntUnaligned(src.sessionImpl(), src.unsafeGetBase(), src.unsafeGetOffset() + srcFromOffset + offset, false);
final int d = SCOPED_MEMORY_ACCESS.getIntUnaligned(dst.sessionImpl(), dst.unsafeGetBase(), dst.unsafeGetOffset() + dstFromOffset + offset, false);
if (s != d) {
return start + offset + mismatch(s, d);
}
offset += 4;
remaining -= 4;
offset += Integer.BYTES;
remaining -= Integer.BYTES;
}
// 0...00X0
if (remaining >= 2) {
if (remaining >= Short.BYTES) {
final short s = SCOPED_MEMORY_ACCESS.getShortUnaligned(src.sessionImpl(), src.unsafeGetBase(), src.unsafeGetOffset() + srcFromOffset + offset, false);
final short d = SCOPED_MEMORY_ACCESS.getShortUnaligned(dst.sessionImpl(), dst.unsafeGetBase(), dst.unsafeGetOffset() + dstFromOffset + offset, false);
if (s != d) {
return start + offset + mismatch(s, d);
}
offset += 2;
remaining -= 2;
offset += Short.BYTES;
remaining -= Short.BYTES;
}
// 0...000X
if (remaining == 1) {
Expand Down
Loading