bug#54728: [PATCH 2/2] gnu: valgrind: fix ld.so symbols not found

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

Hello!

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

>  (define-public valgrind/interactive
> -  (package/inherit
> -   valgrind
> -   (inputs
> -    ;; GDB is needed to provide a sane default for `--db-command'.
> -    `(("gdb" ,gdb)))
> -   (properties '())))
> +  (package/inherit valgrind
> +    (inputs
> +     ;; GDB is needed to provide a sane default for `--db-command'.
> +     `(("gdb" ,gdb)
> +       ("libc:debug" ,(@@ (gnu packages commencement) glibc-final) "debug")))

Rather: ("libc: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 'build '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")
> +                    (("/usr/lib/debug")
> +                     (string-append
> +                       (assoc-ref %build-inputs "libc:debug")
> +                       "/lib/debug")))
> +                  ;; We also need to account for the bigger path in
> +                  ;; the malloc-ed variables
> +                  (substitute* '
> +                    ("coregrind/m_debuginfo/readelf.c")
> +                    (("VG_\\(strlen\\)\\(buildid\\) \\+ 33")
> +                      (string-append
> +                        "VG_(strlen)(buildid) + "
> +                        (number->string
> +                         (+ (string-length
> +                            (string-append
> +                              (assoc-ref %build-inputs "libc:debug")
> +                              "/lib/debug"))
> +                            (string-length "/.build-id//.debug")
> +                            1)))))
> +                  (substitute* '
> +                    ("coregrind/m_debuginfo/readelf.c")
> +                    ((string-append
> +                      "VG_\\(strlen\\)\\(objdir\\) \\+ "
> +                      "VG_\\(strlen\\)\\(debugname\\) \\+ 64")
> +                     (string-append
> +                      "VG_(strlen)(objdir) + VG_(strlen)(debugname) + "
> +                      (number->string
> +                        (+ (string-length
> +                            (string-append
> +                             (assoc-ref
> +                              %build-inputs
> +                              "libc:debug")
> +                             "/lib/debug"))
> +                           (string-length
> +                            "/usr/lib/debug")
> +                           1)))))

I find this patch-as-code snippet rather difficult to follow; it might
also break easily if minor things change in those C files.

How about making it an actual patch?  In the patch, you’d have
placeholders for the store file names, like @LIBC_DEBUG_DIRECTORY@; the
phase would replace those placeholders with ‘substitute*’.

How does that sound?

Thanks,
Ludo’.