llvmPackages.libcxx: remove option for external libcxx

[rhelmot]

Description of changes

This patch was suggested by a, uh, now-deleted user at #311777 (comment)

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[rrbutani]

suggested by a, uh, now-deleted user at #311777 (comment)

I believe it was @annaleeleaves.

cc: @paparodeo

[Ericson2314]

I can see us wanting to go back to the other ABI for FreeBSD, so I don't mind leaving this around. But maybe it is easy enough to not revert too.

[rrbutani]

This looks good to me; left a few notes about cosmetic stuff:

re: whether or not to accept this change: I'll defer to the other llvmPackages maintainers and @paparodeo. I'm personally a little wary of keeping around codepaths that we aren't actively using.

[rrbutani]

nit: I know these bits predate your changes but would you mind merging the unconditional/>=14 snippets so we have fewer optionalString ...s?

i.e.:

  src' = if monorepoSrc != null then
    runCommand "${pname}-src-${version}" {} (''
      mkdir -p "$out/llvm"
      cp -r ${monorepoSrc}/libcxx "$out"
      cp -r ${monorepoSrc}/libcxxabi "$out"
      cp -r ${monorepoSrc}/llvm/cmake "$out/llvm"
      cp -r ${monorepoSrc}/llvm/utils "$out/llvm"
      cp -r ${monorepoSrc}/runtimes "$out"
    '' + (lib.optionalString (lib.versionAtLeast release_version "14") ''
      cp -r ${monorepoSrc}/cmake "$out"
      cp -r ${monorepoSrc}/third-party "$out"
    '')) else src;

[rrbutani]

The comment added in #311777 read:

# note: our setup using libcxxabi instead of libcxxrt on FreeBSD diverges from
# normal FreeBSD. This may cause issues with binary patching down the line.
# If this becomes an issue, try adding as symlink libcxxrt.so -> libc++abi.so

This looks fine regardless; I'm just curious: @rhelmot did y'all end up running into programs that expect libcxxrt.{so,a} or is this a proactive change?

[rhelmot]

I believe the first thing we're gonna run into is the rustc bootstrap. Artemis was working on this two days ago but she's been feeling under the weather. I'll try to pick her work up tonight and see whether this solution works as well as we're hoping.

In our original branch, we solved this by just pulling in libcxxrt built from the FreeBSD source tree (!!!), which was extremely noninvasive, not requiring any diffs to llvm proper.

[rrbutani]

sounds good, no rush!

In our original branch, we solved this by just pulling in libcxxrt built from the FreeBSD source tree (!!!),

nice 😄

Is that libcxxrt materially different from the one in nixpkgs right now?

[rhelmot]

No, its vendored from LLVM16 (in the FreeBSD 14 source tree at least). However I suspect the build instructions are different.

[paparodeo]

linking both libc++abi and libcxxrt seems wrong as they are providing the same symbols.

[rhelmot]

No, its vendored from LLVM16 (in the FreeBSD 14 source tree at least). However I suspect the build instructions are different.

So this was just straight up a lie! The vendored libcxxrt a) is not an LLVM project, egg on my face for thinking it wa. b) is modified to have symbol versioning (!!!!!!!) and some of these symbol versions are explicitly impersonating libstdc++ (!!!!!!!!!!!!!!!!). So it looks like when we're doing binary patching against stuff built for native FreeBSD, we will absolutely need to use the vendored libstdc++. I'll revert this part of the patch for this PR.

[paparodeo]

so just running nm --defined-only --dynamic /lib/libcxxrt.so.1 on freebsd 14.0-RELEASE-p3 FreeBSD 14.0-RELEASE-p3 it looks like the following symbols are defined that are not included in libc++abi nix freebsd cross build.

libc++abi missing symbols

