[SR-2755] False objc_autoreleaseReturnValue dependency building on Linux

[swift-ci]

Previous ID	SR-2755
Radar	None
Original Reporter	gmilos (JIRA User)
Type	Bug

Attachment: Download

Additional Detail from JIRA

Votes	0
Component/s	Compiler
Labels	Bug
Assignee	@slavapestov
Priority	Medium

md5: 89cbd7095952971a44b28957ee5b5db8

relates to:

• SR-5271 Don't emit objc_retainAutoreleasedReturnValue() on non-ObjC targets

Issue Description:

When building Swift code, which calls to a C API taking block with block return value, generates false objc_autoreleaseReturnValue dependency.

From the attached minimal reproduction:

sbs:/tmp/objc_autoreleaseReturnValue_bug$
cat CStuff/c-stuff.c
#include "c-stuff.h"

#ifdef DEFINE_AUTORELEASE_RETURN_VALUE
void *objc_autoreleaseReturnValue(void * a) {
return a;
}
#endif

void bar(SomeBlock b) {
b();
}
sbs:/tmp/objc_autoreleaseReturnValue_bug$
cat CStuff/c-stuff.h
#ifndef C_STUFF
#define C_STUFF

typedef void(^SomeOtherBlock)(void);
typedef SomeOtherBlock(^SomeBlock)(void);
void bar(SomeBlock b);

#endif
sbs:/tmp/objc_autoreleaseReturnValue_bug$
cat
CStuff/ libCStuff.so Makefile test.o test.swift
sbs:/tmp/objc_autoreleaseReturnValue_bug$
cat test.swift
import Dispatch
import CStuff

bar({ return {} })
sbs:/tmp/objc_autoreleaseReturnValue_bug$
make
swiftc -lswiftCore -lswiftSwiftOnoneSupport -o test test.o libCStuff.so
test.o:test.o:function TTRXFooGSqFT_TXFdCbaGSqbT_T**_: error: undefined reference to 'objc_autoreleaseReturnValue'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
<unknown>:0: error: link command failed with exit code 1 (use -v to see invocation)
Makefile:4: recipe for target 'all' failed
make: *** [all] Error 1

The objc_autoreleaseReturnValue dependency doesn't exist on Linux. I was able to verify that the code builds and works when defining the symbol myself (simple identity function). This can be tested building with make clean && make CFLAGS=-DDEFINE_AUTORELEASE_RETURN_VALUE

Explicit Block_retain/Block_release should be used to manage return value lifecycle instead.

[belkadan]

@slavapestov, is this something obvious we can remove?

[slavapestov]

Probably. swift-corelibs-foundation even has a workaround for this, defining an empty stub for objc_retainAutoreleasedReturnValue(). @jckarter should remember the details – IIRC it wasn't as simple as just removing the function, because of memory management issues.

[jckarter]

We should be able to safely call Get-convention Core Foundation APIs by treating their return values as @unowned instead of @autoreleased, which is more accurate anyway. The safety issue is that Swift can't safely implement APIs with this convention without an autorelease pool (or a static ownership model), since it has no way to safely hand back a return value at +0. Core Foundation APIs that return at +0 ought to have been manually implemented such that this is safe, though.

[belkadan]

The advantage of treating them as @autoreleased is that we'll attempt to steal things back out of the autorelease pool on Apple platforms. I have no objection to treating them as @unowned on non-Apple platforms.

[swift-ci]

Comment by Gregor Milos (Grzegorz Miłoś) (JIRA)

FWIW, what I really want in my example is +1 (a.k.a. Create-convention). So the return block in the following line of Swift code: bar({ return {} }) should be +1 when I retrieve it by calling the outer block (from C).

[jckarter]

Yeah, that's what we usually do for native Swift closures. On Darwin we use a +0 convention for returns from blocks because we need to be compatible with ObjC, but we could potentially do something different on other platforms.

[swift-ci]

Comment by Gregor Milos (Grzegorz Miłoś) (JIRA)

@jckarter on Linux, for new code, it makes far more sense to follow same memory management conventions as in native Swift. However, there may be some code in libdispatch and/or Foundation that assumes ObjC conventions. In fact, taking into account what @slavapestov was saying, there likely is some exposure.

[jckarter]

Yeah, that's the tradeoff. In the short term, we could support invoking existing C functions with a +0 convention as "unowned", and flag returning a reference-counted value from a Swift closure used as a block or C function pointer as unsupported.

[swift-ci]

Comment by Gregor Milos (Grzegorz Miłoś) (JIRA)

Oh nooo... Please don't unsupport that 🙂. That's exactly what I want to use! (returning reference counted values [blocks]).

My current usecase for this was wrapping a high-performance C++ server (Proxygen) and exposing matching API in Swift. This was done with a series of callbacks. Since those callbacks need to capture state it forced me to go down the route of using C blocks. The callbacks chain, because a factory class is represented by a block with block return type.

So atm it's easy for me to stub objc_autoreleaseReturnValue out.

[jckarter]

Stubbing out objc_autoreleaseReturnValue is not safe. Without a real autorelease pool, there's nothing to keep the return value alive until the caller can retain it. Instead of returning by value, you could implement the C interface by having it take a function pointer with a `UnsafeMutablePointer` parameter the callback writes out to, and wrap that in a Swift function on the Swift side. With pointers, you can have whatever convention you want when writing to the pointer.

[jckarter]

You could also use an `Unmanaged` return value to manually reference count the object across the boundary.

[swift-ci]

Comment by Gregor Milos (Grzegorz Miłoś) (JIRA)

@jckarter your comment about unsafety confuses me. Especially given the fact the code I wrote seems to be behaving safely.
Could you clarify what is wrong in my understanding below:

• objc_autoreleaseReturnValue is called (through a jmp) once the return value has been constructed, and it receives +1 ref

• if the above wasn't true (i.e. if it received +0) the return value could/would already be destroyed on entry to objc_autoreleaseReturnValue

• in objc world, objc_autoreleaseReturnValue

  makes a best effort to hand off ownership of a retain count

  (see: http://clang.llvm.org/docs/AutomaticReferenceCounting.html#arc-runtime-objc-autoreleasereturnvalue). If this handover is not possible, it is meant to add the value to an autorelease pool.

• the job of the autorelease pool is to release the +1 ref at some point in the future

• in my stub implementation, objc_autoreleaseReturnValue is an identity function, therefore the caller will receive +1 ref (just like with a Create convention) and it is responsible for releasing it when no longer needed (this release is analogous to what the autorelease pool would have done, if we had one).

[belkadan]

The caller can't know whether the value was added to an autorelease pool or not. Therefore, it can't know whether it has to release or not. (The fact that there are no autorelease pools without the ObjC runtime around shouldn't change this.)

Swift has to either always return blocks at +1 on non-ObjC runtimes, or has to disallow returning bare blocks. (Unfortunately Unmanaged doesn't play well with closures right now.)

[swift-ci]

Comment by Gregor Milos (Grzegorz Miłoś) (JIRA)

@belkadan: stubbed objc_autoreleaseReturnValue achieves +1 temporarily. Once the compiler guarantees this 'natively', my stub would become unused and I could get rid of it. The caller assumes +1. Is there anything that would break the +1 'guarantee'?

[belkadan]

It's mostly that it's not consistent with Apple platforms. A cross-platform C caller would not be able to assume +1, and that seems like a pretty lousy story.