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

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

Closed
Lishen1 opened this issue Jun 29, 2021 · 12 comments
Closed

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

Lishen1 opened this issue Jun 29, 2021 · 12 comments
Assignees
Projects

Comments

@Lishen1
Copy link

Lishen1 commented Jun 29, 2021


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
@Lishen1 Lishen1 added the bug label Jun 29, 2021
@pirama-arumuga-nainar
Copy link
Collaborator

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

No, I haven't filed a tracking bug.

@DanAlbert DanAlbert added this to Awaiting triage in LLVM via automation Oct 14, 2021
@DanAlbert DanAlbert moved this from Awaiting triage to Awaiting fix in LLVM Oct 14, 2021
@DanAlbert DanAlbert changed the title [BUG] deleted copy constructor of shared_ptr's deleter does not compile Oct 14, 2021
@DanAlbert DanAlbert pinned this issue Feb 1, 2022
@DanAlbert DanAlbert changed the title [FR] update libc++ Feb 22, 2023
@DanAlbert
Copy link
Member

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

equeim commented Apr 13, 2023

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

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 DanAlbert changed the title [FR] pretty much everything related to libc++ Apr 28, 2023
@DanAlbert DanAlbert changed the title [FR] libc++ hasn't been updated since April 2020, pretty much everything related to libc++ is this bug Apr 28, 2023
@DanAlbert DanAlbert changed the title [FR] libc++ not updated since April 2020, all libc++ bugs go here Apr 28, 2023
@DanAlbert
Copy link
Member

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

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

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

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

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

Zingam commented Jul 29, 2023

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

@DanAlbert
Copy link
Member

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.

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