CFTypeRef problems

447 views
Skip to first unread message

Keith Randall

unread,
Oct 17, 2017, 5:25:06 PM10/17/17
to golang-dev
If you don't care about cgo on Darwin, ignore this.

Some background (see issue 21897 for details):

Darwin has CoreFoundation reference types.  They are named CF*Ref, and act like pointers.  The root of the hierarchy is CFTypeRef, akin to void* in the C world.  There are many "subclasses", like CFStringRef, CFNumberRef, etc.  A subclass can be cast up to CFTypeRef, and a CFTypeRef can be downcast to a subclass (assuming you know which one it should be).  These types are opaque in other respects, requiring calls into CoreFoundation library functions to extract data from them.

When we translate these types to Go using cgo, they become pointer types, because they are declared as such in the CoreFoundation headers.
CFTypeRef -> unsafe.Pointer
CFStringRef -> *struct{...}
and so on.

The problem is that not all of the values stored in these types are actually pointers.  CFNumberRef and CFDateRef, in particular, are implemented (sometimes) by packing their content into the high bits of a pointer and marking them specially using the low bits.  When such a beast reaches the Go side, the garbage collector might barf on it, because it is something which has pointer type, but points somewhere strange (the middle of the first page of memory, or an unused mspan, ...).

As a result, it is fundamentally dangerous to pass a CFNumberRef or a CFDateRef to Go.  And because either of those can be cast up to CFTypeRef, a CFTypeRef is dangerous also.  There's no easy way to defend against this problem, other than to not pass them to Go at all.  But that's hard, as they are both Core and Foundational.  Almost anything you do Darwin-specific in cgo will run into CFTypeRef, at least.

I'm proposing that we change CFTypeRef, CFNumberRef, and CFDateRef to translate on the Go side to uintptr instead of a pointer type.  That way, the garbage collector doesn't treat these items as pointers and the problem goes away.  [Option 2: translate all the CF*Ref types to uintptr.]  Because the only thing you can do with a CF*Ref is to call C functions on it, it shouldn't affect operations we actually want to do on these types - you'd have to do a cgo call to do anything with them.  In particular, even though they are pointers you can't dereference them.

Unfortunately there are two exceptions:  Casts and nil.  Hence this email.

Right now, you can cast from C.CFStringRef to C.CFTypeRef, because both are pointer types (the former is *struct{...}, the latter is unsafe.Pointer).  After my proposal, that is no longer possible, because you can't cast directly from a concrete pointer type to a uintptr.  You need a double cast with unsafe.Pointer as the intermediary.

Right now, you can assign nil to a C.CFTypeRef.  After my proposal, you'd have to assign 0 instead.

[Under option 2, the cast problem goes away, but the nil vs. 0 problem still exists.]

I started to dismiss these shenanigans as unlikely, but it turns out they exist even in the std library.  See crypto/x509/root_cgo_darwin.go.

So what to do?  I've written a go fix to find instances of these problems and fix them.  For casts, it inserts the intermediate unsafe.Pointer cast in the appropriate places, and [still TBD] it replaces nil for 0 when appropriate.  The problem with go fix is that it isn't perfect.  There are cases where the typechecker in go fix is incomplete, and hence can miss instances which would then fail during compilation.

How do people feel about this?  We'd essentially be breaking a possibly substantial fraction of Darwin cgo code and then offering a go fix module which would hopefully fix most, but probably not all, errors.  Any remaining fixes would have to be done by hand.  Hand fixing should be really easy, but could be tedious.

So we'd be trading a moderately painful one-time fix to avoid the random crashes we'd otherwise continue to see in cgo Darwin programs.  Sound worth it?

Option 1 vs. Option 2?  Option 2 is a little bit easier go-fix-wise and manual-fix-wise, but it is changing more types and thus may be impacting more total code.

Bryan C. Mills

unread,
Oct 17, 2017, 6:27:26 PM10/17/17
to Keith Randall, golang-dev
Two other alternatives to consider.

One is to allow `nil` as another name for `uintptr(0)`. Awkward, but gets the job done.

Another is the `unsafe.Void` proposal I previously alluded to here.

It's more complex than either of the options you have proposed, but might address enough other issues to pay for the added cost (I have in mind https://golang.org/issue/22218, https://golang.org/issue/19928, https://golang.org/issue/21878, and perhaps https://golang.org/issue/13467).

We could define a distinguished Go type, which I'm calling `unsafe.Void`, that specifically refers to an object that cannot reside in Go memory.

The differences between `*unsafe.Void`, `unsafe.Pointer`, and `uintptr` would be:
  • uintptr is an integer type: it takes the value 0 instead of nil, and arithmetic can be performed on it.
  • unsafe.Pointer is either nil or a pointer to a valid (Go or non-Go) memory address.
  • unsafe.Void is a zero-sized type that is not instantiable in Go. A struct with a field of a non-instantiable type is itself non-instantiable, as are maps, slices, channels, and arrays with uninstantiable elements. Any expression that produces a value of a non-instantiable type — such as declaring a variable, calling `make`, or dereferencing a pointer except in an address operatotion — is a compile-time error.
  • A pointer to a non-instantiable type is instantiable and has `nil` as its zero-value, but the compiler may safely assume that it never points to Go memory (and thus skip it during garbage collection). A pointer to a non-instantiable type can be converted to an unsafe.Pointer and vice-versa, but if a pointer-to-uninstantiable is converted to unsafe.Pointer, it must then be converted to either uintptr or another pointer-to-uninstantiable. (This is similar to the existing rule for uintptr arithmetic via unsafe.Pointer.)

To stitch that together with cgo, we would map any incomplete C struct type (and, if necessary, complete types in a whitelist) to an uninstantiable Go type.

That solves the nil problem, because a pointer-to-non-instantiable can be nil.

Unfortunately, it introduces a new problem: with that approach, the C type `void*` properly maps to `*unsafe.Void` rather than `unsafe.Pointer`, but we can't change that without breaking compatibility. Perhaps that's not such a big deal since most void* tend to be valid pointers, but since CFTypeRef is a typedef for void* we would probably still have to special-case it.

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Keith Randall

unread,
Nov 2, 2017, 12:12:04 AM11/2/17
to Bryan C. Mills, golang-dev
After some discussion, I've updated my CL (https://go-review.googlesource.com/c/go/+/66332) and went with option 2, translating all the CF*Ref types to uintptr and implementing a go fix that does the nil -> 0 conversions.


On Tue, Oct 17, 2017 at 3:26 PM, Bryan C. Mills <bcm...@google.com> wrote:
Two other alternatives to consider.

One is to allow `nil` as another name for `uintptr(0)`. Awkward, but gets the job done.


I think this is an interesting option, but maybe for Go 2.  A language change means Go 1.10 code couldn't run on Go 1.9.  I want a stronger reason than this bug to go down that road.
It certainly would be nice to have a type-independent way of describing the zero value of a type.  nil works in some places but not everywhere.
Reply all
Reply to author
Forward
0 new messages