[bug#52940] [PATCH] gremlin: Mimic ld.so NEEDED deduplication behavior.

["Ludovic Courtès" <ludo@gnu.org>]

Hi Chris,

Chris Marusich <cmmarusich@gmail.com> skribis:

> Here is the failing test output, including the test source:
>
> ;;; (truth ("linux-vdso64.so.1" "/gnu/store/gahs2sx5snbfkr9vlcjj5c2kvnlhr0zs-guile-3.0.7/lib/libguile-3.0.so.1" "/gnu/store/7x2cjqbmpgwrgmnb234gsxkmsqs5pj09-libgc-8.0.4/lib/libgc.so.1" "/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib/libpthread.so.0" "/gnu/store/521riv2sgv0b0s4j0kzz6i52rf9rarh8-libffi-3.3/lib/../lib/libffi.so.7" "/gnu/store/xj20v8lk2wal0z1rla0yx3bjkasbx6mq-libunistring-0.9.10/lib/libunistring.so.2" "/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib/libcrypt.so.1" "/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib/libdl.so.2" "/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib/libm.so.6" "/gnu/store/ys7b4gr5nbq8sfnff9ry5blb4bhpx6mq-gcc-7.5.0-lib/lib/libgcc_s.so.1" "/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib/libc.so.6" "/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib/ld64.so.2"))
>
> ;;; (needed ("/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib/libc.so.6" "/gnu/store/ys7b4gr5nbq8sfnff9ry5blb4bhpx6mq-gcc-7.5.0-lib/lib/libgcc_s.so.1" "/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib/libm.so.6" "/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib/libdl.so.2" "/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib/libcrypt.so.1" "/gnu/store/xj20v8lk2wal0z1rla0yx3bjkasbx6mq-libunistring-0.9.10/lib/libunistring.so.2" "/gnu/store/521riv2sgv0b0s4j0kzz6i52rf9rarh8-libffi-3.3/lib/../lib/libffi.so.7" "/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib/libpthread.so.0" "/gnu/store/7x2cjqbmpgwrgmnb234gsxkmsqs5pj09-libgc-8.0.4/lib/libgc.so.1" "/gnu/store/gahs2sx5snbfkr9vlcjj5c2kvnlhr0zs-guile-3.0.7/lib/libguile-3.0.so.1" "/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib/ld64.so.2" "/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib/../lib/libc.so.6"))

[...]

> Note that the RUNPATH above contains an entry for
> "/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib" followed
> later by
> "/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib/../lib".  It seems
> that ld.so's tracing mechanism is smart enough to avoid printing the
> second entry.
>
> So, the test fails because the "needed" list is not set-equivalent to
> the "truth" list.  There are two reasons why they are not
> set-equivalent:
>
> A) "truth" contains "linux-vdso64.so.1", but "needed" does not.
>
> B) "needed" contains
> "/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib/../lib/libc.so.6",
> but "truth" does not.  However, both contain
> "/gnu/store/sipyfs2540b48b2sb9j8ypmybja1dvqb-glibc-2.31/lib/libc.so.6",
> which refers to the same file.

[...]

> What is a good solution for (B)?  I can think of the following potential
> solutions:
>
> 1) Try to avoid introducing multiple entries referring to the same thing
> in the first place.  Somehow, somewhere, something is adding the second
> entry to the dynamic section of Guile's ELF file.  It happens on
> powerpc64le-linux but not on x86_64-linux.  What code or tool is doing
> this?  I don't know, but I guess I would start by looking at the
> gnu-build-system code.  I'm not sure if it's a really problem, though,
> so I'm not eager to jump down this rabbit hole just yet.
>
> 2) Change the test so that it passes even if file-needed/recursive
> returns multiple entries referring to the same file.  In other words,
> accept that the current behavior is OK, even if it means that the
> results returned by file-needed/recursive are not always exactly the
> same as the results returned by ld.so.
>
> 3) Try to change file-needed/recursive so that it does not return
> multiple entries referring to the same file.  In other words, make it
> behave more like ld.so.
>
> I can't think of a reason why the current behavior of
> file-needed/recursive is bad, but it was simple enough to make it
> deduplicate entries similarly to ld.so.  So, my patch implements
> solution (3).  Hopefully it's good enough!

Good catch!  We could go fancy and have ‘loop’ in
‘file-needed/recursive’ thread a map of device/inode number pairs to
file names; when calling ‘search-path’, we’d check whether the file we
found already is in the set, possibly under a different name, and we’d
use that name instead of introducing a new one.  That’d be more
efficient that calling ‘canonicalize-path’, especially O(n³) times
roughly.

But… given that this is a corner case, that modifying (guix build
gremlin) entails a full rebuild, and that there just should be that
“/lib/../lib” entry in the first place, I’d lean towards leaving
gremlin.scm unchanged.

WDYT?

However…

> diff --git a/tests/gremlin.scm b/tests/gremlin.scm
> index 9af899c89a..86757e62b4 100644
> --- a/tests/gremlin.scm
> +++ b/tests/gremlin.scm
> @@ -1,5 +1,6 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2015, 2018, 2020 Ludovic Courtès <ludo@gnu.org>
> +;;; Copyright © 2022 Chris Marusich <cmmarusich@gmail.com>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -92,7 +93,9 @@
>                 (loop result))))))
>  
>      (define ground-truth
> -      (remove (cut string-prefix? "linux-vdso.so" <>)
> +      (remove (lambda (entry)
> +                (or (string-prefix? "linux-vdso.so" entry)
> +                    (string-prefix? "linux-vdso64.so" entry)))
>                (read-ldd-output pipe)))
>  
>      (and (zero? (close-pipe pipe))

… I think this part should definitely be committed (‘master’ is fine).

Thanks,
Ludo’.