Skip to content

[libc++] Remove _LIBCPP_DISABLE_AVAILABILITY macro #112952

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 24, 2024
Merged

[libc++] Remove _LIBCPP_DISABLE_AVAILABILITY macro #112952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions libcxx/docs/ReleaseNotes/20.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ Deprecations and Removals
headers as an extension and only deprecates them. The ``_LIBCPP_DISABLE_DEPRECATION_WARNINGS`` macro can be defined to
suppress deprecation for these headers.

- The ``_LIBCPP_DISABLE_AVAILABILITY`` macro that was used to force-disable availability markup has now been removed.
Whether availability markup is used by the library is now solely controlled at configuration-time.
Comment on lines +105 to +106
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks the setup in our distribution pretty essentially. We always ship an up-to-date libc++, but keep compiling for pretty old deployment targets (currently still on 10.13). The only way to make many current-gen C++ libraries work under those constraints is to disable the availability annotations, which has been how we've been supporting both modern packages AND old hardware (docs). This happens at build time for said library, but at runtime from the POV of the already-compiled libc++.

We currently have >250 packages making use of this. What would be an alternative in a world after this PR? Just define _LIBCPP_HAS_NO_VENDOR_AVAILABILITY_ANNOTATIONS?

I realize that we're not a common use-case in and of ourselves, but we do have >2B of monthly downloads (across all platforms, not just osx), so a relatively big multiplier. We believe that keeping compatibility for as long as feasible is an important quality, and since we've got the infrastructural wherewithal to solve the technical issues, it would be a pity to shut this door.

CC @isuruf @xhochy @beckermr @jakirkham

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you ship your own libc++ you shouldn't enable availability annotations. I don't know exactly what kind of hacky solution you're using, but you probably want to inject your own __config_site. That's obviously not officially sanctioned, but should work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know exactly what kind of hacky solution you're using, but you probably want to inject your own __config_site.

If that makes it possible to tell clang to ignore the system libcxx and use the one we're shipping, that'd be great! That's really the key issue we're working around.

If you ship your own libc++ you shouldn't enable availability annotations.

Yeah, I can understand that. Our case is simple though - our availability annotations are equivalent to what the current(==newest) version of libcxx supports, so it's equivalent to _LIBCPP_DISABLE_AVAILABILITY for that version AFAIU. (and there's some details w.r.t. libc++abi that I'm leaving out, see discussion in #110920).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@h-vetinari I think what you want here is simply to define LIBCXX_ENABLE_VENDOR_AVAILABILITY_ANNOTATIONS=OFF at CMake configuration time. In fact, LIBCXX_ENABLE_VENDOR_AVAILABILITY_ANNOTATIONS=OFF is the default, so I think there's something I'm not understanding about your use case. Why do you manually enable LIBCXX_ENABLE_VENDOR_AVAILABILITY_ANNOTATIONS=ON and then attempt to disable it via some other mechanism?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ldionne I think they actually use the Apple SDK headers but ship their own libc++.so to support some programs on older platforms.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, -nostdinc++ works when a SDK is in use. There's nothing different when a SDK is in use, really: it's just that the sysroot where Clang starts looking for stuff is located inside the SDK, but all the behavior is otherwise the same.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So to answer the questions w.r.t. our setup (to the best of my understanding, much of the setup was done before my time, I'm trying to document & improve things as I go).

