[PATCH 1/3] [gdb/testsuite] Mention DEBUGINFOD_VERBOSE in gdb.debuginfod test-cases

[PATCH 1/3] [gdb/testsuite] Mention DEBUGINFOD_VERBOSE in gdb.debuginfod test-cases

[Tom de Vries]

In proc start_debuginfod we start debuginfod with a fair amount of verbosity:
...
	spawn debuginfod -vvvv -d $db -p $port -F $debugdir
...
which gives a lot of information on startup, but none or not much after that.

There's another way to increase verbosity of debuginfod: through the
environment variable DEBUGINFOD_VERBOSE, which gives us the following
additional information (edited for readability):
...
(gdb) file fetch_src_and_symbols^M
 Reading symbols from fetch_src_and_symbols...^M
+debuginfod_find_debuginfo $hex^M
+server urls "http://127.0.0.1:8000"^M
+checking build-id^M
+checking cache dir $client_cache^M
+using timeout 10^M
+init server 0 http://127.0.0.1:8000^M
+url 0 http://127.0.0.1:8000/buildid/$hex/debuginfo^M
+query 1 urls in parallel^M
 Downloading separate debug info for $fetch_src_and_symbols...^M
+server response No error^M
+got file from server^M
+found $client_cache/$hex/debuginfo (fd=18)^M
 Reading symbols from $client_cache/$hex/debuginfo...^M
(gdb)
...

IMO this is too verbose to enable by default, but it's good to at least
mention the option explicitly in the test-cases.

Tested by enabling it in the 3 gdb.debuginfod test-cases.  This generates 2
FAILs, but I don't think it's worthwhile to address these.

Tested on x86_64-linux.
---
 gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp | 3 +++
 gdb/testsuite/gdb.debuginfod/crc_mismatch.exp              | 3 +++
 gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp     | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
