Skip to content

Fix multiple issues with lldb 15-17#252015

Merged
sternenseemann merged 2 commits intoNixOS:masterfrom
Itaros:sideport-lldb-1x
Jan 4, 2024
Merged

Fix multiple issues with lldb 15-17#252015
sternenseemann merged 2 commits intoNixOS:masterfrom
Itaros:sideport-lldb-1x

Conversation

@Itaros
Copy link
Contributor

@Itaros Itaros commented Aug 28, 2023

Description of changes

Not really proud of 9ccc2f36150a1951c2a6126c0638a05dde913fd3 . Very hacky, but I can't figure out why rpath entry become missing after introduction of 251c557 , and that is the only way I could do it without hindering any potential improvements. I guess working, but hacky is better then pretending to be working and failing, at least for a while.

[nix-shell:~/.cache/nixpkgs-review/rev-f4be6f8b21d7e3552a464294121f6ae2c0fde466]$ cat report.json 
{
    "blacklisted": [],
    "broken": [],
    "built": [
        "lldb_15",
        "lldb_15.dev",
        "lldb_15.lib",
        "lldb_16",
        "lldb_16.dev",
        "lldb_16.lib",
        "llvmPackages_15.lldb-manpages",
        "llvmPackages_16.lldb-manpages"
    ],
    "extra-nixpkgs-config": null,
    "failed": [],
    "non-existant": [],
    "pr": null,
    "system": "x86_64-linux",
    "tests": []
}

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • 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/)
  • 23.11 Release Notes (or backporting 23.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.

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Aug 28, 2023
@Artturin Artturin force-pushed the sideport-lldb-1x branch 3 times, most recently from 13fdaf7 to 014a51b Compare September 7, 2023 00:33
@Jackaed
Copy link
Contributor

Jackaed commented Oct 26, 2023

Any reason why this hasn't been merged yet?

@hlolli
Copy link
Member

hlolli commented Oct 31, 2023

I can confirm that these changes fix the python import error message when starting lldb on my darwin system

Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'lldb'

gone...

but I'm still stuck with

error: shell expansion failed (reason: could not get support executable directory for lldb-argdumper tool).

weird one, but probably entierly different issue and these canges are a step forward in improving the current horrible lldb experience. Hope this gets merges, looks like an easy request for changes to implement there.

EDIT: 4wiw, it's the lldb python package that has a broken symlink in it

> ls -l /nix/store/q394yscq32rri095xjwzaf8g92b7r81w-lldb-14.0.6-lib/lib/python3.11/site-packages/lldb/
total 688
-r--r--r-- 1 root nixbld 695990 Jan  1  1970 __init__.py
lrwxr-xr-x 1 root nixbld     22 Jan  1  1970 _lldb.cpython-311-darwin.so -> ../../../liblldb.dylib*
dr-xr-xr-x 5 root nixbld    160 Jan  1  1970 diagnose/
-r--r--r-- 1 root nixbld   4415 Jan  1  1970 embedded_interpreter.py
dr-xr-xr-x 9 root nixbld    288 Jan  1  1970 formatters/
lrwxr-xr-x 1 root nixbld     98 Jan  1  1970 lldb-argdumper -> ../../../../../../../tmp/nix-build-lldb-14.0.6.drv-0/lldb-src-14.0.6/lldb/build/bin/lldb-argdumper
dr-xr-xr-x 6 root nixbld    192 Jan  1  1970 macosx/
dr-xr-xr-x 4 root nixbld    128 Jan  1  1970 plugins/
dr-xr-xr-x 5 root nixbld    160 Jan  1  1970 utils/

@hlolli
Copy link
Member

hlolli commented Nov 1, 2023

it seems the paths keep being wrong, not only are the symlinks pointing to nowhere, but I think also that lldb is looking in the wrong place on osx under nix

_lldb.cpython-311-darwin.so -> ../../../liblldb.dylib

is always incorrect for me, no matter how I try relinking this or copying, the multiple outputs may be throwing me off

I have been able to get this fixed

lldb-argdumper -> ../../../../../../../tmp/nix-build-lldb-14.0.6.drv-0/lldb-src-14.0.6/lldb/build/bin/lldb-argdumper

but it doesn't seem to fix the underlying issue, I'm a bit stuck and I hope someone else takes a look at this.

https://github.com/llvm/llvm-project/blob/6266b964202336a02f40007928719e060bc81694/lldb/source/Host/common/HostInfoBase.cpp#L248

I feel it's giving us a clue that it should be finding this under Framework.LLDB but I haven't seen any framework outputs when I build lldb on darwin+nix. Maybe a patch will fix it for darwin users. I'm not sure if there's a better solution or it's already being worked on elsewhere.

@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. and removed 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. labels Dec 18, 2023
@wegank wegank changed the title Fix multiple issues with lldb 15-16 Fix multiple issues with lldb 15-17 Dec 21, 2023
@ofborg ofborg bot added 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. and removed 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Dec 21, 2023
@wegank wegank force-pushed the sideport-lldb-1x branch 2 times, most recently from 3f1f4b2 to ec44607 Compare December 21, 2023 23:12
@wegank
Copy link
Member

wegank commented Dec 21, 2023

_lldb.cpython-311-darwin.so -> ../../../liblldb.dylib is a working relative symlink.

lldb-argdumper -> ../../../../bin/lldb-argdumper is not, however, but it is broken for all version of LLDB in nixpkgs...

@Nanotwerp
Copy link
Contributor

For the sake of all things good and sacred, we HAVE to get this merged!

In the meantime, this fixed LLDB can be applied by using a flake, although you'll have to compile LLDB during the first build:

{
  description = "Assembly development environment";

  inputs = {
    nixpkgs.url = "nixpkgs/nixos-unstable";
    lldb-fix.url = "github:Itaros/nixpkgs/sideport-lldb-1x";
  };

  outputs = { self, nixpkgs, lldb-fix }:
    let
      allSystems = [ "x86_64-linux" "aarch64-linux" "x86_64-darwin" "aarch64-darwin" ];
    
      forAllSystems = f: nixpkgs.lib.genAttrs allSystems (system: f {
        pkgs = import nixpkgs { inherit system; };
        lldb-pkgs = import lldb-fix { inherit system; };
      });
    in { 
      devShell = forAllSystems
        ({ pkgs, lldb-pkgs }:
          pkgs.mkShell {
            packages = [
              pkgs.yasm
              pkgs.mold
              lldb-pkgs.lldb_17 
            ]; 
        }); 
    };
}

This can then be used with the nix develop command, or the better alternative of nix-direnv, to temporarily enter an environment where all things are well in the world.

Copy link
Member

@sternenseemann sternenseemann left a comment

Choose a reason for hiding this comment

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

Seems good except for the patchelfing, the updated patch would also need to b ported to llvmPackages_git, so future updates will benefit from it.

@patryk4815
Copy link
Contributor

patryk4815 commented Jan 2, 2024

@Itaros Can we add passthru.tests for this bugs?

My patch:

diff --git a/pkgs/development/compilers/llvm/common/lldb.nix b/pkgs/development/compilers/llvm/common/lldb.nix
index 2444c1795a78..82ce2bf0e953 100644
--- a/pkgs/development/compilers/llvm/common/lldb.nix
+++ b/pkgs/development/compilers/llvm/common/lldb.nix
@@ -36,7 +36,7 @@ let
       '' else src;
 in

-stdenv.mkDerivation (rec {
+stdenv.mkDerivation (finalAttrs: {
   passthru.monorepoSrc = monorepoSrc;
   pname = "lldb";
   inherit version;
@@ -46,7 +46,7 @@ stdenv.mkDerivation (rec {

   outputs = [ "out" "lib" "dev" ];

-  sourceRoot = lib.optional (lib.versionAtLeast release_version "13") "${src.name}/${pname}";
+  sourceRoot = lib.optional (lib.versionAtLeast release_version "13") "${finalAttrs.src.name}/${finalAttrs.pname}";

   nativeBuildInputs = [
     cmake
@@ -101,7 +101,7 @@ stdenv.mkDerivation (rec {
   hardeningDisable = [ "format" ];

   cmakeFlags = [
-    "-DLLDB_INCLUDE_TESTS=${if doCheck then "YES" else "NO"}"
+    "-DLLDB_INCLUDE_TESTS=${if finalAttrs.doCheck then "YES" else "NO"}"
     "-DLLVM_ENABLE_RTTI=OFF"
     "-DClang_DIR=${lib.getDev libclang}/lib/cmake"
     "-DLLVM_EXTERNAL_LIT=${lit}/bin/lit"
@@ -122,27 +122,35 @@ stdenv.mkDerivation (rec {
     #
     # so, we just ignore the resulting errors
     "-DSPHINX_WARNINGS_AS_ERRORS=OFF"
-  ]) ++ lib.optionals doCheck [
+  ]) ++ lib.optionals finalAttrs.doCheck [
     "-DLLDB_TEST_C_COMPILER=${stdenv.cc}/bin/${stdenv.cc.targetPrefix}cc"
     "-DLLDB_TEST_CXX_COMPILER=${stdenv.cc}/bin/${stdenv.cc.targetPrefix}c++"
   ];

   doCheck = false;
-  doInstallCheck = lib.versionOlder release_version "15";
-
-  # TODO: cleanup with mass-rebuild
-  installCheckPhase = ''
-    if [ ! -e $lib/${python3.sitePackages}/lldb/_lldb*.so ] ; then
-        echo "ERROR: python files not installed where expected!";
-        return 1;
+
+  # check if lldb works fine, this skip symlinks issues
+  passthru.tests.lldb_check = runCommand "${finalAttrs.pname}-test" { meta.timeout = 60; } (''
+    broken=0
+    ${finalAttrs.finalPackage}/bin/lldb --script-language python -o 'script print("python", None)' -o 'quit' > test_py
+    if ! grep -q 'python None' ./test_py; then
+      echo "Error: broken lldb python interpreter" 1>&2;
+      broken=1
     fi
-  '' # Something lua is built on older versions but this file doesn't exist.
-  + lib.optionalString (lib.versionAtLeast release_version "14") ''
-    if [ ! -e "$lib/lib/lua/${lua5_3.luaversion}/lldb.so" ] ; then
-        echo "ERROR: lua files not installed where expected!";
-        return 1;
+  '' + (lib.optionalString (lib.versionAtLeast release_version "10") ''
+    ${finalAttrs.finalPackage}/bin/lldb --script-language lua -o 'script print("lua", nil)' -o 'quit' > test_lua
+    if ! grep -qP 'lua\tnil' ./test_lua; then
+      echo "Error: broken lldb lua interpreter" 1>&2;
+      broken=1
     fi
-  '';
+  '') + ''
+    if [ $broken -eq 0 ]; then
+      touch $out
+      exit 0
+    else
+      exit 1
+    fi
+  '');

   postInstall = ''
     wrapProgram $out/bin/lldb --prefix PYTHONPATH : $lib/${python3.sitePackages}/

@ofborg ofborg bot requested a review from sternenseemann January 3, 2024 03:16
@ofborg ofborg bot added 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. and removed 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. labels Jan 3, 2024
sternenseemann

This comment was marked as outdated.

Copy link
Member

@sternenseemann sternenseemann left a comment

Choose a reason for hiding this comment

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

I've pushed an improved solution for the rpath problem. What remains to be done is testing the builds (which I've kicked off) and updating the patch for llvmPackages_git which ought not to be forgotten.

Itaros and others added 2 commits January 3, 2024 11:42
It appears that LLVM's build system no longer sets the executable's
rpath to include the faux resource root we pass in, so we need to make
sure cc-wrapper does this.
@ofborg ofborg bot requested a review from sternenseemann January 3, 2024 11:06
@Nanotwerp
Copy link
Contributor

I've pushed an improved solution for the rpath problem. What remains to be done is testing the builds (which I've kicked off) and updating the patch for llvmPackages_git which ought not to be forgotten.

Tested in a nix shell, I can confirm that these patches work for lldb and all of its subcommands.

Copy link
Member

@sternenseemann sternenseemann left a comment

Choose a reason for hiding this comment

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

llvmPackages_git failure is not a regression: #278667.

@sternenseemann sternenseemann merged commit 23cf198 into NixOS:master Jan 4, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Jan 4, 2024

Successfully created backport PR for release-23.11:

@rrbutani rrbutani added the 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related label May 27, 2024
@github-actions
Copy link
Contributor

Backport failed for release-23.11, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-23.11
git worktree add -d .worktree/backport-252015-to-release-23.11 origin/release-23.11
cd .worktree/backport-252015-to-release-23.11
git switch --create backport-252015-to-release-23.11
git cherry-pick -x 21269ffbf6b6eb0385ad2829e5421e1bfd0ad9e1 3c049266e7ae0110e49d9cd7a618b0dc82f9845e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

10 participants