Presumably unsafe `withUnsafeMutablePointer` usage pattern

Using Swift
,
1

Consider this fragment:

import Foundation

struct Foo {
    var a: Int64 = 1
    var b: Int64 = 2
    
    mutating func secondValuePointer() -> UnsafeMutablePointer<Int> {
        return withUnsafeMutablePointer(to: &self) { p in
            return UnsafeMutableRawPointer(p)       // typed pointer -> raw pointer
                .advanced(by: 8)                    // adjust raw pointer
                .assumingMemoryBound(to: Int.self)  // raw pointer -> typed pointer
        }
    }
}

func foo() {
    var v = Foo()
    print(v) // Foo(a: 1, b: 2)
    print(v.secondValuePointer().pointee) // 2
    v.secondValuePointer().pointee = 3
    print(v) // Foo(a: 1, b: 3)
}

foo()

Assuming there is no error in the pointer math and the advanced pointer points to a proper location and properly typed value, does this still trigger UB and work by chance when it returns a pointer like that, or is it ok?

I think it works by chance, and this:

v.secondValuePointer()  /* ... */  .pointee

could break if compiler (for whatever reason) decides to move v to another location and place something else in that location after v.secondValuePointer() called and before .pointee being called. Is this analysis correct?

Is it possible to fix this fragment without changing it significantly (e.g. without changing it to take a callback and do the work inside that callback)?

2

Yes, this is unsafe because structs do not have a fixed memory location. This one is even small enough that the compiler could choose to assign a and b to registers and only copy them to the stack for the period of time where you specifically asked for a pointer to the struct.

One way to make this actually work (but without adding a closure) is to use UnsafeMutablePointer.allocate to explicitly allocate storage on the heaps. Then you’d be able to initialize the pointer and pass it around. Note that you still have to deinitialize and deallocate the pointer once you’re done with it. A slightly more efficient option that does require a closure is withUnsafeTemporaryAllocation, which is likely to give you a stack pointer in a situation like this. Memory on the stack is much cheaper to allocate at the cost of it being unconditionally invalidated when the function that allocated it returns.

3 Likes