Skip to content

Commit 8fadf29

Browse files
author
Roger Riggs
committed
8351443: Improve robustness of StringBuilder
Reviewed-by: liach, rgiulietti, bchristi, jpai
1 parent 68a1185 commit 8fadf29

File tree

8 files changed

+942
-277
lines changed

8 files changed

+942
-277
lines changed

src/java.base/share/classes/java/lang/AbstractStringBuilder.java

Lines changed: 487 additions & 241 deletions
Large diffs are not rendered by default.

src/java.base/share/classes/java/lang/StringLatin1.java

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ public static boolean canEncode(int cp) {
5454
return cp >=0 && cp <= 0xff;
5555
}
5656

57+
public static byte coderFromChar(char cp) {
58+
return (byte)((0xff - cp) >>> (Integer.SIZE - 1));
59+
}
60+
5761
public static int length(byte[] value) {
5862
return value.length;
5963
}
@@ -709,6 +713,21 @@ static Stream<String> lines(byte[] value) {
709713
return StreamSupport.stream(LinesSpliterator.spliterator(value), false);
710714
}
711715

716+
public static void putCharsAt(byte[] value, int i, char c1, char c2, char c3, char c4) {
717+
value[i] = (byte)c1;
718+
value[i + 1] = (byte)c2;
719+
value[i + 2] = (byte)c3;
720+
value[i + 3] = (byte)c4;
721+
}
722+
723+
public static void putCharsAt(byte[] value, int i, char c1, char c2, char c3, char c4, char c5) {
724+
value[i] = (byte)c1;
725+
value[i + 1] = (byte)c2;
726+
value[i + 2] = (byte)c3;
727+
value[i + 3] = (byte)c4;
728+
value[i + 4] = (byte)c5;
729+
}
730+
712731
public static void putChar(byte[] val, int index, int c) {
713732
//assert (canEncode(c));
714733
val[index] = (byte)(c);
@@ -742,10 +761,6 @@ public static String newString(byte[] val, int index, int len) {
742761
LATIN1);
743762
}
744763

