Swift *less* safe than C for imported API that uses opaque struct pointers


(Timothy J. Wood) #1

Currently, APIs that get imported with COpaquePointer make Swift *less* safe than C. Fixing this seems like a breaking change, since it would change the meaning of existing code that uses COpaquePointer.

As a motivating example consider:

import Darwin

var state = copyfile_state_alloc()
print("state = \(state.dynamicType) \(state)")

let acl = acl_init(1)
print("acl = \(acl.dynamicType) \(acl)")

state = acl
print("state = \(state.dynamicType) \(state)")

I’m just using these types since they use opaque structs and are easy to create in this sample, but this pattern is fairly common in other C APIs that try to encapsulate their implementation.

This compiles and builds, silently accepting the bogus code:

DEVELOPER_DIR=/Applications/Xcode-8.0b1.app/Contents/Developer swift c-unsafety.swift
state = Optional<OpaquePointer> Optional(0x00007f9db481b3d0)
acl = Optional<OpaquePointer> Optional(0x00007f9db2944c00)
state = Optional<OpaquePointer> Optional(0x00007f9db2944c00)

The equivalent C version:

copyfile_state_t state = state = copyfile_state_alloc();
acl_t acl = acl_init(1);
state = acl;

produces a warning:

c-unsafety.c:10:8: warning: incompatible pointer types assigning to 'copyfile_state_t' (aka 'struct _copyfile_state *') from 'acl_t' (aka 'struct _acl *')

Would it be feasible to import these sorts of pointers in a safe(r) way by making COpaquePointer generic and faking up a struct tag type? So, these might get imported with something equivalent to:

struct _acl {}
typealias acl_t = COpaquePointer<_acl>

A further problem, though, is that other examples of this use `const` to form a very basic `isa` relationship between two types. For example:

typedef const struct OpaqueJSContext* JSContextRef;
typedef struct OpaqueJSContext* JSGlobalContextRef;

so this approach wouldn’t solve casting between these two C types, but perhaps the name of tagging struct could indicate the `const` (“Const_ OpaqueJSContext”?) or maybe COpaquePointer could be used for `const` and a MutableCOpaquePointer type could be added for the non-const case?

There would also be issues if one Swift module tried to pass one of these to another; the made-up struct tag would need to be in some global namespace to avoid errors passing A.COpaquePointer<OpaqueFoo> to B.COpaquePointer<OpaqueFoo> (though really I don’t consider this to be a big problem -- the surface level API of a Swift module using C libraries that deal with C API should mostly try to hide that internally).

In my “real” cases, I’ve been trying to *immediately* wrap the COpaquePointer in a struct of my own, but this doesn’t handle the ‘isa-like’ relationship between a const/non-const variant of an opaque struct pointer, is much more verbose, and it is easy to mess it up and accidentally cross the streams. And of course, with all the contortions Swift goes to try to be safe, this regression in safety from C is puzzling.

Thanks!

-tim


(Joe Groff) #2

We should be able to synthesize opaque types as structs with inaccessible initializers, and use the existing UnsafePointer types. That would work better in situations where you have some modules that can see the implementation of '_acl', and some that can't.

-Joe

···

On Jun 22, 2016, at 11:20 AM, Timothy J. Wood via swift-evolution <swift-evolution@swift.org> wrote:

Currently, APIs that get imported with COpaquePointer make Swift *less* safe than C. Fixing this seems like a breaking change, since it would change the meaning of existing code that uses COpaquePointer.

As a motivating example consider:

import Darwin

var state = copyfile_state_alloc()
print("state = \(state.dynamicType) \(state)")

let acl = acl_init(1)
print("acl = \(acl.dynamicType) \(acl)")

state = acl
print("state = \(state.dynamicType) \(state)")

I’m just using these types since they use opaque structs and are easy to create in this sample, but this pattern is fairly common in other C APIs that try to encapsulate their implementation.

This compiles and builds, silently accepting the bogus code:

DEVELOPER_DIR=/Applications/Xcode-8.0b1.app/Contents/Developer swift c-unsafety.swift
state = Optional<OpaquePointer> Optional(0x00007f9db481b3d0)
acl = Optional<OpaquePointer> Optional(0x00007f9db2944c00)
state = Optional<OpaquePointer> Optional(0x00007f9db2944c00)

The equivalent C version:

copyfile_state_t state = state = copyfile_state_alloc();
acl_t acl = acl_init(1);
state = acl;

produces a warning:

c-unsafety.c:10:8: warning: incompatible pointer types assigning to 'copyfile_state_t' (aka 'struct _copyfile_state *') from 'acl_t' (aka 'struct _acl *')

Would it be feasible to import these sorts of pointers in a safe(r) way by making COpaquePointer generic and faking up a struct tag type? So, these might get imported with something equivalent to:

struct _acl {}
typealias acl_t = COpaquePointer<_acl>


(Jordan Rose) #3

Right. The concern we’ve had here is that you’ll start off with an opaque struct and then import something else that does have the definition. I think this only comes up in the REPL, though, and maybe it’s rare enough that we could just ignore it or disallow using the type as a value in that case.

Jordan

···

On Jun 22, 2016, at 12:58, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

On Jun 22, 2016, at 11:20 AM, Timothy J. Wood via swift-evolution <swift-evolution@swift.org> wrote:

Currently, APIs that get imported with COpaquePointer make Swift *less* safe than C. Fixing this seems like a breaking change, since it would change the meaning of existing code that uses COpaquePointer.

As a motivating example consider:

import Darwin

var state = copyfile_state_alloc()
print("state = \(state.dynamicType) \(state)")

let acl = acl_init(1)
print("acl = \(acl.dynamicType) \(acl)")

state = acl
print("state = \(state.dynamicType) \(state)")

I’m just using these types since they use opaque structs and are easy to create in this sample, but this pattern is fairly common in other C APIs that try to encapsulate their implementation.

This compiles and builds, silently accepting the bogus code:

DEVELOPER_DIR=/Applications/Xcode-8.0b1.app/Contents/Developer swift c-unsafety.swift
state = Optional<OpaquePointer> Optional(0x00007f9db481b3d0)
acl = Optional<OpaquePointer> Optional(0x00007f9db2944c00)
state = Optional<OpaquePointer> Optional(0x00007f9db2944c00)

The equivalent C version:

copyfile_state_t state = state = copyfile_state_alloc();
acl_t acl = acl_init(1);
state = acl;

produces a warning:

c-unsafety.c:10:8: warning: incompatible pointer types assigning to 'copyfile_state_t' (aka 'struct _copyfile_state *') from 'acl_t' (aka 'struct _acl *')

Would it be feasible to import these sorts of pointers in a safe(r) way by making COpaquePointer generic and faking up a struct tag type? So, these might get imported with something equivalent to:

struct _acl {}
typealias acl_t = COpaquePointer<_acl>

We should be able to synthesize opaque types as structs with inaccessible initializers, and use the existing UnsafePointer types. That would work better in situations where you have some modules that can see the implementation of '_acl', and some that can’t.