How to use withUnsafeMutablePointer with Swift 5.2

I have this code

func getNext<T: FixedWidthInteger>() throws -> T {
    var output = T()
    let buffer = UnsafeMutableBufferPointer(start: &output, count: 1)
    let _ = self.data.copyBytes(to: buffer, from: startIndex..<endIndex)
    return T(bigEndian: output)
}

and after upgrading to Xcode 11.4 I get a warning.

Initialization of 'UnsafeMutableBufferPointer<T>' results in a dangling buffer pointer

and

Use 'withUnsafeMutablePointer' in order to explicitly convert argument to pointer valid for a defined scope

The documentation for withUnsafeMutablePointer and UnsafeMutableBufferPointer does not really help me in any way to tell me how to change my code.

I can get the compiler to not issue any warning with this code:

func getNext<T: FixedWidthInteger>() throws -> T {
    var output = T()
    withUnsafeMutablePointer(to: &output) {
        let buffer = UnsafeMutableBufferPointer(start: $0, count: 1)
        let _ = self.data.copyBytes(to: buffer, from: startIndex..<endIndex)
    }
    return T(bigEndian: output)
}

Will this work and avoid the dangling buffer pointer that the warning told me about?

That'd work.

What happened is that &output is valid only while it's inside the init function. Once you get buffer, the init function already ends, and the &output pointer becomes invalid. The buffer is still using that invalid pointer, hence the warning.

If you can guarantee that the data is properly aligned to read T, you can use load(fromByteOffset:as:)

var output = data.withUnsafeBytes {
    $0.load(fromByteOffset: startIndex, as: T.self)
}
return T(bigEndian: output)

PS

I think you shouldn't let the resulting type be the only thing dictating the generic. It'd be better to do

func getNext<T: FixedWidthInteger>(_: T.Type) -> T { ... }

You can also drop throws if that's not needed.

2 Likes

Ah, so it is not that UnsafeMutableBufferPointer.init() returns something short-lived, it is more that the Swift &output operation is short-lived! I misunderstood the warning and explanation in the doc. Thank you.

The throws is for some of the code I deleted to have only the relevant parts :slight_smile:

The result is always returned into a struct var and if I need to give the type as a parameter it will be redundant. This api should probably be rewritten to use Codable instead (this was made before Codable was a thing) but there not a lot of information out there on how to make your own.

1 Like

There are a few posts about that here and there, maybe you can look it up.

Cool! Thanks. Perhaps I will give it a go when nobody is looking :-)

Filling in the details a little bit more:

&output creates a pointer that is temporarily bound to output during the scope of the call to UnsafeMutableBufferPointer.init. I.e. this line is effectively equivalent to:

let buffer = withUnsafeMutablePointer(to: &output) {
  UnsafeMutableBufferPointer(start: $0, count: 1)
}

Rewritten like this, the problem becomes more clear; because output is a value (at least for the usual FixedWidthInteger types), rather than an object with a fixed memory address, the pointer is no longer semantically bound to output once UnsafeMutableBufferPointer.init returns--buffer is never semantically bound to output at any point in the code!

I am frankly somewhat amazed that this ever "worked"--I would expect it to produce meaningless results even in older versions of Swift, as the optimizer would be free to simply assume that the call to copyBytes does not update output and return 0.

I would note that you may also be able to use the approach I sketched here to avoid any pointer shenanigans at all and still get reasonable performance.

(Tagging @Andrew_Trick, who's the real guru w.r.t. this corner of Swift, in case he wants to make my terminology more precise or offer other suggestions). I should note that none of us, especially not me and Andy, are very happy with the road hazard you're encountering here--the warning will help a lot of people figure out that they did something wrong, but we need to do a much better job of giving people good patterns that let them avoid the problem to begin with.

2 Likes

Ok, that makes it even more clear to me. It is really strange this ever worked. But it do work in production with a lot of data flowing through at a constant rate.

I did actually notice your post back in october 2019 but looked at it as an academic exploration at the time. Now it seems more reasonable :slight_smile:

In this code Igo for clarity in the code more than speed so either solutions would do. I cannot decide wether pointer handling or striding through the bits (your post from oct 2019) is the more maintainable for future generations.

All the answers I've seen here make sense. I always appreciate the "don't use pointers in Swift" answer.

I think the old code worked in practice because the compiler still honors the pointer as valid and copies bytes. The only danger is that something else decides to use output's stack space before copyBytes is called.

1 Like