Commit: Option to disable the use of debuginfod

Commit: Option to disable the use of debuginfod

[Nick Clifton]

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

Hi Guys,

  I have recently seen several bug reports about readelf and objdump
  unexpectedly slowing down.  The problem turns out to be due to
  attempts to access debuginfod servers which are either not there or
  just slow to respond.  So to help alleviate this problem I am applying
  the patch below which adds a new command line option:
  
    --dwarf=do-not-use-debuginfod       (objdump)
    --debug-dump=do-not-use-debuginfod  (readelf)

  Along with the equivalent =use-debuginfod option and short versions of
  both.  The default behaviour is not changed, so the patch should not
  impact any scripts that are currently relying upon the debuginfod
  support.

Cheers
  Nick

binutils/ChangeLog
2022-03-10  Nick Clifton  <nickc@redhat.com>

	* dwarf.c (use_debuginfod): New variable.  Set to 1.
	(load_separate_debug_info): Only call
	debuginfod_fetch_separate_debug_info is use_debuginfod is true.
	(dwarf_select_sections_by_names): Add do-not-use-debuginfod and
	use-debuginfod options.
	(dwarf_select_sections_by_letters): Add D and E options.
	* dwarf.h (use_debuginfod): New extern.
	* objdump.c (usage): Mention the new options.
	* readelf.c (usage): Likewise.
	* doc/binutils.texi: Document the new options.
	* doc/debug-options.texi: Describe the new options.
	* NEWS: Mention the new feature.
	* testsuite/binutils-all/debuginfod.exp: Add tests of the new
	options.


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

diff --git a/binutils/NEWS b/binutils/NEWS
index c188b469a77..3c447083e2e 100644
--- a/binutils/NEWS
+++ b/binutils/NEWS
@@ -1,5 +1,8 @@
 -*- text -*-
 
+* Add an option to objdump and readelf to prevent attempts to access debuginfod
+  servers when following links.
+
 Changes in 2.38:
 
 * elfedit: Add --output-abiversion option to update ABIVERSION.
diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
index 288974be386..c73837ee27b 100644
--- a/binutils/doc/binutils.texi
+++ b/binutils/doc/binutils.texi
@@ -2246,6 +2246,8 @@ objdump [@option{-a}|@option{--archive-headers}]
          @option{--dwarf}[=rawline,=decodedline,=info,=abbrev,=pubnames,=aranges,=macro,=frames,=frames-interp,=str,=str-offsets,=loc,=Ranges,=pubtypes,=trace_info,=trace_abbrev,=trace_aranges,=gdb_index,=addr,=cu_index,=links]]
         [@option{-WK}|@option{--dwarf=follow-links}]
         [@option{-WN}|@option{--dwarf=no-follow-links}]
+        [@option{-wD}|@option{--dwarf=use-debuginfod}]
+        [@option{-wE}|@option{--dwarf=do-not-use-debuginfod}]
         [@option{-L}|@option{--process-links}]
         [@option{--ctf=}@var{section}]
         [@option{-G}|@option{--stabs}]
@@ -4883,6 +4885,8 @@ readelf [@option{-a}|@option{--all}]
          @option{--debug-dump}[=rawline,=decodedline,=info,=abbrev,=pubnames,=aranges,=macro,=frames,=frames-interp,=str,=str-offsets,=loc,=Ranges,=pubtypes,=trace_info,=trace_abbrev,=trace_aranges,=gdb_index,=addr,=cu_index,=links]]
         [@option{-wK}|@option{--debug-dump=follow-links}]
         [@option{-wN}|@option{--debug-dump=no-follow-links}]
+        [@option{-wD}|@option{--debug-dump=use-debuginfod}]
+        [@option{-wE}|@option{--debug-dump=do-not-use-debuginfod}]
         [@option{-P}|@option{--process-links}]
         [@option{--dwarf-depth=@var{n}}]
         [@option{--dwarf-start=@var{n}}]
@@ -5508,7 +5512,8 @@ deduced from the input file
 @cindex separate debug files
 
 debuginfod is a web service that indexes ELF/DWARF debugging resources
