bug#54728: Valgrind not working out of the box due to stripped ld.so

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

[-- Attachment #1: Type: text/plain, Size: 916 bytes --]

Hi Denis,

Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org> skribis:

> * gnu/packages/valgrind.scm (valgrind/interactive)[native-inputs]:
>   Add libc:debug
>   [arguments]: Add patch-default-debuginfo-path phase
>   [source]: Add patch.
>   (valgrind-3.18)[source]: Add patch.
>
> * gnu/packages/commencement.scm (glibc-final): Export variable.
>
> Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>

I had lost track of it, so thanks for pinging.

I’ve applied it and confirmed it works.  I made the following cosmetic
changes (patch below):

  • removing the ‘glibc-final’ export, which is unnecessary;

  • in phases, refer to the ‘inputs’ parameter rather than the
    ‘%build-inputs’ global variable (which is deprecated);

  • avoid repetition of (string-append (assoc-ref …) "/lib/debug");

  • tweak indentation.

Thanks!

Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 6813 bytes --]

diff --git a/gnu/packages/valgrind.scm b/gnu/packages/valgrind.scm
index 1269e07593..0da011087c 100644
--- a/gnu/packages/valgrind.scm
+++ b/gnu/packages/valgrind.scm
@@ -95,83 +95,71 @@ (define-public valgrind
 
 (define-public valgrind/interactive
   (package/inherit
-   valgrind
-   (version "3.17.0")
-   (source (origin
-             (method url-fetch)
-             (uri (list (string-append "https://sourceware.org/pub/valgrind"
-                                       "/valgrind-" version ".tar.bz2")
-                        (string-append "ftp://sourceware.org/pub/valgrind"
-                                       "/valgrind-" version ".tar.bz2")))
-             (sha256
-              (base32
-               "18l5jbk301j3462gipqn9bkfx44mdmwn0pwr73r40gl1irkfqfmd"))
-             (patches (search-patches
-                       "valgrind-enable-arm.patch"
-                       "valgrind-fix-default-debuginfo-path.patch"))))
-   (inputs
-    ;; GDB is needed to provide a sane default for `--db-command'.
-    `(("gdb" ,gdb)
-      ("glibc:debug" ,(canonical-package glibc) "debug")))
-   (arguments
+      valgrind
+    (version "3.17.0")
+    (source (origin
+              (method url-fetch)
+              (uri (list (string-append "https://sourceware.org/pub/valgrind"
+                                        "/valgrind-" version ".tar.bz2")
+                         (string-append "ftp://sourceware.org/pub/valgrind"
+                                        "/valgrind-" version ".tar.bz2")))
+              (sha256
+               (base32
+                "18l5jbk301j3462gipqn9bkfx44mdmwn0pwr73r40gl1irkfqfmd"))
+              (patches (search-patches
+                        "valgrind-enable-arm.patch"
+                        "valgrind-fix-default-debuginfo-path.patch"))))
+    (inputs
+     ;; GDB is needed to provide a sane default for `--db-command'.
+     `(("gdb" ,gdb)
+       ("glibc:debug" ,(canonical-package glibc) "debug")))
+    (arguments
      (substitute-keyword-arguments (package-arguments valgrind)
-       ((#:phases those-phases #~%standard-phases)
-        #~(let* ((those-phases #$those-phases)
-                 (unpack (assoc-ref those-phases 'unpack)))
-            (modify-phases
-              those-phases
-                (add-before 'configure 'patch-default-debuginfo-path
-                  (lambda _
-                    ;; This helps Valgrind find the debug symbols of ld.so.
-                    ;; Without it, Valgrind does not work in a Guix shell
-                    ;; container and cannot be used as-is during packages tests
-                    ;; phases
-                    (substitute* '
-                      ("coregrind/m_debuginfo/readelf.c"
-                       "docs/xml/manual-core-adv.xml"
-                       "docs/xml/manual-core.xml")
-                      (("DEFAULT_DEBUGINFO_PATH")
-                       (string-append (assoc-ref %build-inputs "glibc:debug")
-                                      "/lib/debug")))
-                    ;; We also need to account for the bigger path in
-                    ;; the malloc-ed variables
-                    (substitute* '
-                     ("coregrind/m_debuginfo/readelf.c")
-                     (("DEBUGPATH_EXTRA_BYTES_1")
-                      (number->string
-                       (+ (string-length
-                           (string-append
-                            (assoc-ref %build-inputs "glibc:debug")
-                            "/lib/debug"))
-                          (string-length "/.build-id//.debug")
-                          1))))
-                    (substitute* '
-                     ("coregrind/m_debuginfo/readelf.c")
-                     (("DEBUGPATH_EXTRA_BYTES_2")
-                      (number->string
-                       (+ (string-length
-                           (string-append
-                            (assoc-ref
-                             %build-inputs
-                             "glibc:debug")
-                            "/lib/debug"))
-                          (string-length
-                           "/usr/lib/debug")
-                          1)))))))))))
-   (properties '())))
+       ((#:phases phases #~%standard-phases)
+        #~(modify-phases #$phases
+            (add-before 'configure 'patch-default-debuginfo-path
+              (lambda* (#:key inputs #:allow-other-keys)
+                ;; This helps Valgrind find the debug symbols of ld.so.
+                ;; Without it, Valgrind does not work in a Guix shell
+                ;; container and cannot be used as-is during packages tests
+                ;; phases.
+                ;; TODO: Remove on the next rebuild cycle, when libc is not
+                ;; longer fully stripped.
+                (define libc-debug
+                  (string-append (assoc-ref inputs "glibc:debug")
+                                 "/lib/debug"))
+
+                (substitute* '("coregrind/m_debuginfo/readelf.c"
+                               "docs/xml/manual-core-adv.xml"
+                               "docs/xml/manual-core.xml")
+                  (("DEFAULT_DEBUGINFO_PATH")
+                   libc-debug))
+                ;; We also need to account for the bigger path in
+                ;; the malloc-ed variables.
+                (substitute* '("coregrind/m_debuginfo/readelf.c")
+                  (("DEBUGPATH_EXTRA_BYTES_1")
+                   (number->string
+                    (+ (string-length libc-debug)
+                       (string-length "/.build-id//.debug")
+                       1))))
+                (substitute* '("coregrind/m_debuginfo/readelf.c")
+                  (("DEBUGPATH_EXTRA_BYTES_2")
+                   (number->string
+                    (+ (string-length libc-debug)
+                       (string-length "/usr/lib/debug")
+                       1))))))))))
+    (properties '())))
 
 (define-public valgrind-3.18
   (package
     (inherit valgrind/interactive)
     (version "3.18.1")
     (source (origin
-              (method url-fetch)
+              (inherit (package-source valgrind/interactive))
               (uri (list (string-append "https://sourceware.org/pub/valgrind"
                                         "/valgrind-" version ".tar.bz2")
                          (string-append "ftp://sourceware.org/pub/valgrind"
                                         "/valgrind-" version ".tar.bz2")))
               (sha256
                (base32
-                "1xgph509i6adv9w2glviw3xrmlz0dssg8992hbvxsbkp7ahrm180"))
-              (patches (search-patches "valgrind-enable-arm.patch"
-                                       "valgrind-fix-default-debuginfo-path.patch"))))))
+                "1xgph509i6adv9w2glviw3xrmlz0dssg8992hbvxsbkp7ahrm180"))))))