Bug 435908 - valgrind tries to fetch from deubginfod for files which already have debug information
Summary: valgrind tries to fetch from deubginfod for files which already have debug in...
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (other bugs)
Version First Reported In: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-04-19 09:25 UTC by Tom Hughes
Modified: 2021-09-06 06:06 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
patch (985 bytes, patch)
2021-05-06 18:11 UTC, Aaron Merey
Details
Patch to avoid looking for a debuginfo image if the main image has debugging (1.04 KB, patch)
2021-05-12 10:02 UTC, Tom Hughes
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Hughes 2021-04-19 09:25:33 UTC
The new debuginfod support in valgrind is rather over eager and will try and fetch a debug image for files which already have debug information.

Development builds of the code I work on have about 250 shared libraries which are build with full debugging but are not stripped at all so the debug information is in the main image.

Because valgrind has always tried to find a debug image before doing any debug lookups, and because that code not includes trying to search debuginfod for an image, programs now takes several minutes to start while valgrind runs 250 debuginfod-find's each of which makes an HTTP call which gets a negative result.

There is currently no negative caching in libdebuginfo (though it is planned) so this isn't even a one time thing.
Comment 1 Mark Wielaard 2021-04-19 10:12:42 UTC
The negative caching bug is: https://sourceware.org/bugzilla/show_bug.cgi?id=25628
Comment 2 Tom Hughes 2021-04-19 10:46:45 UTC
Just to be clear, the libraries I'm talking about here have debug information (that is to say that have populated .debug_xxx sections) but they do not have separate debuginfo images.
Comment 3 Aaron Merey 2021-05-06 18:11:07 UTC
Created attachment 138194 [details]
patch

It looks like these unnecessary queries occur when no .gnu_debuglink section is present in a binary but valgrind nevertheless searches for a separate debuginfo file. I've attached a patch to prevent debuginfod queries in this case.
Comment 4 Tom Hughes 2021-05-12 08:55:45 UTC
No that patch won't help.

The debugname variable is just the name of the file to look for and is in fact pretty much always set.

If .gnu_debuglink was present then crc would be set but modern systems don't use that anyway - they use .note.gnu.build-id instead and that is always set even if debug hasn't been separated out.
Comment 5 Tom Hughes 2021-05-12 10:02:25 UTC
Created attachment 138358 [details]
Patch to avoid looking for a debuginfo image if the main image has debugging

This patch does work for me by avoiding looking for a separate debuginfo image if the main image has debugging.

That gets the start time for my case down from 2 minutes to 40 seconds.

That's still not the 10 seconds I see when I disable debuginfod completely but it turns out that 30 seconds is the time it takes us to parse the debug information for the system libraries that I didn't previously have debug information for.
Comment 6 Aaron Merey 2021-05-12 14:42:57 UTC
(In reply to Tom Hughes from comment #4)
> No that patch won't help.
> 
> The debugname variable is just the name of the file to look for and is in
> fact pretty much always set.
> 
> If .gnu_debuglink was present then crc would be set but modern systems don't
> use that anyway - they use .note.gnu.build-id instead and that is always set
> even if debug hasn't been separated out.

debugname is set when a .gnu_debuglink or .gnu_debugaltlink section is present and is NULL when there is a build-id but no .gnu_debuglink. It is the latter case in which debuginfod is unnecessary since there is no separate debug file to be found. So I don't see how my patch fails to stop unnecessary debuginfod calls.

Regardless your patch is better insofar as it avoids calling find_debug_file altogether for unstripped binaries instead of just skipping find_debug_file_debuginfod.
Comment 7 Mark Wielaard 2021-05-20 16:06:40 UTC
diff --git a/coregrind/m_debuginfo/readelf.c b/coregrind/m_debuginfo/readelf.c
index b0f062ddc..e424e3e7e 100644
--- a/coregrind/m_debuginfo/readelf.c
+++ b/coregrind/m_debuginfo/readelf.c
@@ -2879,13 +2879,15 @@ Bool ML_(read_elf_debug_info) ( struct _DebugInfo* di )
       /* Look for a build-id */
       HChar* buildid = find_buildid(mimg, False, False);
 
-      /* Look for a debug image that matches either the build-id or
+      /* If we don't have a .debug_info section in the main image then
+         look for a debug image that matches either the build-id or
          the debuglink-CRC32 in the main image.  If the main image
          doesn't contain either of those then this won't even bother
          to try looking.  This looks in all known places, including
          the --extra-debuginfo-path if specified and on the
          --debuginfo-server if specified. */
-      if (buildid != NULL || debuglink_escn.img != NULL) {
+      if (debug_info_escn.img == NULL &&
+          (buildid != NULL || debuglink_escn.img != NULL)) {
          /* Do have a debuglink section? */
          if (debuglink_escn.img != NULL) {
             UInt crc_offset
Comment 8 Tom Hughes 2021-05-20 16:19:34 UTC
The patch has been committed as 93104368952c37268da724231487058ea3eaf1dc.
Comment 9 Tom Hughes 2021-09-06 06:06:21 UTC
*** Bug 442061 has been marked as a duplicate of this bug. ***