sage: fix segfaults and test timeout on darwin

[t0yv0]

After #541155 disabled ntl threading on Darwin to bypass a compilation failure, the sage tests started failing with segfaults caused by with-threading and without-threading ntl assumptions present in the graph of libraries installed by the build. This change ensures that all dependencies that link against ntl use the same copy, that is, the no-threading copy on Darwin.

With these change the tests now pass.

Fixes #545650

Besides fixing tests this likely resolves end-user segfaults in the sage build.

Things done

• Built on platform:
  • x86_64-linux
  • aarch64-linux
  • aarch64-darwin
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
  • Tests in lib/tests or pkgs/test for functions and "core" functionality.
• Ran nixpkgs-review on this PR. See nixpkgs-review usage.
• Tested basic functionality of all binary files, usually in ./result/bin/.
• Nixpkgs Release Notes
  • Package update: when the change is major or breaking.
• NixOS Release Notes
  • Module addition: when adding a new NixOS module.
  • Module update: when the change is significant.
• Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.
• Follows the automation/AI policy.

[collares]

@t0yv0 I'm not opposed to merging this to get Sage tests working ASAP, but, to be clear, I think this is going in the wrong direction, in the sense that ideally we'd find out why enabling threads is a problem (brew ships a working Sage with NTL threads enabled, for example). Can you remind me what went wrong with the aarch64-darwin Sage build with NTL threads enabled?

[collares]

Huh, apparently Homebrew merely installs the Sage dmg from the 3-manifolds project. Very weird.

[collares]

@t0yv0 Can you check if this PR also works if you replace NTL_THREADS=off by NTL_TLS_HACK=off? You don't need to push a commit, testing locally is fine. If this works, I think we could actually disable this hack for all Darwin NTL builds, and perhaps even for all platforms. The point is that I don't think this is Sage-specific, and it could affect everything that uses NTL; it's just that Sage is the only "big project" (in the sense of having other TLS-using dependencies) to currently do so.

Reason: The TLS hack was introduced in NTL 9.8.0 (see the release notes) due to a lack of proper thread_local support in compilers. In general, many projects hacked around thread_local support bugs by using GCC's special __thread support or pthreads's TLS implementation, but that caused linking issues for Darwin. Nowadays it seems like compilers implement thread_local in a more standards-compliant manner, and there are reports it doesn't suffer from the linking issues you encountered. See, e.g., grpc/grpc#24247 for related discussion.

[collares]

Setting NTL_TLS_HACK=off doesn't cause problems on x86_64-linux. I've asked Shoup whether this is a good idea: libntl/ntl#36.

[t0yv0]

I've done some digging on my end and not having thread-local support seems to resonate quite a bit with what I am seeing. If I use regular ntl I get a failure of this kind:

clang++  -o src/sage/libs/eclib/mwrank.cpython-314-darwin.so src/sage/libs/eclib/mwrank.cpython-314-darwin.so.p/meson-generated_src_sage_libs_eclib_mwrank.pyx.cpp.o -Wl,-dead_strip_dylibs -Wl,-headerpad_max_install_names -Wl,-undefined,dynamic_lookup -bundle -Wl,-undefined,dynamic_lookup 
/nix/store/rdygrnmrxhmzsrg63wb6m8mx8mcmad7n-eclib-20250627/lib/libec.dylib -lecl 
/nix/store/saaj0vlz8bmxd8z5l3a6wm85ss8fdbv8-flint-3.6.0/lib/libflint.dylib 
/nix/store/c8i80yixjlm4ljgyg96bx57nalcbpsly-gmp-with-cxx-6.3.0/lib/libgmp.dylib 
/nix/store/72vjb4bvkbb0ykglvdndvqplrhpsl8v7-mpfr-4.2.2/lib/libmpfr.dylib
ld: illegal thread local variable reference to regular symbol __ZN3NTL2RR4precE for architecture arm64

This is about NTL::RR::prec, in NTL/RR.h:

static NTL_CHEAP_THREAD_LOCAL long prec;
static void SetPrecision(long p);
static long precision() { return prec; }

https://github.com/libntl/ntl/blob/v11.6.0/include/NTL/RR.h#L44-L50

[t0yv0]

Interesting data point before I try NTL_TLS_HACK=off. If I force the linker to be aware of ntl, the build succeeds for me with threading in the ntl library. I have attempted a crude workaround of this kind:

+  env =
+    lib.optionalAttrs stdenv.cc.isClang {
+      # code tries to assign a unsigned long to an int in an initialized list
+      # leading to this error.
+      # https://github.com/sagemath/sage/pull/39249
+      NIX_CFLAGS_COMPILE = "-Wno-error=c++11-narrowing-const-reference";
+    }
+    // lib.optionalAttrs stdenv.hostPlatform.isDarwin {
+      # Crude Darwin workaround: force direct NTL linkage so bundles that
+      # inline RR::prec do not rely on transitive TLS symbol resolution.
+      NIX_LDFLAGS = "-L${ntl}/lib -lntl";
+    };

But perhaps it can be made more precise into a patch of the upstream sources so that meson and ninja build propagates ntl dependency to the darwin linker.

[t0yv0]

NTL_TLS_HACK=off data point: I still get the same linker failure as without it. This does not seem to be the right fix.

[collares]

NTL_TLS_HACK=off data point: I still get the same linker failure as without it. This does not seem to be the right fix.

Just to check, this is on top of the present PR, right? That is, making all dependencies use a NTL built with NTL_TLS_HACK=off (but with threads) as well.

[t0yv0]

Indeed. That is what I tested. All dependencies using ntl with threads and NTL_TLS_HACK=off still fails to link on darwin.

I am now isolating a minimal patch to sage that makes ntl passed explicitly to the linker, this seems the most promising avenue. I'll report if I can pass tests and use sage this way, and post a PR if so.

[collares]

I think I prefer this PR to tweaking linker options at Sage build time. Feels a bit funny to build eclib (say) with one version of NTL and then link it with another.

[collares]

Unless you're passing the threaded version of NTL (same used to build other dependencies) directly to the linker at Sage build time, of course.

[t0yv0]

Let's have a look at both directly to see which one we prefer. Here's the other change: #546056

I verified it gives me a working sage, but the sage-tests are still running. I'll post the result here when I have it.

The appeal of #546056 is that we get to use stock ntl builds. If this is the right fix it might also be possible to upstream the patch.