android / ndk

The Android Native Development Kit
1.96k stars 254 forks source link

[FR] libc++ not updated since April 2020, all libc++ issues #1530

Closed Lishen1 closed 1 year ago

Lishen1 commented 3 years ago

Description

for c++17 standard this code must be compiled: doc

template< class Y, class Deleter >shared_ptr( Y* ptr, Deleter d ); | (4) -- | --
4-5) Uses the specified deleter d as the deleter. The expression d(ptr) must be well formed, have well-defined behavior and not throw any exceptions. The construction of d and of the stored deleter from d must not throw exceptions. Deleter must be CopyConstructible. | (until C++17) -- | -- These constructors additionally do not participate in overload resolution if the expression d(ptr) is not well-formed, or if std::is_move_constructible::value is false. | (since C++17)
struct A{
    int d;
};

template <typename C>
struct CDeleter {
    std::function<void(C*)> primary;
    std::vector<std::function<void()>> also;

    CDeleter() = default;
    explicit CDeleter(std::function<void(C*)> primary)
            : primary{std::move(primary)} {}

    CDeleter(const CDeleter&) = delete;

    CDeleter(CDeleter&&) noexcept = default;
    CDeleter& operator=(const CDeleter&) = delete;
    CDeleter& operator=(CDeleter&&) noexcept = default;

    void operator()(C* c) {
        if (primary) {
            primary(c);
        }
        for (auto&& fn : also) {
            fn();
        }
        delete c;
    }
};

int ff()
{
    CDeleter<A> deleter;
    auto s = std::shared_ptr<A>(new A, std::move(deleter));

    return 0;
}

but it gives an error:

