patchelf-0.17.2 seems to corrupt emacs on `staging-next`

[trofi]

On current staging-next iteration quite a few emacs-dependent packages are failing. The failures seem to stem from the fact that emacs is incorrectly modified by patchelf-0.17.2 (0.15.0 works, bisected in nixpkgs by @mweinelt).

$ nix run https://github.com/NixOS/nixpkgs/archive/staging-next.tar.gz#emacs
Segmentation fault (core dumped)

$ patchelf --version
patchelf 0.17.2

It seems to have something to do with modified library list:

$ nix shell https://github.com/NixOS/nixpkgs/archive/staging-next.tar.gz#emacs
$ gdb emacs
Reading symbols from emacs...

warning: Loadable section ".dynstr" outside of ELF segments
  in /nix/store/lzahvwakhghr8b3ri40s935bwhn7nf0x-emacs-28.2/bin/emacs-28.2

warning: Loadable section ".dynamic" outside of ELF segments
  in /nix/store/lzahvwakhghr8b3ri40s935bwhn7nf0x-emacs-28.2/bin/emacs-28.2
(No debugging symbols found in emacs)
(gdb) run
Starting program: /nix/store/lzahvwakhghr8b3ri40s935bwhn7nf0x-emacs-28.2/bin/emacs

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7fe6597 in dl_main ()
   from /nix/store/8xk4yl1r3n6kbyn05qhan7nbag7npymx-glibc-2.35-224/lib/ld-linux-x86-64.so.2
(gdb) bt
#0  0x00007ffff7fe6597 in dl_main ()
   from /nix/store/8xk4yl1r3n6kbyn05qhan7nbag7npymx-glibc-2.35-224/lib/ld-linux-x86-64.so.2
#1  0x00007ffff7fe2a06 in _dl_sysdep_start ()
   from /nix/store/8xk4yl1r3n6kbyn05qhan7nbag7npymx-glibc-2.35-224/lib/ld-linux-x86-64.so.2
#2  0x00007ffff7fe45ad in _dl_start ()
   from /nix/store/8xk4yl1r3n6kbyn05qhan7nbag7npymx-glibc-2.35-224/lib/ld-linux-x86-64.so.2
#3  0x00007ffff7fe33a8 in _start ()
   from /nix/store/8xk4yl1r3n6kbyn05qhan7nbag7npymx-glibc-2.35-224/lib/ld-linux-x86-64.so.2
#4  0x0000000000000001 in ?? ()
#5  0x00007fffffffd1f3 in ?? ()
#6  0x0000000000000000 in ?? ()

If gdb is to be believed loadable program headers that contain ".dynstr" and ".dynamic" are not what they should be.

LD_DEBUG also suggests very little could be loaded by ld.so:

