Skip to content

Commit defcb54

Browse files
committed
Use raw pointers instead of typed pointers.
In general, it is better to use raw pointers instead of typed pointers whenever those pointers are provided to you by external users. The reason for this is that Swift requires that each pointer be bound to one and only one data type at any point in the program execution. If you are vended a raw pointer (e.g. from a Data), your program does not unequivocally know what the type of that pointer may be, and so to assume that it is bound to UInt8 or Int8 is to risk violating Swift's pointer aliasing rules. We've learned lessons in the past that violating the pointer aliasing rules in programming languages can lead to disaster. In our case, we can avoid the majority of this trouble by electing to use raw pointers instead of typed pointers. This also helps us by removing tricky pointer management code. This patch contains essentially no function change: wherever possible I made straightforward, targeted refactors. I think once or twice I turned a pointer into a buffer pointer, but I generally resisted getting too frisky with changes.
1 parent d04ea43 commit defcb54

24 files changed

+228
-177
lines changed

Sources/SwiftProtobuf/AnyMessageStorage.swift

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,8 @@ fileprivate func unpack(contentJSON: Data,
6767

6868
var value = String()
6969
try contentJSON.withUnsafeBytes { (body: UnsafeRawBufferPointer) in
70-
if let baseAddress = body.baseAddress, body.count > 0 {
71-
let bytes = baseAddress.assumingMemoryBound(to: UInt8.self)
72-
73-
let buffer = UnsafeBufferPointer(start: bytes, count: body.count)
74-
var scanner = JSONScanner(source: buffer,
70+
if body.count > 0 {
71+
var scanner = JSONScanner(source: body,
7572
messageDepthLimit: options.messageDepthLimit,
7673
ignoreUnknownFields: options.ignoreUnknownFields)
7774
let key = try scanner.nextQuotedString()

Sources/SwiftProtobuf/BinaryDecoder.swift

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,13 @@ import Foundation
1919

2020
internal struct BinaryDecoder: Decoder {
2121
// Current position
22-
private var p : UnsafePointer<UInt8>
22+
private var p : UnsafeRawPointer
2323
// Remaining bytes in input.
2424
private var available : Int
2525
// Position of start of field currently being parsed
26-
private var fieldStartP : UnsafePointer<UInt8>
26+
private var fieldStartP : UnsafeRawPointer
2727
// Position of end of field currently being parsed, nil if we don't know.
28-
private var fieldEndP : UnsafePointer<UInt8>?
28+
private var fieldEndP : UnsafeRawPointer?
2929
// Whether or not the field value has actually been parsed
3030
private var consumed = true
3131
// Wire format for last-examined field
@@ -51,7 +51,7 @@ internal struct BinaryDecoder: Decoder {
5151
private var complete: Bool {return available == 0}
5252

5353
internal init(
54-
forReadingFrom pointer: UnsafePointer<UInt8>,
54+
forReadingFrom pointer: UnsafeRawPointer,
5555
count: Int,
5656
options: BinaryDecodingOptions,
5757
extensions: ExtensionMap? = nil
@@ -66,7 +66,7 @@ internal struct BinaryDecoder: Decoder {
6666
}
6767

6868
internal init(
69-
forReadingFrom pointer: UnsafePointer<UInt8>,
69+
forReadingFrom pointer: UnsafeRawPointer,
7070
count: Int,
7171
parent: BinaryDecoder
7272
) {
@@ -878,8 +878,7 @@ internal struct BinaryDecoder: Decoder {
878878
var field = Data(count: fieldSize)
879879
field.withUnsafeMutableBytes { (body: UnsafeMutableRawBufferPointer) in
880880
if let baseAddress = body.baseAddress, body.count > 0 {
881-
let pointer = baseAddress.assumingMemoryBound(to: UInt8.self)
882-
var encoder = BinaryEncoder(forWritingInto: pointer)
881+
var encoder = BinaryEncoder(forWritingInto: baseAddress)
883882
encoder.startField(tag: fieldTag)
884883
encoder.putVarInt(value: Int64(bodySize))
885884
for v in extras {
@@ -1209,8 +1208,7 @@ internal struct BinaryDecoder: Decoder {
12091208
var wasDecoded = false
12101209
try data.withUnsafeBytes { (body: UnsafeRawBufferPointer) in
12111210
if let baseAddress = body.baseAddress, body.count > 0 {
1212-
let pointer = baseAddress.assumingMemoryBound(to: UInt8.self)
1213-
var extDecoder = BinaryDecoder(forReadingFrom: pointer,
1211+
var extDecoder = BinaryDecoder(forReadingFrom: baseAddress,
12141212
count: body.count,
12151213
parent: self)
12161214
// Prime the decode to be correct.
@@ -1253,8 +1251,7 @@ internal struct BinaryDecoder: Decoder {
12531251
var payload = Data(count: payloadSize)
12541252
payload.withUnsafeMutableBytes { (body: UnsafeMutableRawBufferPointer) in
12551253
if let baseAddress = body.baseAddress, body.count > 0 {
1256-
let pointer = baseAddress.assumingMemoryBound(to: UInt8.self)
1257-
var encoder = BinaryEncoder(forWritingInto: pointer)
1254+
var encoder = BinaryEncoder(forWritingInto: baseAddress)
12581255
encoder.putBytesValue(value: data)
12591256
}
12601257
}
@@ -1297,14 +1294,14 @@ internal struct BinaryDecoder: Decoder {
12971294
if available < 1 {
12981295
throw BinaryDecodingError.truncated
12991296
}
1300-
var c = p[0]
1297+
var c = p.load(fromByteOffset: 0, as: UInt8.self)
13011298
while (c & 0x80) != 0 {
13021299
p += 1
13031300
available -= 1
13041301
if available < 1 {
13051302
throw BinaryDecodingError.truncated
13061303
}
1307-
c = p[0]
1304+
c = p.load(fromByteOffset: 0, as: UInt8.self)
13081305
}
13091306
p += 1
13101307
available -= 1
@@ -1382,7 +1379,7 @@ internal struct BinaryDecoder: Decoder {
13821379
}
13831380
var start = p
13841381
var length = available
1385-
var c = start[0]
1382+
var c = start.load(fromByteOffset: 0, as: UInt8.self)
13861383
start += 1
13871384
length -= 1
13881385
if c & 0x80 == 0 {
@@ -1396,7 +1393,7 @@ internal struct BinaryDecoder: Decoder {
13961393
if length < 1 || shift > 63 {
13971394
throw BinaryDecodingError.malformedProtobuf
13981395
}
1399-
c = start[0]
1396+
c = start.load(fromByteOffset: 0, as: UInt8.self)
14001397
start += 1
14011398
length -= 1
14021399
value |= UInt64(c & 0x7f) << shift
@@ -1448,10 +1445,8 @@ internal struct BinaryDecoder: Decoder {
14481445
/// helper handles all four-byte number types.
14491446
private mutating func decodeFourByteNumber<T>(value: inout T) throws {
14501447
guard available >= 4 else {throw BinaryDecodingError.truncated}
1451-
withUnsafeMutablePointer(to: &value) { ip -> Void in
1452-
let dest = UnsafeMutableRawPointer(ip).assumingMemoryBound(to: UInt8.self)
1453-
let src = UnsafeRawPointer(p).assumingMemoryBound(to: UInt8.self)
1454-
dest.initialize(from: src, count: 4)
1448+
withUnsafeMutableBytes(of: &value) { dest -> Void in
1449+
dest.copyMemory(from: UnsafeRawBufferPointer(start: p, count: 4))
14551450
}
14561451
consume(length: 4)
14571452
}
@@ -1460,10 +1455,8 @@ internal struct BinaryDecoder: Decoder {
14601455
/// helper handles all eight-byte number types.
14611456
private mutating func decodeEightByteNumber<T>(value: inout T) throws {
14621457
guard available >= 8 else {throw BinaryDecodingError.truncated}
1463-
withUnsafeMutablePointer(to: &value) { ip -> Void in
1464-
let dest = UnsafeMutableRawPointer(ip).assumingMemoryBound(to: UInt8.self)
1465-
let src = UnsafeRawPointer(p).assumingMemoryBound(to: UInt8.self)
1466-
dest.initialize(from: src, count: 8)
1458+
withUnsafeMutableBytes(of: &value) { dest -> Void in
1459+
dest.copyMemory(from: UnsafeRawBufferPointer(start: p, count: 8))
14671460
}
14681461
consume(length: 8)
14691462
}
@@ -1490,7 +1483,7 @@ internal struct BinaryDecoder: Decoder {
14901483

14911484
/// Private: Get the start and length for the body of
14921485
// a length-delimited field.
1493-
private mutating func getFieldBodyBytes(count: inout Int) throws -> UnsafePointer<UInt8> {
1486+
private mutating func getFieldBodyBytes(count: inout Int) throws -> UnsafeRawPointer {
14941487
let length = try decodeVarint()
14951488
if length <= UInt64(available) {
14961489
count = Int(length)

Sources/SwiftProtobuf/BinaryDelimited.swift

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,17 @@ public enum BinaryDelimited {
5757
var data = Data(count: totalSize)
5858
data.withUnsafeMutableBytes { (body: UnsafeMutableRawBufferPointer) in
5959
if let baseAddress = body.baseAddress, body.count > 0 {
60-
let pointer = baseAddress.assumingMemoryBound(to: UInt8.self)
61-
var encoder = BinaryEncoder(forWritingInto: pointer)
60+
var encoder = BinaryEncoder(forWritingInto: baseAddress)
6261
encoder.putBytesValue(value: serialized)
6362
}
6463
}
6564

6665
var written: Int = 0
6766
data.withUnsafeBytes { (body: UnsafeRawBufferPointer) in
6867
if let baseAddress = body.baseAddress, body.count > 0 {
68+
// This assumingMemoryBound is technically unsafe, but without SR-11078
69+
// (https://bugs.swift.org/browse/SR-11087) we don't have another option.
70+
// It should be "safe enough".
6971
let pointer = baseAddress.assumingMemoryBound(to: UInt8.self)
7072
written = stream.write(pointer, maxLength: totalSize)
7173
}
@@ -162,6 +164,9 @@ public enum BinaryDelimited {
162164
var bytesRead: Int = 0
163165
data.withUnsafeMutableBytes { (body: UnsafeMutableRawBufferPointer) in
164166
if let baseAddress = body.baseAddress, body.count > 0 {
167+
// This assumingMemoryBound is technically unsafe, but without SR-11078
168+
// (https://bugs.swift.org/browse/SR-11087) we don't have another option.
169+
// It should be "safe enough".
165170
let pointer = baseAddress.assumingMemoryBound(to: UInt8.self)
166171
bytesRead = stream.read(pointer, maxLength: length)
167172
}

Sources/SwiftProtobuf/BinaryEncoder.swift

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,30 +19,35 @@ import Foundation
1919
* Encoder for Binary Protocol Buffer format
2020
*/
2121
internal struct BinaryEncoder {
22-
private var pointer: UnsafeMutablePointer<UInt8>
22+
private var pointer: UnsafeMutableRawPointer
2323

24-
init(forWritingInto pointer: UnsafeMutablePointer<UInt8>) {
24+
init(forWritingInto pointer: UnsafeMutableRawPointer) {
2525
self.pointer = pointer
2626
}
2727

2828
private mutating func append(_ byte: UInt8) {
29-
pointer.pointee = byte
30-
pointer = pointer.successor()
29+
pointer.storeBytes(of: byte, as: UInt8.self)
30+
pointer = pointer.advanced(by: 1)
3131
}
3232

3333
private mutating func append(contentsOf data: Data) {
34-
let count = data.count
35-
data.copyBytes(to: pointer, count: count)
36-
pointer = pointer.advanced(by: count)
34+
data.withUnsafeBytes { dataPointer in
35+
if let baseAddress = dataPointer.baseAddress, dataPointer.count > 0 {
36+
pointer.copyMemory(from: baseAddress, byteCount: dataPointer.count)
37+
pointer = pointer.advanced(by: dataPointer.count)
38+
}
39+
}
3740
}
3841

39-
private mutating func append(contentsOf bufferPointer: UnsafeBufferPointer<UInt8>) {
42+
private mutating func append(contentsOf bufferPointer: UnsafeRawBufferPointer) {
4043
let count = bufferPointer.count
41-
pointer.assign(from: bufferPointer.baseAddress!, count: count)
44+
if let baseAddress = bufferPointer.baseAddress, count > 0 {
45+
memcpy(pointer, baseAddress, count)
46+
}
4247
pointer = pointer.advanced(by: count)
4348
}
4449

45-
func distance(pointer: UnsafeMutablePointer<UInt8>) -> Int {
50+
func distance(pointer: UnsafeMutableRawPointer) -> Int {
4651
return pointer.distance(to: self.pointer)
4752
}
4853

@@ -123,8 +128,8 @@ internal struct BinaryEncoder {
123128
let count = value.utf8.count
124129
putVarInt(value: count)
125130
for b in value.utf8 {
126-
pointer.pointee = b
127-
pointer = pointer.successor()
131+
pointer.storeBytes(of: b, as: UInt8.self)
132+
pointer = pointer.advanced(by: 1)
128133
}
129134
}
130135

Sources/SwiftProtobuf/BinaryEncodingVisitor.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ internal struct BinaryEncodingVisitor: Visitor {
2626
/// - Precondition: `pointer` must point to an allocated block of memory that
2727
/// is large enough to hold the entire encoded message. For performance
2828
/// reasons, the encoder does not make any attempts to verify this.
29-
init(forWritingInto pointer: UnsafeMutablePointer<UInt8>) {
29+
init(forWritingInto pointer: UnsafeMutableRawPointer) {
3030
encoder = BinaryEncoder(forWritingInto: pointer)
3131
}
3232

Sources/SwiftProtobuf/DoubleParser.swift

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -21,46 +21,39 @@ internal class DoubleParser {
2121
// In theory, JSON writers should be able to represent any IEEE Double
2222
// in at most 25 bytes, but many writers will emit more digits than
2323
// necessary, so we size this generously.
24-
#if swift(>=4.1)
25-
private var work =
26-
UnsafeMutableRawBufferPointer.allocate(byteCount: 128,
27-
alignment: MemoryLayout<UInt8>.alignment)
28-
#else
29-
private var work = UnsafeMutableRawBufferPointer.allocate(count: 128)
30-
#endif
24+
private var work =
25+
UnsafeMutableBufferPointer<Int8>.allocate(capacity: 128)
3126

3227
deinit {
3328
work.deallocate()
3429
}
3530

36-
func utf8ToDouble(bytes: UnsafeBufferPointer<UInt8>,
37-
start: UnsafeBufferPointer<UInt8>.Index,
38-
end: UnsafeBufferPointer<UInt8>.Index) -> Double? {
39-
return utf8ToDouble(bytes: bytes.baseAddress! + start, count: end - start)
31+
func utf8ToDouble(bytes: UnsafeRawBufferPointer,
32+
start: UnsafeRawBufferPointer.Index,
33+
end: UnsafeRawBufferPointer.Index) -> Double? {
34+
return utf8ToDouble(bytes: UnsafeRawBufferPointer(rebasing: bytes[start..<end]))
4035
}
4136

42-
func utf8ToDouble(bytes: UnsafePointer<UInt8>, count: Int) -> Double? {
37+
func utf8ToDouble(bytes: UnsafeRawBufferPointer) -> Double? {
4338
// Reject unreasonably long or short UTF8 number
44-
if work.count <= count || count < 1 {
39+
if work.count <= bytes.count || bytes.count < 1 {
4540
return nil
4641
}
47-
// Copy it to the work buffer and null-terminate it
48-
let source = UnsafeRawBufferPointer(start: bytes, count: count)
42+
4943
#if swift(>=4.1)
50-
work.copyMemory(from:source)
44+
UnsafeMutableRawBufferPointer(work).copyMemory(from: bytes)
5145
#else
52-
work.copyBytes(from:source)
46+
UnsafeMutableRawBufferPointer(work).copyBytes(from: bytes)
5347
#endif
54-
work[count] = 0
48+
work[bytes.count] = 0
5549

5650
// Use C library strtod() to parse it
57-
let start = work.baseAddress!.assumingMemoryBound(to: Int8.self)
58-
var e: UnsafeMutablePointer<Int8>? = start
59-
let d = strtod(start, &e)
51+
var e: UnsafeMutablePointer<Int8>? = work.baseAddress
52+
let d = strtod(work.baseAddress!, &e)
6053

6154
// Fail if strtod() did not consume everything we expected
6255
// or if strtod() thought the number was out of range.
63-
if e != start + count || !d.isFinite {
56+
if e != work.baseAddress! + bytes.count || !d.isFinite {
6457
return nil
6558
}
6659
return d

Sources/SwiftProtobuf/Enum.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ extension Enum {
8383
/// to distinguish them.
8484
///
8585
/// - Parameter name: Buffer holding the UTF-8 bytes of the desired name.
86-
internal init?(rawUTF8: UnsafeBufferPointer<UInt8>) {
86+
internal init?(rawUTF8: UnsafeRawBufferPointer) {
8787
guard let nameProviding = Self.self as? _ProtoNameProviding.Type,
8888
let number = nameProviding._protobuf_nameMap.number(forJSONName: rawUTF8) else {
8989
return nil

Sources/SwiftProtobuf/Google_Protobuf_Any+Extensions.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,9 @@ extension Google_Protobuf_Any {
6868
if let data = textFormatString.data(using: String.Encoding.utf8) {
6969
try data.withUnsafeBytes { (body: UnsafeRawBufferPointer) in
7070
if let baseAddress = body.baseAddress, body.count > 0 {
71-
let bytes = baseAddress.assumingMemoryBound(to: UInt8.self)
72-
7371
var textDecoder = try TextFormatDecoder(
7472
messageType: Google_Protobuf_Any.self,
75-
utf8Pointer: bytes,
73+
utf8Pointer: baseAddress,
7674
count: body.count,
7775
extensions: extensions)
7876
try decodeTextFormat(decoder: &textDecoder)

Sources/SwiftProtobuf/JSONDecoder.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ internal struct JSONDecoder: Decoder {
2525
throw JSONDecodingError.conflictingOneOf
2626
}
2727

28-
internal init(source: UnsafeBufferPointer<UInt8>, options: JSONDecodingOptions) {
28+
internal init(source: UnsafeRawBufferPointer, options: JSONDecodingOptions) {
2929
self.options = options
3030
self.scanner = JSONScanner(source: source,
3131
messageDepthLimit: self.options.messageDepthLimit,

Sources/SwiftProtobuf/JSONEncoder.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -323,9 +323,7 @@ internal struct JSONEncoder {
323323
data.append(asciiDoubleQuote)
324324
if value.count > 0 {
325325
value.withUnsafeBytes { (body: UnsafeRawBufferPointer) in
326-
if let baseAddress = body.baseAddress, body.count > 0 {
327-
let p = baseAddress.assumingMemoryBound(to: UInt8.self)
328-
326+
if let p = body.baseAddress, body.count > 0 {
329327
var t: Int = 0
330328
var bytesInGroup: Int = 0
331329
for i in 0..<body.count {

0 commit comments

Comments
 (0)