Build command failed.
Error while executing process C:\Users\username\AppData\Local\Android\Sdk\cmake\3.18.1\bin\ninja.exe with arguments {-C D:\development\test-projects\libaipc\app\.cxx\cmake\debug\x86_64 alephzero libaipc}
ninja: Entering directory `D:\development\test-projects\libaipc\app\.cxx\cmake\debug\x86_64'
[1/2] Building CXX object src/CMakeFiles/libaipc.dir/libaipc.cpp.o
FAILED: src/CMakeFiles/libaipc.dir/libaipc.cpp.o 
C:\Users\username\AppData\Local\Android\Sdk\ndk\22.1.7171670\toolchains\llvm\prebuilt\windows-x86_64\bin\clang++.exe --target=x86_64-none-linux-android29 --gcc-toolchain=C:/Users/username/AppData/Local/Android/Sdk/ndk/22.1.7171670/toolchains/llvm/prebuilt/windows-x86_64 --sysroot=C:/Users/username/AppData/Local/Android/Sdk/ndk/22.1.7171670/toolchains/llvm/prebuilt/windows-x86_64/sysroot -DUSE_BAR -DUSE_FOO -D_LIBCPP_NO_EXCEPTIONS -Dlibaipc_EXPORTS -ID:/development/test-projects/libaipc/app/src/main/cpp/include -I_deps/alephzero-src/include -g -DANDROID -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security  -std=c++17 -O0 -fno-limit-debug-info  -fPIC -Wall -Wextra -Wshadow -Wnon-virtual-dtor -Wold-style-cast -Wcast-align -Wunused -Woverloaded-virtual -Wpedantic -Wconversion -Wsign-conversion -Wnull-dereference -Wdouble-promotion -Wformat=2 -std=gnu++17 -MD -MT src/CMakeFiles/libaipc.dir/libaipc.cpp.o -MF src\CMakeFiles\libaipc.dir\libaipc.cpp.o.d -o src/CMakeFiles/libaipc.dir/libaipc.cpp.o -c D:/development/test-projects/libaipc/app/src/main/cpp/src/libaipc.cpp
D:/development/test-projects/libaipc/app/src/main/cpp/src/libaipc.cpp:56:72: warning: unused parameter 'thiz' [-Wunused-parameter]
Java_com_android_libipc_IpcProvider_stringFromJNI(JNIEnv *env, jobject thiz) {
                                                                       ^
In file included from D:/development/test-projects/libaipc/app/src/main/cpp/src/libaipc.cpp:4:
In file included from C:/Users/username/AppData/Local/Android/Sdk/ndk/22.1.7171670/toolchains/llvm/prebuilt/windows-x86_64/sysroot/usr/include/c++/v1\string:504:
In file included from C:/Users/username/AppData/Local/Android/Sdk/ndk/22.1.7171670/toolchains/llvm/prebuilt/windows-x86_64/sysroot/usr/include/c++/v1\string_view:175:
In file included from C:/Users/username/AppData/Local/Android/Sdk/ndk/22.1.7171670/toolchains/llvm/prebuilt/windows-x86_64/sysroot/usr/include/c++/v1\__string:57:
In file included from C:/Users/username/AppData/Local/Android/Sdk/ndk/22.1.7171670/toolchains/llvm/prebuilt/windows-x86_64/sysroot/usr/include/c++/v1\algorithm:643:
C:/Users/username/AppData/Local/Android/Sdk/ndk/22.1.7171670/toolchains/llvm/prebuilt/windows-x86_64/sysroot/usr/include/c++/v1\memory:4013:39: error: call to deleted constructor of 'CDeleter<A>'
        __cntrl_ = new _CntrlBlk(__p, __d, _AllocT());
                                      ^~~
D:/development/test-projects/libaipc/app/src/main/cpp/src/libaipc.cpp:48:14: note: in instantiation of function template specialization 'std::__ndk1::shared_ptr<A>::shared_ptr<A, CDeleter<A>>' requested here
    auto s = std::shared_ptr<A>(new A, std::move(deleter));
             ^
D:/development/test-projects/libaipc/app/src/main/cpp/src/libaipc.cpp:28:5: note: 'CDeleter' has been explicitly marked deleted here
    CDeleter(const CDeleter&) = delete;
    ^
C:/Users/username/AppData/Local/Android/Sdk/ndk/22.1.7171670/toolchains/llvm/prebuilt/windows-x86_64/sysroot/usr/include/c++/v1\memory:3575:39: note: passing argument to parameter '__d' here
    __shared_ptr_pointer(_Tp __p, _Dp __d, _Alloc __a)
                                      ^
1 warning and 1 error generated.
ninja: build stopped: subcommand failed.

Environment Details

  • NDK Version: 22.1.7171670
  • Build system: ndk's cmake 3.18.1
  • Host OS: Windows
  • ABI: x86_64, x86, aarch64, arm
  • NDK API level: 29
  • Device API level: 30
pirama-arumuga-nainar commented 3 years ago

IIUC, this is part of LWG 2875, which is supported in libcxx from Feb 2021. @DanAlbert is there a github issue for updating the libcxx in the NDK?

DanAlbert commented 3 years ago

No, I haven't filed a tracking bug.

DanAlbert commented 1 year ago

For anyone landing here from the pinned issue: we do not need more reports of the problems caused by libc++ being out of date. This is being actively worked on (and has been for a very long time, it's just a lot of work). If you want updates, click the subscribe button on the right side of the page. We'll update this bug when we have something useful to say, but reviving the cross-testing support that was removed from upstream is taking a significant amount of time.

equeim commented 1 year ago

I think it would be nice to explicitly mention that "based on LLVM 14 development" in NDK changelog doesn't include C++ standard library (I learned this the hard way).

DanAlbert commented 1 year ago

Work still ongoing. Highly unlikely that it will be done in time for r26, which is kicking off Very Soon. We'll pull it back into this release if it miraculously takes less time.

DanAlbert commented 1 year ago

We finally reached a point where some work could be done in parallel and that sped things up pretty significantly. https://android-review.googlesource.com/c/platform/ndk/+/2579155 ostensibly fixes this, but it still needs quite a lot of testing (and one more toolchain respin because the one we have now missed an important flag when building libc++abi).

If the tests pass (it will take a few days at least to run them; it's now possible to run them but it's still not easy), this will be included in r26. If they don't we'll need to evaluate the failures and make a guess about how much longer it would take to get them passing. If it can be done in a few weeks we'll wait before moving on with r26, but we can't really delay r26 beta 1 much longer.

justusranvier commented 1 year ago

It's not really clear from the thread but how recent of a libc++ will we get in the next update, either in r26 or r27?

The upstream LLVM 16 version got much closer to finishing C++17 support compared to version 15, and those additions would be highly welcome on Android.

DanAlbert commented 1 year ago

This is now available in the canary builds. It'd be a huge help if folks could try that out and let us know if there are any bugs. Updating required us to redo our testing process for libc++ from the ground up (that's why it took so damn long). Knowing that it still works in the real world would put my mind at ease. (We are not interested in "it still doesn't include support for $FEATURE", that's beyond our power to fix; we're just looking for regressions.)

how recent of a libc++ will we get in the next update, either in r26 or r27?

You can see the upcoming r26 changelog here: https://android.googlesource.com/platform/ndk/+/master/docs/changelogs/Changelog-r26.md

Updated LLVM to clang-r487747b, based on LLVM 17 development.

You can download the canary and see:

$ cat android-ndk-r26-canary/toolchains/llvm/prebuilt/darwin-x86_64/AndroidVersion.txt
17.0.1
based on r487747b
for additional information on LLVM revision and cherry-picks, see clang_source_info.md

Which in turn says:

$ cat android-ndk-r26-canary/toolchains/llvm/prebuilt/darwin-x86_64/clang_source_info.md 
Base revision: [c4c5e79dd4b4c78eee7cffd9b0d7394b5bedcf12](https://github.com/llvm/llvm-project/commits/c4c5e79dd4b4c78eee7cffd9b0d7394b5bedcf12)

- [Revert "[LoopVectorize] Enable integer Mul and Add as select](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/cherry/4468e27d9fff153af9826eaf12e0044e67a701a8.patch)
- [[CMake] Support undefined LLVM_NATIVE_ARCH in](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/cherry/2e3153059c268700d4b399a8cbba28e9c2514e09.patch)
- [[RISCV] Allow mismatched SmallDataLimit and use Min for](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/cherry/af128791464810123bcd60a6d9d0902b5c550aef.patch)
- [[RISCV] Fix miscompile in SExtWRemoval due to early return](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/cherry/db6bee5fec0d7fdfc18005c5c5ccd15f1ede945d.patch)
- [[MTE stack] fix incorrect offset for st2g](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/cherry/a4ab294bc01c8f538951ec223b81bfc1b2c2af6b.patch)
- [Clear read_fd_set if EINTR received](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/cherry/d8bd179a173876a7a9ee11828b63efffe145356c.patch)
- [Revert "[Clang] Implement fix for DR2628"](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/cherry/3b6c88331bcd0531d627fe27de5dbd0ac3165300.patch)
- [[HWASAN][LSAN] Only initialize Symbolizer if leak checking is](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/cherry/7b7db789ff3d8750d1098dcc84aa29d11877d610.patch)
- [Revert "[Darwin] Apply workaround to make symbolication in](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/cherry/d8b8911d58dba73fd7a28210d8d3e780ae881179.patch)
- [[llvm-objdump] Fix help message for --print-imm-hex](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/cherry/3d65cd405d64afd86a59c1f58098dfe891841271.patch)
- [[clang][driver] Pass `-femulated-tls` through to the linker](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/cherry/a78816a6b6debb548efbf1717aeeb490df42f401.patch)
- [[StackProtector] don't check stack protector before calling](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/cherry/fc4494dffa5422b2be5442c235554e76bed79c8a.patch)
- [[codegen][riscv] Emit CFI directives when using shadow call](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/cherry/ade336d6e14140134728b9991be8e4e68d6205ea.patch)
- [[patches] Cherry pick CLS for: [PATCH] [CodeGen][RISCV]](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/cherry/aa1d2693c25622ea4a8ee2b622ba2a617e18ef88.patch)
- [Add-stubs-and-headers-for-nl_types-APIs.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/Add-stubs-and-headers-for-nl_types-APIs.patch)
- [Ensure-that-we-use-our-toolchain-s-lipo-and-not-the-.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/Ensure-that-we-use-our-toolchain-s-lipo-and-not-the-.patch)
- [BOLT-Increase-max-allocation-size-to-allow-BOLTing-clang-and-rustc.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/BOLT-Increase-max-allocation-size-to-allow-BOLTing-clang-and-rustc.patch)
- [Revert-clang-Improve-diagnostics-for-expansion-length-mismatch-v3.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/Revert-clang-Improve-diagnostics-for-expansion-length-mismatch-v3.patch)
- [Revert-clang-fix-missing-initialization-of-original-number-of-expansions-v3.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/Revert-clang-fix-missing-initialization-of-original-number-of-expansions-v3.patch)
- [Revert-Enable-IAS-In-Backend-v2.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/Revert-Enable-IAS-In-Backend-v2.patch)
- [Disable-vfork-fork-events.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/Disable-vfork-fork-events.patch)
- [Enable-targeting-riscv64-linux-android.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/Enable-targeting-riscv64-linux-android.patch)
- [Revert-Driver-Allow-target-override-containing-.-in-executable-name.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/Revert-Driver-Allow-target-override-containing-.-in-executable-name.patch)
- [Wasm-omit-64-bit-function-pointer-cast.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/Wasm-omit-64-bit-function-pointer-cast.patch)
- [compiler-rt-Allow-finding-LLVMConfig-if-CMAKE_FIND_ROOT_PATH_MODE_PACKAGE-is-set-to-ONLY.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/compiler-rt-Allow-finding-LLVMConfig-if-CMAKE_FIND_ROOT_PATH_MODE_PACKAGE-is-set-to-ONLY.patch)
- [enable-libcxx-testing-using-adb.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/enable-libcxx-testing-using-adb.patch)
- [move-cxa-demangle-into-libcxxdemangle.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/move-cxa-demangle-into-libcxxdemangle.patch)
- [avoid-triggering-fdsan-in-filebuf-test.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/avoid-triggering-fdsan-in-filebuf-test.patch)
- [hide-locale-lit-features-for-bionic.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/hide-locale-lit-features-for-bionic.patch)
- [android-temp-dir-is-data-local-tmp.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/android-temp-dir-is-data-local-tmp.patch)
- [bionic-includes-plus-sign-for-nan.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/bionic-includes-plus-sign-for-nan.patch)
- [disable-symlink-resolve-test-on-android.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/disable-symlink-resolve-test-on-android.patch)
- [avoid-fifo-socket-hardlink-in-libcxx-tests.patch](https://android.googlesource.com/toolchain/llvm_android/+/49c608f1d5500df01197e1e9a8eade0e764c54bf/patches/avoid-fifo-socket-hardlink-in-libcxx-tests.patch)

those additions would be highly welcome on Android.

We understand, but the NDK can't move any faster than what is made available to us. r26 will be shipping the latest that's available to us (as is true of every NDK from here on out). In practice the LLVM in the NDK will be 0-6 weeks old at the time we take the final update before shipping beta 1. By the time that's a stable release it will be 3-5 months old. The only way that number can be reduced is by skipping betas, but no amount of testing we can do will compare to testing in the wild so that'd pretty significantly impact release quality and I'm not willing to do that.

DanAlbert commented 1 year ago

Forgot to mention: the current canary does have one known regression: if you dlclose libc++ (directly or indirectly) before thread_local destructors run, programs will crash at thread exit. If you encounter that regression you should probably be removing the dlclose call anyway. However the fix has been submitted and we'll have it in the NDK before we release to stable.

DanAlbert commented 1 year ago

We're reasonably confident that what's in the current r26 canary doesn't introduce any regressions (at least, none that aren't also regressions in the upstream release). This isn't as solid as we'd like because the tests are not in CI, but given that that's probably still ~months of work and the manual testing is showing good results, I think that's preferable to missing the r26 release, so we'll be shipping the update that's currently in the canary.

Zingam commented 1 year ago

Did you resolve your toolchain bandwidth? Are there going to be more timely LLVM updates again soon?

DanAlbert commented 1 year ago

https://github.com/android/ndk/wiki/Changelog-r26

libc++ has been updated. The NDK's libc++ now comes directly from our LLVM toolchain, so every future LLVM update is also a libc++ update. Future changelogs will not explicitly mention libc++ updates.