Skip to content

Commit f66ba9e

Browse files
committed
Revert "Foundation: fix missing string terminator"
This reverts commit f3a6f50. It also fixes the underlying issue of the heap corruption. There is no guarantee that the underlying storage for an allocation is done by `malloc`. It is possible that the size may be small enough to be pushed down to a stack allocation, which cannot be released by `free`. Buffers record how they are allocated and adjust the release accordingly. The safe way to release the memory is to use `deallocate`. Although the initial approach fixed the heap corruption, it did so at the single site, but this is a problem with a public interface. Use the `deallocate` method on the buffer when releasing the memory to ensure that the correct mechanism is used for releasing the resources. This is not specific to Windows but to any platform where a stack allocated buffer or an aligned allocation may not be passed to `free`. Thanks to Lily Vulcano for pressing me to investigate this further and root cause the failure in the public interface!
1 parent cd0bc7b commit f66ba9e

File tree

3 files changed

+9
-4
lines changed

3 files changed

+9
-4
lines changed

Docs/ReleaseNotes_Swift5.md

+7
Original file line numberDiff line numberDiff line change
@@ -156,3 +156,10 @@ A few notes:
156156
* The exception-throwing API will be deprecated in a future release. It is now marked as `@available(…, deprecated: 100000, …)`, which matches what you would see if `API_TO_BE_DEPRECATED` was used in a Objective-C header.
157157

158158
* Subclassing `NSFileHandle` is strongly discouraged. Many of the new methods are `public` and cannot be overridden.
159+
160+
## `NSString(bytesNoCopy:length:encoding:freeWhenDone:)` deviations from reference implementation
161+
162+
On Windows, `NSString(bytesNoCopy:length:encoding:freeWhenDone:)` deviates from
163+
the behaviour on Darwin and uses the buffer's `deallocate` routine rather than
164+
`free`. This is done to ensure that small buffers which are stack allocated
165+
through `alloca` can be properly released.

Sources/Foundation/NSData.swift

+1-3
Original file line numberDiff line numberDiff line change
@@ -599,12 +599,10 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding {
599599

600600
let capacity = estimateBase64Size(length: dataLength)
601601
let ptr = UnsafeMutableRawPointer.allocate(byteCount: capacity, alignment: 1)
602-
defer { ptr.deallocate() }
603602
let buffer = UnsafeMutableRawBufferPointer(start: ptr, count: capacity)
604603
let length = NSData.base64EncodeBytes(self, options: options, buffer: buffer)
605604

606-
let utf8buffer = UnsafeBufferPointer<UInt8>(start: ptr.assumingMemoryBound(to: UInt8.self), count: length)
607-
return String(decoding: utf8buffer, as: UTF8.self)
605+
return String(bytesNoCopy: ptr, length: length, encoding: .ascii, freeWhenDone: true)!
608606
}
609607

610608
/// Creates a Base64, UTF-8 encoded Data from the data object using the given options.

Sources/Foundation/NSString.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -1359,7 +1359,7 @@ extension NSString {
13591359
// just copy for now since the internal storage will be a copy anyhow
13601360
self.init(bytes: bytes, length: len, encoding: encoding)
13611361
if freeBuffer { // don't take the hint
1362-
free(bytes)
1362+
bytes.deallocate()
13631363
}
13641364
}
13651365

0 commit comments

Comments
 (0)