binding-tools-for-swift icon indicating copy to clipboard operation
binding-tools-for-swift copied to clipboard

Please don't try to reach into closure contexts because there are no ABI guarantees

Open jckarter opened this issue 6 years ago • 4 comments

There's some fishy code here:

https://github.com/xamarin/binding-tools-for-swift/blob/6f26f61aa73d641b8a9d4e3e8b11997293cfb4bc/SwiftRuntimeLibrary/SwiftClosureRepresentation.cs#L24

		static IntPtr LocateRefPtrFromPartialApplicationForwarder(IntPtr p)
		{
			p = Marshal.ReadIntPtr (p + IntPtr.Size);
			return Marshal.ReadIntPtr (p + 3 * IntPtr.Size);
		}

There's no guarantee that a closure context contains anything in particular, or is even a dereferenceable pointer. Partial application forwarders are an implementation detail that will likely disappear when we change our representation of closures in SIL. The only ABI guarantees on a closure value are that the context word can be passed as an argument to the invocation function, and (if it's escaping) that the context word can be passed to swift_retain and swift_release.

It'd be interesting to hear what you're trying to do by looking through the forwarder. If you want to invoke a closure, you ought to pass the context to the invocation function as given, and if you want to create closures, there's no reason to mimic what the Swift compiler does. If you're trying to divine what the closure context contains…you can't yet, sorry.

jckarter avatar Nov 13 '19 20:11 jckarter

Hey Joe - thanks for pointing this out!

Closures are a funny beast to handle in C#. The main problem is that the swift ABI and the platform ABI for functions are divergent, so I can't directly pass a C# delegate to swift and vice versa, so I have to create adapters that contain reference constants that I can use on one side or the other to refer back to the original closure. In particular, there are three general forms of closures:

()->()
(args)->()
()->return
(args)->return

The first I can call directly. The second through fourth I have to transform into:

(UnsafePointer<(args)>)->()
(UnsafePointer<return>,UnsafePointer<(args)>)->()

Which is clunky but a working lingua franca between the two languages since I can always handle functions with pointers for their arguments.

When I want to pass a C# delegate into swift, I make a SwiftDotNetCapsule with a handle in it which gets attached to the closure. I use the handle to associate it with the original code. So when a C# closure gets called, the capusule finds it way back into C# where we grab it, find the original C# delegate and invoke it.

In swift 3, a closure was (IIRC) two pointers, a data handle for the closure and a pointer to code to get executes. Sometime between swift 3 and 4 this was changed to have an extra layer of wrapping - the partial application forwarder. The data pointer was wrapped inside the payload for the forwarder. What you're seeing is the result of a unit test failing and me rooting around in memory to find where things had gone. For better or for worse, it's what I need to do.

When you change your partial application forwarders, I expect the unit tests to fail and I'll try to figure out how to do this a different way.

I appreciate the heads up and I'll definitely keep an eye on it when we bump to the next swift versions.

stephen-hawley avatar Nov 13 '19 21:11 stephen-hawley

In swift 3, a closure was (IIRC) two pointers, a data handle for the closure and a pointer to code to get executes. Sometime between swift 3 and 4 this was changed to have an extra layer of wrapping - the partial application forwarder. The data pointer was wrapped inside the payload for the forwarder. What you're seeing is the result of a unit test failing and me rooting around in memory to find where things had gone. For better or for worse, it's what I need to do.

Trust me—you don't need to look into closure contexts. I'm not sure what changed, but partial application forwarders have been an implementation detail going back all the way to Swift 1, but they've always been just that—an implementation detail, and whether we thunk or what kinds of thunks we generate is not something that's reliably observable. A closure has only ever been two pointers, the data handle for the closure and the function pointer to execute. If there does happen to be a forwarder, then that forwarder is the function you should execute.

Perhaps what happened is that Swift 4 is when we implemented Swift's own calling convention, which would make it so that the context argument is passed in the self register, when it had previously been passed as a trailing argument in the C calling convention. If you're relying on similarities between the C and Swift calling convention, then that might be your problem, because there'd be no way to invoke a Swift closure invocation function using a C calling convention anymore. Since you're able to override and invoke methods, it seems like there must be at least some swiftcc support in place, maybe you can use that?

When you change your partial application forwarders, I expect the unit tests to fail and I'll try to figure out how to do this a different way.

I appreciate the heads up and I'll definitely keep an eye on it when we bump to the next swift versions.

It'd be better to follow the ABI now then have your developers' apps all break when their users upgrade their phones…

jckarter avatar Nov 13 '19 21:11 jckarter

@jckarter - Is there a supported way to invoke closures in a way we need without digging into implementation details here? If not, can we request one be added?

Steve is correct, our tests will catch when this breaks, but if we can sidestep it into supported territory that's even better.

chamons avatar Nov 14 '19 19:11 chamons

Yes, you can pass the context word to the invocation function in the self register (r13 for x86-64). Both of these words are directly available in the closure value, and you don't need to look inside either of them (and there's nothing you can count on being there):

https://swift.godbolt.org/z/PA-CxQ

(Ignore the retain/release, as long as nothing else can release the closure while it's being executed that's not strictly necessary.) It isn't clear to me why it needs to be more complicated than that.

jckarter avatar Nov 14 '19 19:11 jckarter