Because our users are necessarily installing a full environment (where we control dependencies and distribution), we can rely on the fact that we can always ship an up-to-date libc++.dylib. We also use up-to-date the libc++ headers when compiling things, but we still need to use point to some Apple SDK (for which older versions are available, e.g. from https://github.com/phracker/MacOSX-SDKs or https://github.com/alexey-lysiuk/macos-sdk), and we're generally on a 10.13 baseline for now.

We need the SDK for platform-related things and the C stdlib, so this is kind of unavoidable. However, the libc++ headers will trigger availability warnings based on the detected macOS version and therefore report false positives (from our POV, because our C++ stdlib is always up-to-date and available).

These things happen at runtime (from the POV of libc++), because different packages may require a newer SDK for some reason, and we can provide that based on the configuration of a given build (often while still leaving MACOSX_DEPLOYMENT_TARGET unchanged).

We're also relying on ABI stability of libc++, in the sense that artefacts compiled against libc++ vN get a runtime dependency libcxx >=N, so we that if we later have libc++.{N+2}.dylib in the environment, things still work in the vast majority of situations. I don't want to scare people here in that this is what the ironclad expectation is, but rather that this is something that has worked for us for years, and rebuilding literally 10's of thousands of artefacts for every new libc++ version is simply not realistic.

So if _LIBCPP_DISABLE_AVAILABILITY gets removed, AFAIU we'd have to either:

  • Patch things so that we recreated the effect of _LIBCPP_DISABLE_AVAILABILITY
  • Provide our own vendor availability annotations that match that effect

Neither sounds impossible, but we try to minimize the amount of patches we carry, obviously...

Actually there is one exception where we do modify the default availability annotations, which has to do with another wrinkle. While we could build and ship libc++abi just fine (and we did in the past), there are situations where certain libraries or frameworks (including CPython) will bypass the fact that everything points to our custom $PREFIX and simply go to the system libc++abi straight away. Because it's basically impossible to chase down and patch all cases where this happens, we've stopped shipping libc++abi and now always rely on the system-provided one, which is why we unconditionally disable _LIBCPP_AVAILABILITY_HAS_INIT_PRIMARY_EXCEPTION for example, as the required ___cxa_init_primary_exception isn't available before macOS 15 AFAIU (and our deployment target baseline is much lower). This is also why I'm interested in -DLIBCXX_CXX_ABI=system-libcxxabi, c.f. #110920.

Finally, where we're aware of it, we do patch relevant ABI differences for older macOS system libs, compare e.g. this patch to realign a struct, resp. the related discussion (I think the original email thread got split a bit by the discourse migration). This isn't great, but given that the system ABI isn't 100% stable, this is +/- the best we can do under the given constraints.

Hope that explains the situation a bit more. Happy to answer more questions of course, and thanks for giving us a chance to make the case!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@h-vetinari All of this doesn't explain why you can't use your own libc++ headers via -nostdinc++. Is it just that you've never though of that or is there an issue with using that strategy?

Copy link
Member Author

@ldionne ldionne Nov 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because our users are necessarily installing a full environment (where we control dependencies and distribution), we can rely on the fact that we can always ship an up-to-date libc++.dylib. We also use up-to-date the libc++ headers when compiling things, but we still need to use point to some Apple SDK (for which older versions are available, e.g. from https://github.com/phracker/MacOSX-SDKs or https://github.com/alexey-lysiuk/macos-sdk), and we're generally on a 10.13 baseline for now.

Ok, this makes sense. I assume you're doing something like -isysroot <PATH-TO-OLDER-SDK>, which means you're getting the C library headers (and other system headers) from that SDK. However, as a side effect, you're also getting the older C++ Standard Library headers located in SDKROOT/usr/include/c++/v1, and those have undesirable availability markup in them.

You don't want that. In particular, you really really don't want to be using Apple "system libc++" headers from the SDK but linking against your own self-built libc++.a or libc++.dylib. That's brittle and can break badly at any point. Those are two distinct libraries and they only happen to share the same name and the same symbol names for the most part, but pedantically you're using entirely incompatible headers and built library. If you want to use the SDK but provide your own C++ Standard Library, you should do this:

-isysroot <SDK>
-nostdinc++ -isystem <path-to-your-libcxx-headers>
-nostdlib++ -L <path-to-your-libcxx-dylib> -l c++

That way, you're overriding both the library and the header search paths to make sure you find consistent ones. I think this is where @philnik777 was going above. Please let me know if this makes sense to you.

Edit: Going even further, it's a shame that we didn't select a different namespace and install-name for the Apple system libc++. It would have made these issues so much clearer. For example, if the Apple system libc++ were using an inline namespace like std::__applesdk:: instead of std::__1:: like upstream LLVM, you'd be getting a clear linker error when you try to mix the two, and many subtly incorrect setups would have been caught. Unfortunately, changing this has serious challenges at this point.

Finally, where we're aware of it, we do patch relevant ABI differences for older macOS system libs, compare e.g. this patch to realign a struct, resp. the related discussion (I think the original email thread got split a bit by the discourse migration). This isn't great, but given that the system ABI isn't 100% stable, this is +/- the best we can do under the given constraints.

We do strive to be 100% backwards compatible. I added a reply to the Discourse thread explaining what I think is happening here and why I don't think that's libc++'s "fault": https://discourse.llvm.org/t/shipping-custom-libc-on-macos/58606/7. TLDR, it looks to me like some programs end up with multiple copies of libc++ in them, which is an ODR violation. Reordering these lines as you do in the patch simply makes that ODR violation be benign and inconsequential, but doesn't technically solve the problem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the comprehensive response here. It was a fair amount for me to chew on, which is why I ended up postponing having to deal with it a few times - apologies 😅

I'll try to use "just the C bits" of the SDK as you describe. It sounds like a more solid approach already so I'm hoping that'll work. 🤞

Also 💯 on the different symbol namespaces, that would have been excellent. Finally, the case you commented on in discourse really is unusual (also for us), in it's triggered by an uncommon case of flags (using -Wl,-rpath, but not -L to point to $PREFIX/lib, where our libc++ lives.)


Upcoming Deprecations and Removals
----------------------------------

Expand Down
7 changes: 0 additions & 7 deletions libcxx/include/__configuration/availability.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,6 @@
//
// [1]: https://clang.llvm.org/docs/AttributeReference.html#availability

// For backwards compatibility, allow users to define _LIBCPP_DISABLE_AVAILABILITY
// for a while.
#if defined(_LIBCPP_DISABLE_AVAILABILITY)
# undef _LIBCPP_HAS_VENDOR_AVAILABILITY_ANNOTATIONS
# define _LIBCPP_HAS_VENDOR_AVAILABILITY_ANNOTATIONS 0
#endif

// Availability markup is disabled when building the library, or when a non-Clang
// compiler is used because only Clang supports the necessary attributes.
#if defined(_LIBCPP_BUILDING_LIBRARY) || defined(_LIBCXXABI_BUILDING_LIBRARY) || !defined(_LIBCPP_COMPILER_CLANG_BASED)
Expand Down
Loading