gnustep / libobjc2

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

Rework ARC code as C++. #119

Closed davidchisnall closed 4 years ago

davidchisnall commented 5 years ago

Move the weak references hash table to use a well-tested third-party implementation instead of the hand-rolled version.

triplef commented 5 years ago

FYI I tried building this branch for Android but get the following linker error:

clang --target=armv7-none-linux-androideabi21 --gcc-toolchain=/Users/frederik/Library/Android/android-ndk-r20-clang-r353983d/toolchains/llvm/prebuilt/darwin-x86_64 --sysroot=/Users/frederik/Library/Android/android-ndk-r20-clang-r353983d/toolchains/llvm/prebuilt/darwin-x86_64/sysroot -fPIC -g -DANDROID -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -fno-addrsig -march=armv7-a -mthumb -Wa,--noexecstack -Wformat -Werror=format-security -DDEBUG_EXCEPTIONS=1 -Xclang -fexceptions -Xclang -fobjc-exceptions -O0 -Xclang -fno-inline -O0 -fno-limit-debug-info  -Wl,--exclude-libs,libgcc.a -Wl,--exclude-libs,libatomic.a -static-libstdc++ -Wl,--build-id -Wl,--warn-shared-textrel -Wl,--fatal-warnings -Wl,--exclude-libs,libunwind.a -Wl,--no-undefined -Qunused-arguments -Wl,-z,noexecstack -shared -Wl,-soname,libobjc.so -o libobjc.so CMakeFiles/objc.dir/alias_table.c.o CMakeFiles/objc.dir/block_to_imp.c.o CMakeFiles/objc.dir/caps.c.o CMakeFiles/objc.dir/category_loader.c.o CMakeFiles/objc.dir/class_table.c.o CMakeFiles/objc.dir/dtable.c.o CMakeFiles/objc.dir/encoding2.c.o CMakeFiles/objc.dir/hooks.c.o CMakeFiles/objc.dir/ivar.c.o CMakeFiles/objc.dir/loader.c.o CMakeFiles/objc.dir/mutation.m.o CMakeFiles/objc.dir/protocol.c.o CMakeFiles/objc.dir/runtime.c.o CMakeFiles/objc.dir/sarray2.c.o CMakeFiles/objc.dir/selector_table.c.o CMakeFiles/objc.dir/sendmsg2.c.o CMakeFiles/objc.dir/eh_personality.c.o CMakeFiles/objc.dir/block_trampolines.S.o CMakeFiles/objc.dir/objc_msgSend.S.o CMakeFiles/objc.dir/NSBlocks.m.o CMakeFiles/objc.dir/Protocol2.m.o CMakeFiles/objc.dir/associate.m.o CMakeFiles/objc.dir/blocks_runtime.m.o CMakeFiles/objc.dir/properties.m.o CMakeFiles/objc.dir/gc_none.c.o CMakeFiles/objc.dir/arc.mm.o CMakeFiles/objc.dir/objcxx_eh.cc.o -L/Users/frederik/Library/Android/GNUstep/armeabi-v7a/lib -lcxxrt -latomic -lm && :
/Users/frederik/Library/Android/android-ndk-r20-clang-r353983d/toolchains/llvm/prebuilt/darwin-x86_64/sysroot/usr/include/c++/v1/stdexcept:0: error: undefined reference to 'typeinfo for std::length_error'
/Users/frederik/Library/Android/android-ndk-r20-clang-r353983d/toolchains/llvm/prebuilt/darwin-x86_64/sysroot/usr/include/c++/v1/stdexcept:0: error: undefined reference to 'std::length_error::~length_error()'
/Users/frederik/Library/Android/android-ndk-r20-clang-r353983d/toolchains/llvm/prebuilt/darwin-x86_64/sysroot/usr/include/c++/v1/stdexcept:136: error: undefined reference to 'std::logic_error::logic_error(char const*)'
/Users/frederik/Library/Android/android-ndk-r20-clang-r353983d/toolchains/llvm/prebuilt/darwin-x86_64/sysroot/usr/include/c++/v1/stdexcept:0: error: undefined reference to 'vtable for std::length_error'
/Users/frederik/Library/Android/android-ndk-r20-clang-r353983d/toolchains/llvm/prebuilt/darwin-x86_64/lib/gcc/arm-linux-androideabi/4.9.x/../../../../arm-linux-androideabi/bin/ld: the vtable symbol may be undefined because the class is missing its key function
clang: error: linker command failed with exit code 1 (use -v to see invocation)
davidchisnall commented 5 years ago

Can you see (from the preprocessed output of arc.mm) what is pulling in those symbols? The map table is compiled to not throw exceptions and there's a separate allocator implementation so they shouldn't come from std::allocator. On FreeBSD, none of the libc++ headers pull them in and on Ubuntu we won't see this failure because we're using libstdc++ (not libsupc++) as the C++ runtime.

triplef commented 5 years ago

If I read the preprocessor output correctly it comes from robin_map.h including functional, which in the end includes memory and stdexcept. The former contains an allocator implementation referencing __throw_length_error(). Not sure if that’s expected?

Here is the full preprocessor output: arc.mm.txt

davidchisnall commented 5 years ago

It looks as if the use of std::vector by the map is pulling in exceptions. Can you try adding #define _LIBCPP_NO_EXCEPTIONS 1 at the top of the .mm file (before any #includes) and see if that fixes it?

triplef commented 5 years ago

Yep that fixes it! 👍 I’ll try the weak references tests now and report back.

triplef commented 5 years ago

Using this branch, the weak references tests pass on ARMv7, i.e. it fixes #107.