public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
Subject: Re: [PATCH v2] gdb: defer warnings when loading separate debug files
Date: Thu, 19 Jan 2023 11:38:41 +0000	[thread overview]
Message-ID: <87o7queou6.fsf@redhat.com> (raw)
In-Reply-To: <20230103130324.1329734-1-ahajkova@redhat.com>

AlexandraHájková@sourceware.org writes:

> From: Alexandra Hájková <ahajkova@redhat.com>
>
> Currently, when GDB loads debug information from a separate debug file,
> there are a couple of warnings that could be produced if things go
> wrong.
>
> In find_separate_debug_file_by_buildid (build-id.c) GDB can give a
> warning if the separate debug file doesn't include any actual debug
> information, and in separate_debug_file_exists (symfile.c) we can warn
> if the CRC checksum in the separate debug file doesn't match the
> checksum in the original executable.
>
> The problem here is that, when looking up debug information, GDB will
> try several different approaches, lookup by build-id, lookup by
> debug-link, and then a lookup from debuginfod.  GDB can potentially give
> a warning from an earlier attempt, and then succeed with a later
> attempt.  In the cases I have run into this is primarily a warning about
> some out of date debug information on my machine, but then GDB finds the
> correct information using debuginfod.  This can be confusing to a user,
> they will see warnings from GDB when really everything is working just
> fine.
>
> For example:
> warning: the debug information found in "/usr/lib/debug//lib64/ld-2.32.so.debug"
> does not match "/lib64/ld-linux-x86-64.so.2" (CRC mismatch).
>
> This diagnostic was printed on Fedora 33 even when the correct debuginfo
> was downloaded.
>
> In this patch I propose that we defer any warnings related to looking up
> debug information from a separate debug file.  If any of the approaches
> are successful then GDB will not print any of the warnings.  As far as
> the user is concerned, everything "just worked".  Only if GDB completely
> fails to find any suitable debug information will the warnings be
> printed.
>
> The crc_mismatch test compiles two executables: crc_mismatch and crc_mismatch-2
> and then strips them of debuginfo creating separate debug files. The test then
> replaces crc_mismatch-2.debug with crc_mismatch.debug to trigger "CRC mismatch"
> warning. A local debuginfod server is setup to supply the correct debug file,
> now when GDB looks up the debug info no warning is given.
>
> The build-id-no-debug-warning.exp is similar to the previous test. It triggers
> the "separate debug info file has no debug info" warning by replacing
> the build-id based .debug file with the stripped binary and then loading it to gdb.
> It then also sets up local debuginfod server with the correct debug file to download
> to make sure no warnings are emmitted.
> ---
> Version 2 addresses Simon's concern advanced users/developers wants to be notified
> about problems along the way even if gdb fetches the correct debuginfo eventually.
> Print the warning immediatly if separate_debug_file_debug is set.
>
>  gdb/build-id.c                                |  16 +-
>  gdb/build-id.h                                |  10 +-
>  gdb/coffread.c                                |  12 +-
>  gdb/elfread.c                                 |  12 +-
>  gdb/symfile.c                                 |  59 +++++---
>  gdb/symfile.h                                 |   9 +-
>  .../build-id-no-debug-warning.c               |  20 +++
>  .../build-id-no-debug-warning.exp             | 142 ++++++++++++++++++
>  gdb/testsuite/gdb.debuginfod/crc_mismatch-2.c |  22 +++
>  gdb/testsuite/gdb.debuginfod/crc_mismatch.c   |  20 +++
>  gdb/testsuite/gdb.debuginfod/crc_mismatch.exp | 114 ++++++++++++++
>  gdb/testsuite/lib/gdb.exp                     |  38 +++--
>  12 files changed, 427 insertions(+), 47 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.c
>  create mode 100644 gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
>  create mode 100644 gdb/testsuite/gdb.debuginfod/crc_mismatch-2.c
>  create mode 100644 gdb/testsuite/gdb.debuginfod/crc_mismatch.c
>  create mode 100644 gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
>
> diff --git a/gdb/build-id.c b/gdb/build-id.c
> index c82f96402c8..f13ef52eaf3 100644
> --- a/gdb/build-id.c
> +++ b/gdb/build-id.c
> @@ -202,7 +202,8 @@ build_id_to_exec_bfd (size_t build_id_len, const bfd_byte *build_id)
>  /* See build-id.h.  */
>  
>  std::string
> -find_separate_debug_file_by_buildid (struct objfile *objfile)
> +find_separate_debug_file_by_buildid (struct objfile *objfile,
> +				     std::vector<std::string> *warnings_vector)
>  {
>    const struct bfd_build_id *build_id;
>  
> @@ -219,9 +220,16 @@ find_separate_debug_file_by_buildid (struct objfile *objfile)
>        /* Prevent looping on a stripped .debug file.  */
>        if (abfd != NULL
>  	  && filename_cmp (bfd_get_filename (abfd.get ()),
> -			   objfile_name (objfile)) == 0)
> -	warning (_("\"%s\": separate debug info file has no debug info"),
> -		 bfd_get_filename (abfd.get ()));
> +			   objfile_name (objfile)) == 0) {
> +          if (separate_debug_file_debug)
> +              gdb_printf (gdb_stdlog,
> +                          (_( "\"%s\": separate debug info file has no debug info"),
> +                          bfd_get_filename (abfd.get ())));
> +          else warnings_vector->emplace_back
> +              (string_printf
> +               (_("\"%s\": separate debug info file has no debug info"),
> +                bfd_get_filename (abfd.get ())));
> +      }
>        else if (abfd != NULL)
>  	return std::string (bfd_get_filename (abfd.get ()));
>      }
> diff --git a/gdb/build-id.h b/gdb/build-id.h
> index 4c3f8e0479a..191720ddf28 100644
> --- a/gdb/build-id.h
> +++ b/gdb/build-id.h
> @@ -49,10 +49,16 @@ extern gdb_bfd_ref_ptr build_id_to_exec_bfd (size_t build_id_len,
>  
>  /* Find the separate debug file for OBJFILE, by using the build-id
>     associated with OBJFILE's BFD.  If successful, returns the file name for the
> -   separate debug file, otherwise, return an empty string.  */
> +   separate debug file, otherwise, return an empty string.
> +
> +   Any warnings that are generated by the lookup process should be added to
> +   WARNINGS_VECTOR, one std::string per warning.  If some other mechanism can
> +   be used to lookup the debug information then the warning will not be shown,
> +   however, if GDB fails to find suitable debug information using any
> +   approach, then any warnings will be printed.  */
>  
>  extern std::string find_separate_debug_file_by_buildid
> -  (struct objfile *objfile);
> +  (struct objfile *objfile, std::vector<std::string> *warnings_vector);
>  
>  /* Return an hex-string representation of BUILD_ID.  */
>  
> diff --git a/gdb/coffread.c b/gdb/coffread.c
> index 4950a7324c8..8f2a8673e10 100644
> --- a/gdb/coffread.c
> +++ b/gdb/coffread.c
> @@ -734,10 +734,13 @@ coff_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
>    /* Try to add separate debug file if no symbols table found.   */
>    if (!objfile->has_partial_symbols ())
>      {
> -      std::string debugfile = find_separate_debug_file_by_buildid (objfile);
> +      std::vector<std::string> warnings_vector;
> +      std::string debugfile
> +	= find_separate_debug_file_by_buildid (objfile, &warnings_vector);
>  
>        if (debugfile.empty ())
> -	debugfile = find_separate_debug_file_by_debuglink (objfile);
> +	debugfile
> +	  = find_separate_debug_file_by_debuglink (objfile, &warnings_vector);
>  
>        if (!debugfile.empty ())
>  	{
> @@ -746,6 +749,11 @@ coff_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
>  	  symbol_file_add_separate (debug_bfd, debugfile.c_str (),
>  				    symfile_flags, objfile);
>  	}
> +      /* If all the methods to collect the debuginfo failed, print any
> +	 warnings that were collected.  */
> +      if (debugfile.empty () && !warnings_vector.empty ())
> +	for (const std::string &w : warnings_vector)
> +	  warning ("%s", w.c_str ());
>      }
>  }
>  
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index ceaf81f0fca..0f6b64b0768 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -1194,6 +1194,7 @@ elf_symfile_read_dwarf2 (struct objfile *objfile,
>  			 symfile_add_flags symfile_flags)
>  {
>    bool has_dwarf2 = true;
> +  std::vector<std::string> warnings_vector;
>  
>    if (dwarf2_has_info (objfile, NULL, true))
>      dwarf2_initialize_objfile (objfile);
> @@ -1213,10 +1214,12 @@ elf_symfile_read_dwarf2 (struct objfile *objfile,
>  	   && objfile->separate_debug_objfile == NULL
>  	   && objfile->separate_debug_objfile_backlink == NULL)
>      {
> -      std::string debugfile = find_separate_debug_file_by_buildid (objfile);
> +      std::string debugfile
> +	= find_separate_debug_file_by_buildid (objfile, &warnings_vector);
>  
>        if (debugfile.empty ())
> -	debugfile = find_separate_debug_file_by_debuglink (objfile);
> +	debugfile = find_separate_debug_file_by_debuglink (objfile,
> +							   &warnings_vector);
>  
>        if (!debugfile.empty ())
>  	{
> @@ -1258,6 +1261,11 @@ elf_symfile_read_dwarf2 (struct objfile *objfile,
>  		}
>  	    }
>  	}
> +      /* If all the methods to collect the debuginfo failed, print
> +	 the warnings, if there're any. */
> +      if (debugfile.empty () && !has_dwarf2 && !warnings_vector.empty ())
> +	for (const std::string &w : warnings_vector)
> +	  warning ("%s", w.c_str ());
>      }
>  
>    return has_dwarf2;
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index e0942dfb22d..58016b7d3f8 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -1244,7 +1244,8 @@ bool separate_debug_file_debug = false;
>  
>  static int
>  separate_debug_file_exists (const std::string &name, unsigned long crc,
> -			    struct objfile *parent_objfile)
> +			    struct objfile *parent_objfile,
> +			    std::vector<std::string> *warnings_vector)
>  {
>    unsigned long file_crc;
>    int file_crc_p;
> @@ -1335,13 +1336,16 @@ separate_debug_file_exists (const std::string &name, unsigned long crc,
>  	    }
>  	}
>  
> -      if (verified_as_different || parent_crc != file_crc)
> -	warning (_("the debug information found in \"%s\""
> -		   " does not match \"%s\" (CRC mismatch).\n"),
> -		 name.c_str (), objfile_name (parent_objfile));
> -
> -      if (separate_debug_file_debug)
> -	gdb_printf (gdb_stdlog, _(" no, CRC doesn't match.\n"));
> +      if (verified_as_different || parent_crc != file_crc) {
> +          if (separate_debug_file_debug)
> +              gdb_printf (gdb_stdlog, _("the debug information found in \"%s\""
> +                                        " does not match \"%s\" (CRC mismatch).\n"),
> +                                      name.c_str (), objfile_name (parent_objfile));
> +          else warnings_vector->emplace_back
> +              (string_printf (_("the debug information found in \"%s\""
> +                                " does not match \"%s\" (CRC mismatch).\n"),
> +                              name.c_str (), objfile_name (parent_objfile)));

I don't think we want to do this.  We shouldn't be switching what the
user sees based on whether a debug flag is on or not.  I think what you
mean is:

  if (verified_as_different || parent_crc != file_crc)
    {
      if (separate_debug_file_debug)
        gdb_printf (gdb_stdlog, _("the debug information found in \"%s\""
                                  " does not match \"%s\" (CRC mismatch).\n"),
                                  name.c_str (), objfile_name (parent_objfile));
      warnings_vector->emplace_back
              (string_printf (_("the debug information found in \"%s\""
                                " does not match \"%s\" (CRC mismatch).\n"),
                                name.c_str (), objfile_name (parent_objfile)));
    }

Notice that the GNU style is not to use trailing '{' characters.  Also,
lines should be indented with a mix of tabs and spaces.

Thanks,
Andrew

> +      }
>  
>        return 0;
>      }
> @@ -1373,13 +1377,17 @@ show_debug_file_directory (struct ui_file *file, int from_tty,
>     looking for.  CANON_DIR is the "realpath" form of DIR.
>     DIR must contain a trailing '/'.
>     Returns the path of the file with separate debug info, or an empty
> -   string.  */
> +   string.
> +
> +   Any warnings generated as part of the lookup process are added to
> +   WARNINGS_VECTOR, one std::string per warning.  */
>  
>  static std::string
>  find_separate_debug_file (const char *dir,
>  			  const char *canon_dir,
>  			  const char *debuglink,
> -			  unsigned long crc32, struct objfile *objfile)
> +			  unsigned long crc32, struct objfile *objfile,
> +			  std::vector<std::string> *warnings_vector)
>  {
>    if (separate_debug_file_debug)
>      gdb_printf (gdb_stdlog,
> @@ -1390,7 +1398,7 @@ find_separate_debug_file (const char *dir,
>    std::string debugfile = dir;
>    debugfile += debuglink;
>  
> -  if (separate_debug_file_exists (debugfile, crc32, objfile))
> +  if (separate_debug_file_exists (debugfile, crc32, objfile, warnings_vector))
>      return debugfile;
>  
>    /* Then try in the subdirectory named DEBUG_SUBDIRECTORY.  */
> @@ -1399,7 +1407,7 @@ find_separate_debug_file (const char *dir,
>    debugfile += "/";
>    debugfile += debuglink;
>  
> -  if (separate_debug_file_exists (debugfile, crc32, objfile))
> +  if (separate_debug_file_exists (debugfile, crc32, objfile, warnings_vector))
>      return debugfile;
>  
>    /* Then try in the global debugfile directories.
> @@ -1444,7 +1452,8 @@ find_separate_debug_file (const char *dir,
>        debugfile += dir_notarget;
>        debugfile += debuglink;
>  
> -      if (separate_debug_file_exists (debugfile, crc32, objfile))
> +      if (separate_debug_file_exists (debugfile, crc32, objfile,
> +				      warnings_vector))
>  	return debugfile;
>  
>        const char *base_path = NULL;
> @@ -1466,7 +1475,8 @@ find_separate_debug_file (const char *dir,
>  	  debugfile += "/";
>  	  debugfile += debuglink;
>  
> -	  if (separate_debug_file_exists (debugfile, crc32, objfile))
> +	  if (separate_debug_file_exists (debugfile, crc32, objfile,
> +					  warnings_vector))
>  	    return debugfile;
>  
>  	  /* If the file is in the sysroot, try using its base path in
> @@ -1492,10 +1502,11 @@ find_separate_debug_file (const char *dir,
>  	      debugfile += "/";
>  	      debugfile += debuglink;
>  
> -	      if (separate_debug_file_exists (debugfile, crc32, objfile))
> -		return debugfile;
> -	    }
> -	}
> +              if (separate_debug_file_exists (debugfile, crc32, objfile,
> +                                              warnings_vector))
> +                  return debugfile;
> +            }
> +        }
>      }
>  
>    return std::string ();
> @@ -1520,11 +1531,11 @@ terminate_after_last_dir_separator (char *path)
>    path[i + 1] = '\0';
>  }
>  
> -/* Find separate debuginfo for OBJFILE (using .gnu_debuglink section).
> -   Returns pathname, or an empty string.  */
> +/* See symtab.h.  */
>  
>  std::string
> -find_separate_debug_file_by_debuglink (struct objfile *objfile)
> +find_separate_debug_file_by_debuglink
> +  (struct objfile *objfile, std::vector<std::string> *warnings_vector)
>  {
>    unsigned long crc32;
>  
> @@ -1544,7 +1555,8 @@ find_separate_debug_file_by_debuglink (struct objfile *objfile)
>  
>    std::string debugfile
>      = find_separate_debug_file (dir.c_str (), canon_dir.get (),
> -				debuglink.get (), crc32, objfile);
> +				debuglink.get (), crc32, objfile,
> +				warnings_vector);
>  
>    if (debugfile.empty ())
>      {
> @@ -1568,7 +1580,8 @@ find_separate_debug_file_by_debuglink (struct objfile *objfile)
>  							symlink_dir.get (),
>  							debuglink.get (),
>  							crc32,
> -							objfile);
> +							objfile,
> +							warnings_vector);
>  		}
>  	    }
>  	}
> diff --git a/gdb/symfile.h b/gdb/symfile.h
> index fb5fda795e3..b4d9a10e711 100644
> --- a/gdb/symfile.h
> +++ b/gdb/symfile.h
> @@ -241,7 +241,14 @@ extern struct objfile *symbol_file_add_from_bfd (const gdb_bfd_ref_ptr &,
>  extern void symbol_file_add_separate (const gdb_bfd_ref_ptr &, const char *,
>  				      symfile_add_flags, struct objfile *);
>  
> -extern std::string find_separate_debug_file_by_debuglink (struct objfile *);
> +/* Find separate debuginfo for OBJFILE (using .gnu_debuglink section).
> +   Returns pathname, or an empty string.
> +
> +   Any warnings generated as part of this lookup are added to
> +   WARNINGS_VECTOR, one std::string per warning.  */
> +
> +extern std::string find_separate_debug_file_by_debuglink
> +  (struct objfile *objfile, std::vector<std::string> *warnings_vector);
>  
>  /* Build (allocate and populate) a section_addr_info struct from an
>     existing section table.  */
> diff --git a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.c b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.c
> new file mode 100644
> index 00000000000..223a703bb4f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.c
> @@ -0,0 +1,20 @@
> +/* Copyright 2022 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +int
> +main (void)
> +{
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> new file mode 100644
> index 00000000000..b328e19f438
> --- /dev/null
> +++ b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> @@ -0,0 +1,142 @@
> +# Copyright 2022 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +# This test triggers the "separate debug info file has no debug info" warning by replacing
> +# the build-id based .debug file with the stripped binary and then loading it to gdb.
> +# It then also sets up local debuginfod server with the correct debug file to download
> +# to make sure no warnings are emmitted.
> +
> +
> +standard_testfile
> +
> +load_lib debuginfod-support.exp
> +
> +if { [skip_debuginfod_tests] } { return -1 }
> +
> +if {[build_executable "build executable" ${testfile} ${srcfile} \
> +	 {debug build-id}] == -1} {
> +    return -1
> +}
> +
> +# Split BINFILE into BINFILE.stripped and BINFILE.debug, the first is
> +# the executable with the debug information removed, and the second is
> +# the debug information.
> +#
> +# However, by passing the "no-debuglink" flag we prevent this proc
> +# from adding a .gnu_debuglink section to the executable.  Any lookup
> +# of the debug information by GDB will need to be done based on the
> +# build-id.
> +if [gdb_gnu_strip_debug $binfile no-debuglink] {
> +    unsupported "cannot produce separate debug info files"
> +    return -1
> +}
> +
> +# Get the .build-id/PREFIX/SUFFIX.debug file name, and convert it to
> +# an absolute path, this is where we will place the debug information.
> +set build_id_debug_file \
> +    [standard_output_file [build_id_debug_filename_get $binfile]]
> +
> +# Get the BINFILE.debug filename.  This is the file we should be
> +# moving to the BUILD_ID_DEBUG_FILE location, but we wont, we're going
> +# to move something else there instead.
> +set debugfile [standard_output_file "${binfile}.debug"]
> +
> +# Move debugfile to the directory to be used by the debuginfod
> +# server.
> +set debuginfod_debugdir [standard_output_file "debug"]
> +remote_exec build "mkdir $debuginfod_debugdir"
> +remote_exec build "mv $debugfile $debuginfod_debugdir"
> +
> +# This is BINFILE with the debug information removed.  We are going to
> +# place this in the BUILD_ID_DEBUG_FILE location, this would usually
> +# represent a mistake by the user, and will trigger a warning from
> +# GDB, this is the warning we are checking for.
> +set stripped_binfile [standard_output_file "${binfile}.stripped"]
> +
> +# Create the .build-id/PREFIX directory name from
> +# .build-id/PREFIX/SUFFIX.debug filename.
> +set debugdir [file dirname ${build_id_debug_file}]
> +remote_exec build "mkdir -p $debugdir"
> +
> +# Now move the stripped executable into the .build-id directory
> +# instead of the debug information.  Later on we're going to try and
> +# load this into GDB.  GDB will then try to find the separate debug
> +# information, which will point back at this file, which also doesn't
> +# have debug information, which could cause a loop.  But GDB will spot
> +# this and give a warning.
> +remote_exec build "mv ${stripped_binfile} ${build_id_debug_file}"
> +
> +# Now start GDB.
> +clean_restart
> +
> +# Tell GDB where to look for the .build-id directory.
> +set debug_file_directory [standard_output_file ""]
> +gdb_test_no_output "set debug-file-directory ${debug_file_directory}" \
> +    "set debug-file-directory"
> +
> +# Now load the file into GDB, and look for the warning.
> +gdb_test "file ${build_id_debug_file}" \
> +    [multi_line \
> +    ".*Reading symbols from.*debuginfo.*" \
> +    ".*separate debug info file has no debug info.*"] \
> +    "load test file, expect a warning"
> +
> +# Now we should close GDB.
> +gdb_exit
> +
> +# Create CACHE and DB directories ready for debuginfod to use.
> +prepare_for_debuginfod cache db
> +
> +# Start debuginfod server and test debuginfo is downloaded from
> +# it and we can se no warnings anymore.
> +proc_with_prefix local_debuginfod { } {
> +    global db debuginfod_debugdir cache build_id_debug_file
> +
> +    set url [start_debuginfod $db $debuginfod_debugdir]
> +    if { $url == "" } {
> +	unresolved "failed to start debuginfod server"
> +	return
> +    }
> +
> +    # Point the client to the server.
> +    setenv DEBUGINFOD_URLS $url
> +
> +    # GDB should now find the symbol and source files.
> +    clean_restart
> +
> +    # Enable debuginfod and fetch the debuginfo.
> +    gdb_test_no_output "set debuginfod enabled on"
> +
> +    # "separate debug info file has no debug info" warning should not be
> +    # reported now because the correct debuginfo should be fetched from
> +    # debuginfod.
> +    gdb_test "file ${build_id_debug_file}" \
> +	[multi_line \
> +	     "Reading symbols from ${build_id_debug_file}\\.\\.\\." \
> +	     "Downloading 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"
> +}
> +
> +# Restart GDB, and load the file, this time we should correctly get
> +# the debug symbols from the server, and should not see the warning.
> +with_debuginfod_env $cache {
> +    local_debuginfod
> +}
> +
> +stop_debuginfod
> +# Spare debug files may confuse testsuite runs in the future.
> +remote_exec build "rm -f $debugfile"
> +
> diff --git a/gdb/testsuite/gdb.debuginfod/crc_mismatch-2.c b/gdb/testsuite/gdb.debuginfod/crc_mismatch-2.c
> new file mode 100644
> index 00000000000..6b3984dc7d2
> --- /dev/null
> +++ b/gdb/testsuite/gdb.debuginfod/crc_mismatch-2.c
> @@ -0,0 +1,22 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2022 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +int
> +main (void)
> +{
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.debuginfod/crc_mismatch.c b/gdb/testsuite/gdb.debuginfod/crc_mismatch.c
> new file mode 100644
> index 00000000000..1801a9c953a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.debuginfod/crc_mismatch.c
> @@ -0,0 +1,20 @@
> +/* Copyright 2022 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +int
> +main (int argc, char **argv, char **envp)
> +{
> +  return argc;
> +}
> diff --git a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
> new file mode 100644
> index 00000000000..76cc771814b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
> @@ -0,0 +1,114 @@
> +# Copyright 2022 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +# This test compiles two executables: crc_mismatch and crc_mismatch-2
> +# and then strips them of debuginfo creating separate debug files. The test
> +# then replaces crc_mismatch-2.debug with crc_mismatch.debug to trigger
> +# "CRC mismatch" warning. A local debuginfod server is setup to supply
> +# the correct debug file, now when GDB looks up the debug info no warning
> +# is given.
> +
> +standard_testfile .c -2.c
> +
> +load_lib debuginfod-support.exp
> +
> +if { [skip_debuginfod_tests] } { return -1 }
> +
> +if  {[build_executable "build executable" $testfile $srcfile debug] == -1 } {
> +    untested "failed to compile"
> +    return -1
> +}
> +
> +# The procedure gdb_gnu_strip_debug will produce an executable called
> +# ${binfile}, which is just like the executable ($binfile) but without
> +# the debuginfo. Instead $binfile has a .gnu_debuglink section which
> +# contains the name of a debuginfo only file.
> +if [gdb_gnu_strip_debug $binfile] {
> +    # Check that you have a recent version of strip and objcopy installed.
> +    unsupported "cannot produce separate debug info files"
> +    return -1
> +}
> +
> +set debugfile "$[standard_output_file ${testfile}.debug]"
> +set debugdir [standard_output_file "debug"]
> +remote_exec build "mkdir $debugdir"
> +remote_exec build "mkdir -p [file dirname $debugfile]"
> +remote_exec build "mv -f [standard_output_file ${testfile}.debug] $debugfile"
> +
> +# Test CRC mismatch is reported.
> +if {[build_executable crc_mismatch.exp crc_mismatch-2 crc_mismatch-2.c debug] != -1
> +    && ![gdb_gnu_strip_debug [standard_output_file crc_mismatch-2]]} {
> +
> +    # Copy the correct debug file for crc_mismatch-2 to the debugdir
> +    # which is going to be used by local debuginfod.
> +    remote_exec build "cp [standard_output_file crc_mismatch-2.debug] ${debugdir}"
> +    # Move the unmatching debug file for crc_mismatch-2 instead of its real one
> +    # to trigger the "CRC mismatch" warning.
> +    remote_exec build "mv ${debugfile} [standard_output_file crc_mismatch-2.debug]"
> +
> +    gdb_exit
> +    gdb_start
> +
> +    set escapedobjdirsubdir [string_to_regexp [standard_output_file {}]]
> +
> +    gdb_test "file [standard_output_file crc_mismatch-2]" "warning: the debug information found in \"${escapedobjdirsubdir}/crc_mismatch-2\\.debug\" does not match \"${escapedobjdirsubdir}/crc_mismatch-2\" \\(CRC mismatch\\)\\..*\\(No debugging symbols found in .*\\).*" "CRC mismatch is reported"
> +}
> +
> +# Create CACHE and DB directories ready for debuginfod to use.
> +prepare_for_debuginfod cache db
> +
> +# Start debuginfod server, test the correct debuginfo was fetched
> +# from the server so there're not warnings anymore.
> +proc_with_prefix local_debuginfod { } {
> +    global binfile db debugdir cache
> +    set escapedobjdirsubdir [string_to_regexp [standard_output_file {}]]
> +
> +    set url [start_debuginfod $db $debugdir]
> +    if { $url == "" } {
> +	unresolved "failed to start debuginfod server"
> +	return
> +    }
> +
> +    # Point the client to the server.
> +    setenv DEBUGINFOD_URLS $url
> +
> +    # GDB should now find the symbol and source files.
> +    clean_restart
> +
> +    # Enable debuginfod and fetch the debuginfo.
> +    gdb_test_no_output "set debuginfod enabled on"
> +    gdb_test "file $binfile" ".*Reading symbols from.*debuginfo.*" \
> +	"file [file tail $binfile] cmd on"
> +
> +    # 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]" \
> +	[multi_line \
> +	     "Reading symbols from ${escapedobjdirsubdir}/crc_mismatch-2\\.\\.\\." \
> +	     "Downloading separate debug info for ${escapedobjdirsubdir}/crc_mismatch-2\\.\\.\\." \
> +	     "Reading symbols from ${cache}/\[^\r\n\]+\\.\\.\\.(?:\r\nExpanding full symbols from \[^\r\n\]+)*"] \
> +	 "debuginfod running, info downloaded, no CRC mismatch"
> +
> +    #
> +
> +}
> +
> +with_debuginfod_env $cache {
> +    local_debuginfod
> +}
> +
> +stop_debuginfod
> +# Spare debug files may confuse testsuite runs in the future.
> +remote_exec build "rm -f $debugfile"
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index f94ec1b77f3..82bbf7513c4 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -7153,9 +7153,19 @@ proc build_id_debug_filename_get { filename } {
>      return ".build-id/${data}.debug"
>  }
>  
> -# Create stripped files for DEST, replacing it.  If ARGS is passed, it is a
> -# list of optional flags.  The only currently supported flag is no-main,
> -# which removes the symbol entry for main from the separate debug file.
> +# DEST should be a file compiled with debug information.  This proc
> +# creates two new files DEST.debug which contains the debug
> +# information extracted from DEST, and DEST.stripped, which is a copy
> +# of DEST with the debug information removed.  A '.gnu_debuglink'
> +# section will be added to DEST.stripped that points to DEST.debug.
> +#
> +# If ARGS is passed, it is a list of optional flags.  The currently
> +# supported flags are:
> +#
> +#   - no-main : remove the symbol entry for main from the separate
> +#               debug file DEST.debug,
> +#   - no-debuglink : don't add the '.gnu_debuglink' section to
> +#                    DEST.stripped.
>  #
>  # Function returns zero on success.  Function will return non-zero failure code
>  # on some targets not supporting separate debug info (such as i386-msdos).
> @@ -7200,7 +7210,7 @@ proc gdb_gnu_strip_debug { dest args } {
>      # leaves the symtab in the original file only.  There's no way to get
>      # objcopy or strip to remove the symbol table without also removing the
>      # debugging sections, so this is as close as we can get.
> -    if { [llength $args] == 1 && [lindex $args 0] == "no-main" } {
> +    if {[lsearch -exact $args "no-main"] != -1} {
>  	set result [catch "exec $objcopy_program -N main ${debug_file} ${debug_file}-tmp" output]
>  	verbose "result is $result"
>  	verbose "output is $output"
> @@ -7211,15 +7221,17 @@ proc gdb_gnu_strip_debug { dest args } {
>  	file rename "${debug_file}-tmp" "${debug_file}"
>      }
>  
> -    # Link the two previous output files together, adding the .gnu_debuglink
> -    # section to the stripped_file, containing a pointer to the debug_file,
> -    # save the new file in dest.
> -    # This will be the regular executable filename, in the usual location.
> -    set result [catch "exec $objcopy_program --add-gnu-debuglink=${debug_file} ${stripped_file} ${dest}" output]
> -    verbose "result is $result"
> -    verbose "output is $output"
> -    if {$result == 1} {
> -      return 1
> +    # Unless the "no-debuglink" flag is passed, then link the two
> +    # previous output files together, adding the .gnu_debuglink
> +    # section to the stripped_file, containing a pointer to the
> +    # debug_file, save the new file in dest.
> +    if {[lsearch -exact $args "no-debuglink"] == -1} {
> +	set result [catch "exec $objcopy_program --add-gnu-debuglink=${debug_file} ${stripped_file} ${dest}" output]
> +	verbose "result is $result"
> +	verbose "output is $output"
> +	if {$result == 1} {
> +	    return 1
> +	}
>      }
>  
>      # Workaround PR binutils/10802:
> -- 
> 2.38.1


      parent reply	other threads:[~2023-01-19 11:38 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-03 13:03 
2023-01-09 12:56 ` Alexandra Petlanova Hajkova
2023-01-19 11:21   ` Alexandra Petlanova Hajkova
2023-01-19 11:38 ` Andrew Burgess [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87o7queou6.fsf@redhat.com \
    --to=aburgess@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).