8345120: A likely bug in StringSupport::chunkedStrlenShort

Reviewed-by: mcimadamore

src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java

@@ -889,23 +889,27 @@ public abstract sealed class AbstractMemorySegmentImpl
         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);

src/java.base/share/classes/jdk/internal/foreign/SegmentBulkOperations.java

@@ -28,7 +28,6 @@ package jdk.internal.foreign;
 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;
 
@@ -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.
@@ -75,21 +75,21 @@ public final class SegmentBulkOperations {
             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;
+                offset += Integer.BYTES;
-                remaining -= 4;
+                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;
+                offset += Short.BYTES;
-                remaining -= 2;
+                remaining -= Short.BYTES;
             }
             // 0...000X
             if (remaining == 1) {
@@ -123,26 +123,26 @@ public final class SegmentBulkOperations {
             // 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;
+                offset += Integer.BYTES;
-                remaining -= 4;
+                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;
+                offset += Short.BYTES;
-                remaining -=2;
+                remaining -= Short.BYTES;
             }
             // 0...000X
             if (remaining == 1) {
@@ -202,9 +202,9 @@ public final class SegmentBulkOperations {
             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]
@@ -218,24 +218,24 @@ public final class SegmentBulkOperations {
         }
         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;
+            fromOffset += Integer.BYTES;
-            remaining -= 4;
+            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;
+            fromOffset += Short.BYTES;
-            remaining -= 2;
+            remaining -= Short.BYTES;
         }
         // 0...000X
         if (remaining == 1) {
@@ -288,7 +288,7 @@ public final class SegmentBulkOperations {
                                  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) {
@@ -298,24 +298,24 @@ public final class SegmentBulkOperations {
         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;
+            offset += Integer.BYTES;
-            remaining -= 4;
+            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;
+            offset += Short.BYTES;
-            remaining -= 2;
+            remaining -= Short.BYTES;
         }
         // 0...000X
         if (remaining == 1) {

src/java.base/share/classes/jdk/internal/foreign/StringSupport.java

@@ -27,8 +27,10 @@ package jdk.internal.foreign;
 
 import jdk.internal.access.JavaLangAccess;
 import jdk.internal.access.SharedSecrets;
-import jdk.internal.foreign.abi.SharedUtils;
+import jdk.internal.misc.ScopedMemoryAccess;
+import jdk.internal.util.Architecture;
 import jdk.internal.util.ArraysSupport;
+import jdk.internal.vm.annotation.ForceInline;
 
 import java.lang.foreign.MemorySegment;
 import java.nio.charset.Charset;
@@ -40,11 +42,14 @@ import static java.lang.foreign.ValueLayout.*;
  */
 public final class StringSupport {
 
-    static final JavaLangAccess JAVA_LANG_ACCESS = SharedSecrets.getJavaLangAccess();
+    private static final JavaLangAccess JAVA_LANG_ACCESS = SharedSecrets.getJavaLangAccess();
+    private static final ScopedMemoryAccess SCOPED_MEMORY_ACCESS = ScopedMemoryAccess.getScopedMemoryAccess();
+    private static final long LONG_MASK = ~7L; // The last three bits are zero
 
     private StringSupport() {}
 
-    public static String read(MemorySegment segment, long offset, Charset charset) {
+    @ForceInline
+    public static String read(AbstractMemorySegmentImpl segment, long offset, Charset charset) {
         return switch (CharsetKind.of(charset)) {
             case SINGLE_BYTE -> readByte(segment, offset, charset);
             case DOUBLE_BYTE -> readShort(segment, offset, charset);
@@ -52,7 +57,8 @@ public final class StringSupport {
         };
     }
 
-    public static void write(MemorySegment segment, long offset, Charset charset, String string) {
+    @ForceInline
+    public static void write(AbstractMemorySegmentImpl segment, long offset, Charset charset, String string) {
         switch (CharsetKind.of(charset)) {
             case SINGLE_BYTE -> writeByte(segment, offset, charset, string);
             case DOUBLE_BYTE -> writeShort(segment, offset, charset, string);
@@ -60,111 +66,183 @@ public final class StringSupport {
         }
     }
 
-    private static String readByte(MemorySegment segment, long offset, Charset charset) {
+    @ForceInline
-        long len = chunkedStrlenByte(segment, offset);
+    private static String readByte(AbstractMemorySegmentImpl segment, long offset, Charset charset) {
-        byte[] bytes = new byte[(int)len];
+        final int len = strlenByte(segment, offset, segment.byteSize());
-        MemorySegment.copy(segment, JAVA_BYTE, offset, bytes, 0, (int)len);
+        final byte[] bytes = new byte[len];
+        MemorySegment.copy(segment, JAVA_BYTE, offset, bytes, 0, len);
         return new String(bytes, charset);
     }
 
-    private static void writeByte(MemorySegment segment, long offset, Charset charset, String string) {
+    @ForceInline
+    private static void writeByte(AbstractMemorySegmentImpl segment, long offset, Charset charset, String string) {
         int bytes = copyBytes(string, segment, charset, offset);
         segment.set(JAVA_BYTE, offset + bytes, (byte)0);
     }
 
-    private static String readShort(MemorySegment segment, long offset, Charset charset) {
+    @ForceInline
-        long len = chunkedStrlenShort(segment, offset);
+    private static String readShort(AbstractMemorySegmentImpl segment, long offset, Charset charset) {
-        byte[] bytes = new byte[(int)len];
+        int len = strlenShort(segment, offset, segment.byteSize());
-        MemorySegment.copy(segment, JAVA_BYTE, offset, bytes, 0, (int)len);
+        byte[] bytes = new byte[len];
+        MemorySegment.copy(segment, JAVA_BYTE, offset, bytes, 0, len);
         return new String(bytes, charset);
     }
 
-    private static void writeShort(MemorySegment segment, long offset, Charset charset, String string) {
+    @ForceInline
+    private static void writeShort(AbstractMemorySegmentImpl segment, long offset, Charset charset, String string) {
         int bytes = copyBytes(string, segment, charset, offset);
         segment.set(JAVA_SHORT_UNALIGNED, offset + bytes, (short)0);
     }
 
-    private static String readInt(MemorySegment segment, long offset, Charset charset) {
+    @ForceInline
-        long len = strlenInt(segment, offset);
+    private static String readInt(AbstractMemorySegmentImpl segment, long offset, Charset charset) {
-        byte[] bytes = new byte[(int)len];
+        int len = strlenInt(segment, offset, segment.byteSize());
-        MemorySegment.copy(segment, JAVA_BYTE, offset, bytes, 0, (int)len);
+        byte[] bytes = new byte[len];
+        MemorySegment.copy(segment, JAVA_BYTE, offset, bytes, 0, len);
         return new String(bytes, charset);
     }
 
-    private static void writeInt(MemorySegment segment, long offset, Charset charset, String string) {
+    @ForceInline
+    private static void writeInt(AbstractMemorySegmentImpl segment, long offset, Charset charset, String string) {
         int bytes = copyBytes(string, segment, charset, offset);
         segment.set(JAVA_INT_UNALIGNED, offset + bytes, 0);
     }
 
     /**
-     * {@return the shortest distance beginning at the provided {@code start}
+     * {@return the index of the first zero byte beginning at the provided
-     *  to the encountering of a zero byte in the provided {@code segment}}
+     *          {@code fromOffset} to the encountering of a zero byte in the provided
+     *          {@code segment} checking bytes before the {@code toOffset}}
      * <p>
-     * The method divides the region of interest into three distinct regions:
+     * The method is using a heuristic method to determine if a long word contains a
-     * <ul>
+     * zero byte. The method might have false positives but never false negatives.
-     *     <li>head (access made on a byte-by-byte basis) (if any)</li>
-     *     <li>body (access made with eight bytes at a time at physically 64-bit-aligned memory) (if any)</li>
-     *     <li>tail (access made on a byte-by-byte basis) (if any)</li>
-     * </ul>
-     * <p>
-     * The body is using a heuristic method to determine if a long word
-     * contains a zero byte. The method might have false positives but
-     * never false negatives.
      * <p>
      * This method is inspired by the `glibc/string/strlen.c` implementation
      *
-     * @param segment to examine
+     * @param segment    to examine
-     * @param start   from where examination shall begin
+     * @param fromOffset from where examination shall begin (inclusive)
+     * @param toOffset   to where examination shall end (exclusive)
      * @throws IllegalArgumentException if the examined region contains no zero bytes
      *                                  within a length that can be accepted by a String
      */
-    public static int chunkedStrlenByte(MemorySegment segment, long start) {
+    @ForceInline
-
+    public static int strlenByte(final AbstractMemorySegmentImpl segment,
-        // Handle the first unaligned "head" bytes separately
+                                 final long fromOffset,
-        int headCount = (int)SharedUtils.remainsToAlignment(segment.address() + start, Long.BYTES);
+                                 final long toOffset) {
-
+        final long length = toOffset - fromOffset;
-        int offset = 0;
+        segment.checkBounds(fromOffset, length);
-        for (; offset < headCount; offset++) {
+        if (length == 0) {
-            byte curr = segment.get(JAVA_BYTE, start + offset);
+            // The state has to be checked explicitly for zero-length segments
-            if (curr == 0) {
+            segment.scope.checkValidState();
-                return offset;
+            throw nullNotFound(segment, fromOffset, toOffset);
-            }
         }
-
+        final long longBytes = length & LONG_MASK;
-        // We are now on a long-aligned boundary so this is the "body"
+        final long longLimit = fromOffset + longBytes;
-        int bodyCount = bodyCount(segment.byteSize() - start - headCount);
+        long offset = fromOffset;
-
+        for (; offset < longLimit; offset += Long.BYTES) {
-        for (; offset < bodyCount; offset += Long.BYTES) {
+            long val = SCOPED_MEMORY_ACCESS.getLongUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + offset, !Architecture.isLittleEndian());
-            // We know we are `long` aligned so, we can save on alignment checking here
+            if (mightContainZeroByte(val)) {
-            long curr = segment.get(JAVA_LONG_UNALIGNED, start + offset);
+                for (int j = 0; j < Long.BYTES; j++) {
-            // Is this a candidate?
+                    if (SCOPED_MEMORY_ACCESS.getByte(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + offset + j) == 0) {
-            if (mightContainZeroByte(curr)) {
+                        return requireWithinStringSize(offset + j - fromOffset, segment, fromOffset, toOffset);
-                for (int j = 0; j < 8; j++) {
-                    if (segment.get(JAVA_BYTE, start + offset + j) == 0) {
-                        return offset + j;
                     }
                 }
             }
         }
-
+        // Handle the tail
-        // Handle the "tail"
+        for (; offset < toOffset; offset++) {
-        return requireWithinArraySize((long) offset + strlenByte(segment, start + offset));
+            byte val = SCOPED_MEMORY_ACCESS.getByte(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + offset);
+            if (val == 0) {
+                return requireWithinStringSize(offset - fromOffset, segment, fromOffset, toOffset);
+            }
+        }
+        throw nullNotFound(segment, fromOffset, toOffset);
     }
 
-    /* Bits 63 and N * 8 (N = 1..7) of this number are zero.  Call these bits
+    @ForceInline
-       the "holes".  Note that there is a hole just to the left of
+    public static int strlenShort(final AbstractMemorySegmentImpl segment,
-       each byte, with an extra at the end:
+                                  final long fromOffset,
+                                  final long toOffset) {
+        final long length = toOffset - fromOffset;
+        segment.checkBounds(fromOffset, length);
+        if (length == 0) {
+            segment.scope.checkValidState();
+            throw nullNotFound(segment, fromOffset, toOffset);
+        }
+        final long longBytes = length & LONG_MASK;
+        final long longLimit = fromOffset + longBytes;
+        long offset = fromOffset;
+        for (; offset < longLimit; offset += Long.BYTES) {
+            long val = SCOPED_MEMORY_ACCESS.getLongUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + offset, !Architecture.isLittleEndian());
+            if (mightContainZeroShort(val)) {
+                for (int j = 0; j < Long.BYTES; j += Short.BYTES) {
+                    if (SCOPED_MEMORY_ACCESS.getShortUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + offset + j, !Architecture.isLittleEndian()) == 0) {
+                        return requireWithinStringSize(offset + j - fromOffset, segment, fromOffset, toOffset);
+                    }
+                }
+            }
+        }
+        // Handle the tail
+        // Prevent over scanning as we step by 2
+        final long endScan = toOffset & ~1; // The last bit is zero
+        for (; offset < endScan; offset += Short.BYTES) {
+            short val = SCOPED_MEMORY_ACCESS.getShortUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + offset, !Architecture.isLittleEndian());
+            if (val == 0) {
+                return requireWithinStringSize(offset - fromOffset, segment, fromOffset, toOffset);
+            }
+        }
+        throw nullNotFound(segment, fromOffset, toOffset);
+    }
 
-       bits:  01111110 11111110 11111110 11111110 11111110 11111110 11111110 11111111
+    @ForceInline
-       bytes: AAAAAAAA BBBBBBBB CCCCCCCC DDDDDDDD EEEEEEEE FFFFFFFF GGGGGGGG HHHHHHHH
+    public static int strlenInt(final AbstractMemorySegmentImpl segment,
+                                final long fromOffset,
+                                final long toOffset) {
+        final long length = toOffset - fromOffset;
+        segment.checkBounds(fromOffset, length);
+        if (length == 0) {
+            segment.scope.checkValidState();
+            throw nullNotFound(segment, fromOffset, toOffset);
+        }
+        final long longBytes = length & LONG_MASK;
+        final long longLimit = fromOffset + longBytes;
+        long offset = fromOffset;
+        for (; offset < longLimit; offset += Long.BYTES) {
+            long val = SCOPED_MEMORY_ACCESS.getLongUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + offset, !Architecture.isLittleEndian());
+            if (mightContainZeroInt(val)) {
+                for (int j = 0; j < Long.BYTES; j += Integer.BYTES) {
+                    if (SCOPED_MEMORY_ACCESS.getIntUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + offset + j, !Architecture.isLittleEndian()) == 0) {
+                        return requireWithinStringSize(offset + j - fromOffset, segment, fromOffset, toOffset);
+                    }
+                }
+            }
+        }
+        // Handle the tail
+        // Prevent over scanning as we step by 4
+        final long endScan = toOffset & ~3; // The last two bit are zero
+        for (; offset < endScan; offset += Integer.BYTES) {
+            int val = SCOPED_MEMORY_ACCESS.getIntUnaligned(segment.sessionImpl(), segment.unsafeGetBase(), segment.unsafeGetOffset() + offset, !Architecture.isLittleEndian());
+            if (val == 0) {
+                return requireWithinStringSize(offset - fromOffset, segment, fromOffset, toOffset);
+            }
+        }
+        throw nullNotFound(segment, fromOffset, toOffset);
+    }
 
-       The 1-bits make sure that carries propagate to the next 0-bit.
+    /*
-       The 0-bits provide holes for carries to fall into.
+    Bits 63 and N * 8 (N = 1..7) of this number are zero.  Call these bits
+    the "holes".  Note that there is a hole just to the left of
+    each byte, with an extra at the end:
+
+    bits:  01111110 11111110 11111110 11111110 11111110 11111110 11111110 11111111
+    bytes: AAAAAAAA BBBBBBBB CCCCCCCC DDDDDDDD EEEEEEEE FFFFFFFF GGGGGGGG HHHHHHHH
+
+    The 1-bits make sure that carries propagate to the next 0-bit.
+    The 0-bits provide holes for carries to fall into.
     */
     private static final long HIMAGIC_FOR_BYTES = 0x8080_8080_8080_8080L;
     private static final long LOMAGIC_FOR_BYTES = 0x0101_0101_0101_0101L;
 
-    static boolean mightContainZeroByte(long l) {
+    private static boolean mightContainZeroByte(long l) {
         return ((l - LOMAGIC_FOR_BYTES) & (~l) & HIMAGIC_FOR_BYTES) != 0;
     }
 
@@ -175,99 +253,40 @@ public final class StringSupport {
         return ((l - LOMAGIC_FOR_SHORTS) & (~l) & HIMAGIC_FOR_SHORTS) != 0;
     }
 
-    static int requireWithinArraySize(long size) {
+    private static final long HIMAGIC_FOR_INTS = 0x8000_0000_8000_0000L;
+    private static final long LOMAGIC_FOR_INTS = 0x0000_0001_0000_0001L;
+
+    static boolean mightContainZeroInt(long l) {
+        return ((l - LOMAGIC_FOR_INTS) & (~l) & HIMAGIC_FOR_INTS) != 0;
+    }
+
+
+    private static int requireWithinStringSize(long size,
+                                               AbstractMemorySegmentImpl segment,
+                                               long fromOffset,
+                                               long toOffset) {
         if (size > ArraysSupport.SOFT_MAX_ARRAY_LENGTH) {
-            throw newIaeStringTooLarge();
+            throw stringTooLarge(segment, fromOffset, toOffset);
         }
         return (int) size;
     }
 
-    static int bodyCount(long remaining) {
+    private static IllegalArgumentException stringTooLarge(AbstractMemorySegmentImpl segment,
-        return (int) Math.min(
+                                                           long fromOffset,
-                // Make sure we do not wrap around
+                                                           long toOffset) {
-                Integer.MAX_VALUE - Long.BYTES,
+        return new IllegalArgumentException("String too large: " + exceptionInfo(segment, fromOffset, toOffset));
-                // Remaining bytes to consider
-                remaining)
-                & -Long.BYTES; // Mask 0xFFFFFFF8
     }
 
-    private static int strlenByte(MemorySegment segment, long start) {
+    private static IndexOutOfBoundsException nullNotFound(AbstractMemorySegmentImpl segment,
-        for (int offset = 0; offset < ArraysSupport.SOFT_MAX_ARRAY_LENGTH; offset += 1) {
+                                                          long fromOffset,
-            byte curr = segment.get(JAVA_BYTE, start + offset);
+                                                          long toOffset) {
-            if (curr == 0) {
+        return new IndexOutOfBoundsException("No null terminator found: " + exceptionInfo(segment, fromOffset, toOffset));
-                return offset;
-            }
-        }
-        throw newIaeStringTooLarge();
     }
 
-    /**
+    private static String exceptionInfo(AbstractMemorySegmentImpl segment,
-     * {@return the shortest distance beginning at the provided {@code start}
+                                        long fromOffset,
-     *  to the encountering of a zero short in the provided {@code segment}}
+                                        long toOffset) {
-     * <p>
+        return segment + " using region [" + fromOffset + ", " + toOffset + ")";
-     * Note: The inspected region must be short aligned.
-     *
-     * @see #chunkedStrlenByte(MemorySegment, long) for more information
-     *
-     * @param segment to examine
-     * @param start   from where examination shall begin
-     * @throws IllegalArgumentException if the examined region contains no zero shorts
-     *                                  within a length that can be accepted by a String
-     */
-    public static int chunkedStrlenShort(MemorySegment segment, long start) {
-
-        // Handle the first unaligned "head" bytes separately
-        int headCount = (int)SharedUtils.remainsToAlignment(segment.address() + start, Long.BYTES);
-
-        int offset = 0;
-        for (; offset < headCount; offset += Short.BYTES) {
-            short curr = segment.get(JAVA_SHORT_UNALIGNED, start + offset);
-            if (curr == 0) {
-                return offset;
-            }
-        }
-
-        // We are now on a long-aligned boundary so this is the "body"
-        int bodyCount = bodyCount(segment.byteSize() - start - headCount);
-
-        for (; offset < bodyCount; offset += Long.BYTES) {
-            // We know we are `long` aligned so, we can save on alignment checking here
-            long curr = segment.get(JAVA_LONG_UNALIGNED, start + offset);
-            // Is this a candidate?
-            if (mightContainZeroShort(curr)) {
-                for (int j = 0; j < Long.BYTES; j += Short.BYTES) {
-                    if (segment.get(JAVA_SHORT_UNALIGNED, start + offset + j) == 0) {
-                        return offset + j;
-                    }
-                }
-            }
-        }
-
-        // Handle the "tail"
-        return requireWithinArraySize((long) offset + strlenShort(segment, start + offset));
-    }
-
-    private static int strlenShort(MemorySegment segment, long start) {
-        for (int offset = 0; offset < ArraysSupport.SOFT_MAX_ARRAY_LENGTH; offset += Short.BYTES) {
-            short curr = segment.get(JAVA_SHORT_UNALIGNED, start + offset);
-            if (curr == (short)0) {
-                return offset;
-            }
-        }
-        throw newIaeStringTooLarge();
-    }
-
-    // The gain of using `long` wide operations for `int` is lower than for the two other `byte` and `short` variants
-    // so, there is only one method for ints.
-    public static int strlenInt(MemorySegment segment, long start) {
-        for (int offset = 0; offset < ArraysSupport.SOFT_MAX_ARRAY_LENGTH; offset += Integer.BYTES) {
-            // We are guaranteed to be aligned here so, we can use unaligned access.
-            int curr = segment.get(JAVA_INT_UNALIGNED, start + offset);
-            if (curr == 0) {
-                return offset;
-            }
-        }
-        throw newIaeStringTooLarge();
     }
 
     public enum CharsetKind {
@@ -323,9 +342,4 @@ public final class StringSupport {
     public static void copyToSegmentRaw(String string, MemorySegment segment, long offset) {
         JAVA_LANG_ACCESS.copyToSegmentRaw(string, segment, offset);
     }
-
-    private static IllegalArgumentException newIaeStringTooLarge() {
-        return new IllegalArgumentException("String too large");
-    }
-
 }

test/jdk/java/foreign/TestStringEncoding.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
  *  DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  *  This code is free software; you can redistribute it and/or modify it
@@ -39,6 +39,7 @@ import java.util.List;
 import java.util.Random;
 import java.util.function.UnaryOperator;
 
+import jdk.internal.foreign.AbstractMemorySegmentImpl;
 import jdk.internal.foreign.StringSupport;
 import org.testng.annotations.*;
 
@@ -53,6 +54,20 @@ import static org.testng.Assert.*;
 
 public class TestStringEncoding {
 
+    @Test
+    public void emptySegment() {
+        for (Charset charset : standardCharsets()) {
+            for (Arena arena : arenas()) {
+                try (arena) {
+                    var segment = arena.allocate(0);
+                    var e = expectThrows(IndexOutOfBoundsException.class, () ->
+                            segment.getString(0, charset));
+                    assertTrue(e.getMessage().contains("No null terminator found"));
+                }
+            }
+        }
+    }
+
     @Test(dataProvider = "strings")
     public void testStrings(String testString) {
         for (Charset charset : Charset.availableCharsets().values()) {
@@ -87,7 +102,6 @@ public class TestStringEncoding {
         }
     }
 
-
     @Test(dataProvider = "strings")
     public void testStringsHeap(String testString) {
         for (Charset charset : singleByteCharsets()) {
@@ -198,8 +212,9 @@ public class TestStringEncoding {
                 try (arena) {
                     MemorySegment inSegment = arena.allocateFrom(testString, charset);
                     for (int i = 0; i < 3; i++) {
+                        String expected = testString.substring(i);
                         String actual = inSegment.getString(i, charset);
-                        assertEquals(actual, testString.substring(i));
+                        assertEquals(actual, expected);
                     }
                 }
             }
@@ -249,6 +264,32 @@ public class TestStringEncoding {
         }
     }
 
+    // This test ensures that we do not address outside the segment even though there
+    // are odd bytes at the end.
+    @Test(dataProvider = "strings")
+    public void offBoundaryTrailingBytes(String testString) {
+        if (testString.length() < 3 || !containsOnlyRegularCharacters(testString)) {
+            return;
+        }
+        for (var charset : standardCharsets()) {
+            for (var arena: arenas()) {
+                try (arena) {
+                    MemorySegment strSegment = arena.allocateFrom(testString, charset);
+                    // Add an odd byte at the end
+                    MemorySegment inSegment = arena.allocate(strSegment.byteSize() + 1);
+                    // Make sure there are no null-terminators so that we will try to scan
+                    // the entire segment.
+                    inSegment.fill((byte) 1);
+                    for (int i = 0; i < 4; i++) {
+                        final int offset = i;
+                        var e = expectThrows(IndexOutOfBoundsException.class, () -> inSegment.getString(offset, charset));
+                        assertTrue(e.getMessage().contains("No null terminator found"));
+                    }
+                }
+            }
+        }
+    }
+
     private static final int TEST_LENGTH_MAX = 277;
 
     private Random deterministicRandom() {
@@ -271,9 +312,15 @@ public class TestStringEncoding {
                     }
                     segment.setAtIndex(JAVA_BYTE, len, (byte) 0);
                     for (int j = 0; j < len; j++) {
-                        int actual = StringSupport.chunkedStrlenByte(segment, j);
+                        int actual = StringSupport.strlenByte((AbstractMemorySegmentImpl) segment, j, segment.byteSize());
                         assertEquals(actual, len - j);
                     }
+                    // Test end offset
+                    for (int j = 0; j < len - 1; j++) {
+                        final long toOffset = j;
+                        expectThrows(IndexOutOfBoundsException.class, () ->
+                                StringSupport.strlenByte((AbstractMemorySegmentImpl) segment, 0, toOffset));
+                    }
                 }
             }
         }
@@ -295,7 +342,7 @@ public class TestStringEncoding {
                     }
                     segment.setAtIndex(JAVA_SHORT, len, (short) 0);
                     for (int j = 0; j < len; j++) {
-                        int actual = StringSupport.chunkedStrlenShort(segment, j * Short.BYTES);
+                        int actual = StringSupport.strlenShort((AbstractMemorySegmentImpl) segment, j * Short.BYTES, segment.byteSize());
                         assertEquals(actual, (len - j) * Short.BYTES);
                     }
                 }
@@ -319,7 +366,7 @@ public class TestStringEncoding {
                     }
                     segment.setAtIndex(JAVA_INT, len, 0);
                     for (int j = 0; j < len; j++) {
-                        int actual = StringSupport.strlenInt(segment, j * Integer.BYTES);
+                        int actual = StringSupport.strlenInt((AbstractMemorySegmentImpl) segment, j * Integer.BYTES, segment.byteSize());
                         assertEquals(actual, (len - j) * Integer.BYTES);
                     }
                 }

test/micro/org/openjdk/bench/java/lang/foreign/InternalStrLen.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -22,6 +22,8 @@
  */
 package org.openjdk.bench.java.lang.foreign;
 
+import jdk.internal.foreign.AbstractMemorySegmentImpl;
+import jdk.internal.foreign.StringSupport;
 import org.openjdk.jmh.annotations.Benchmark;
 import org.openjdk.jmh.annotations.BenchmarkMode;
 import org.openjdk.jmh.annotations.Fork;
@@ -43,20 +45,20 @@ import java.util.stream.IntStream;
 import java.util.stream.Stream;
 
 import static java.lang.foreign.ValueLayout.*;
-import static jdk.internal.foreign.StringSupport.*;
 
 @BenchmarkMode(Mode.AverageTime)
 @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
 @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
 @State(Scope.Benchmark)
 @OutputTimeUnit(TimeUnit.NANOSECONDS)
-@Fork(value = 3, jvmArgs = {"--add-exports=java.base/jdk.internal.foreign=ALL-UNNAMED", "--enable-native-access=ALL-UNNAMED", "--enable-preview"})
+@Fork(value = 3, jvmArgs = {"--add-exports=java.base/jdk.internal.foreign=ALL-UNNAMED",
+                            "--enable-native-access=ALL-UNNAMED"})
 public class InternalStrLen {
 
-    private MemorySegment singleByteSegment;
+    private AbstractMemorySegmentImpl singleByteSegment;
-    private MemorySegment singleByteSegmentMisaligned;
+    private AbstractMemorySegmentImpl singleByteSegmentMisaligned;
-    private MemorySegment doubleByteSegment;
+    private AbstractMemorySegmentImpl doubleByteSegment;
-    private MemorySegment quadByteSegment;
+    private AbstractMemorySegmentImpl quadByteSegment;
 
     @Param({"1", "4", "16", "251", "1024"})
     int size;
@@ -64,10 +66,9 @@ public class InternalStrLen {
     @Setup
     public void setup() {
         var arena = Arena.ofAuto();
-        singleByteSegment = arena.allocate((size + 1L) * Byte.BYTES);
+        singleByteSegment = (AbstractMemorySegmentImpl) arena.allocate((size + 1L) * Byte.BYTES);
-        singleByteSegmentMisaligned = arena.allocate((size + 1L) * Byte.BYTES);
+        doubleByteSegment = (AbstractMemorySegmentImpl) arena.allocate((size + 1L) * Short.BYTES);
-        doubleByteSegment = arena.allocate((size + 1L) * Short.BYTES);
+        quadByteSegment = (AbstractMemorySegmentImpl) arena.allocate((size + 1L) * Integer.BYTES);
-        quadByteSegment = arena.allocate((size + 1L) * Integer.BYTES);
         Stream.of(singleByteSegment, doubleByteSegment, quadByteSegment)
                 .forEach(s -> IntStream.range(0, (int) s.byteSize() - 1)
                         .forEach(i -> s.set(
@@ -78,7 +79,7 @@ public class InternalStrLen {
         singleByteSegment.set(ValueLayout.JAVA_BYTE, singleByteSegment.byteSize() - Byte.BYTES, (byte) 0);
         doubleByteSegment.set(ValueLayout.JAVA_SHORT, doubleByteSegment.byteSize() - Short.BYTES, (short) 0);
         quadByteSegment.set(ValueLayout.JAVA_INT, quadByteSegment.byteSize() - Integer.BYTES, 0);
-        singleByteSegmentMisaligned = arena.allocate(singleByteSegment.byteSize() + 1).
+        singleByteSegmentMisaligned = (AbstractMemorySegmentImpl) arena.allocate(singleByteSegment.byteSize() + 1).
                 asSlice(1);
         MemorySegment.copy(singleByteSegment, 0, singleByteSegmentMisaligned, 0, singleByteSegment.byteSize());
     }
@@ -105,22 +106,22 @@ public class InternalStrLen {
 
     @Benchmark
     public int chunkedSingle() {
-        return chunkedStrlenByte(singleByteSegment, 0);
+        return StringSupport.strlenByte(singleByteSegment, 0, singleByteSegment.byteSize());
     }
 
     @Benchmark
     public int chunkedSingleMisaligned() {
-        return chunkedStrlenByte(singleByteSegmentMisaligned, 0);
+        return StringSupport.strlenByte(singleByteSegmentMisaligned, 0, singleByteSegment.byteSize());
     }
 
     @Benchmark
     public int chunkedDouble() {
-        return chunkedStrlenShort(doubleByteSegment, 0);
+        return StringSupport.strlenShort(doubleByteSegment, 0, doubleByteSegment.byteSize());
     }
 
     @Benchmark
     public int changedElementQuad() {
-        return strlenInt(quadByteSegment, 0);
+        return StringSupport.strlenInt(quadByteSegment, 0, quadByteSegment.byteSize());
     }
 
     // These are the legacy methods