Skip to content

Commit 23c1933

Browse files
authored
[stdlib] Fix String.reserveCapacity underallocation (#65902)
When called on a string that is not uniquely referenced, `String.reserveCapacity(_:)` ignores the current capacity, using the passed-in capacity for the size of its new storage. This can result in an underallocation and write past the end of the new buffer. This fix changes the new size calculation to use the current UTF-8 count as the minimum. Non-native or non-unique strings now allocate the requested capacity (or space enough for the current contents, if that's larger than what's requested). rdar://109275875 Fixes #53483
1 parent 34faa58 commit 23c1933

File tree

2 files changed

+57
-2
lines changed

2 files changed

+57
-2
lines changed

stdlib/public/core/StringGutsRangeReplaceable.swift

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,14 +100,28 @@ extension _StringGuts {
100100
// Grow to accommodate at least `n` code units
101101
@usableFromInline
102102
internal mutating func grow(_ n: Int) {
103-
defer { self._invariantCheck() }
103+
defer {
104+
self._invariantCheck()
105+
_internalInvariant(
106+
self.uniqueNativeCapacity != nil && self.uniqueNativeCapacity! >= n)
107+
}
104108

105109
_internalInvariant(
106110
self.uniqueNativeCapacity == nil || self.uniqueNativeCapacity! < n)
107111

112+
// If unique and native, apply a 2x growth factor to avoid problematic
113+
// performance when used in a loop. If one if those doesn't apply, we
114+
// can just use the requested capacity (at least the current utf-8 count).
108115
// TODO: Don't do this! Growth should only happen for append...
109-
let growthTarget = Swift.max(n, (self.uniqueNativeCapacity ?? 0) * 2)
116+
let growthTarget: Int
117+
if let capacity = self.uniqueNativeCapacity {
118+
growthTarget = Swift.max(n, capacity * 2)
119+
} else {
120+
growthTarget = Swift.max(n, self.utf8Count)
121+
}
110122

123+
// `isFastUTF8` is not the same as `isNative`. It can include small
124+
// strings or foreign strings that provide contiguous UTF-8 access.
111125
if _fastPath(isFastUTF8) {
112126
let isASCII = self.isASCII
113127
let storage = self.withFastUTF8 {

validation-test/stdlib/String.swift

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2370,4 +2370,45 @@ StringTests.test("SmallString.zeroTrailingBytes") {
23702370
}
23712371
}
23722372

2373+
StringTests.test("String.CoW.reserveCapacity") {
2374+
// Test that reserveCapacity(_:) doesn't actually shrink capacity
2375+
var str = String(repeating: "a", count: 20)
2376+
str.reserveCapacity(10)
2377+
expectGE(str.capacity, 20)
2378+
str.reserveCapacity(30)
2379+
expectGE(str.capacity, 30)
2380+
let preGrowCapacity = str.capacity
2381+
2382+
// Growth shouldn't be linear
2383+
str.append(contentsOf: String(repeating: "z", count: 20))
2384+
expectGE(str.capacity, preGrowCapacity * 2)
2385+
2386+
// Capacity can shrink when copying, but not below the count
2387+
var copy = str
2388+
copy.reserveCapacity(30)
2389+
expectGE(copy.capacity, copy.count)
2390+
expectLT(copy.capacity, str.capacity)
2391+
}
2392+
2393+
StringTests.test("NSString.CoW.reserveCapacity") {
2394+
#if _runtime(_ObjC)
2395+
func newNSString() -> NSString {
2396+
NSString(string: String(repeating: "a", count: 20))
2397+
}
2398+
2399+
// Test that reserveCapacity(_:) doesn't actually shrink capacity
2400+
var str = newNSString() as String
2401+
var copy = str
2402+
copy.reserveCapacity(10)
2403+
copy.reserveCapacity(30)
2404+
expectGE(copy.capacity, 30)
2405+
2406+
var str2 = newNSString() as String
2407+
var copy2 = str2
2408+
copy2.append(contentsOf: String(repeating: "z", count: 10))
2409+
expectGE(copy2.capacity, 30)
2410+
expectLT(copy2.capacity, 40)
2411+
#endif
2412+
}
2413+
23732414
runAllTests()

0 commit comments

Comments
 (0)