{'_ZN10__cxxabiv116__enum_type_infoD0Ev',
 '_ZN10__cxxabiv116__enum_type_infoD1Ev',
 '_ZN10__cxxabiv116__enum_type_infoD2Ev',
 '_ZN10__cxxabiv117__array_type_infoD0Ev',
 '_ZN10__cxxabiv117__array_type_infoD1Ev',
 '_ZN10__cxxabiv117__array_type_infoD2Ev',
 '_ZN10__cxxabiv117__class_type_infoD0Ev',
 '_ZN10__cxxabiv117__class_type_infoD1Ev',
 '_ZN10__cxxabiv117__class_type_infoD2Ev',
 '_ZN10__cxxabiv117__pbase_type_infoD0Ev',
 '_ZN10__cxxabiv117__pbase_type_infoD1Ev',
 '_ZN10__cxxabiv117__pbase_type_infoD2Ev',
 '_ZN10__cxxabiv119__pointer_type_infoD0Ev',
 '_ZN10__cxxabiv119__pointer_type_infoD1Ev',
 '_ZN10__cxxabiv119__pointer_type_infoD2Ev',
 '_ZN10__cxxabiv120__function_type_infoD0Ev',
 '_ZN10__cxxabiv120__function_type_infoD1Ev',
 '_ZN10__cxxabiv120__function_type_infoD2Ev',
 '_ZN10__cxxabiv120__si_class_type_infoD0Ev',
 '_ZN10__cxxabiv120__si_class_type_infoD1Ev',
 '_ZN10__cxxabiv120__si_class_type_infoD2Ev',
 '_ZN10__cxxabiv121__vmi_class_type_infoD0Ev',
 '_ZN10__cxxabiv121__vmi_class_type_infoD1Ev',
 '_ZN10__cxxabiv121__vmi_class_type_infoD2Ev',
 '_ZN10__cxxabiv123__fundamental_type_infoD0Ev',
 '_ZN10__cxxabiv123__fundamental_type_infoD1Ev',
 '_ZN10__cxxabiv123__fundamental_type_infoD2Ev',
 '_ZN10__cxxabiv129__pointer_to_member_type_infoD0Ev',
 '_ZN10__cxxabiv129__pointer_to_member_type_infoD1Ev',
 '_ZN10__cxxabiv129__pointer_to_member_type_infoD2Ev',
 '_ZN9pathscale13set_terminateEPFvvE',
 '_ZN9pathscale14set_unexpectedEPFvvE',
 '_ZN9pathscale29set_use_thread_local_handlersEb',
 '_ZNKSt9type_info10__do_catchEPKS_PPvj',
 '_ZNKSt9type_info11__do_upcastEPKN10__cxxabiv117__class_type_infoEPPv',
 '_ZNKSt9type_info14__is_pointer_pEv',
 '_ZNKSt9type_info15__is_function_pEv',
 '_ZNKSt9type_info4nameEv',
 '_ZNKSt9type_info6beforeERKS_',
 '_ZNKSt9type_infoeqERKS_',
 '_ZNKSt9type_infoneERKS_',
 '_ZNSt10bad_typeidC1ERKS_',
 '_ZNSt10bad_typeidC2ERKS_',
 '_ZNSt10bad_typeidaSERKS_',
 '_ZNSt20bad_array_new_lengthC1ERKS_',
 '_ZNSt20bad_array_new_lengthC2ERKS_',
 '_ZNSt20bad_array_new_lengthaSERKS_',
 '_ZNSt8bad_castC1ERKS_',
 '_ZNSt8bad_castC2ERKS_',
 '_ZNSt8bad_castaSERKS_',
 '_ZNSt9bad_allocC1ERKS_',
 '_ZNSt9bad_allocC2ERKS_',
 '_ZNSt9bad_allocaSERKS_',
 '_ZNSt9exceptionC1ERKS_',
 '_ZNSt9exceptionC1Ev',
 '_ZNSt9exceptionC2ERKS_',
 '_ZNSt9exceptionC2Ev',
 '_ZNSt9exceptionaSERKS_',
 '_ZNSt9type_infoC1ERKS_',
 '_ZNSt9type_infoC2ERKS_',
 '_ZNSt9type_infoaSERKS_',
 '_ZSt18uncaught_exceptionv',
 '_ZSt19uncaught_exceptionsv'}

and when looking at nix libcxxrt most symbols are included except for the following two

{'_ZdaPvm', '_ZdlPvm'}
# operator delete[](void*, unsigned long), operator delete(void*, unsigned long)

so the libcxxrt included with nix has most symbols and the last two can easily be added

so it kind of seems to me that the path of least resistance given the symbol mismatch is to just stick with libcxxrt (which needs libcxx to be built with -DDLIBCXX_ENABLE_NEW_DELETE_DEFINITIONS=ON as noted in #311777 (comment).

[rhelmot]

I don't think that conclusion is correct. I have yet to find a package that cannot be built against libcxxabi on FreeBSD, and I was building whole desktop environments out of nixpkgs before I switched to trying to get stuff merged.

The only situation in which you would absolutely need libcxxrt would be if you're trying to ingest and patch a binary built for non-nix FreeBSD, in which case you can't use standard nix libcxxrt because of the above symbol versioning problem. When that situation happens (rust bootstrap being the primary example right now) it is not actually a problem if you just use libcxx with this PR applied and then supply freebsd libcxxrt as a build input. That library plays just fine with existing nixpkgs libcxx as built for libcxxabi. The only weird part of this arrangement is that you have libc++abi.so and libcxxrt.so loaded into the same process space, but I have yet to see this actually cause problems.

I am willing to be swayed on this patch, but I will need at least one of the following:

1. An explanation of why it's bad to have libc++abi.so and libcxxrt.so loaded into the same process space, as per my plan for binary patching, or
2. An example of a package which cannot build from source in nixpkgs without libcxxrt. Based on grep, there are none, but I might be missing something.

[paparodeo]

yeah, i don't think that building packages is really the issue it is that it is not ABI compatible due to the missing symbols. given that and the weird thing you're doing building rust or rust bootstrap and how trivial it is to use libcxxrt over libcxxabi was what swayed my opinion. removing all the external cxxabi / libcxxrt code from llvm.libcxx is something i'd like to see i just think it's not the correct option for freebsd.

yeah -- having two libraries with the same symbols is problematic as the behaviour can change based on the order the libraries get loaded and how the symbols get bound. you probably don't want code passing objects between libraries that get allocated with new from libcxxabi and delete from libcxxrt. not saying it is not possible to have shared objects with the same symbols co-exist in the same address space but you're not really making a strong argument why you need to do it.

anyway -- freebsd is your mess. i've said more than i want to say about this code so i'm going to check out. good luck!

[rhelmot]

Based on @paparodeo's comments, I think this is probably not a good change given that we do need to accommodate binary-patched libraries and possibly applications and have them play nice linking against other parts of the nixpkgs ecosystem. I'll investigate the feasibility of a parallel stdenv that uses libcxxrt, but the answer may just be that we have to roll back the prior patch using libcxxabi for FreeBSD. Quite sad, but I think it will simplify our work down the line.