From 0da407258286793d3c1189b2b0ef496063acac4d Mon Sep 17 00:00:00 2001 From: Alyssa Ross Date: Tue, 28 Feb 2023 15:28:42 +0000 Subject: [PATCH 1/3] llvmPackages_git.llvm: run the tests on macOS Port of c7231c0b6d8 ("llvmPackages_15.llvm: run the tests on macOS"). The sysctl native check input was taken from 6d0c87602fe ("llvmPackages_15.llvm: add in a missing check dep"), because it looks like it was supposed to be part of c7231c0b6d8 instead. --- .../compilers/llvm/git/llvm/default.nix | 69 ++++++++++++---- ...-script-runner-set-dyld-library-path.patch | 26 ++++++ .../llvm-lit-cfg-add-libs-to-dylib-path.patch | 79 +++++++++++++++++++ ...polly-lit-cfg-add-libs-to-dylib-path.patch | 24 ++++++ 4 files changed, 181 insertions(+), 17 deletions(-) create mode 100644 pkgs/development/compilers/llvm/git/llvm/lit-shell-script-runner-set-dyld-library-path.patch create mode 100644 pkgs/development/compilers/llvm/git/llvm/llvm-lit-cfg-add-libs-to-dylib-path.patch create mode 100644 pkgs/development/compilers/llvm/git/llvm/polly-lit-cfg-add-libs-to-dylib-path.patch diff --git a/pkgs/development/compilers/llvm/git/llvm/default.nix b/pkgs/development/compilers/llvm/git/llvm/default.nix index 1daeb745e588..d944ee949fd9 100644 --- a/pkgs/development/compilers/llvm/git/llvm/default.nix +++ b/pkgs/development/compilers/llvm/git/llvm/default.nix @@ -16,9 +16,10 @@ , release_version , zlib , which +, sysctl , buildLlvmTools , debugVersion ? false -, doCheck ? stdenv.isLinux && (!stdenv.isx86_32) && (!stdenv.hostPlatform.isMusl) +, doCheck ? (!stdenv.isx86_32 /* TODO: why */) && (!stdenv.hostPlatform.isMusl) && (stdenv.hostPlatform == stdenv.buildPlatform) , enableManpages ? false , enableSharedLibraries ? !stdenv.hostPlatform.isStatic @@ -85,11 +86,59 @@ in stdenv.mkDerivation (rec { propagatedBuildInputs = [ ncurses zlib ]; - nativeCheckInputs = [ which ]; + nativeCheckInputs = [ + which + ] ++ lib.optional stdenv.isDarwin sysctl; patches = [ ./gnu-install-dirs.patch - ] ++ lib.optional enablePolly ./gnu-install-dirs-polly.patch; + + # Running the tests involves invoking binaries (like `opt`) that depend on + # the LLVM dylibs and reference them by absolute install path (i.e. their + # nix store path). + # + # Because we have not yet run the install phase (we're running these tests + # as part of `checkPhase` instead of `installCheckPhase`) these absolute + # paths do not exist yet; to work around this we point the loader (`ld` on + # unix, `dyld` on macOS) at the `lib` directory which will later become this + # package's `lib` output. + # + # Previously we would just set `LD_LIBRARY_PATH` to include the build `lib` + # dir but: + # - this doesn't generalize well to other platforms; `lit` doesn't forward + # `DYLD_LIBRARY_PATH` (macOS): + # + https://github.com/llvm/llvm-project/blob/0d89963df354ee309c15f67dc47c8ab3cb5d0fb2/llvm/utils/lit/lit/TestingConfig.py#L26 + # - even if `lit` forwarded this env var, we actually cannot set + # `DYLD_LIBRARY_PATH` in the child processes `lit` launches because + # `DYLD_LIBRARY_PATH` (and `DYLD_FALLBACK_LIBRARY_PATH`) is cleared for + # "protected processes" (i.e. the python interpreter that runs `lit`): + # https://stackoverflow.com/a/35570229 + # - other LLVM subprojects deal with this issue by having their `lit` + # configuration set these env vars for us; it makes sense to do the same + # for LLVM: + # + https://github.com/llvm/llvm-project/blob/4c106cfdf7cf7eec861ad3983a3dd9a9e8f3a8ae/clang-tools-extra/test/Unit/lit.cfg.py#L22-L31 + # + # !!! TODO: look into upstreaming this patch + ./llvm-lit-cfg-add-libs-to-dylib-path.patch + + # `lit` has a mode where it executes run lines as a shell script which is + # constructs; this is problematic for macOS because it means that there's + # another process in between `lit` and the binaries being tested. As noted + # above, this means that `DYLD_LIBRARY_PATH` is cleared which means that our + # tests fail with dyld errors. + # + # To get around this we patch `lit` to reintroduce `DYLD_LIBRARY_PATH`, when + # present in the test configuration. + # + # It's not clear to me why this isn't an issue for LLVM developers running + # on macOS (nothing about this _seems_ nix specific).. + ./lit-shell-script-runner-set-dyld-library-path.patch + ] ++ lib.optionals enablePolly [ + ./gnu-install-dirs-polly.patch + + # Just like the `llvm-lit-cfg` patch, but for `polly`. + ./polly-lit-cfg-add-libs-to-dylib-path.patch + ]; postPatch = optionalString stdenv.isDarwin '' substituteInPlace cmake/modules/AddLLVM.cmake \ @@ -134,12 +183,6 @@ in stdenv.mkDerivation (rec { ) ''; - # hacky fix: created binaries need to be run before installation - preBuild = '' - mkdir -p $out/ - ln -sv $PWD/lib $out - ''; - # E.g. mesa.drivers use the build-id as a cache key (see #93946): LDFLAGS = optionalString (enableSharedLibraries && !stdenv.isDarwin) "-Wl,--build-id=sha1"; @@ -216,14 +259,6 @@ in stdenv.mkDerivation (rec { ) ]; - postBuild = '' - rm -fR $out - ''; - - preCheck = '' - export LD_LIBRARY_PATH=$LD_LIBRARY_PATH''${LD_LIBRARY_PATH:+:}$PWD/lib - ''; - postInstall = '' mkdir -p $python/share mv $out/share/opt-viewer $python/share/opt-viewer diff --git a/pkgs/development/compilers/llvm/git/llvm/lit-shell-script-runner-set-dyld-library-path.patch b/pkgs/development/compilers/llvm/git/llvm/lit-shell-script-runner-set-dyld-library-path.patch new file mode 100644 index 000000000000..32f1d13a9dc2 --- /dev/null +++ b/pkgs/development/compilers/llvm/git/llvm/lit-shell-script-runner-set-dyld-library-path.patch @@ -0,0 +1,26 @@ +diff --git a/utils/lit/lit/TestRunner.py b/utils/lit/lit/TestRunner.py +index 0242e0b75af3..d732011306f7 100644 +--- a/utils/lit/lit/TestRunner.py ++++ b/utils/lit/lit/TestRunner.py +@@ -1029,6 +1029,12 @@ def executeScript(test, litConfig, tmpBase, commands, cwd): + f.write('@echo off\n') + f.write('\n@if %ERRORLEVEL% NEQ 0 EXIT\n'.join(commands)) + else: ++ # This env var is *purged* when invoking subprocesses so we have to ++ # manually set it from within the bash script in order for the commands ++ # in run lines to see this var: ++ if "DYLD_LIBRARY_PATH" in test.config.environment: ++ f.write(f'export DYLD_LIBRARY_PATH="{test.config.environment["DYLD_LIBRARY_PATH"]}"\n') ++ + for i, ln in enumerate(commands): + match = re.match(kPdbgRegex, ln) + if match: +@@ -1363,7 +1369,7 @@ def applySubstitutions(script, substitutions, conditions={}, + return processed + + process = processLine if recursion_limit is None else processLineToFixedPoint +- ++ + return [unescapePercents(process(ln)) for ln in script] + + diff --git a/pkgs/development/compilers/llvm/git/llvm/llvm-lit-cfg-add-libs-to-dylib-path.patch b/pkgs/development/compilers/llvm/git/llvm/llvm-lit-cfg-add-libs-to-dylib-path.patch new file mode 100644 index 000000000000..d824516c0a16 --- /dev/null +++ b/pkgs/development/compilers/llvm/git/llvm/llvm-lit-cfg-add-libs-to-dylib-path.patch @@ -0,0 +1,79 @@ +diff --git a/test/Unit/lit.cfg.py b/test/Unit/lit.cfg.py +index 81e8dc04acea..479ff95681e2 100644 +--- a/test/Unit/lit.cfg.py ++++ b/test/Unit/lit.cfg.py +@@ -3,6 +3,7 @@ + # Configuration file for the 'lit' test runner. + + import os ++import platform + import subprocess + + import lit.formats +@@ -55,3 +56,26 @@ if sys.platform in ['win32', 'cygwin'] and os.path.isdir(config.shlibdir): + # Win32 may use %SYSTEMDRIVE% during file system shell operations, so propogate. + if sys.platform == 'win32' and 'SYSTEMDRIVE' in os.environ: + config.environment['SYSTEMDRIVE'] = os.environ['SYSTEMDRIVE'] ++ ++# Add the LLVM dynamic libs to the platform-specific loader search path env var: ++# ++# TODO: this is copied from `clang`'s `lit.cfg.py`; should unify.. ++def find_shlibpath_var(): ++ if platform.system() in ['Linux', 'FreeBSD', 'NetBSD', 'OpenBSD', 'SunOS']: ++ yield 'LD_LIBRARY_PATH' ++ elif platform.system() == 'Darwin': ++ yield 'DYLD_LIBRARY_PATH' ++ elif platform.system() == 'Windows': ++ yield 'PATH' ++ elif platform.system() == 'AIX': ++ yield 'LIBPATH' ++ ++for shlibpath_var in find_shlibpath_var(): ++ shlibpath = os.path.pathsep.join( ++ (config.shlibdir, ++ config.environment.get(shlibpath_var, ''))) ++ config.environment[shlibpath_var] = shlibpath ++ break ++else: ++ lit_config.warning("unable to inject shared library path on '{}'" ++ .format(platform.system())) +diff --git a/test/lit.cfg.py b/test/lit.cfg.py +index 75a38b4c5dad..856fc75c9d74 100644 +--- a/test/lit.cfg.py ++++ b/test/lit.cfg.py +@@ -42,6 +42,26 @@ llvm_config.with_environment('PATH', config.llvm_tools_dir, append_path=True) + llvm_config.with_system_environment( + ['HOME', 'INCLUDE', 'LIB', 'TMP', 'TEMP']) + ++# Add the LLVM dynamic libs to the platform-specific loader search path env var: ++# ++# TODO: this is copied from `clang`'s `lit.cfg.py`; should unify.. ++def find_shlibpath_var(): ++ if platform.system() in ['Linux', 'FreeBSD', 'NetBSD', 'OpenBSD', 'SunOS']: ++ yield 'LD_LIBRARY_PATH' ++ elif platform.system() == 'Darwin': ++ yield 'DYLD_LIBRARY_PATH' ++ elif platform.system() == 'Windows': ++ yield 'PATH' ++ elif platform.system() == 'AIX': ++ yield 'LIBPATH' ++ ++for shlibpath_var in find_shlibpath_var(): ++ shlibpath = config.llvm_shlib_dir ++ llvm_config.with_environment(shlibpath_var, shlibpath, append_path = True) ++ break ++else: ++ lit_config.warning("unable to inject shared library path on '{}'" ++ .format(platform.system())) + + # Set up OCAMLPATH to include newly built OCaml libraries. + top_ocaml_lib = os.path.join(config.llvm_lib_dir, 'ocaml') +@@ -318,7 +338,7 @@ def have_cxx_shared_library(): + + try: + readobj_cmd = subprocess.Popen( +- [readobj_exe, '--needed-libs', readobj_exe], stdout=subprocess.PIPE) ++ [readobj_exe, '--needed-libs', readobj_exe], stdout=subprocess.PIPE, env=config.environment) + except OSError: + print('could not exec llvm-readobj') + return False diff --git a/pkgs/development/compilers/llvm/git/llvm/polly-lit-cfg-add-libs-to-dylib-path.patch b/pkgs/development/compilers/llvm/git/llvm/polly-lit-cfg-add-libs-to-dylib-path.patch new file mode 100644 index 000000000000..1354ad267314 --- /dev/null +++ b/pkgs/development/compilers/llvm/git/llvm/polly-lit-cfg-add-libs-to-dylib-path.patch @@ -0,0 +1,24 @@ +diff --git a/tools/polly/test/lit.cfg b/tools/polly/test/lit.cfg +index 41e3a589c61e..09f3b17498b0 100644 +--- a/tools/polly/test/lit.cfg ++++ b/tools/polly/test/lit.cfg +@@ -36,9 +36,17 @@ base_paths = [config.llvm_tools_dir, config.environment['PATH']] + path = os.path.pathsep.join(base_paths + config.extra_paths) + config.environment['PATH'] = path + ++# (Copied from polly/test/Unit/lit.cfg) ++if platform.system() == 'Darwin': ++ shlibpath_var = 'DYLD_LIBRARY_PATH' ++elif platform.system() == 'Windows': ++ shlibpath_var = 'PATH' ++else: ++ shlibpath_var = 'LD_LIBRARY_PATH' ++ + path = os.path.pathsep.join((config.llvm_libs_dir, +- config.environment.get('LD_LIBRARY_PATH',''))) +-config.environment['LD_LIBRARY_PATH'] = path ++ config.environment.get(shlibpath_var,''))) ++config.environment[shlibpath_var] = path + + llvm_config.use_default_substitutions() + From 3f9a9ad9f27bb6c82485cee964da4b05def00f78 Mon Sep 17 00:00:00 2001 From: Rahul Butani Date: Sun, 8 Jan 2023 04:31:14 -0800 Subject: [PATCH 2/3] llvmPackages_git.llvm: adjust a path, skip an xfail test on darwin Port of 0ee52512f0fd4f5b675ba9f742a300b50adf27d3 ("llvmPackages_15.llvm: adjust a path, skip an xfail test on darwin") --- pkgs/development/compilers/llvm/git/llvm/default.nix | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkgs/development/compilers/llvm/git/llvm/default.nix b/pkgs/development/compilers/llvm/git/llvm/default.nix index d944ee949fd9..4f0e68dc7b5f 100644 --- a/pkgs/development/compilers/llvm/git/llvm/default.nix +++ b/pkgs/development/compilers/llvm/git/llvm/default.nix @@ -4,6 +4,7 @@ , runCommand , fetchpatch , cmake +, darwin , ninja , python3 , libffi @@ -144,6 +145,14 @@ in stdenv.mkDerivation (rec { substituteInPlace cmake/modules/AddLLVM.cmake \ --replace 'set(_install_name_dir INSTALL_NAME_DIR "@rpath")' "set(_install_name_dir)" \ --replace 'set(_install_rpath "@loader_path/../''${CMAKE_INSTALL_LIBDIR}''${LLVM_LIBDIR_SUFFIX}" ''${extra_libdir})' "" + # As of LLVM 15, marked as XFAIL on arm64 macOS but lit doesn't seem to pick + # this up: https://github.com/llvm/llvm-project/blob/c344d97a125b18f8fed0a64aace73c49a870e079/llvm/test/MC/ELF/cfi-version.ll#L7 + rm test/MC/ELF/cfi-version.ll + + # This test tries to call `sw_vers` by absolute path (`/usr/bin/sw_vers`) + # thus fails under the sandbox: + substituteInPlace unittests/Support/Host.cpp \ + --replace '/usr/bin/sw_vers' "${(builtins.toString darwin.DarwinTools) + "/bin/sw_vers" }" '' + '' # FileSystem permissions tests fail with various special bits substituteInPlace unittests/Support/CMakeLists.txt \ From 5a4cfac5ecdd2e56efe3596dfbb35caee9fe46b2 Mon Sep 17 00:00:00 2001 From: Rahul Butani Date: Fri, 27 Jan 2023 11:26:14 -0800 Subject: [PATCH 3/3] llvmPackages_git.llvm: fix the tests on `x86_64-darwin` Port of eafb8fbd3215f0fe6b1d4a0360a9c4f761677179 ("llvmPackages_15.llvm: fix the tests on `x86_64-darwin`") --- .../compilers/llvm/git/llvm/default.nix | 53 ++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/pkgs/development/compilers/llvm/git/llvm/default.nix b/pkgs/development/compilers/llvm/git/llvm/default.nix index 4f0e68dc7b5f..ef7ff66e7464 100644 --- a/pkgs/development/compilers/llvm/git/llvm/default.nix +++ b/pkgs/development/compilers/llvm/git/llvm/default.nix @@ -150,10 +150,59 @@ in stdenv.mkDerivation (rec { rm test/MC/ELF/cfi-version.ll # This test tries to call `sw_vers` by absolute path (`/usr/bin/sw_vers`) - # thus fails under the sandbox: + # and thus fails under the sandbox: substituteInPlace unittests/Support/Host.cpp \ --replace '/usr/bin/sw_vers' "${(builtins.toString darwin.DarwinTools) + "/bin/sw_vers" }" - '' + '' + '' + optionalString (stdenv.isDarwin && stdenv.hostPlatform.isx86) '' + # This test tries to call the intrinsics `@llvm.roundeven.f32` and + # `@llvm.roundeven.f64` which seem to (incorrectly?) lower to `roundevenf` + # and `roundeven` on x86_64 macOS. + # + # However these functions are glibc specific so the test fails: + # - https://www.gnu.org/software/gnulib/manual/html_node/roundevenf.html + # - https://www.gnu.org/software/gnulib/manual/html_node/roundeven.html + # + # TODO(@rrbutani): this seems to run fine on `aarch64-darwin`, why does it + # pass there? + substituteInPlace test/ExecutionEngine/Interpreter/intrinsics.ll \ + --replace "%roundeven32 = call float @llvm.roundeven.f32(float 0.000000e+00)" "" \ + --replace "%roundeven64 = call double @llvm.roundeven.f64(double 0.000000e+00)" "" + + # This test fails on darwin x86_64 because `sw_vers` reports a different + # macOS version than what LLVM finds by reading + # `/System/Library/CoreServices/SystemVersion.plist` (which is passed into + # the sandbox on macOS). + # + # The `sw_vers` provided by nixpkgs reports the macOS version associated + # with the `CoreFoundation` framework with which it was built. Because + # nixpkgs pins the SDK for `aarch64-darwin` and `x86_64-darwin` what + # `sw_vers` reports is not guaranteed to match the macOS version of the host + # that's building this derivation. + # + # Astute readers will note that we only _patch_ this test on aarch64-darwin + # (to use the nixpkgs provided `sw_vers`) instead of disabling it outright. + # So why does this test pass on aarch64? + # + # Well, it seems that `sw_vers` on aarch64 actually links against the _host_ + # CoreFoundation framework instead of the nixpkgs provided one. + # + # Not entirely sure what the right fix is here. I'm assuming aarch64 + # `sw_vers` doesn't intentionally link against the host `CoreFoundation` + # (still digging into how this ends up happening, will follow up) but that + # aside I think the more pertinent question is: should we be patching LLVM's + # macOS version detection logic to use `sw_vers` instead of reading host + # paths? This *is* a way in which details about builder machines can creep + # into the artifacts that are produced, affecting reproducibility, but it's + # not clear to me when/where/for what this even gets used in LLVM. + # + # TODO(@rrbutani): fix/follow-up + substituteInPlace unittests/Support/Host.cpp \ + --replace "getMacOSHostVersion" "DISABLED_getMacOSHostVersion" + + # This test fails with a `dysmutil` crash; have not yet dug into what's + # going on here (TODO(@rrbutani)). + rm test/tools/dsymutil/ARM/obfuscated.test + '' + '' # FileSystem permissions tests fail with various special bits substituteInPlace unittests/Support/CMakeLists.txt \ --replace "Path.cpp" ""