-by build-id and serves them over HTTP.
+by build-id and serves them over HTTP.  For more information see:
+@emph{https://sourceware.org/elfutils/Debuginfod.html}
 
 Binutils can be built with the debuginfod client library
 @code{libdebuginfod} using the @option{--with-debuginfod} configure option.
@@ -5520,6 +5525,10 @@ separate debug files when the files are otherwise not found.
 debuginfod is packaged with elfutils, starting with version 0.178.
 You can get the latest version from `https://sourceware.org/elfutils/'.
 
+The DWARF info dumping tools (@command{readelf} and @command{objdump})
+have options to control when they should access the debuginfod
+servers.  By default this access is enabled.
+
 @node Reporting Bugs
 @chapter Reporting Bugs
 @cindex bugs
diff --git a/binutils/doc/debug.options.texi b/binutils/doc/debug.options.texi
index dbe74ff45f7..26477ec1f95 100644
--- a/binutils/doc/debug.options.texi
+++ b/binutils/doc/debug.options.texi
@@ -68,10 +68,27 @@ chosen when configuring the binutils via the
 @option{--enable-follow-debug-links=no} options.  If these are not
 used then the default is to enable the following of debug links.
 
+Note - if support for the debuginfod protocol was enabled when the
+binutils were built then this option will also include an attempt to
+contact any debuginfod servers mentioned in the @var{DEBUGINFOD_URLS}
+environment variable.  This could take some time to resolve.  This
+behaviour can be disabled via the @option{=do-not-use-debuginfod} debug
+option.
+
 @item N
 @itemx =no-follow-links
 Disables the following of links to separate debug info files.
 
+@item D
+@itemx =use-debuginfod
+Enables contacting debuginfod servers if there is a need to follow
+debug links.  This is the default behaviour.
+
+@item E
+@itemx =do-not-use-debuginfod
+Disables contacting debuginfod servers when there is a need to follow
+debug links.
+
 @item l
 @itemx =rawline
 Displays the contents of the @samp{.debug_line} section in a raw
diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index bc33e9f0814..15b3c81a138 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -109,6 +109,9 @@ int do_debug_cu_index;
 int do_wide;
 int do_debug_links;
 int do_follow_links = DEFAULT_FOR_FOLLOW_LINKS;
+#ifdef HAVE_LIBDEBUGINFOD
+int use_debuginfod = 1;
+#endif
 bool do_checks;
 
 int dwarf_cutoff_level = -1;
@@ -11038,7 +11041,7 @@ debuginfod_fetch_separate_debug_info (struct dwarf_section * section,
 
   return false;
 }
-#endif
+#endif /* HAVE_LIBDEBUGINFOD  */
 
 static void *
 load_separate_debug_info (const char *            main_filename,
@@ -11157,9 +11160,10 @@ load_separate_debug_info (const char *            main_filename,
   {
     char * tmp_filename;
 
-    if (debuginfod_fetch_separate_debug_info (xlink,
-                                              & tmp_filename,
-                                              file))
+    if (use_debuginfod
+	&& debuginfod_fetch_separate_debug_info (xlink,
+						 & tmp_filename,
+						 file))
       {
         /* File successfully downloaded from server, replace
            debug_filename with the file's path.  */
@@ -11207,13 +11211,15 @@ load_separate_debug_info (const char *            main_filename,
       warn (_("tried: %s\n"), debug_filename);
 
 #if HAVE_LIBDEBUGINFOD
-      {
-	char *urls = getenv (DEBUGINFOD_URLS_ENV_VAR);
-	if (urls == NULL)
-	  urls = "";
+      if (use_debuginfod)
+	{
+	  char *urls = getenv (DEBUGINFOD_URLS_ENV_VAR);
 
-	warn (_("tried: DEBUGINFOD_URLS=%s\n"), urls);
-      }
+	  if (urls == NULL)
+	    urls = "";
+
+	  warn (_("tried: DEBUGINFOD_URLS=%s\n"), urls);
+	}
 #endif
     }
 
@@ -11707,6 +11713,9 @@ dwarf_select_sections_by_names (const char *names)
       { "aranges", & do_debug_aranges, 1 },
       { "cu_index", & do_debug_cu_index, 1 },
       { "decodedline", & do_debug_lines, FLAG_DEBUG_LINES_DECODED },
+#ifdef HAVE_LIBDEBUGINFOD
+      { "do-not-use-debuginfod", & use_debuginfod, 0 },
+#endif
       { "follow-links", & do_follow_links, 1 },
       { "frames", & do_debug_frames, 1 },
       { "frames-interp", & do_debug_frames_interp, 1 },
@@ -11730,6 +11739,9 @@ dwarf_select_sections_by_names (const char *names)
       { "trace_abbrev", & do_trace_abbrevs, 1 },
       { "trace_aranges", & do_trace_aranges, 1 },
       { "trace_info", & do_trace_info, 1 },
+#ifdef HAVE_LIBDEBUGINFOD
+      { "use-debuginfod", & use_debuginfod, 1 },
+#endif
       { NULL, NULL, 0 }
     };
 
@@ -11783,6 +11795,10 @@ dwarf_select_sections_by_letters (const char *letters)
       case 'A':	do_debug_addr = 1; break;
       case 'a':	do_debug_abbrevs = 1; break;
       case 'c':	do_debug_cu_index = 1; break;
+#ifdef HAVE_LIBDEBUGINFOD
+      case 'D': use_debuginfod = 1; break;
+      case 'E': use_debuginfod = 0; break;
+#endif
       case 'F':	do_debug_frames_interp = 1; /* Fall through.  */
       case 'f':	do_debug_frames = 1; break;
       case 'g':	do_gdb_index = 1; break;
diff --git a/binutils/dwarf.h b/binutils/dwarf.h
index 18430719d48..4fc62abfa4c 100644
--- a/binutils/dwarf.h
+++ b/binutils/dwarf.h
@@ -224,6 +224,9 @@ extern int do_debug_cu_index;
 extern int do_wide;
 extern int do_debug_links;
 extern int do_follow_links;
+#ifdef HAVE_LIBDEBUGINFOD
+extern int use_debuginfod;
+#endif
 extern bool do_checks;
 
 extern int dwarf_cutoff_level;
diff --git a/binutils/objdump.c b/binutils/objdump.c
index 24e91869bfd..8e1c9cb0c21 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -281,6 +281,14 @@ usage (FILE *stream, int status)
   -WN,--dwarf=no-follow-links\n\
                            Do not follow links to separate debug info files\n\
                             (default)\n"));
+#endif
+#if HAVE_LIBDEBUGINFOD
+  fprintf (stream, _("\
+  -WD --dwarf=use-debuginfod\n\
+                           When following links, also query debuginfod servers (default)\n"));
+  fprintf (stream, _("\
+  -WE --dwarf=do-not-use-debuginfod\n\
+                           When following links, do not query debuginfod servers\n"));
 #endif
   fprintf (stream, _("\
   -L, --process-links      Display the contents of non-debug sections in\n\
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 16efe1dfd2d..8b46052c7de 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -5126,6 +5126,14 @@ usage (FILE * stream)
   -wN --debug-dump=no-follow-links\n\
                          Do not follow links to separate debug info files\n\
                           (default)\n"));
+#endif
+#if HAVE_LIBDEBUGINFOD
+  fprintf (stream, _("\
+  -wD --debug-dump=use-debuginfod\n\
+                         When following links, also query debuginfod servers (default)\n"));
+  fprintf (stream, _("\
+  -wE --debug-dump=do-not-use-debuginfod\n\
+                         When following links, do not query debuginfod servers\n"));
 #endif
   fprintf (stream, _("\
   --dwarf-depth=N        Do not display DIEs at depth N or greater\n"));
diff --git a/binutils/testsuite/binutils-all/debuginfod.exp b/binutils/testsuite/binutils-all/debuginfod.exp
index 96543b2037a..f5935ad0348 100644
--- a/binutils/testsuite/binutils-all/debuginfod.exp
+++ b/binutils/testsuite/binutils-all/debuginfod.exp
@@ -185,8 +185,14 @@ proc test_fetch_debugaltlink { prog progargs } {
 }
 
 if { [regexp ".*DEBUGINFOD.*" $conf_objdump] } {
-    test_fetch_debuglink $OBJDUMP "-W"
+    test_fetch_debuglink $OBJDUMP "-W -WD"
     test_fetch_debugaltlink $OBJDUMP "-Wk"
+
+    set test "disabling debuginfod access"
+    setup_xfail *-*-*
+    test_fetch_debuglink $OBJDUMP "-W -WE"
+    set test "debuginfod"
+
 } else {
     untested "$test (objdump not configured with debuginfod)"
 }
@@ -194,6 +200,12 @@ if { [regexp ".*DEBUGINFOD.*" $conf_objdump] } {
 if { [regexp ".*DEBUGINFOD.*" $conf_readelf] } {
     test_fetch_debuglink $READELF "-w"
     test_fetch_debugaltlink $READELF "-wk"
+
+    set test "disabling debuginfod access"
+    setup_xfail *-*-*
+    test_fetch_debuglink $READELF "-w -wE"
+    set test "debuginfod"
+
 } else {
     untested "$test (readelf not configured with debuginfod)"
 }

Re: Commit: Option to disable the use of debuginfod

[Luis Machado]

Hi Nick,

On 3/10/22 09:11, Nick Clifton via Binutils wrote:
> Hi Guys,
> 
>    I have recently seen several bug reports about readelf and objdump
>    unexpectedly slowing down.  The problem turns out to be due to
>    attempts to access debuginfod servers which are either not there or
>    just slow to respond.  So to help alleviate this problem I am applying
>    the patch below which adds a new command line option:

If it slows things down by default, shouldn't we change the default to 
disabled (I'm assuming it is enabled by default) and/or look into 
improving the code so it doesn't slow things down?

Re: Commit: Option to disable the use of debuginfod

[Nick Clifton]

Hi Luis,

>>    I have recently seen several bug reports about readelf and objdump
>>    unexpectedly slowing down.  The problem turns out to be due to
>>    attempts to access debuginfod servers which are either not there or
>>    just slow to respond.  So to help alleviate this problem I am applying
>>    the patch below which adds a new command line option:
> 
> If it slows things down by default, shouldn't we change the default to 
> disabled (I'm assuming it is enabled by default) 

Yes, it is enabled by default.

I did think of changing the default to be "disabled" but then this would
mean introducing a conflict between the current binutils and future
binutils releases.  Plus my feeling is that it is better to provide the
user with as much information as possible by default, since these tools
are basically used when investigating something.  Skipping information
because it might slow things down ought to be definite action taken by
the user rather than a default action taken by the tool.


> and/or look into improving the code so it doesn't slow things down?

Well the slow down is in the debuginfod client library code, or more
accurately in the communication with the servers, so there is not a lot
that we can do on our end.  At least I don't think so.  Maybe there is
a timeout setting that we could tweak.  I have to confess that I am not
exactly an expert in the debuginfod code...

Cheers
   Nick