745-
public static void fillNull(byte[] val, int index, int end) {
746-
Arrays.fill(val, index, end, (byte)0);
747-
}
748-
749764
// inflatedCopy byte[] -> char[]
750765
@IntrinsicCandidate
751766
public static void inflate(byte[] src, int srcOff, char[] dst, int dstOff, int len) {

src/java.base/share/classes/java/lang/StringUTF16.java

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1331,10 +1331,6 @@ public static String newString(byte[] val, int index, int len) {
13311331
return new String(Arrays.copyOfRange(val, index << 1, last << 1), UTF16);
13321332
}
13331333

1334-
public static void fillNull(byte[] val, int index, int end) {
1335-
Arrays.fill(val, index << 1, end << 1, (byte)0);
1336-
}
1337-
13381334
static class CharsSpliterator implements Spliterator.OfInt {
13391335
private final byte[] array;
13401336
private int index; // current index, modified on advance/split
@@ -1532,27 +1528,23 @@ public static boolean contentEquals(byte[] value, CharSequence cs, int len) {
15321528
return true;
15331529
}
15341530

1535-
public static int putCharsAt(byte[] value, int i, char c1, char c2, char c3, char c4) {
1531+
public static void putCharsAt(byte[] value, int i, char c1, char c2, char c3, char c4) {
15361532
int end = i + 4;
15371533
checkBoundsBeginEnd(i, end, value);
1538-
putChar(value, i++, c1);
1539-
putChar(value, i++, c2);
1540-
putChar(value, i++, c3);
1541-
putChar(value, i++, c4);
1542-
assert(i == end);
1543-
return end;
1534+
putChar(value, i, c1);
1535+
putChar(value, i + 1, c2);
1536+
putChar(value, i + 2, c3);
1537+
putChar(value, i + 3, c4);
15441538
}
15451539

1546-
public static int putCharsAt(byte[] value, int i, char c1, char c2, char c3, char c4, char c5) {
1540+
public static void putCharsAt(byte[] value, int i, char c1, char c2, char c3, char c4, char c5) {
15471541
int end = i + 5;
15481542
checkBoundsBeginEnd(i, end, value);
1549-
putChar(value, i++, c1);
1550-
putChar(value, i++, c2);
1551-
putChar(value, i++, c3);
1552-
putChar(value, i++, c4);
1553-
putChar(value, i++, c5);
1554-
assert(i == end);
1555-
return end;
1543+
putChar(value, i, c1);
1544+
putChar(value, i + 1, c2);
1545+
putChar(value, i + 2, c3);
1546+
putChar(value, i + 3, c4);
1547+
putChar(value, i + 4, c5);
15561548
}
15571549

15581550
public static char charAt(byte[] value, int index) {

test/hotspot/jtreg/compiler/patches/java.base/java/lang/Helper.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2016, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -140,12 +140,12 @@ public static boolean contentEquals(byte[] value, CharSequence cs, int len) {
140140
return StringUTF16.contentEquals(value, cs, len);
141141
}
142142

143-
public static int putCharsAt(byte[] value, int i, char c1, char c2, char c3, char c4) {
144-
return StringUTF16.putCharsAt(value, i, c1, c2, c3, c4);
143+
public static void putCharsAt(byte[] value, int i, char c1, char c2, char c3, char c4) {
144+
StringUTF16.putCharsAt(value, i, c1, c2, c3, c4);
145145
}
146146

147-
public static int putCharsAt(byte[] value, int i, char c1, char c2, char c3, char c4, char c5) {
148-
return StringUTF16.putCharsAt(value, i, c1, c2, c3, c4, c5);
147+
public static void putCharsAt(byte[] value, int i, char c1, char c2, char c3, char c4, char c5) {
148+
StringUTF16.putCharsAt(value, i, c1, c2, c3, c4, c5);
149149
}
150150

151151
public static char charAt(byte[] value, int index) {

test/jdk/java/lang/StringBuilder/CompactStringBuilder.java

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -30,11 +30,12 @@
3030

3131
/*
3232
* @test
33-
* @bug 8054307 8077559
33+
* @bug 8054307 8077559 8351443
3434
* @summary Tests Compact String. This test is testing StringBuilder
3535
* behavior related to Compact String.
3636
* @run testng/othervm -XX:+CompactStrings CompactStringBuilder
3737
* @run testng/othervm -XX:-CompactStrings CompactStringBuilder
38+
* @run testng/othervm -Xcomp CompactStringBuilder
3839
*/
3940

4041
public class CompactStringBuilder {
@@ -54,6 +55,10 @@ public void testCompactStringBuilderForLatinA() {
5455
check(new StringBuilder(ORIGIN).append(new StringBuffer("\uFF21")),
5556
"A\uFF21");
5657
check(new StringBuilder(ORIGIN).append("\uFF21"), "A\uFF21");
58+
check(new StringBuilder(ORIGIN).append(new char[] { 'a', 'b', 'c'}),
59+
"Aabc");
60+
check(new StringBuilder(ORIGIN).append(new char[] { 'a', 'b', 'c'}, 1, 2),
61+
"Abc");
5762
check(new StringBuilder(ORIGIN).append(new StringBuffer("\uFF21")),
5863
"A\uFF21");
5964
check(new StringBuilder(ORIGIN).delete(0, 1), "");
@@ -404,6 +409,54 @@ public void testCompactStringForMaybeLatin1() {
404409
check(new StringBuilder().append(sb), "AB");
405410
}
406411

412+
// Test cases to force expanding the capacity during replace.
413+
// Start with a known capacity and a known initial value that almost fills it.
414+
// Use both latin1 and utf16 initial values and replacement values.
415+
// Iterate through cases of SB.replace start and end values and various lengths of replacement.
416+
// The results are checked by composing the new string using concatenation of the segments.
417+
@Test
418+
public void testGrowingCapacityReplace() {
419+
final int INIT_CAPACITY = 8;
420+
final String[] INITIAL_CHARS = {"A", "\u0100"};
421+
final String[] REPLACEMENT_CHARS = {"B", "\u0101"};
422+
for (String INITIAL : INITIAL_CHARS) {
423+
for (String REPLACEMENT : REPLACEMENT_CHARS) {
424+
for (int initLen = INIT_CAPACITY - 1; initLen < INIT_CAPACITY + 2; initLen++) {
425+
final String orig = INITIAL.repeat(initLen);
426+
for (int start = INIT_CAPACITY - 4; start < orig.length(); start++) {
427+
for (int end = start; end < orig.length(); end++) {
428+
for (int insLen = 0; insLen < 2; insLen++) {
429+
final String repl = REPLACEMENT.repeat(insLen);
430+
var sb = new StringBuilder(INIT_CAPACITY)
431+
.append(orig);
432+
int capBefore = sb.capacity();
433+
sb.replace(start, end, repl);
434+
int capAfter = sb.capacity();
435+
String expected = genReplacementString(orig, start, end, repl);
436+
try {
437+
check(sb, expected);
438+
} catch (Throwable ex) {
439+
System.out.printf("repl: \"%s\", actual: %s, expected: %s%n", repl, sb, expected);
440+
System.out.printf(" insLen: %d, gap: %d, beforeLen: %d, afterLen: %d, capBefore: %d, capAfter: %d%n",
441+
insLen, (end - start), orig.length(), sb.length(),
442+
capBefore, capAfter);
443+
throw ex;
444+
}
445+
}
446+
}
447+
}
448+
}
449+
}
450+
}
451+
}
452+
453+
// Construct the replacement string using string concat of the segments
454+
private static String genReplacementString(String orig, int start, int end, String repl) {
455+
return orig.substring(0, start) +
456+
repl +
457+
orig.substring(end, orig.length());
458+
}
459+
407460
private void checkGetChars(StringBuilder sb, int srcBegin, int srcEnd,
408461
char expected[]) {
409462
char[] dst = new char[srcEnd - srcBegin];

test/jdk/java/lang/StringBuilder/HugeCapacity.java

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2016, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2016, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -23,9 +23,11 @@
2323

2424
import jdk.internal.util.ArraysSupport;
2525

26+
import java.util.Arrays;
27+
2628
/**
2729
* @test
28-
* @bug 8149330 8218227
30+
* @bug 8149330 8218227 8351443
2931
* @summary Capacity should not get close to Integer.MAX_VALUE unless
3032
* necessary
3133
* @modules java.base/jdk.internal.util
@@ -47,6 +49,7 @@ public static void main(String[] args) {
4749
testUtf16();
4850
testHugeInitialString();
4951
testHugeInitialCharSequence();
52+
testHugePlus(isCompact);
5053
if (failures > 0) {
5154
throw new RuntimeException(failures + " tests failed");
5255
}
@@ -108,4 +111,23 @@ public CharSequence subSequence(int st, int e) {
108111
}
109112
public String toString() { return ""; }
110113
}
114+
115+
// Test creating and appending the max size string for -XX:+CompactStrings and -XX:-CompactStrings
116+
private static void testHugePlus(boolean isCompact) {
117+
int repeatCount = (isCompact) ? Integer.MAX_VALUE / 2 - 1 : Integer.MAX_VALUE / 4 - 1;
118+
char[] chars = new char[repeatCount];
119+
char[] aChar = {'A', '\uff21'};
120+
for (char ch : aChar) {
121+
try {
122+
int size = (ch > 0xff) ? repeatCount / 2 : repeatCount;
123+
Arrays.fill(chars, 0, size, ch);
124+
StringBuilder b = new StringBuilder(0);
125+
b.append(chars, 0, size);
126+
b.append(chars, 0, size);
127+
} catch (Throwable unexpected) {
128+
unexpected.printStackTrace();
129+
failures++;
130+
}
131+
}
132+
}
111133
}

0 commit comments

Comments
 (0)