index b86622543ef..25d800a378b 100644
--- a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
+++ b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
@@ -127,6 +127,9 @@ prepare_for_debuginfod cache db
 proc_with_prefix local_debuginfod { } {
     global db debuginfod_debugdir cache build_id_debug_file
 
+    # Uncomment to increase verbosity of debuginfod when handling query.
+    #setenv DEBUGINFOD_VERBOSE 1
+
     set url [start_debuginfod $db $debuginfod_debugdir]
     if {$url eq ""} {
 	unresolved "failed to start debuginfod server"
diff --git a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
index 017ef132573..27069b580dc 100644
--- a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
+++ b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
@@ -99,6 +99,9 @@ proc_with_prefix local_debuginfod { } {
     global binfile db debugdir cache
     set escapedobjdirsubdir [string_to_regexp [standard_output_file {}]]
 
+    # Uncomment to increase verbosity of debuginfod when handling query.
+    #setenv DEBUGINFOD_VERBOSE 1
+
     set url [start_debuginfod $db $debugdir]
     if {$url eq ""} {
 	unresolved "failed to start debuginfod server"
diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
index 401af0df0d2..0fb752ff6bd 100644
--- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
+++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
@@ -209,6 +209,9 @@ proc disable_delete_breakpoints {} {
 proc_with_prefix local_url { } {
     global binfile outputdir debugdir db
 
+    # Uncomment to increase verbosity of debuginfod when handling query.
+    #setenv DEBUGINFOD_VERBOSE 1
+
     set url [start_debuginfod $db $debugdir]
     if { $url == "" } {
 	unresolved "failed to start debuginfod server"

base-commit: 4250085217f2011335257fd3291cb50c939e9746
-- 
2.35.3

[PATCH 2/3] [gdb/symtab] Add debuginfod verbose level 2

[Tom de Vries]

In print_outcome in debuginfod-support.c, download failures are not mentioned
if the errno is ENOENT, aka as "No such file or directory":
...
static void
print_outcome (int fd, const char *desc, const char *fname)
{
  if (fd < 0 && fd != -ENOENT)
    {
      ui_file *outstream = get_unbuffered (gdb_stdout);
      gdb_printf (outstream,
                  _("Download failed: %s.  Continuing without %s %ps.\n"),
                  safe_strerror (-fd),
                  desc,
                  styled_string (file_name_style.style (), fname));
    }
}
...

Define a new debuginfod verbose level 2 at which also this type of download
failure is mentioned, and use it in the gdb.debuginfod test-cases, such that
we get the explicit:
...
Downloading separate debug info for /lib64/ld-linux-x86-64.so.2...^M
Download failed: No such file or directory.  Continuing without separate \
  debug info for /lib64/ld-linux-x86-64.so.2.^M
...

Tested on x86_64-linux.
---
 gdb/debuginfod-support.c                      |  2 +-
 gdb/doc/gdb.texinfo                           |  4 +--
 .../build-id-no-debug-warning.exp             |  6 +++-
 gdb/testsuite/gdb.debuginfod/crc_mismatch.exp |  7 ++++-
 .../gdb.debuginfod/fetch_src_and_symbols.exp  | 31 ++++++++++++-------
 5 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
index 841b6f2078c..e65afb7523f 100644
--- a/gdb/debuginfod-support.c
+++ b/gdb/debuginfod-support.c
@@ -288,7 +288,7 @@ debuginfod_is_enabled ()
 static void
 print_outcome (int fd, const char *desc, const char *fname)
 {
-  if (fd < 0 && fd != -ENOENT)
+  if (fd < 0 && (fd != -ENOENT || debuginfod_verbose >= 2))
     {
       ui_file *outstream = get_unbuffered (gdb_stdout);
       gdb_printf (outstream,
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 61f91ef4ad6..fa1812124c1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -50330,8 +50330,8 @@ Display the list of URLs that @code{debuginfod} will attempt to query.
 @item set debuginfod verbose
 @itemx set debuginfod verbose @var{n}
 Enable or disable @code{debuginfod}-related output.  Use a non-zero value
-to enable and @code{0} to disable.  @code{debuginfod} output is shown by
-default.
+to enable and @code{0} to disable.  Higher values may produce more
+output.  Set to @code{1} by default.
 
 @kindex show debuginfod verbose
 @item show debuginfod verbose
diff --git a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
index 25d800a378b..d63957ac05c 100644
--- a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
+++ b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
@@ -142,9 +142,13 @@ proc_with_prefix local_debuginfod { } {
     # GDB should now find the symbol and source files.
     clean_restart
 
-    # Enable debuginfod and fetch the debuginfo.
+    # Enable debuginfod.
     gdb_test_no_output "set debuginfod enabled on"
 
+    # Be verbose about observed debuginfod behaviour.
+    gdb_test_no_output "set debuginfod verbose 2"
+
+    # Fetch the debuginfo.
     # "separate debug info file has no debug info" warning should not be
     # reported now because the correct debuginfo should be fetched from
     # debuginfod.
diff --git a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
index 27069b580dc..6dcabf60357 100644
--- a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
+++ b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
@@ -114,8 +114,13 @@ proc_with_prefix local_debuginfod { } {
     # GDB should now find the symbol and source files.
     clean_restart
 
-    # Enable debuginfod and fetch the debuginfo.
+    # Enable debuginfod.
     gdb_test_no_output "set debuginfod enabled on"
+
+    # Be verbose about observed debuginfod behaviour.
+    gdb_test_no_output -nopass "set debuginfod verbose 2"
+
+    # Fetch the debuginfo.
     gdb_test "file $binfile" ".*Reading symbols from.*debuginfo.*" \
 	"file [file tail $binfile] cmd on"
 
diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
index 0fb752ff6bd..4d2f70261fe 100644
--- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
+++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
@@ -42,6 +42,13 @@ if { [gdb_compile "$sourcetmp" "${binfile}2" executable {debug build-id}] != ""
     return -1
 }
 
+proc clean_restart_local {{executable ""}} {
+    clean_restart $executable
+
+    # Be verbose about observed debuginfod behaviour.
+    gdb_test_no_output -nopass "set debuginfod verbose 2"
+}
+
 # Write some assembly that just has a .gnu_debugaltlink section.
 # Copied from testsuite/gdb.dwarf2/dwzbuildid.exp.
 proc write_just_debugaltlink {filename dwzname buildid} {
@@ -107,7 +114,7 @@ proc_with_prefix no_url { } {
     setenv DEBUGINFOD_URLS ""
 
     # Test that GDB cannot find source without debuginfod.
-    clean_restart $binfile
+    clean_restart_local $binfile
     gdb_test_no_output "set substitute-path $outputdir /dev/null" \
 	"set substitute-path"
     gdb_test "list" ".*No such file or directory.*"
@@ -126,7 +133,7 @@ proc_with_prefix no_url { } {
     file rename -force $debuginfo $debugdir
 
     # Test that GDB cannot find symbols without debuginfod.
-    clean_restart $binfile
+    clean_restart_local $binfile
     gdb_test "file" ".*No symbol file.*"
 
     set buildid "01234567890abcdef0123456"
@@ -150,14 +157,14 @@ proc_with_prefix no_url { } {
     file rename -force ${binfile}_dwz.o $debugdir
 
     # Test that GDB cannot find dwz without debuginfod.
-    clean_restart
+    clean_restart_local
     gdb_test "file ${binfile}_alt.o" \
 	".*could not find '.gnu_debugaltlink'.*" \
 	"file [file tail ${binfile}_alt.o]"
 
     # Generate a core file and test that GDB cannot find the
     # executable.
-    clean_restart ${binfile}2
+    clean_restart_local ${binfile}2
     if ![runto_main] {
 	return -1
     }
@@ -169,7 +176,7 @@ proc_with_prefix no_url { } {
 	"file [file tail $::corefile] gen"
     file rename -force ${binfile}2 $debugdir
 
-    clean_restart
+    clean_restart_local
     gdb_test "core $::corefile" ".*in ?? ().*" "file [file tail $::corefile]"
 }
 
@@ -180,7 +187,7 @@ proc_with_prefix no_url { } {
 
 proc test_urls {urls pattern_re test} {
     setenv DEBUGINFOD_URLS $urls
-    clean_restart
+    clean_restart_local
 
     if {$pattern_re != ""} {
 	set urls_re " +${pattern_re}\r\n"
@@ -222,7 +229,7 @@ proc_with_prefix local_url { } {
     setenv DEBUGINFOD_URLS $url
 
     # GDB should now find the symbol and source files.
-    clean_restart
+    clean_restart_local
     gdb_test_no_output "set debuginfod enabled on" \
 	"enabled debuginfod for initial test"
     gdb_load $binfile
@@ -248,12 +255,12 @@ proc_with_prefix local_url { } {
     # GDB should now find the executable file.
     set enable_debuginfod_question \
 	"Enable debuginfod for this session. \\(y or \\\[n\\\]\\) "
-    clean_restart
+    clean_restart_local
     gdb_test "core $::corefile" ".*return 0.*" "file [file tail $::corefile]" \
 	$enable_debuginfod_question "y"
 
     # GDB should now find the debugaltlink file.
-    clean_restart
+    clean_restart_local
     gdb_test "file ${binfile}_alt.o" \
 	".*Downloading.*separate debug info.*" \
 	"file [file tail ${binfile}_alt.o]" \
@@ -261,7 +268,7 @@ proc_with_prefix local_url { } {
 
     # Configure debuginfod with commands.
     unsetenv DEBUGINFOD_URLS
-    clean_restart
+    clean_restart_local
     gdb_test "file $binfile" ".*No debugging symbols.*" \
 	"file [file tail $binfile] cmd"
     gdb_test_no_output "set debuginfod enabled off"
@@ -282,7 +289,7 @@ proc_with_prefix local_url { } {
 
     # Empty URLS disables Debuginfod.
     setenv DEBUGINFOD_URLS ""
-    clean_restart
+    clean_restart_local
     # Disable confirmation to avoid having to deal with a query.  See
     # test_urls.
     set file_cmd "with confirm off -- file $binfile"
@@ -297,7 +304,7 @@ proc_with_prefix local_url { } {
 
     # Whitespace-only URLS disables Debuginfod.
     setenv DEBUGINFOD_URLS "    "
-    clean_restart
+    clean_restart_local
     gdb_test_multiple $file_cmd "notice whitespace URL" {
 	-re -wrap "This GDB supports auto-downloading.*" {
 	    fail $gdb_test_name
-- 
2.35.3

[PATCH 3/3] [gdb/symtab] Note success at debuginfod verbose level 2

[Tom de Vries]

Expand debuginfod verbose level 2 by also mentioning download successes:
...
Downloading separate debug info for crc_mismatch-2...^M
Download succeeded: separate debug info for crc_mismatch-2.^M
Reading symbols from $client_cache/$hex/debuginfo...^M
...

Tested on x86_64-linux.
---
 gdb/debuginfod-support.c                               |  8 ++++++++
 .../gdb.debuginfod/build-id-no-debug-warning.exp       |  2 ++
 gdb/testsuite/gdb.debuginfod/crc_mismatch.exp          | 10 +++++++---
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
index e65afb7523f..fe735756f72 100644
--- a/gdb/debuginfod-support.c
+++ b/gdb/debuginfod-support.c
@@ -297,6 +297,14 @@ print_outcome (int fd, const char *desc, const char *fname)
 		  desc,
 		  styled_string (file_name_style.style (), fname));
     }
+  if (fd >= 0 && debuginfod_verbose >= 2)
+    {
+      ui_file *outstream = get_unbuffered (gdb_stdout);
+      gdb_printf (outstream,
+		  _("Download succeeded: %s %ps.\n"),
+		  desc,
+		  styled_string (file_name_style.style (), fname));
+    }
 }
 
 /* See debuginfod-support.h  */
diff --git a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
index d63957ac05c..1f80dee9e33 100644
--- a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
+++ b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
@@ -156,6 +156,8 @@ proc_with_prefix local_debuginfod { } {
 	[multi_line \
 	     "Reading symbols from ${build_id_debug_file}\\.\\.\\." \
 	     "Downloading separate debug info for ${build_id_debug_file}\\.\\.\\." \
+	     [string_to_regexp \
+		  "Download succeeded: separate debug info for ${build_id_debug_file}."] \
 	     "Reading symbols from ${cache}/\[^\r\n\]+\\.\\.\\.(?:\r\nExpanding full symbols from \[^\r\n\]+)*"] \
 	"debuginfod running, info downloaded, no warnings"
 }
diff --git a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
index 6dcabf60357..8391e8f4a01 100644
--- a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
+++ b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
@@ -126,10 +126,14 @@ proc_with_prefix local_debuginfod { } {
 
     # CRC mismatch should not be reported now because the correct debuginfo
     # should be fetched from debuginfod.
-    gdb_test "file [standard_output_file crc_mismatch-2]" \
+    set binfile2 [standard_output_file crc_mismatch-2]
+    gdb_test "file $binfile2" \
 	[multi_line \
-	     "Reading symbols from ${escapedobjdirsubdir}/crc_mismatch-2\\.\\.\\." \
-	     "Downloading.*separate debug info for ${escapedobjdirsubdir}/crc_mismatch-2\\.\\.\\." \
+	     [string_to_regexp "Reading symbols from $binfile2..."] \
+	     [string_to_regexp \
+		  "Downloading separate debug info for $binfile2..."] \
+	     [string_to_regexp \
+		  "Download succeeded: separate debug info for $binfile2."] \
 	     "Reading symbols from ${cache}/\[^\r\n\]+\\.\\.\\.(?:\r\nExpanding full symbols from \[^\r\n\]+)*"] \
 	 "debuginfod running, info downloaded, no CRC mismatch"
 }
-- 
2.35.3

Re: [PATCH 1/3] [gdb/testsuite] Mention DEBUGINFOD_VERBOSE in gdb.debuginfod test-cases

[Andrew Burgess]

Tom de Vries <tdevries@suse.de> writes:

> In proc start_debuginfod we start debuginfod with a fair amount of verbosity:
> ...
> 	spawn debuginfod -vvvv -d $db -p $port -F $debugdir
> ...
> which gives a lot of information on startup, but none or not much after that.
>
> There's another way to increase verbosity of debuginfod: through the
> environment variable DEBUGINFOD_VERBOSE, which gives us the following
> additional information (edited for readability):
> ...
> (gdb) file fetch_src_and_symbols^M
>  Reading symbols from fetch_src_and_symbols...^M
> +debuginfod_find_debuginfo $hex^M
> +server urls "http://127.0.0.1:8000"^M
> +checking build-id^M
> +checking cache dir $client_cache^M
> +using timeout 10^M
> +init server 0 http://127.0.0.1:8000^M
> +url 0 http://127.0.0.1:8000/buildid/$hex/debuginfo^M
> +query 1 urls in parallel^M
>  Downloading separate debug info for $fetch_src_and_symbols...^M
> +server response No error^M
> +got file from server^M
> +found $client_cache/$hex/debuginfo (fd=18)^M
>  Reading symbols from $client_cache/$hex/debuginfo...^M
> (gdb)
> ...
>
> IMO this is too verbose to enable by default, but it's good to at least
> mention the option explicitly in the test-cases.
>
> Tested by enabling it in the 3 gdb.debuginfod test-cases.  This generates 2
> FAILs, but I don't think it's worthwhile to address these.
>
> Tested on x86_64-linux.
> ---
>  gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp | 3 +++
>  gdb/testsuite/gdb.debuginfod/crc_mismatch.exp              | 3 +++
>  gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp     | 3 +++
>  3 files changed, 9 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> index b86622543ef..25d800a378b 100644
> --- a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> +++ b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> @@ -127,6 +127,9 @@ prepare_for_debuginfod cache db
>  proc_with_prefix local_debuginfod { } {
>      global db debuginfod_debugdir cache build_id_debug_file
>  
> +    # Uncomment to increase verbosity of debuginfod when handling query.
> +    #setenv DEBUGINFOD_VERBOSE 1
> +

I'm not a huge fan of leaving comment out code like this.  I have some
patches pending that add more debuginfod tests.  Am I expected to add
this same commented out code before every call to start_debuginfod?

Some alternative suggestions:

  - Add this comment to start_debuginfod's comment, as in: "If
    additional debugging is required try 'setenv DEBUGINFOD_VERBOSE 1'."

  - We could document this in testsuite/README as a hint for debugging
    debuginfod tests.

  - We could add an 'args' list argument to start_debuginfod, and accept
    a 'verbose=1' argument within args.

Hopefully you might find one of these an acceptable alternative...

thanks,
Andrew


>      set url [start_debuginfod $db $debuginfod_debugdir]
>      if {$url eq ""} {
>  	unresolved "failed to start debuginfod server"
> diff --git a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
> index 017ef132573..27069b580dc 100644
> --- a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
> +++ b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
> @@ -99,6 +99,9 @@ proc_with_prefix local_debuginfod { } {
>      global binfile db debugdir cache
>      set escapedobjdirsubdir [string_to_regexp [standard_output_file {}]]
>  
> +    # Uncomment to increase verbosity of debuginfod when handling query.
> +    #setenv DEBUGINFOD_VERBOSE 1
> +
>      set url [start_debuginfod $db $debugdir]
>      if {$url eq ""} {
>  	unresolved "failed to start debuginfod server"
> diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> index 401af0df0d2..0fb752ff6bd 100644
> --- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> +++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> @@ -209,6 +209,9 @@ proc disable_delete_breakpoints {} {
>  proc_with_prefix local_url { } {
>      global binfile outputdir debugdir db
>  
> +    # Uncomment to increase verbosity of debuginfod when handling query.
> +    #setenv DEBUGINFOD_VERBOSE 1
> +
>      set url [start_debuginfod $db $debugdir]
>      if { $url == "" } {
>  	unresolved "failed to start debuginfod server"
>
> base-commit: 4250085217f2011335257fd3291cb50c939e9746
> -- 
> 2.35.3

Re: [PATCH 2/3] [gdb/symtab] Add debuginfod verbose level 2

[Andrew Burgess]

Tom de Vries <tdevries@suse.de> writes:

> In print_outcome in debuginfod-support.c, download failures are not mentioned
> if the errno is ENOENT, aka as "No such file or directory":
> ...
> static void
> print_outcome (int fd, const char *desc, const char *fname)
> {
>   if (fd < 0 && fd != -ENOENT)
>     {
>       ui_file *outstream = get_unbuffered (gdb_stdout);
>       gdb_printf (outstream,
>                   _("Download failed: %s.  Continuing without %s %ps.\n"),
>                   safe_strerror (-fd),
>                   desc,
>                   styled_string (file_name_style.style (), fname));
>     }
> }

I believe ENOENT is what we get back when the debuginfod server doesn't
have anything to give us, in which case, in normal usage we prefer not
to spam the user that the server couldn't help us.

But I can see how this might be useful.

There is some possible cross over here between a flag to make user
commands more or less verbose, and the possibility of adding a 'set
debug debuginfod on/off' option.  When does "extra verbose user output"
become "debug output only useful for GDB maintainers"?  I'm not really
sure what the answer is.

Anyway, I figure we can always tweak these messages later if needed.  So
I'd be happy with these going in.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

> ...
>
> Define a new debuginfod verbose level 2 at which also this type of download
> failure is mentioned, and use it in the gdb.debuginfod test-cases, such that
> we get the explicit:
> ...
> Downloading separate debug info for /lib64/ld-linux-x86-64.so.2...^M
> Download failed: No such file or directory.  Continuing without separate \
>   debug info for /lib64/ld-linux-x86-64.so.2.^M
> ...
>
> Tested on x86_64-linux.
> ---
>  gdb/debuginfod-support.c                      |  2 +-
>  gdb/doc/gdb.texinfo                           |  4 +--
>  .../build-id-no-debug-warning.exp             |  6 +++-
>  gdb/testsuite/gdb.debuginfod/crc_mismatch.exp |  7 ++++-
>  .../gdb.debuginfod/fetch_src_and_symbols.exp  | 31 ++++++++++++-------
>  5 files changed, 33 insertions(+), 17 deletions(-)
>
> diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
> index 841b6f2078c..e65afb7523f 100644
> --- a/gdb/debuginfod-support.c
> +++ b/gdb/debuginfod-support.c
> @@ -288,7 +288,7 @@ debuginfod_is_enabled ()
>  static void
>  print_outcome (int fd, const char *desc, const char *fname)
>  {
> -  if (fd < 0 && fd != -ENOENT)
> +  if (fd < 0 && (fd != -ENOENT || debuginfod_verbose >= 2))
>      {
>        ui_file *outstream = get_unbuffered (gdb_stdout);
>        gdb_printf (outstream,
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 61f91ef4ad6..fa1812124c1 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -50330,8 +50330,8 @@ Display the list of URLs that @code{debuginfod} will attempt to query.
>  @item set debuginfod verbose
>  @itemx set debuginfod verbose @var{n}
>  Enable or disable @code{debuginfod}-related output.  Use a non-zero value
> -to enable and @code{0} to disable.  @code{debuginfod} output is shown by
> -default.
> +to enable and @code{0} to disable.  Higher values may produce more
> +output.  Set to @code{1} by default.
>  
>  @kindex show debuginfod verbose
>  @item show debuginfod verbose
> diff --git a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> index 25d800a378b..d63957ac05c 100644
> --- a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> +++ b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> @@ -142,9 +142,13 @@ proc_with_prefix local_debuginfod { } {
>      # GDB should now find the symbol and source files.
>      clean_restart
>  
> -    # Enable debuginfod and fetch the debuginfo.
> +    # Enable debuginfod.
>      gdb_test_no_output "set debuginfod enabled on"
>  
> +    # Be verbose about observed debuginfod behaviour.
> +    gdb_test_no_output "set debuginfod verbose 2"
> +
> +    # Fetch the debuginfo.
>      # "separate debug info file has no debug info" warning should not be
>      # reported now because the correct debuginfo should be fetched from
>      # debuginfod.
> diff --git a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
> index 27069b580dc..6dcabf60357 100644
> --- a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
> +++ b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
> @@ -114,8 +114,13 @@ proc_with_prefix local_debuginfod { } {
>      # GDB should now find the symbol and source files.
>      clean_restart
>  
> -    # Enable debuginfod and fetch the debuginfo.
> +    # Enable debuginfod.
>      gdb_test_no_output "set debuginfod enabled on"
> +
> +    # Be verbose about observed debuginfod behaviour.
> +    gdb_test_no_output -nopass "set debuginfod verbose 2"
> +
> +    # Fetch the debuginfo.
>      gdb_test "file $binfile" ".*Reading symbols from.*debuginfo.*" \
>  	"file [file tail $binfile] cmd on"
>  
> diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> index 0fb752ff6bd..4d2f70261fe 100644
> --- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> +++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> @@ -42,6 +42,13 @@ if { [gdb_compile "$sourcetmp" "${binfile}2" executable {debug build-id}] != ""
>      return -1
>  }
>  
> +proc clean_restart_local {{executable ""}} {
> +    clean_restart $executable
> +
> +    # Be verbose about observed debuginfod behaviour.
> +    gdb_test_no_output -nopass "set debuginfod verbose 2"
> +}
> +
>  # Write some assembly that just has a .gnu_debugaltlink section.
>  # Copied from testsuite/gdb.dwarf2/dwzbuildid.exp.
>  proc write_just_debugaltlink {filename dwzname buildid} {
> @@ -107,7 +114,7 @@ proc_with_prefix no_url { } {
>      setenv DEBUGINFOD_URLS ""
>  
>      # Test that GDB cannot find source without debuginfod.
> -    clean_restart $binfile
> +    clean_restart_local $binfile
>      gdb_test_no_output "set substitute-path $outputdir /dev/null" \
>  	"set substitute-path"
>      gdb_test "list" ".*No such file or directory.*"
> @@ -126,7 +133,7 @@ proc_with_prefix no_url { } {
>      file rename -force $debuginfo $debugdir
>  
>      # Test that GDB cannot find symbols without debuginfod.
> -    clean_restart $binfile
> +    clean_restart_local $binfile
>      gdb_test "file" ".*No symbol file.*"
>  
>      set buildid "01234567890abcdef0123456"
> @@ -150,14 +157,14 @@ proc_with_prefix no_url { } {
>      file rename -force ${binfile}_dwz.o $debugdir
>  
>      # Test that GDB cannot find dwz without debuginfod.
> -    clean_restart
> +    clean_restart_local
>      gdb_test "file ${binfile}_alt.o" \
>  	".*could not find '.gnu_debugaltlink'.*" \
>  	"file [file tail ${binfile}_alt.o]"
>  
>      # Generate a core file and test that GDB cannot find the
>      # executable.
> -    clean_restart ${binfile}2
> +    clean_restart_local ${binfile}2
>      if ![runto_main] {
>  	return -1
>      }
> @@ -169,7 +176,7 @@ proc_with_prefix no_url { } {
>  	"file [file tail $::corefile] gen"
>      file rename -force ${binfile}2 $debugdir
>  
> -    clean_restart
> +    clean_restart_local
>      gdb_test "core $::corefile" ".*in ?? ().*" "file [file tail $::corefile]"
>  }
>  
> @@ -180,7 +187,7 @@ proc_with_prefix no_url { } {
>  
>  proc test_urls {urls pattern_re test} {
>      setenv DEBUGINFOD_URLS $urls
> -    clean_restart
> +    clean_restart_local
>  
>      if {$pattern_re != ""} {
>  	set urls_re " +${pattern_re}\r\n"
> @@ -222,7 +229,7 @@ proc_with_prefix local_url { } {
>      setenv DEBUGINFOD_URLS $url
>  
>      # GDB should now find the symbol and source files.
> -    clean_restart
> +    clean_restart_local
>      gdb_test_no_output "set debuginfod enabled on" \
>  	"enabled debuginfod for initial test"
>      gdb_load $binfile
> @@ -248,12 +255,12 @@ proc_with_prefix local_url { } {
>      # GDB should now find the executable file.
>      set enable_debuginfod_question \
>  	"Enable debuginfod for this session. \\(y or \\\[n\\\]\\) "
> -    clean_restart
> +    clean_restart_local
>      gdb_test "core $::corefile" ".*return 0.*" "file [file tail $::corefile]" \
>  	$enable_debuginfod_question "y"
>  
>      # GDB should now find the debugaltlink file.
> -    clean_restart
> +    clean_restart_local
>      gdb_test "file ${binfile}_alt.o" \
>  	".*Downloading.*separate debug info.*" \
>  	"file [file tail ${binfile}_alt.o]" \
> @@ -261,7 +268,7 @@ proc_with_prefix local_url { } {
>  
>      # Configure debuginfod with commands.
>      unsetenv DEBUGINFOD_URLS
> -    clean_restart
> +    clean_restart_local
>      gdb_test "file $binfile" ".*No debugging symbols.*" \
>  	"file [file tail $binfile] cmd"
>      gdb_test_no_output "set debuginfod enabled off"
> @@ -282,7 +289,7 @@ proc_with_prefix local_url { } {
>  
>      # Empty URLS disables Debuginfod.
>      setenv DEBUGINFOD_URLS ""
> -    clean_restart
> +    clean_restart_local
>      # Disable confirmation to avoid having to deal with a query.  See
>      # test_urls.
>      set file_cmd "with confirm off -- file $binfile"
> @@ -297,7 +304,7 @@ proc_with_prefix local_url { } {
>  
>      # Whitespace-only URLS disables Debuginfod.
>      setenv DEBUGINFOD_URLS "    "
> -    clean_restart
> +    clean_restart_local
>      gdb_test_multiple $file_cmd "notice whitespace URL" {
>  	-re -wrap "This GDB supports auto-downloading.*" {
>  	    fail $gdb_test_name
> -- 
> 2.35.3

Re: [PATCH 3/3] [gdb/symtab] Note success at debuginfod verbose level 2

[Andrew Burgess]

Tom de Vries <tdevries@suse.de> writes:

> Expand debuginfod verbose level 2 by also mentioning download successes:
> ...
> Downloading separate debug info for crc_mismatch-2...^M
> Download succeeded: separate debug info for crc_mismatch-2.^M
> Reading symbols from $client_cache/$hex/debuginfo...^M
> ...

LGTM.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


>
> Tested on x86_64-linux.
> ---
>  gdb/debuginfod-support.c                               |  8 ++++++++
>  .../gdb.debuginfod/build-id-no-debug-warning.exp       |  2 ++
>  gdb/testsuite/gdb.debuginfod/crc_mismatch.exp          | 10 +++++++---
>  3 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
> index e65afb7523f..fe735756f72 100644
> --- a/gdb/debuginfod-support.c
> +++ b/gdb/debuginfod-support.c
> @@ -297,6 +297,14 @@ print_outcome (int fd, const char *desc, const char *fname)
>  		  desc,
>  		  styled_string (file_name_style.style (), fname));
>      }
> +  if (fd >= 0 && debuginfod_verbose >= 2)
> +    {
> +      ui_file *outstream = get_unbuffered (gdb_stdout);
> +      gdb_printf (outstream,
> +		  _("Download succeeded: %s %ps.\n"),
> +		  desc,
> +		  styled_string (file_name_style.style (), fname));
> +    }
>  }
>  
>  /* See debuginfod-support.h  */
> diff --git a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> index d63957ac05c..1f80dee9e33 100644
> --- a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> +++ b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> @@ -156,6 +156,8 @@ proc_with_prefix local_debuginfod { } {
>  	[multi_line \
>  	     "Reading symbols from ${build_id_debug_file}\\.\\.\\." \
>  	     "Downloading separate debug info for ${build_id_debug_file}\\.\\.\\." \
> +	     [string_to_regexp \
> +		  "Download succeeded: separate debug info for ${build_id_debug_file}."] \
>  	     "Reading symbols from ${cache}/\[^\r\n\]+\\.\\.\\.(?:\r\nExpanding full symbols from \[^\r\n\]+)*"] \
>  	"debuginfod running, info downloaded, no warnings"
>  }
> diff --git a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
> index 6dcabf60357..8391e8f4a01 100644
> --- a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
> +++ b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
> @@ -126,10 +126,14 @@ proc_with_prefix local_debuginfod { } {
>  
>      # CRC mismatch should not be reported now because the correct debuginfo
>      # should be fetched from debuginfod.
> -    gdb_test "file [standard_output_file crc_mismatch-2]" \
> +    set binfile2 [standard_output_file crc_mismatch-2]
> +    gdb_test "file $binfile2" \
>  	[multi_line \
> -	     "Reading symbols from ${escapedobjdirsubdir}/crc_mismatch-2\\.\\.\\." \
> -	     "Downloading.*separate debug info for ${escapedobjdirsubdir}/crc_mismatch-2\\.\\.\\." \
> +	     [string_to_regexp "Reading symbols from $binfile2..."] \
> +	     [string_to_regexp \
> +		  "Downloading separate debug info for $binfile2..."] \
> +	     [string_to_regexp \
> +		  "Download succeeded: separate debug info for $binfile2."] \
>  	     "Reading symbols from ${cache}/\[^\r\n\]+\\.\\.\\.(?:\r\nExpanding full symbols from \[^\r\n\]+)*"] \
>  	 "debuginfod running, info downloaded, no CRC mismatch"
>  }
> -- 
> 2.35.3