$ LD_DEBUG=all emacs
   2608983:     symbol=__vdso_clock_gettime;  lookup in file=linux-vdso.so.1 [0]
   2608983:     binding file linux-vdso.so.1 [0] to linux-vdso.so.1 [0]: normal symbol `__vdso_clock_gettime' [LINUX_2.6]
   2608983:     symbol=__vdso_gettimeofday;  lookup in file=linux-vdso.so.1 [0]
   2608983:     binding file linux-vdso.so.1 [0] to linux-vdso.so.1 [0]: normal symbol `__vdso_gettimeofday' [LINUX_2.6]
   2608983:     symbol=__vdso_time;  lookup in file=linux-vdso.so.1 [0]
   2608983:     binding file linux-vdso.so.1 [0] to linux-vdso.so.1 [0]: normal symbol `__vdso_time' [LINUX_2.6]
   2608983:     symbol=__vdso_getcpu;  lookup in file=linux-vdso.so.1 [0]
   2608983:     binding file linux-vdso.so.1 [0] to linux-vdso.so.1 [0]: normal symbol `__vdso_getcpu' [LINUX_2.6]
   2608983:     symbol=__vdso_clock_getres;  lookup in file=linux-vdso.so.1 [0]
   2608983:     binding file linux-vdso.so.1 [0] to linux-vdso.so.1 [0]: normal symbol `__vdso_clock_getres' [LINUX_2.6]
Segmentation fault (core dumped)

eu-elflint is also unhappy:

$ eu-elflint /nix/store/lzahvwakhghr8b3ri40s935bwhn7nf0x-emacs-28.2/bin/emacs
section [ 7] '.dynstr' not fully contained in segment of program header entry 2
section [ 8] '.dynamic': alloc flag set but section not in any loaded segment
section [29] '.symtab': symbol 1 (__abi_tag): st_value out of bounds
section [29] '.symtab': _GLOBAL_OFFSET_TABLE_ symbol size 0 does not match .got section size 9736
section [29] '.symtab': symbol 5599 (_DYNAMIC): st_value out of bounds
section [29] '.symtab': _DYNAMIC_ symbol value 0x6ab560 does not match dynamic segment address 0x40ee60
section [29] '.symtab': _DYNAMIC symbol size 0 does not match dynamic segment size 1184
section [29] '.symtab': symbol 6650 (__bss_start): st_value out of bounds
loadable segment [2] is writable but contains no writable sections

[brenoguim]

Thanks for reporting the issue.
I'll investigate when I get home.

I think I will be able to reproduce the issue easily using nix, but if you could attach the result of readelf -a -W for the binary, it always helps.

[mweinelt]

https://gist.github.com/mweinelt/af1bd9f663f9b2c320d64733961b7bd0

[trofi]

Using emacs as an example bisected patchelf down to 42394e8 write out replace sections in original order.

It's gist st change of traversal from

    for (auto & i : replacedSections) {
        const std::string & sectionName = i.first;
        auto & shdr = findSectionHeader(sectionName);

to

    /* We iterate over the sorted section headers here, so that the relative
       position between replaced sections stays the same.  */
    for (auto & shdr : shdrs) {
        std::string sectionName = getSectionName(shdr);
        auto i = replacedSections.find(sectionName);
        if (i == replacedSections.end())
            continue;

I suspect it has a chance to miss newly added sections if patchelf ever does that. But maybe not in emacs case. readelf (attached below "before" and "after") says both have 31 sections. But I'm not sure I believe it.

Attaching readelf -a -W:

• readelf-aw-bad.txt: at this commit
• readelf-aw-good.txt: just before this commit

Looks like one of program headers got lost (or merged into existing one):

diff -u readelf-aw-good.txt  readelf-aw-bad.txt | cat
--- readelf-aw-good.txt 2023-03-18 18:30:42.604009844 +0000
+++ readelf-aw-bad.txt  2023-03-18 18:31:55.812435119 +0000
@@ -10,11 +10,11 @@
   Version:                           0x1
   Entry point address:               0x427ea0
   Start of program headers:          64 (bytes into file)
-  Start of section headers:          6757680 (bytes into file)
+  Start of section headers:          6753584 (bytes into file)
   Flags:                             0x0
   Size of this header:               64 (bytes)
   Size of program headers:           56 (bytes)
-  Number of program headers:         15
+  Number of program headers:         14

[brenoguim]

I debugged something like this. It had to do with rounding of load segments overlapping.
Thanks for this info!

[brenoguim]

I think this is the same scenario I saw in: #446

We can see the following LOAD segments with different read/write permissions:

  LOAD           0x000000 0x00000000003ff000 0x00000000003ff000 0x01057c 0x01057c RW  0x1000
  LOAD           0x01057c 0x000000000040f57c 0x000000000040f57c 0x0085cc 0x0085cc R   0x1000

The first one goes from 0x3ff000 to 0x40f57c
And the second goes from 0x40f57c to 0x417b48

Notice that the alignment is 0x1000 so the OS will have to map the pages:

1. 0x3ff000 to 0x410000
2. 0x40e000 to 0x418000

There is an overlap between the two. So the fix #469 should hopefully fix this issue.
I'll try the "master" commit tomorrow and see if it fixes the issue.

[trofi]

I tried master as well and it did not fix the issue for me. Tested as:

--- a/pkgs/applications/editors/emacs/generic.nix
+++ b/pkgs/applications/editors/emacs/generic.nix
@@ -46,6 +46,7 @@
   else "lucid")
 , withSystemd ? lib.meta.availableOn stdenv.hostPlatform systemd, systemd
 , withTreeSitter ? lib.versionAtLeast version "29", tree-sitter ? null
+, patchelfUnstable
 }:
 
 assert (libXft != null) -> libpng != null;      # probably a bug
@@ -135,7 +136,7 @@ assert withTreeSitter -> tree-sitter != null;
     ""
   ];
 
-  nativeBuildInputs = [ pkg-config makeWrapper ]
+  nativeBuildInputs = [ pkg-config makeWrapper patchelfUnstable ]
     ++ lib.optionals (srcRepo || withMacport) [ texinfo ]
     ++ lib.optionals srcRepo [ autoreconfHook ]
     ++ lib.optional (withX && (withGTK3 || withXwidgets)) wrapGAppsHook;
diff --git a/pkgs/development/tools/misc/patchelf/unstable.nix b/pkgs/development/tools/misc/patchelf/unstable.nix
index 66c14bd07e0..3f20cb7834f 100644
--- a/pkgs/development/tools/misc/patchelf/unstable.nix
+++ b/pkgs/development/tools/misc/patchelf/unstable.nix
@@ -2,13 +2,13 @@
 
 stdenv.mkDerivation rec {
   pname = "patchelf";
-  version = "unstable-2023-03-07";
+  version = "unstable-2023-03-18";
 
   src = fetchFromGitHub {
     owner = "NixOS";
     repo = "patchelf";
-    rev = "ea2fca765c440fff1ff74e1463444dea7b819db2";
-    sha256 = "sha256-IH80NcLhwjGpIXEjHuV+NgaSC+Y/PXquxZ/C8Bl+CLk=";
+    rev = "265b31ae22c6e1d20b01295aaa7bcf28fd31a5cf";
+    sha256 = "sha256-+iGvdjXvhk5mN8jp3u+M9fICKFqbtyZCx+WjQszaB1o=";
   };
 
   # Drop test that fails on musl (?)

[trofi]

NixOS/nixpkgs#221900 was merged into staging-next to fix emacs. You might need to revert the change locally to reproduce it on staging-next.

[brenoguim]

Weird, I can't reproduce the crash using the commit before the merge.

# before merge
nix run https://github.com/NixOS/nixpkgs/archive/6c70dbc.gz#emacs

# after merge
nix run https://github.com/NixOS/nixpkgs/archive/ce7e136.gz#emacs

(I'm new to Nix, so I might be doing something wrong)

I can reproduce the messages from gdb and eu-elflint in both hashes.

[brenoguim]

I reverted it locally and now I can reproduce it. Not sure what is the difference, but let me get to it.

[trofi]

This commit fails for me (it's the one directly preceeding the patchelf-0.15.0 pin):

$ nix run https://github.com/NixOS/nixpkgs/archive/403b148aa51073bc343febbbfd041ecd495dbe3e.tar.gz#emacs
Segmentation fault (core dumped)

This should allow extracting exact binary:

$ nix build https://github.com/NixOS/nixpkgs/archive/403b148aa51073bc343febbbfd041ecd495dbe3e.tar.gz#emacs
$ result/bin/emacs
Segmentation fault (core dumped)

[brenoguim]

Note so far:

1. I generated an unpatched emacs by removing the patchelf invocation on emacs/generic.nix and I'm running them manually. Seems to be a good way to debug these things.
2. patching emacs with the new layout engine from Implement an alternative layout engine #468 #477 generates good binaries.
3. emacs is patched to change rpath and then add a needed so. Issue happens only after the "add needed".

Still investigating. Looks like the strtab is falling out of the LOAD segment that is supposed to map it into memory.

[brenoguim]

I'm quite sure the missing thing is an else to this if:

patchelf/src/patchelf.cc

Line 1015 in 265b31a

if (neededSpace > startOffset) {

When we enter that if, we split a LOAD segment in two: the one the loads the replaced sections and the one that keeps loading the sections that stay in place.
However, for the "else", when there is enough space before the first non-replaced section, we don't check if the LOAD is large enough to map all the new rewritten sections. Then some of the sections may be dangling out of the load.

It's a bit hard though to nail the exact fix

[brenoguim]

#485