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

Consider adding an option to disable the use of __ulock_wait in libc++ #60466

Closed
bc-lee opened this issue Feb 2, 2023 · 3 comments
Closed

Consider adding an option to disable the use of __ulock_wait in libc++ #60466

bc-lee opened this issue Feb 2, 2023 · 3 comments
Assignees
Labels
invalid Resolved as invalid, i.e. not a bug libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. platform:ios iOS-related issues platform:macos

Comments

@bc-lee
Copy link
Contributor

bc-lee commented Feb 2, 2023

Uses of __ulock_wait / __ulock_wake in libc++ have been added to support C++20's std::atomic<T>::wait.

https://reviews.llvm.org/D68480

extern "C" int __ulock_wait(uint32_t operation, void *addr, uint64_t value,

https://github.com/llvm/llvm-project/blob/0ce25b12357b24d06cf08cc02719c144d567d5db/libcxx/src/include/apple_availability.h

These APIs are considered private APIs and will result in App Store rejection.

boostorg/atomic
rust-lang/rust

libc++ comes natively on Apple platforms, but some projects, e.g., Chromium, WebRTC, etc., prefer to static link their own libc++ builds.

https://source.chromium.org/chromium/chromium/src/+/a2a378674abbcd56a824df4b779f90a2b5a0fb3e:build/config/c++/c++.gni;l=13

  # Use in-tree libc++ (buildtools/third_party/libc++ and
  # buildtools/third_party/libc++abi) instead of the system C++ library for C++
  # standard library support.
  # Don't check in changes that set this to false for more platforms; doing so
  # is not supported.
  use_custom_libcxx = is_fuchsia || is_android || is_apple || is_linux ||
                      is_chromeos || (is_win && is_clang)

Fortunately, Chromium does not currently use C++20's std::atomic<T>::wait, so code using __ublock_wait is not currently shipped, but this may change in the future.

So it would be nice to have an option (macro) to disable the use of __ulock_wait on Apple platforms.

@fsb4000 fsb4000 added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. platform:macos platform:ios iOS-related issues and removed new issue labels Feb 2, 2023
@bc-lee
Copy link
Contributor Author

bc-lee commented Feb 10, 2023

I uploaded a patch for this: https://reviews.llvm.org/D143689

@ldionne ldionne added the invalid Resolved as invalid, i.e. not a bug label Jun 13, 2023
@ldionne
Copy link
Member

ldionne commented Jun 13, 2023

As explained in https://reviews.llvm.org/D143689:

When shipping on the App Store, you should be using the system-provided libc++. There are various reasons for that, but long story short it'll make your app a good citizen in the ecosystem.

Is there a reason why you're going for a static link instead? If you are not careful, I can imagine various ways this could behave unexpectedly.

I'll tentatively close this as NTBF, let's reopen if there's more discussion.

@ldionne ldionne closed this as completed Jun 13, 2023
@thiagomacieira
Copy link

(My old llvm.org login is not working, so I'm adding the comment here)

Given that the API is really meant to be private in Darwin and may be removed without warning, this shouldn't be "an option to disable" but instead should be "an option to enable", thus leaving the default to the fallback implementation. Therefore, users will not mistakenly enable this API and have their applications suddenly break in a couple of years.

@EugeneZelenko EugeneZelenko closed this as not planned Won't fix, can't repro, duplicate, stale Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid Resolved as invalid, i.e. not a bug libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. platform:ios iOS-related issues platform:macos
Projects
None yet
Development

No branches or pull requests

5 participants