gnustep / libobjc2

Objective-C runtime library intended for use with Clang.
http://www.gnustep.org/
MIT License
434 stars 118 forks source link

Block.h of libobjc2 conflicts with Block.h from libdispatch (and Swift) #199

Closed fff7d1bc closed 4 months ago

fff7d1bc commented 3 years ago

The libobjc2 installs root level Block.h header that conflicts with libdispatch, which is also provided by Swift on Linux, making it impossible to install both gnustep's libobjc2 and libdispatch (https://github.com/apple/swift-corelibs-libdispatch)

Is there any option to maybe put this header of libobjc2 into subdirectory in new releases?

davidchisnall commented 3 years ago

It's odd for libdispatch to provide it, since I libdispatch doesn't (didn't?) include a blocks runtime. We did before there was another open source one and compiler-rt also provides one but without very good Objective-C interop for non-Apple platforms. All versions of this header should be compatible, so the simplest thing to do is add an option to not install that header. Anyone packaging libobjc2 can simply exclude it from the set of installed files, but I'm also happy to accept a PR that makes manual installs skip it if the user requests it.

ngrewe commented 3 years ago

A while back, we tried to upstream support for libobjc2 into libdispatch, but were shot down because the Swift team insisted on always shipping their bundled version of the blocks runtime. So unless the stance on that on the libdispatch/Swift side has changed, making installation of the header optional seems like the best approach.

triplef commented 3 years ago

So we’ve been using the Swift libdispatch + libobjc2 on Android without any patches (also not https://github.com/apple/swift-corelibs-libdispatch/pull/534), and are using blocks and GCD extensively, and haven’t run into any issues, with libdispatch linking against their libBlocksRuntime. Can one of you elaborate on the differences of the different blocks runtimes, and potential issues mixing them?

And just to put this out there, would it make sense to go the other way around and add support for using libdispatch’s blocks runtime in libobjc2?

davidchisnall commented 3 years ago

I haven't looked at the Swift one but the LLVM one did not include the right hooks for Objective-C interop with anything other than the Apple runtime. This won't matter at all if you use ARC (ARC just generates block destructors that call ARC functions). If this isn't working, then you can't do things like send a -retain message to a block. In EtoileFoundation, we used to have Smalltalk-style -value, -value:, and so on methods declared on blocks that would invoke them if they had matching type signatures so that you could use blocks or any other Objective-C object with the same signature, but Apple never went in that direction and blocks are a pretty tacked-on feature now. In Objective-C++, I tend to use C++ lambdas instead of C blocks, because the ownership semantics are a lot clearer. The _Block_release implementation needs to call out to the ARC implementation if you want __weak pointers to blocks to work - I don't know if you're doing that but it's the only way of cleaning up after some blocks.

triplef commented 3 years ago

Thanks for the explanation @davidchisnall, that’s very helpful! Too bad Apple didn’t take @ngrewe’s patch to allow using the libobjc2 blocks runtime. Not sure if it could help if the PR came from you David with a more technical explanation along the lines of the above as to why this is needed?

davidchisnall commented 3 years ago

I haven't tried using the Swift blocks runtime. It might be possible - I don't know what hooks they provide for blocks support. I'm not particularly attached to keeping a blocks runtime in libobjc2 - we shipped one because it was the only open source one at the time, not because I believe it's the right place for one to live. If Apple's runtime provides useable hooks for integrating with weak references and so on then we should use theirs.

buzzdeee commented 7 months ago

This just did hit me here on OpenBSD amd64 as well. Tested libobjc2 2.2, which finally builds, and produces binaries, that run and don't crash. Up to now, I had libobjc 1.8.1 installed and gnustep-base was linking against libdispatch. Uninstalling and disabling picking up libdispatch make all work. There are only 2 or 3 other ports depending on libdispatch, so I probably get away here with a @conflict marker for the package for the time being. However, it would be nice if things could be de-conflicted one way or another.

qmfrederik commented 7 months ago

Yes, there's a similar situation when packaging GNUstep for MSYS2. GNUstep depends on libobjc2 and optionally libdispatch, both which provide a blocks runtime. Additionally, MSYS2 contains libBlocksRuntime based on compiler-rt.

ethanc8 commented 7 months ago

@davidchisnall When building GNUstep, should we patch out libdispatch's own Blocks Runtime and use libobjc2's Blocks Runtime? If so, should we keep a repo which tracks libdispatch's stable releases except that we apply the patch to remove libdispatch's Blocks Runtime? In that case, do we need to build libobjc2 before libdispatch? I'm not exactly sure how all of these dependencies fit together.

triplef commented 7 months ago

We carry @ngrewe’s patch in the Windows and Android toolchains and apply it via the build scripts. Not sure what would be a good setup for other platforms.

I believe libdispatch needs to be built after libobjc2 in order for libobjc2’s blocks runtime to be found.

davidchisnall commented 7 months ago

All of the versions of this header should be interoperable. I thought we added a configuration option to not install ours. That should be sufficient if you have one from another source.

ethanc8 commented 6 months ago

If we have a choice, is it better to use libobjc2's or libdispatch's? I heard that libobjc2 had support for using the Blocks as Objective-C objects, while libdispatch's didn't, but I don't know if anything has changed.

davidchisnall commented 6 months ago

The headers should be compatible. We don’t currently have an option to build libobjc2 without the blocks runtime (when we shipped it, it was the only open source implementation). We use the hooks libdispatch exposes for autorelease pools. I’ve not seen what happens if you compile out our blocks runtime and use the libdispatch or clang one. It may just work now, which would mean we could default to not building ours.

ngrewe commented 4 months ago

It may just work now

Not quite, I'm afraid. I've started working on this over here and the initial result is that most of the blocks related tests continue to pass, but the WeakBlock_arc test doesn't (as you predicted above). That might be related to the fact that I'm not doing anything about the _Block_use_RR2 hooks that are used to install Obj-C retain/release personalities. I haven't look at it in greater detail yet, though.

ngrewe commented 4 months ago

That might be related to the fact that I'm not doing anything about the _Block_use_RR2 hooks

yupp, that turned out to be the main bit here. There also were a few additional oddities, but I've got an initial PR ready (#293)

ethanc8 commented 4 months ago

The headers should be compatible. We don’t currently have an option to build libobjc2 without the blocks runtime (when we shipped it, it was the only open source implementation). We use the hooks libdispatch exposes for autorelease pools. I’ve not seen what happens if you compile out our blocks runtime and use the libdispatch or clang one. It may just work now, which would mean we could default to not building ours.

@davidchisnall Even if the headers are compatible, having both the libdispatch and libobjc2 blocks runtime built would mean that linking any Objective-C application against libdispatch should end up with a symbol conflict. I don't know why this doesn't happen.

ngrewe commented 4 months ago

@davidchisnall Even if the headers are compatible, having both the libdispatch and libobjc2 blocks runtime built would mean that linking any Objective-C application against libdispatch should end up with a symbol conflict. I don't know why this doesn't happen.

I think the answer is that some packagers (e.g. Ubuntu) make the libBlocksRuntime objects into weak symbols, so that the ones from libobjc2 take precedence:

> objdump -T libBlocksRuntime.so

libBlocksRuntime.so:     file format elf64-x86-64

DYNAMIC SYMBOL TABLE:
0000000000000000      DF *UND*  0000000000000000 (GLIBC_2.2.5) free
0000000000000000  w   D  *UND*  0000000000000000  Base        _ITM_deregisterTMCloneTable
0000000000000000      DF *UND*  0000000000000000 (GLIBC_2.3.4) __memmove_chk
0000000000000000  w   D  *UND*  0000000000000000  Base        __gmon_start__
0000000000000000      DF *UND*  0000000000000000 (GLIBC_2.14) memcpy
0000000000000000      DF *UND*  0000000000000000 (GLIBC_2.2.5) malloc
0000000000000000      DF *UND*  0000000000000000 (GLIBC_2.3.4) __printf_chk
0000000000000000      DF *UND*  0000000000000000 (GLIBC_2.2.5) exit
0000000000000000  w   D  *UND*  0000000000000000  Base        _ITM_registerTMCloneTable
0000000000000000  w   DF *UND*  0000000000000000 (GLIBC_2.2.5) __cxa_finalize
0000000000000000      DF *UND*  0000000000000000 (GLIBC_2.3.4) __sprintf_chk
0000000000004350  w   DO .bss   0000000000000100  Base        _NSConcreteStackBlock
0000000000004650  w   DO .bss   0000000000000100  Base        _NSConcreteFinalizingBlock
0000000000004550  w   DO .bss   0000000000000100  Base        _NSConcreteAutoBlock
0000000000001350  w   DF .text  00000000000001a4  Base        _Block_object_assign
0000000000001290  w   DF .text  0000000000000095  Base        _Block_release
0000000000004450  w   DO .bss   0000000000000100  Base        _NSConcreteMallocBlock
0000000000004850  w   DO .bss   0000000000000100  Base        _NSConcreteWeakBlockVariable
0000000000004750  w   DO .bss   0000000000000100  Base        _NSConcreteGlobalBlock
0000000000001330  w   DF .text  0000000000000012  Base        Block_size
00000000000015b0  w   DF .text  0000000000000397  Base        _Block_dump
0000000000001950  w   DF .text  000000000000014c  Base        _Block_byref_dump
0000000000001500  w   DF .text  00000000000000a9  Base        _Block_object_dispose
00000000000011e0  w   DF .text  00000000000000a7  Base        _Block_copy
ethanc8 commented 4 months ago

@davidchisnall Even if the headers are compatible, having both the libdispatch and libobjc2 blocks runtime built would mean that linking any Objective-C application against libdispatch should end up with a symbol conflict. I don't know why this doesn't happen.

I think the answer is that some packagers (e.g. Ubuntu) make the libBlocksRuntime objects into weak symbols, so that the ones from libobjc2 take precedence:

> objdump -T libBlocksRuntime.so

libBlocksRuntime.so:     file format elf64-x86-64

DYNAMIC SYMBOL TABLE:
0000000000000000      DF *UND*    0000000000000000 (GLIBC_2.2.5) free
0000000000000000  w   D  *UND*    0000000000000000  Base        _ITM_deregisterTMCloneTable
0000000000000000      DF *UND*    0000000000000000 (GLIBC_2.3.4) __memmove_chk
0000000000000000  w   D  *UND*    0000000000000000  Base        __gmon_start__
0000000000000000      DF *UND*    0000000000000000 (GLIBC_2.14) memcpy
0000000000000000      DF *UND*    0000000000000000 (GLIBC_2.2.5) malloc
0000000000000000      DF *UND*    0000000000000000 (GLIBC_2.3.4) __printf_chk
0000000000000000      DF *UND*    0000000000000000 (GLIBC_2.2.5) exit
0000000000000000  w   D  *UND*    0000000000000000  Base        _ITM_registerTMCloneTable
0000000000000000  w   DF *UND*    0000000000000000 (GLIBC_2.2.5) __cxa_finalize
0000000000000000      DF *UND*    0000000000000000 (GLIBC_2.3.4) __sprintf_chk
0000000000004350  w   DO .bss 0000000000000100  Base        _NSConcreteStackBlock
0000000000004650  w   DO .bss 0000000000000100  Base        _NSConcreteFinalizingBlock
0000000000004550  w   DO .bss 0000000000000100  Base        _NSConcreteAutoBlock
0000000000001350  w   DF .text    00000000000001a4  Base        _Block_object_assign
0000000000001290  w   DF .text    0000000000000095  Base        _Block_release
0000000000004450  w   DO .bss 0000000000000100  Base        _NSConcreteMallocBlock
0000000000004850  w   DO .bss 0000000000000100  Base        _NSConcreteWeakBlockVariable
0000000000004750  w   DO .bss 0000000000000100  Base        _NSConcreteGlobalBlock
0000000000001330  w   DF .text    0000000000000012  Base        Block_size
00000000000015b0  w   DF .text    0000000000000397  Base        _Block_dump
0000000000001950  w   DF .text    000000000000014c  Base        _Block_byref_dump
0000000000001500  w   DF .text    00000000000000a9  Base        _Block_object_dispose
00000000000011e0  w   DF .text    00000000000000a7  Base        _Block_copy

That must be it! Thanks! (I still feel like it'd be better to patch out libdispatch's blocks runtime though.)

ngrewe commented 4 months ago

I still feel like it'd be better to patch out libdispatch's blocks runtime though.

Or, if you're feeling adventurous, you could test drive #293 (configured with -DEMBEDDED_BLOCKS_RUNTIME=OFF), which goes the opposite route and swaps the blocks runtime in libobjc2 for the one from libdispatch 😅