Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
fff7d1bc opened this issue Mar 13, 2021 · 19 comments · Fixed by #293
Closed

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

fff7d1bc opened this issue Mar 13, 2021 · 19 comments · Fixed by #293

Comments

@fff7d1bc
Copy link

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
Copy link
Member

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
Copy link
Member

ngrewe commented Mar 14, 2021

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
Copy link
Member

triplef commented Mar 15, 2021

So we’ve been using the Swift libdispatch + libobjc2 on Android without any patches (also not swiftlang/swift-corelibs-libdispatch#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
Copy link
Member

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
Copy link
Member

triplef commented Mar 19, 2021

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
Copy link
Member

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
Copy link

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
Copy link
Collaborator

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
Copy link

ethanc8 commented Mar 6, 2024

@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
Copy link
Member

triplef commented Mar 7, 2024

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
Copy link
Member

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
Copy link

ethanc8 commented Mar 20, 2024

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
Copy link
Member

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
Copy link
Member

ngrewe commented May 24, 2024

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
Copy link
Member

ngrewe commented May 29, 2024

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
Copy link

ethanc8 commented May 31, 2024

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
Copy link
Member

ngrewe commented May 31, 2024

@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
Copy link

ethanc8 commented May 31, 2024

@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
Copy link
Member

ngrewe commented May 31, 2024

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 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

7 participants