[swift-users] Pointer conversions between different sockaddr types

[Andrew Trick]

> On Aug 19, 2016, at 4:49 PM, Michael Ferenduros via swift-users <swift-users at swift.org> wrote:
> 
> 
>>> On Aug 18, 2016, at 12:28 AM, Quinn The Eskimo! via swift-users <swift-users at swift.org> wrote: 
>>> 
>>> 
>>> On 17 Aug 2016, at 18:55, Martin R via swift-users <swift-users at swift.org> wrote: 
>>> 
>>>> - Are both solutions correct, should one be preferred, or are both wrong? 
>>> 
>>> Your `withMemoryRebound` solution is correct. 
>> Absolutely, withMemoryRebound is always safe. You can use it whenever you just need to reinterpret memory at a call site and know the number of values stored that memory location. In this case it’s “easy" because you’re dealing a single sockaddr_in. The UnsafeRawPointer proposal is the definitive reference 
>> https://github.com/apple/swift-evolution/blob/master/proposals/0107-unsaferawpointer.md <https://github.com/apple/swift-evolution/blob/master/proposals/0107-unsaferawpointer.md> But this migration guide is more approachable… it’s still WIP: 
>> https://gist.github.com/atrick/0283ae0e284610fd21ad6ed3f454a585
> 
> I’m running into the same issues, which is making me wonder withMemoryRebound - socket functions expect an UnsafePointer<sockaddr>, but the thing pointed to can actually be larger than a sockaddr (eg. sockaddr_in6 or sockaddr_storage). Is withMemoryRebound safe to use to cast pointers to differently-sized structures? In the case of UnsafeMutablePointer<sockaddr> it seems like you’re lying to the API about what memory may be modified… But if withMemoryRebound isn’t about providing that guarantee, what does it actually do vs eg. a dumb cast via OpaquePointer?

Good point. I replied too hastily, but you’re about to find out why I didn’t go into details…

As you can see <https://github.com/apple/swift/blob/master/stdlib/public/core/UnsafePointer.swift.gyb#L371>, withMemoryRebound(to: T.self, capacity: count) actually does this:
- (re)binds memory to `count` `T` values
- executes the closure
- (re)binds memory to `count` `Pointee` values

For `count == 1` and `MemoryLayout<T>.size < MemoryLayout<Pointee>.size` this is valid and safe for "compatible" structs. The precondition on `withMemoryRebound` states that `T` must be layout compatible with `Pointee`.
  
  aggregates (tuples, array storage, and structs), are layout
  compatible with larger aggregates of the same kind if their common
  elements are mutually layout compatible.

I should really update withMemoryRebound's precondition to read:

  If `count == 1` then `T` must be layout compatible with
  `Pointee`. If `count > 1`, then `T` must be mutually layout
  compatible with `Pointee`.

You may be wondering why it's necessary to bind and rebind memory if the structures are "compatible". In general, you could have Swift code inside the closure that accesses `sockaddr` and swift code outside that accesses `sockaddr_in6`. We don't have a formal rule that says those two types are "related", so that could be miscompiled. "Related" is a type system concept that has to do with child/subclass relationships, "compatible" is an ABI concept that has to do with in-memory representation of types.

That's the theory. But in practice, the optimizer is going to have to treat sockaddr and sockaddr_in6 as related types simply because they are both imported from C and are allowed to alias in C land. In fact, if they were defined in Swift they wouldn't even be compatible because their overlapping members are not mutually compatible.

Even if one of the types were not imported, an opaque pointer cast would still work in this particular case because Swift code is never actually dereferencing the sockaddr pointer. It's just being passed off to an external C API.

So, the only real reason not to use an opaque pointer cast in this case is that someone reading your code likely won't understand what makes it valid and why they can't do the same thing in their code.

Now, what about Martin's first version:

  UnsafeRawPointer($0).assumingMemoryBound(to: sockaddr_in.self)

The code that this generates is just as safe or unsafe as your opaque pointer cast except that you're now making an untrue assertion about the memory type (per the precondition on the assumingMemoryBound API <https://github.com/apple/swift/blob/master/stdlib/public/core/UnsafeRawPointer.swift.gyb#L187>). A code verification tool--like a hypothetcial pointer sanitizer--could call you out on this.

>>>> - Can the same be achieved simpler? 
>>> 
>>> Not without introducing a layer of abstraction. 
>>> 
>>> In my case I introduced an abstract `Address` type (basically a wrapper around `sockaddr_storage`) and then added a method to that object which calls a closure with the right parameters (actually, multiple such methods, depending on whether I’m calling something like `connect` which takes an address, or `getpeername`, which returns one). This approach concentrates all the ugly in one place, making the rest of my BSD Sockets code much cleaner. 
>> This is an annoying UpdatePointer migration case because it falls under the category of misbehaving C APIs that we deliberately don't want to encourage in Swift. The only good answer is to provide a Swift wrapper on top of the socket API as Quinn has done. It would be nice to post that code at some point so users of the socket API can copy-paste into their project. -Andy
> 
> I do something similar in a sockaddr_in6 extension here, but it’s not great code at this point and not suggested for use: https://github.com/mike-ferenduros/SwiftySockets <https://github.com/mike-ferenduros/SwiftySockets>

That's a start. And another great example of why we need UnsafeBytes!

-Andy

> PS: Apologies for the broken reply-chain, I just subscribed to the list and I missed the mail I’m replying to.
> 
> _______________________________________________
> swift-users mailing list
> swift-users at swift.org
> https://lists.swift.org/mailman/listinfo/swift-users

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-users/attachments/20160819/cfd266af/attachment.html>