[PATCH v2] Use styled_string when defering warnings when loading separate debug files

[PATCH v2] Use styled_string when defering warnings when loading separate debug files

[Alexandra Hájková]

This improves commit 6647f05df023b63bbe056e9167e9e234172fa2ca, the
filenames used in the warnings are styled properly now.
---
This version uses ui_file rather instead of vector of callback functions that emit warnings.
Addresses various cosmetic issues.

 gdb/build-id.c | 16 ++++++++++------
 gdb/build-id.h |  4 ++--
 gdb/coffread.c |  4 ++--
 gdb/elfread.c  |  4 ++--
 gdb/symfile.c  | 26 ++++++++++++++++----------
 gdb/symfile.h  |  4 ++--
 gdb/ui-file.h  |  7 +++++++
 7 files changed, 41 insertions(+), 24 deletions(-)

diff --git a/gdb/build-id.c b/gdb/build-id.c
index 00250c20ae9..5d5114e2ace 100644
--- a/gdb/build-id.c
+++ b/gdb/build-id.c
@@ -18,6 +18,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "cli/cli-style.h"
 #include "bfd.h"
 #include "gdb_bfd.h"
 #include "build-id.h"
@@ -209,7 +210,7 @@ build_id_to_exec_bfd (size_t build_id_len, const bfd_byte *build_id)
 
 std::string
 find_separate_debug_file_by_buildid (struct objfile *objfile,
-				     std::vector<std::string> *warnings_vector)
+				     std::vector<string_file> *warnings_vector)
 {
   const struct bfd_build_id *build_id;
 
@@ -228,12 +229,15 @@ find_separate_debug_file_by_buildid (struct objfile *objfile,
 	  && filename_cmp (bfd_get_filename (abfd.get ()),
 			   objfile_name (objfile)) == 0)
 	{
-	  std::string msg
-	    = string_printf (_("\"%s\": separate debug info file has no "
-			       "debug info"), bfd_get_filename (abfd.get ()));
+	  string_file warnings (true);
+	  warnings.printf ("\"%ps\":separate debug info file has no debug info",
+			   styled_string (file_name_style.style (),
+					  bfd_get_filename (abfd.get ())));
 	  if (separate_debug_file_debug)
-	    gdb_printf (gdb_stdlog, "%s", msg.c_str ());
-	  warnings_vector->emplace_back (std::move (msg));
+	    /* Avoid styling for the debug output.  */
+	    gdb_printf (gdb_stdlog, _("%s: separate debug info file has no debug info"),
+			bfd_get_filename (abfd.get ()));
+	  warnings_vector->emplace_back (std::move (warnings));
 	}
       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 191720ddf28..0c2c4181b4e 100644
--- a/gdb/build-id.h
+++ b/gdb/build-id.h
@@ -52,13 +52,13 @@ extern gdb_bfd_ref_ptr build_id_to_exec_bfd (size_t build_id_len,
    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
+   WARNINGS_VECTOR, one string_file 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, std::vector<std::string> *warnings_vector);
+  (struct objfile *objfile, std::vector<string_file> *warnings_vector);
 
 /* Return an hex-string representation of BUILD_ID.  */
 
diff --git a/gdb/coffread.c b/gdb/coffread.c
index e993b17db09..32705c215fd 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -729,7 +729,7 @@ 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::vector<std::string> warnings_vector;
+      std::vector<string_file> warnings_vector;
       std::string debugfile
 	= find_separate_debug_file_by_buildid (objfile, &warnings_vector);
 
@@ -747,7 +747,7 @@ coff_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
       /* 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)
+	for (const string_file &w : warnings_vector)
 	  warning ("%s", w.c_str ());
     }
 }
diff --git a/gdb/elfread.c b/gdb/elfread.c
index b414da9ed21..4bc89d09d65 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1217,7 +1217,7 @@ elf_symfile_read_dwarf2 (struct objfile *objfile,
 	   && objfile->separate_debug_objfile == NULL
 	   && objfile->separate_debug_objfile_backlink == NULL)
     {
-      std::vector<std::string> warnings_vector;
+      std::vector<string_file> warnings_vector;
 
       std::string debugfile
 	= find_separate_debug_file_by_buildid (objfile, &warnings_vector);
@@ -1269,7 +1269,7 @@ 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)
+	for (const string_file &w : warnings_vector)
 	  warning ("%s", w.c_str ());
     }
 
diff --git a/gdb/symfile.c b/gdb/symfile.c
index bb9981a4634..64d29ea71f9 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1244,7 +1244,7 @@ bool separate_debug_file_debug = false;
 static int
 separate_debug_file_exists (const std::string &name, unsigned long crc,
 			    struct objfile *parent_objfile,
-			    std::vector<std::string> *warnings_vector)
+			    std::vector<string_file> *warnings_vector)
 {
   unsigned long file_crc;
   int file_crc_p;
@@ -1337,13 +1337,19 @@ separate_debug_file_exists (const std::string &name, unsigned long crc,
 
       if (verified_as_different || parent_crc != file_crc)
 	{
-	  std::string msg
-	    = string_printf (_("the debug information found in \"%s\""
-			       " does not match \"%s\" (CRC mismatch).\n"),
-			     name.c_str (), objfile_name (parent_objfile));
+	  string_file warnings (true);
+	  warnings.printf (_("the debug information found in \"%ps\""
+			     " does not match \"%ps\" (CRC mismatch)."),
+			   styled_string (file_name_style.style (),
+					  name.c_str ()),
+			   styled_string (file_name_style.style (),
+					  objfile_name (parent_objfile)));
+	  /* Avoid styling for the debug output.  */
 	  if (separate_debug_file_debug)
-	    gdb_printf (gdb_stdlog, "%s", msg.c_str ());
-	  warnings_vector->emplace_back (std::move (msg));
+	    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 (std::move (warnings));
 	}
 
       return 0;
@@ -1379,14 +1385,14 @@ show_debug_file_directory (struct ui_file *file, int from_tty,
    string.
 
    Any warnings generated as part of the lookup process are added to
-   WARNINGS_VECTOR, one std::string per warning.  */
+   WARNINGS_VECTOR, one string_file 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,
-			  std::vector<std::string> *warnings_vector)
+			  std::vector<string_file> *warnings_vector)
 {
   if (separate_debug_file_debug)
     gdb_printf (gdb_stdlog,
@@ -1534,7 +1540,7 @@ terminate_after_last_dir_separator (char *path)
 
 std::string
 find_separate_debug_file_by_debuglink
-  (struct objfile *objfile, std::vector<std::string> *warnings_vector)
+  (struct objfile *objfile, std::vector<string_file> *warnings_vector)
 {
   unsigned long crc32;
 
diff --git a/gdb/symfile.h b/gdb/symfile.h
index b433e2be31a..041f6f74f9c 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -245,10 +245,10 @@ extern void symbol_file_add_separate (const gdb_bfd_ref_ptr &, const char *,
    Returns pathname, or an empty string.
 
    Any warnings generated as part of this lookup are added to
-   WARNINGS_VECTOR, one std::string per warning.  */
+   WARNINGS_VECTOR, one string_file per warning.  */
 
 extern std::string find_separate_debug_file_by_debuglink
-  (struct objfile *objfile, std::vector<std::string> *warnings_vector);
+  (struct objfile *objfile, std::vector<string_file> *warnings_vector);
 
 /* Build (allocate and populate) a section_addr_info struct from an
    existing section table.  */
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index de24620e247..4444c8adcdb 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -212,6 +212,13 @@ class string_file : public ui_file
     return *this;
   }
 
+  string_file(string_file &&other)
+    : m_string (std::move (other.c_str ())),
+    m_term_out (other.m_term_out)
+  {
+
+  }
+
   /* Provide a few convenience methods with the same API as the
      underlying std::string.  */
   const char *data () const { return m_string.data (); }
-- 
2.39.1

Re: [PATCH v2] Use styled_string when defering warnings when loading separate debug files

[Alexandra Petlanova Hajkova]

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

Ping

On Mon, Mar 20, 2023 at 3:56 PM Alexandra Hájková via Gdb-patches <
gdb-patches@sourceware.org> wrote:

> This improves commit 6647f05df023b63bbe056e9167e9e234172fa2ca, the
> filenames used in the warnings are styled properly now.
> ---
> This version uses ui_file rather instead of vector of callback functions
> that emit warnings.
> Addresses various cosmetic issues.
>
>  gdb/build-id.c | 16 ++++++++++------
>  gdb/build-id.h |  4 ++--
>  gdb/coffread.c |  4 ++--
>  gdb/elfread.c  |  4 ++--
>  gdb/symfile.c  | 26 ++++++++++++++++----------
>  gdb/symfile.h  |  4 ++--
>  gdb/ui-file.h  |  7 +++++++
>  7 files changed, 41 insertions(+), 24 deletions(-)
>
> diff --git a/gdb/build-id.c b/gdb/build-id.c
> index 00250c20ae9..5d5114e2ace 100644
> --- a/gdb/build-id.c
> +++ b/gdb/build-id.c
> @@ -18,6 +18,7 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.
> */
>
>  #include "defs.h"
> +#include "cli/cli-style.h"
>  #include "bfd.h"
>  #include "gdb_bfd.h"
>  #include "build-id.h"
> @@ -209,7 +210,7 @@ build_id_to_exec_bfd (size_t build_id_len, const
> bfd_byte *build_id)
>
>  std::string
>  find_separate_debug_file_by_buildid (struct objfile *objfile,
> -                                    std::vector<std::string>
> *warnings_vector)
> +                                    std::vector<string_file>
> *warnings_vector)
>  {
>    const struct bfd_build_id *build_id;
>
> @@ -228,12 +229,15 @@ find_separate_debug_file_by_buildid (struct objfile
> *objfile,
>           && filename_cmp (bfd_get_filename (abfd.get ()),
>                            objfile_name (objfile)) == 0)
>         {
> -         std::string msg
> -           = string_printf (_("\"%s\": separate debug info file has no "
> -                              "debug info"), bfd_get_filename (abfd.get
> ()));
> +         string_file warnings (true);
> +         warnings.printf ("\"%ps\":separate debug info file has no debug
> info",
> +                          styled_string (file_name_style.style (),
> +                                         bfd_get_filename (abfd.get ())));
>           if (separate_debug_file_debug)
> -           gdb_printf (gdb_stdlog, "%s", msg.c_str ());
> -         warnings_vector->emplace_back (std::move (msg));
> +           /* Avoid styling for the debug output.  */
> +           gdb_printf (gdb_stdlog, _("%s: separate debug info file has no
> debug info"),
> +                       bfd_get_filename (abfd.get ()));
> +         warnings_vector->emplace_back (std::move (warnings));
>         }
>        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 191720ddf28..0c2c4181b4e 100644
> --- a/gdb/build-id.h
> +++ b/gdb/build-id.h
> @@ -52,13 +52,13 @@ extern gdb_bfd_ref_ptr build_id_to_exec_bfd (size_t
> build_id_len,
>     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
> +   WARNINGS_VECTOR, one string_file 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, std::vector<std::string> *warnings_vector);
> +  (struct objfile *objfile, std::vector<string_file> *warnings_vector);
>
>  /* Return an hex-string representation of BUILD_ID.  */
>
> diff --git a/gdb/coffread.c b/gdb/coffread.c
> index e993b17db09..32705c215fd 100644
> --- a/gdb/coffread.c
> +++ b/gdb/coffread.c
> @@ -729,7 +729,7 @@ 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::vector<std::string> warnings_vector;
> +      std::vector<string_file> warnings_vector;
>        std::string debugfile
>         = find_separate_debug_file_by_buildid (objfile, &warnings_vector);
>
> @@ -747,7 +747,7 @@ coff_symfile_read (struct objfile *objfile,
> symfile_add_flags symfile_flags)
>        /* 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)
> +       for (const string_file &w : warnings_vector)
>           warning ("%s", w.c_str ());
>      }
>  }
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index b414da9ed21..4bc89d09d65 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -1217,7 +1217,7 @@ elf_symfile_read_dwarf2 (struct objfile *objfile,
>            && objfile->separate_debug_objfile == NULL
>            && objfile->separate_debug_objfile_backlink == NULL)
>      {
> -      std::vector<std::string> warnings_vector;
> +      std::vector<string_file> warnings_vector;
>
>        std::string debugfile
>         = find_separate_debug_file_by_buildid (objfile, &warnings_vector);
> @@ -1269,7 +1269,7 @@ 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)
> +       for (const string_file &w : warnings_vector)
>           warning ("%s", w.c_str ());
>      }
>
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index bb9981a4634..64d29ea71f9 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -1244,7 +1244,7 @@ bool separate_debug_file_debug = false;
>  static int
>  separate_debug_file_exists (const std::string &name, unsigned long crc,
>                             struct objfile *parent_objfile,
> -                           std::vector<std::string> *warnings_vector)
> +                           std::vector<string_file> *warnings_vector)
>  {
>    unsigned long file_crc;
>    int file_crc_p;
> @@ -1337,13 +1337,19 @@ separate_debug_file_exists (const std::string
> &name, unsigned long crc,
>
>        if (verified_as_different || parent_crc != file_crc)
>         {
> -         std::string msg
> -           = string_printf (_("the debug information found in \"%s\""
> -                              " does not match \"%s\" (CRC mismatch).\n"),
> -                            name.c_str (), objfile_name (parent_objfile));
> +         string_file warnings (true);
> +         warnings.printf (_("the debug information found in \"%ps\""
> +                            " does not match \"%ps\" (CRC mismatch)."),
> +                          styled_string (file_name_style.style (),
> +                                         name.c_str ()),
> +                          styled_string (file_name_style.style (),
> +                                         objfile_name (parent_objfile)));
> +         /* Avoid styling for the debug output.  */
>           if (separate_debug_file_debug)
> -           gdb_printf (gdb_stdlog, "%s", msg.c_str ());
> -         warnings_vector->emplace_back (std::move (msg));
> +           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 (std::move (warnings));
>         }
>
>        return 0;
> @@ -1379,14 +1385,14 @@ show_debug_file_directory (struct ui_file *file,
> int from_tty,
>     string.
>
>     Any warnings generated as part of the lookup process are added to
> -   WARNINGS_VECTOR, one std::string per warning.  */
> +   WARNINGS_VECTOR, one string_file 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,
> -                         std::vector<std::string> *warnings_vector)
> +                         std::vector<string_file> *warnings_vector)
>  {
>    if (separate_debug_file_debug)
>      gdb_printf (gdb_stdlog,
> @@ -1534,7 +1540,7 @@ terminate_after_last_dir_separator (char *path)
>
>  std::string
>  find_separate_debug_file_by_debuglink
> -  (struct objfile *objfile, std::vector<std::string> *warnings_vector)
> +  (struct objfile *objfile, std::vector<string_file> *warnings_vector)
>  {
>    unsigned long crc32;
>
> diff --git a/gdb/symfile.h b/gdb/symfile.h
> index b433e2be31a..041f6f74f9c 100644
> --- a/gdb/symfile.h
> +++ b/gdb/symfile.h
> @@ -245,10 +245,10 @@ extern void symbol_file_add_separate (const
> gdb_bfd_ref_ptr &, const char *,
>     Returns pathname, or an empty string.
>
>     Any warnings generated as part of this lookup are added to
> -   WARNINGS_VECTOR, one std::string per warning.  */
> +   WARNINGS_VECTOR, one string_file per warning.  */
>
>  extern std::string find_separate_debug_file_by_debuglink
> -  (struct objfile *objfile, std::vector<std::string> *warnings_vector);
> +  (struct objfile *objfile, std::vector<string_file> *warnings_vector);
>
>  /* Build (allocate and populate) a section_addr_info struct from an
>     existing section table.  */
> diff --git a/gdb/ui-file.h b/gdb/ui-file.h
> index de24620e247..4444c8adcdb 100644
> --- a/gdb/ui-file.h
> +++ b/gdb/ui-file.h
> @@ -212,6 +212,13 @@ class string_file : public ui_file
>      return *this;
>    }
>
> +  string_file(string_file &&other)
> +    : m_string (std::move (other.c_str ())),
> +    m_term_out (other.m_term_out)
> +  {
> +
> +  }
> +
>    /* Provide a few convenience methods with the same API as the
>       underlying std::string.  */
>    const char *data () const { return m_string.data (); }
> --
> 2.39.1
>
>

Re: [PATCH v2] Use styled_string when defering warnings when loading separate debug files

[Alexandra Petlanova Hajkova]

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

Ping2

On Thu, Mar 30, 2023 at 10:11 AM Alexandra Petlanova Hajkova <
ahajkova@redhat.com> wrote:

> Ping
>
> On Mon, Mar 20, 2023 at 3:56 PM Alexandra Hájková via Gdb-patches <
> gdb-patches@sourceware.org> wrote:
>
>> This improves commit 6647f05df023b63bbe056e9167e9e234172fa2ca, the
>> filenames used in the warnings are styled properly now.
>> ---
>> This version uses ui_file rather instead of vector of callback functions
>> that emit warnings.
>> Addresses various cosmetic issues.
>>
>>  gdb/build-id.c | 16 ++++++++++------
>>  gdb/build-id.h |  4 ++--
>>  gdb/coffread.c |  4 ++--
>>  gdb/elfread.c  |  4 ++--
>>  gdb/symfile.c  | 26 ++++++++++++++++----------
>>  gdb/symfile.h  |  4 ++--
>>  gdb/ui-file.h  |  7 +++++++
>>  7 files changed, 41 insertions(+), 24 deletions(-)
>>
>> diff --git a/gdb/build-id.c b/gdb/build-id.c
>> index 00250c20ae9..5d5114e2ace 100644
>> --- a/gdb/build-id.c
>> +++ b/gdb/build-id.c
>> @@ -18,6 +18,7 @@
>>     along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> */
>>
>>  #include "defs.h"
>> +#include "cli/cli-style.h"
>>  #include "bfd.h"
>>  #include "gdb_bfd.h"
>>  #include "build-id.h"
>> @@ -209,7 +210,7 @@ build_id_to_exec_bfd (size_t build_id_len, const
>> bfd_byte *build_id)
>>
>>  std::string
>>  find_separate_debug_file_by_buildid (struct objfile *objfile,
>> -                                    std::vector<std::string>
>> *warnings_vector)
>> +                                    std::vector<string_file>
>> *warnings_vector)
>>  {
>>    const struct bfd_build_id *build_id;
>>
>> @@ -228,12 +229,15 @@ find_separate_debug_file_by_buildid (struct objfile
>> *objfile,
>>           && filename_cmp (bfd_get_filename (abfd.get ()),
>>                            objfile_name (objfile)) == 0)
>>         {
>> -         std::string msg
>> -           = string_printf (_("\"%s\": separate debug info file has no "
>> -                              "debug info"), bfd_get_filename (abfd.get
>> ()));
>> +         string_file warnings (true);
>> +         warnings.printf ("\"%ps\":separate debug info file has no debug
>> info",
>> +                          styled_string (file_name_style.style (),
>> +                                         bfd_get_filename (abfd.get
>> ())));
>>           if (separate_debug_file_debug)
>> -           gdb_printf (gdb_stdlog, "%s", msg.c_str ());
>> -         warnings_vector->emplace_back (std::move (msg));
>> +           /* Avoid styling for the debug output.  */
>> +           gdb_printf (gdb_stdlog, _("%s: separate debug info file has
>> no debug info"),
>> +                       bfd_get_filename (abfd.get ()));
>> +         warnings_vector->emplace_back (std::move (warnings));
>>         }
>>        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 191720ddf28..0c2c4181b4e 100644
>> --- a/gdb/build-id.h
>> +++ b/gdb/build-id.h
>> @@ -52,13 +52,13 @@ extern gdb_bfd_ref_ptr build_id_to_exec_bfd (size_t
>> build_id_len,
>>     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
>> +   WARNINGS_VECTOR, one string_file 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, std::vector<std::string> *warnings_vector);
>> +  (struct objfile *objfile, std::vector<string_file> *warnings_vector);
>>
>>  /* Return an hex-string representation of BUILD_ID.  */
>>
>> diff --git a/gdb/coffread.c b/gdb/coffread.c
>> index e993b17db09..32705c215fd 100644
>> --- a/gdb/coffread.c
>> +++ b/gdb/coffread.c
>> @@ -729,7 +729,7 @@ 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::vector<std::string> warnings_vector;
>> +      std::vector<string_file> warnings_vector;
>>        std::string debugfile
>>         = find_separate_debug_file_by_buildid (objfile, &warnings_vector);
>>
>> @@ -747,7 +747,7 @@ coff_symfile_read (struct objfile *objfile,
>> symfile_add_flags symfile_flags)
>>        /* 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)
>> +       for (const string_file &w : warnings_vector)
>>           warning ("%s", w.c_str ());
>>      }
>>  }
>> diff --git a/gdb/elfread.c b/gdb/elfread.c
>> index b414da9ed21..4bc89d09d65 100644
>> --- a/gdb/elfread.c
>> +++ b/gdb/elfread.c
>> @@ -1217,7 +1217,7 @@ elf_symfile_read_dwarf2 (struct objfile *objfile,
>>            && objfile->separate_debug_objfile == NULL
>>            && objfile->separate_debug_objfile_backlink == NULL)
>>      {
>> -      std::vector<std::string> warnings_vector;
>> +      std::vector<string_file> warnings_vector;
>>
>>        std::string debugfile
>>         = find_separate_debug_file_by_buildid (objfile, &warnings_vector);
>> @@ -1269,7 +1269,7 @@ 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)
>> +       for (const string_file &w : warnings_vector)
>>           warning ("%s", w.c_str ());
>>      }
>>
>> diff --git a/gdb/symfile.c b/gdb/symfile.c
>> index bb9981a4634..64d29ea71f9 100644
>> --- a/gdb/symfile.c
>> +++ b/gdb/symfile.c
>> @@ -1244,7 +1244,7 @@ bool separate_debug_file_debug = false;
>>  static int
>>  separate_debug_file_exists (const std::string &name, unsigned long crc,
>>                             struct objfile *parent_objfile,
>> -                           std::vector<std::string> *warnings_vector)
>> +                           std::vector<string_file> *warnings_vector)
>>  {
>>    unsigned long file_crc;
>>    int file_crc_p;
>> @@ -1337,13 +1337,19 @@ separate_debug_file_exists (const std::string
>> &name, unsigned long crc,
>>
>>        if (verified_as_different || parent_crc != file_crc)
>>         {
>> -         std::string msg
>> -           = string_printf (_("the debug information found in \"%s\""
>> -                              " does not match \"%s\" (CRC
>> mismatch).\n"),
>> -                            name.c_str (), objfile_name
>> (parent_objfile));
>> +         string_file warnings (true);
>> +         warnings.printf (_("the debug information found in \"%ps\""
>> +                            " does not match \"%ps\" (CRC mismatch)."),
>> +                          styled_string (file_name_style.style (),
>> +                                         name.c_str ()),
>> +                          styled_string (file_name_style.style (),
>> +                                         objfile_name (parent_objfile)));
>> +         /* Avoid styling for the debug output.  */
>>           if (separate_debug_file_debug)
>> -           gdb_printf (gdb_stdlog, "%s", msg.c_str ());
>> -         warnings_vector->emplace_back (std::move (msg));
>> +           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 (std::move (warnings));
>>         }
>>
>>        return 0;
>> @@ -1379,14 +1385,14 @@ show_debug_file_directory (struct ui_file *file,
>> int from_tty,
>>     string.
>>
>>     Any warnings generated as part of the lookup process are added to
>> -   WARNINGS_VECTOR, one std::string per warning.  */
>> +   WARNINGS_VECTOR, one string_file 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,
>> -                         std::vector<std::string> *warnings_vector)
>> +                         std::vector<string_file> *warnings_vector)
>>  {
>>    if (separate_debug_file_debug)
>>      gdb_printf (gdb_stdlog,
>> @@ -1534,7 +1540,7 @@ terminate_after_last_dir_separator (char *path)
>>
>>  std::string
>>  find_separate_debug_file_by_debuglink
>> -  (struct objfile *objfile, std::vector<std::string> *warnings_vector)
>> +  (struct objfile *objfile, std::vector<string_file> *warnings_vector)
>>  {
>>    unsigned long crc32;
>>
>> diff --git a/gdb/symfile.h b/gdb/symfile.h
>> index b433e2be31a..041f6f74f9c 100644
>> --- a/gdb/symfile.h
>> +++ b/gdb/symfile.h
>> @@ -245,10 +245,10 @@ extern void symbol_file_add_separate (const
>> gdb_bfd_ref_ptr &, const char *,
>>     Returns pathname, or an empty string.
>>
>>     Any warnings generated as part of this lookup are added to
>> -   WARNINGS_VECTOR, one std::string per warning.  */
>> +   WARNINGS_VECTOR, one string_file per warning.  */
>>
>>  extern std::string find_separate_debug_file_by_debuglink
>> -  (struct objfile *objfile, std::vector<std::string> *warnings_vector);
>> +  (struct objfile *objfile, std::vector<string_file> *warnings_vector);
>>
>>  /* Build (allocate and populate) a section_addr_info struct from an
>>     existing section table.  */
>> diff --git a/gdb/ui-file.h b/gdb/ui-file.h
>> index de24620e247..4444c8adcdb 100644
>> --- a/gdb/ui-file.h
>> +++ b/gdb/ui-file.h
>> @@ -212,6 +212,13 @@ class string_file : public ui_file
>>      return *this;
>>    }
>>
>> +  string_file(string_file &&other)
>> +    : m_string (std::move (other.c_str ())),
>> +    m_term_out (other.m_term_out)
>> +  {
>> +
>> +  }
>> +
>>    /* Provide a few convenience methods with the same API as the
>>       underlying std::string.  */
>>    const char *data () const { return m_string.data (); }
>> --
>> 2.39.1
>>
>>

Re: [PATCH v2] Use styled_string when defering warnings when loading separate debug files

[Tom Tromey]

>>>>> "Alexandra" == Alexandra Hájková via Gdb-patches <gdb-patches@sourceware.org> writes:

Alexandra> This version uses ui_file rather instead of vector of
Alexandra> callback functions that emit warnings.

I think instead of a std::vector<string_file>, what I meant was just use
a single string_file.

The caller can make it, using something like:

  string_file file (gdb_stderr->can_emit_style_escape ());

Alexandra> +	  string_file warnings (true);

... since I think 'true' isn't always correct.

Alexandra> +	  warnings.printf ("\"%ps\":separate debug info file has no debug info",
Alexandra> +			   styled_string (file_name_style.style (),
Alexandra> +					  bfd_get_filename (abfd.get ())));

Then this printf can either add a 'warning:' prefix like warning() does,
or we can add an overload of "warning" that takes a ui_file as its first
parameter.

Alexandra> -	for (const std::string &w : warnings_vector)
Alexandra> +	for (const string_file &w : warnings_vector)
Alexandra>  	  warning ("%s", w.c_str ());

The approach I mention seems fine because the caller ends up not doing
anything except possibly printing the strings.

Alexandra> +  string_file(string_file &&other)
Alexandra> +    : m_string (std::move (other.c_str ())),
Alexandra> +    m_term_out (other.m_term_out)
Alexandra> +  {
Alexandra> +
Alexandra> +  }

Then you wouldn't need this, either.  But for future reference I think
this is just:

  string_file (string_file &&other) = default;

thanks,
Tom

Re: [PATCH v2] Use styled_string when defering warnings when loading separate debug files

[Alexandra Petlanova Hajkova]

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

I think instead of a std::vector<string_file>, what I meant was just use
> a single string_file.
>
> The caller can make it, using something like:
>
>   string_file file (gdb_stderr->can_emit_style_escape ());
>
> Alexandra> +      string_file warnings (true);
>
> ... since I think 'true' isn't always correct.
>

When is it not true?

>
> Alexandra> +      warnings.printf ("\"%ps\":separate debug info file has
> no debug info",
> Alexandra> +                       styled_string (file_name_style.style (),
> Alexandra> +                                      bfd_get_filename
> (abfd.get ())));
>
> Then this printf can either add a 'warning:' prefix like warning() does,
> or we can add an overload of "warning" that takes a ui_file as its first
> parameter.
>

So you would prefer me to use a single string_file instead of a vector of
those. That means I'll have just a single string and I'll be appending each
new warning to it? I used a vector because I don't know how to easily split
the warnings when printing them. Do you mean I should add an overload of a
"warning" that would be capable of splitting strings? But why is such a
solution better then simply using a vector of string_file?

Thank you for the review,
Alexandra

Re: [PATCH v2] Use styled_string when defering warnings when loading separate debug files

[Tom Tromey]

Alexandra> +      string_file warnings (true);
>> 
>> ... since I think 'true' isn't always correct.
>> 

Alexandra> When is it not true?

I suspect it would be the case if output is redirected or if styling is
disabled globally.

Alexandra> So you would prefer me to use a single string_file instead of a vector of
Alexandra> those. That means I'll have just a single string and I'll be appending each
Alexandra> new warning to it? I used a vector because I don't know how to easily split
Alexandra> the warnings when printing them. Do you mean I should add an overload of a
Alexandra> "warning" that would be capable of splitting strings? But why is such a
Alexandra> solution better then simply using a vector of string_file?

If you feel very strongly about it, I will OK it.  I don't really care
that much.  It just seems to me that there's no reason to construct all
these objects -- a vector of string files -- when the goal is just to
collect some output and maybe print it.  Collecting output like this is
really what a string_file is for.

Adding an overload of warning -- it's up to you, I don't mind either
way.

Printing the output in the end is:

  string_file blah;
  ...
  if (something)
     gdb_puts (gdb_stderr, blah.str ().c_str ());

or something like that.

Tom

[PATCH] Use styled_string when defering warnings when loading separate debug files

[Alexandra Hájková]

This improves commit 6647f05df023b63bbe056e9167e9e234172fa2ca, the
filenames used in the warnings are styled properly now.
---
 gdb/build-id.c | 19 ++++++++++++++++---
 gdb/build-id.h |  5 ++++-
 gdb/coffread.c |  6 +++---
 gdb/elfread.c  |  6 +++---
 gdb/symfile.c  | 21 ++++++++++++++++-----
 gdb/symfile.h  |  5 ++++-
 6 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/gdb/build-id.c b/gdb/build-id.c
index 00250c20ae9..74605bb702c 100644
--- a/gdb/build-id.c
+++ b/gdb/build-id.c
@@ -18,6 +18,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "cli/cli-style.h"
 #include "bfd.h"
 #include "gdb_bfd.h"
 #include "build-id.h"
@@ -205,11 +206,21 @@ build_id_to_exec_bfd (size_t build_id_len, const bfd_byte *build_id)
   return build_id_to_bfd_suffix (build_id_len, build_id, "");
 }
 
+static void
+add_warning (warnings_vec *vec, std::string msg)
+{
+  vec->emplace_back ([msg] () {
+		     gdb_printf (gdb_stdlog, "%ps",
+				 styled_string (file_name_style. style (),
+						msg.c_str ()));
+		     });
+}
+
 /* See build-id.h.  */
 
 std::string
 find_separate_debug_file_by_buildid (struct objfile *objfile,
-				     std::vector<std::string> *warnings_vector)
+				     warnings_vec *vec)
 {
   const struct bfd_build_id *build_id;
 
@@ -232,8 +243,10 @@ find_separate_debug_file_by_buildid (struct objfile *objfile,
 	    = string_printf (_("\"%s\": separate debug info file has no "
 			       "debug info"), bfd_get_filename (abfd.get ()));
 	  if (separate_debug_file_debug)
-	    gdb_printf (gdb_stdlog, "%s", msg.c_str ());
-	  warnings_vector->emplace_back (std::move (msg));
+	    gdb_printf (gdb_stdlog, "%ps",
+			styled_string (file_name_style. style (),
+				       msg.c_str ()));
+	  add_warning(vec, msg);
 	}
       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 191720ddf28..8e72e6ecff6 100644
--- a/gdb/build-id.h
+++ b/gdb/build-id.h
@@ -47,6 +47,9 @@ extern gdb_bfd_ref_ptr build_id_to_debug_bfd (size_t build_id_len,
 extern gdb_bfd_ref_ptr build_id_to_exec_bfd (size_t build_id_len,
 					     const bfd_byte *build_id);
 
+using warning_cb = std::function<void (void)>;
+using warnings_vec = std::vector<warning_cb>;
+
 /* 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.
@@ -58,7 +61,7 @@ extern gdb_bfd_ref_ptr build_id_to_exec_bfd (size_t build_id_len,
    approach, then any warnings will be printed.  */
 
 extern std::string find_separate_debug_file_by_buildid
-  (struct objfile *objfile, std::vector<std::string> *warnings_vector);
+  (struct objfile *objfile, warnings_vec *vec);
 
 /* Return an hex-string representation of BUILD_ID.  */
 
diff --git a/gdb/coffread.c b/gdb/coffread.c
index 65d7828e933..c534eb504cf 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -734,7 +734,7 @@ 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::vector<std::string> warnings_vector;
+      warnings_vec warnings_vector;
       std::string debugfile
 	= find_separate_debug_file_by_buildid (objfile, &warnings_vector);
 
@@ -752,8 +752,8 @@ coff_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
       /* 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 ());
+	for (const auto &cb : warnings_vector)
+	  cb ();
     }
 }
 
diff --git a/gdb/elfread.c b/gdb/elfread.c
index ca684aab57e..d1c231c2f8c 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1213,7 +1213,7 @@ elf_symfile_read_dwarf2 (struct objfile *objfile,
 	   && objfile->separate_debug_objfile == NULL
 	   && objfile->separate_debug_objfile_backlink == NULL)
     {
-      std::vector<std::string> warnings_vector;
+      warnings_vec warnings_vector;
 
       std::string debugfile
 	= find_separate_debug_file_by_buildid (objfile, &warnings_vector);
@@ -1265,8 +1265,8 @@ 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 ());
+	for (const auto &cb : warnings_vector)
+	  cb ();
     }
 
   return has_dwarf2;
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 373f5592107..1fcb5fd5ae4 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1241,10 +1241,20 @@ symbol_file_clear (int from_tty)
 
 bool separate_debug_file_debug = false;
 
+static void
+add_warning (warnings_vec *vec, std::string msg)
+{
+  vec->emplace_back ([msg] () {
+		     gdb_printf (gdb_stdlog, "warning: %ps",
+				 styled_string (file_name_style. style (),
+						msg.c_str ()));
+		     });
+}
+
 static int
 separate_debug_file_exists (const std::string &name, unsigned long crc,
 			    struct objfile *parent_objfile,
-			    std::vector<std::string> *warnings_vector)
+			    warnings_vec *warnings_vector)
 {
   unsigned long file_crc;
   int file_crc_p;
@@ -1342,8 +1352,9 @@ separate_debug_file_exists (const std::string &name, unsigned long crc,
 			       " does not match \"%s\" (CRC mismatch).\n"),
 			     name.c_str (), objfile_name (parent_objfile));
 	  if (separate_debug_file_debug)
-	    gdb_printf (gdb_stdlog, "%s", msg.c_str ());
-	  warnings_vector->emplace_back (std::move (msg));
+	    gdb_printf (gdb_stdlog, "%ps", styled_string (file_name_style. style (),
+							  msg.c_str ()));
+	  add_warning(warnings_vector, msg);
 	}
 
       return 0;
@@ -1386,7 +1397,7 @@ find_separate_debug_file (const char *dir,
 			  const char *canon_dir,
 			  const char *debuglink,
 			  unsigned long crc32, struct objfile *objfile,
-			  std::vector<std::string> *warnings_vector)
+			  warnings_vec *warnings_vector)
 {
   if (separate_debug_file_debug)
     gdb_printf (gdb_stdlog,
@@ -1534,7 +1545,7 @@ terminate_after_last_dir_separator (char *path)
 
 std::string
 find_separate_debug_file_by_debuglink
-  (struct objfile *objfile, std::vector<std::string> *warnings_vector)
+  (struct objfile *objfile, warnings_vec *warnings_vector)
 {
   unsigned long crc32;
 
diff --git a/gdb/symfile.h b/gdb/symfile.h
index b433e2be31a..0cb12176152 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -241,6 +241,9 @@ 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 *);
 
+using warning_cb = std::function<void (void)>;
+using warnings_vec = std::vector<warning_cb>;
+
 /* Find separate debuginfo for OBJFILE (using .gnu_debuglink section).
    Returns pathname, or an empty string.
 
@@ -248,7 +251,7 @@ extern void symbol_file_add_separate (const gdb_bfd_ref_ptr &, const char *,
    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);
+  (struct objfile *objfile, warnings_vec *warnings_vector);
 
 /* Build (allocate and populate) a section_addr_info struct from an
    existing section table.  */
-- 
2.39.1

[PATCH v2] Use styled_string when defering warnings when loading separate debug files

[Alexandra Hájková]

This improves commit 6647f05df023b63bbe056e9167e9e234172fa2ca, the
filenames used in the warnings are styled properly now.
---
This version: - adds comments to the new add_warning function
              - style filenames only, not the whole warning string
	      - avoids creating duplicate type definitions

 gdb/build-id.c | 21 ++++++++++++++++++---
 gdb/build-id.h |  3 ++-
 gdb/coffread.c |  6 +++---
 gdb/elfread.c  |  6 +++---
 gdb/symfile.c  | 30 +++++++++++++++++++++++++-----
 gdb/symfile.h  |  5 ++++-
 6 files changed, 55 insertions(+), 16 deletions(-)

diff --git a/gdb/build-id.c b/gdb/build-id.c
index 00250c20ae9..6e48ec2e4ae 100644
--- a/gdb/build-id.c
+++ b/gdb/build-id.c
@@ -18,6 +18,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "cli/cli-style.h"
 #include "bfd.h"
 #include "gdb_bfd.h"
 #include "build-id.h"
@@ -205,11 +206,23 @@ build_id_to_exec_bfd (size_t build_id_len, const bfd_byte *build_id)
   return build_id_to_bfd_suffix (build_id_len, build_id, "");
 }
 
+/* Use callbacks to produce the warnings.  */
+
+static void
+add_warning (warnings_vec *vec, std::string filename)
+{
+  vec->emplace_back ([filename] () {
+		     warning (_("\"%ps\": separate debug info file has no debug info"),
+			      styled_string (file_name_style.style (),
+					     filename.c_str()));
+		     });
+}
+
 /* See build-id.h.  */
 
 std::string
 find_separate_debug_file_by_buildid (struct objfile *objfile,
-				     std::vector<std::string> *warnings_vector)
+				     warnings_vec *vec)
 {
   const struct bfd_build_id *build_id;
 
@@ -232,8 +245,10 @@ find_separate_debug_file_by_buildid (struct objfile *objfile,
 	    = string_printf (_("\"%s\": separate debug info file has no "
 			       "debug info"), bfd_get_filename (abfd.get ()));
 	  if (separate_debug_file_debug)
-	    gdb_printf (gdb_stdlog, "%s", msg.c_str ());
-	  warnings_vector->emplace_back (std::move (msg));
+	    warning (_("\"%ps\": separate debug info file has no debug info"),
+		     styled_string (file_name_style.style (),
+				    bfd_get_filename (abfd.get ())));
+	  add_warning(vec, 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 191720ddf28..addaec82526 100644
--- a/gdb/build-id.h
+++ b/gdb/build-id.h
@@ -21,6 +21,7 @@
 #define BUILD_ID_H
 
 #include "gdb_bfd.h"
+#include "gdb/symfile.h"
 #include "gdbsupport/rsp-low.h"
 
 /* Locate NT_GNU_BUILD_ID from ABFD and return its content.  */
@@ -58,7 +59,7 @@ extern gdb_bfd_ref_ptr build_id_to_exec_bfd (size_t build_id_len,
    approach, then any warnings will be printed.  */
 
 extern std::string find_separate_debug_file_by_buildid
-  (struct objfile *objfile, std::vector<std::string> *warnings_vector);
+  (struct objfile *objfile, warnings_vec *vec);
 
 /* Return an hex-string representation of BUILD_ID.  */
 
diff --git a/gdb/coffread.c b/gdb/coffread.c
index 65d7828e933..c534eb504cf 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -734,7 +734,7 @@ 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::vector<std::string> warnings_vector;
+      warnings_vec warnings_vector;
       std::string debugfile
 	= find_separate_debug_file_by_buildid (objfile, &warnings_vector);
 
@@ -752,8 +752,8 @@ coff_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
       /* 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 ());
+	for (const auto &cb : warnings_vector)
+	  cb ();
     }
 }
 
diff --git a/gdb/elfread.c b/gdb/elfread.c
index ca684aab57e..d1c231c2f8c 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1213,7 +1213,7 @@ elf_symfile_read_dwarf2 (struct objfile *objfile,
 	   && objfile->separate_debug_objfile == NULL
 	   && objfile->separate_debug_objfile_backlink == NULL)
     {
-      std::vector<std::string> warnings_vector;
+      warnings_vec warnings_vector;
 
       std::string debugfile
 	= find_separate_debug_file_by_buildid (objfile, &warnings_vector);
@@ -1265,8 +1265,8 @@ 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 ());
+	for (const auto &cb : warnings_vector)
+	  cb ();
     }
 
   return has_dwarf2;
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 373f5592107..2571623dbc8 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1241,10 +1241,25 @@ symbol_file_clear (int from_tty)
 
 bool separate_debug_file_debug = false;
 
+/* Use callbacks to produce the warnings.  */
+
+static void
+add_warning (warnings_vec *vec, std::string filename, std:: string obj_filename)
+{
+  vec->emplace_back ([filename, obj_filename] () {
+		     warning (_("the debug information found in \"%ps\""
+				" does not match \"%ps\" (CRC mismatch).\n"),
+			      styled_string (file_name_style.style (),
+					     filename.c_str()),
+			      styled_string (file_name_style.style (),
+					     obj_filename.c_str()));
+		     });
+}
+
 static int
 separate_debug_file_exists (const std::string &name, unsigned long crc,
 			    struct objfile *parent_objfile,
-			    std::vector<std::string> *warnings_vector)
+			    warnings_vec *warnings_vector)
 {
   unsigned long file_crc;
   int file_crc_p;
@@ -1342,8 +1357,13 @@ separate_debug_file_exists (const std::string &name, unsigned long crc,
 			       " does not match \"%s\" (CRC mismatch).\n"),
 			     name.c_str (), objfile_name (parent_objfile));
 	  if (separate_debug_file_debug)
-	    gdb_printf (gdb_stdlog, "%s", msg.c_str ());
-	  warnings_vector->emplace_back (std::move (msg));
+	    warning (_("the debug information found in \"%ps\""
+		       " does not match \"%ps\" (CRC mismatch).\n"),
+		     styled_string (file_name_style.style (),
+				    name.c_str()),
+		     styled_string (file_name_style.style (),
+				    objfile_name (parent_objfile)));
+	  add_warning(warnings_vector, name.c_str (), objfile_name (parent_objfile));
 	}
 
       return 0;
@@ -1386,7 +1406,7 @@ find_separate_debug_file (const char *dir,
 			  const char *canon_dir,
 			  const char *debuglink,
 			  unsigned long crc32, struct objfile *objfile,
-			  std::vector<std::string> *warnings_vector)
+			  warnings_vec *warnings_vector)
 {
   if (separate_debug_file_debug)
     gdb_printf (gdb_stdlog,
@@ -1534,7 +1554,7 @@ terminate_after_last_dir_separator (char *path)
 
 std::string
 find_separate_debug_file_by_debuglink
-  (struct objfile *objfile, std::vector<std::string> *warnings_vector)
+  (struct objfile *objfile, warnings_vec *warnings_vector)
 {
   unsigned long crc32;
 
diff --git a/gdb/symfile.h b/gdb/symfile.h
index b433e2be31a..0cb12176152 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -241,6 +241,9 @@ 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 *);
 
+using warning_cb = std::function<void (void)>;
+using warnings_vec = std::vector<warning_cb>;
+
 /* Find separate debuginfo for OBJFILE (using .gnu_debuglink section).
    Returns pathname, or an empty string.
 
@@ -248,7 +251,7 @@ extern void symbol_file_add_separate (const gdb_bfd_ref_ptr &, const char *,
    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);
+  (struct objfile *objfile, warnings_vec *warnings_vector);
 
 /* Build (allocate and populate) a section_addr_info struct from an
    existing section table.  */
-- 
2.39.1

Re: [PATCH v2] Use styled_string when defering warnings when loading separate debug files

[Andrew Burgess]

Alexandra Hájková via Gdb-patches <gdb-patches@sourceware.org> writes:

> This improves commit 6647f05df023b63bbe056e9167e9e234172fa2ca, the
> filenames used in the warnings are styled properly now.
> ---
> This version: - adds comments to the new add_warning function
>               - style filenames only, not the whole warning string
> 	      - avoids creating duplicate type definitions
>
>  gdb/build-id.c | 21 ++++++++++++++++++---
>  gdb/build-id.h |  3 ++-
>  gdb/coffread.c |  6 +++---
>  gdb/elfread.c  |  6 +++---
>  gdb/symfile.c  | 30 +++++++++++++++++++++++++-----
>  gdb/symfile.h  |  5 ++++-
>  6 files changed, 55 insertions(+), 16 deletions(-)
>
> diff --git a/gdb/build-id.c b/gdb/build-id.c
> index 00250c20ae9..6e48ec2e4ae 100644
> --- a/gdb/build-id.c
> +++ b/gdb/build-id.c
> @@ -18,6 +18,7 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
>  #include "defs.h"
> +#include "cli/cli-style.h"
>  #include "bfd.h"
>  #include "gdb_bfd.h"
>  #include "build-id.h"
> @@ -205,11 +206,23 @@ build_id_to_exec_bfd (size_t build_id_len, const bfd_byte *build_id)
>    return build_id_to_bfd_suffix (build_id_len, build_id, "");
>  }
>  
> +/* Use callbacks to produce the warnings.  */

I don't think this comment is very helpful.  Something like:

  /* Add a callback to VEC that will produce a warning that separate
     debug info file FILENAME actually contains no debug info.  */

> +
> +static void
> +add_warning (warnings_vec *vec, std::string filename)

Given you are creating a copy of filename within the callback I think
you can pass filename as 'const std::string &filename'.

> +{
> +  vec->emplace_back ([filename] () {
> +		     warning (_("\"%ps\": separate debug info file has no debug info"),
> +			      styled_string (file_name_style.style (),
> +					     filename.c_str()));

You dropped the space before '()' here.

> +		     });
> +}
> +
>  /* See build-id.h.  */
>  
>  std::string
>  find_separate_debug_file_by_buildid (struct objfile *objfile,
> -				     std::vector<std::string> *warnings_vector)
> +				     warnings_vec *vec)
>  {
>    const struct bfd_build_id *build_id;
>  
> @@ -232,8 +245,10 @@ find_separate_debug_file_by_buildid (struct objfile *objfile,
>  	    = string_printf (_("\"%s\": separate debug info file has no "
>  			       "debug info"), bfd_get_filename (abfd.get ()));
>  	  if (separate_debug_file_debug)
> -	    gdb_printf (gdb_stdlog, "%s", msg.c_str ());
> -	  warnings_vector->emplace_back (std::move (msg));
> +	    warning (_("\"%ps\": separate debug info file has no debug info"),
> +		     styled_string (file_name_style.style (),
> +				    bfd_get_filename (abfd.get ())));

This doesn't seem right.  If `separate_debug_file_debug` is true then we
should be producing debug output, not additional warnings.  The
gdb_printf call as it was is fine; you just need to get rid of the `msg`
temporary variable.

> +	  add_warning(vec, bfd_get_filename (abfd.get ()));

Honestly I'd get rid of the add_warning call completely, and just inline
it here, with only one caller I don't think it adds much value, but
that's just my opinion, not a requirement.

>  	}
>        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 191720ddf28..addaec82526 100644
> --- a/gdb/build-id.h
> +++ b/gdb/build-id.h
> @@ -21,6 +21,7 @@
>  #define BUILD_ID_H
>  
>  #include "gdb_bfd.h"
> +#include "gdb/symfile.h"
>  #include "gdbsupport/rsp-low.h"
>  
>  /* Locate NT_GNU_BUILD_ID from ABFD and return its content.  */
> @@ -58,7 +59,7 @@ extern gdb_bfd_ref_ptr build_id_to_exec_bfd (size_t build_id_len,
>     approach, then any warnings will be printed.  */
>  
>  extern std::string find_separate_debug_file_by_buildid
> -  (struct objfile *objfile, std::vector<std::string> *warnings_vector);
> +  (struct objfile *objfile, warnings_vec *vec);

The comment for this function is out of date, it still talks about
WARNINGS_VECTOR, not VEC, and still thinks the vector holds strings.

>  
>  /* Return an hex-string representation of BUILD_ID.  */
>  
> diff --git a/gdb/coffread.c b/gdb/coffread.c
> index 65d7828e933..c534eb504cf 100644
> --- a/gdb/coffread.c
> +++ b/gdb/coffread.c
> @@ -734,7 +734,7 @@ 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::vector<std::string> warnings_vector;
> +      warnings_vec warnings_vector;
>        std::string debugfile
>  	= find_separate_debug_file_by_buildid (objfile, &warnings_vector);
>  
> @@ -752,8 +752,8 @@ coff_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
>        /* 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 ());
> +	for (const auto &cb : warnings_vector)
> +	  cb ();
>      }
>  }
>  
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index ca684aab57e..d1c231c2f8c 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -1213,7 +1213,7 @@ elf_symfile_read_dwarf2 (struct objfile *objfile,
>  	   && objfile->separate_debug_objfile == NULL
>  	   && objfile->separate_debug_objfile_backlink == NULL)
>      {
> -      std::vector<std::string> warnings_vector;
> +      warnings_vec warnings_vector;
>  
>        std::string debugfile
>  	= find_separate_debug_file_by_buildid (objfile, &warnings_vector);
> @@ -1265,8 +1265,8 @@ 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 ());
> +	for (const auto &cb : warnings_vector)
> +	  cb ();
>      }
>  
>    return has_dwarf2;
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 373f5592107..2571623dbc8 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -1241,10 +1241,25 @@ symbol_file_clear (int from_tty)
>  
>  bool separate_debug_file_debug = false;
>  
> +/* Use callbacks to produce the warnings.  */
> +
> +static void
> +add_warning (warnings_vec *vec, std::string filename, std:: string obj_filename)

Again, the comment needs updating, and the strings can be passed as
`const std::string &' I think.

> +{
> +  vec->emplace_back ([filename, obj_filename] () {
> +		     warning (_("the debug information found in \"%ps\""
> +				" does not match \"%ps\" (CRC mismatch).\n"),

Should be no `\n` on the end of the warning.

> +			      styled_string (file_name_style.style (),
> +					     filename.c_str()),
> +			      styled_string (file_name_style.style (),
> +					     obj_filename.c_str()));

You dropped the space before '()' in a couple of places here.

> +		     });
> +}
> +
>  static int
>  separate_debug_file_exists (const std::string &name, unsigned long crc,
>  			    struct objfile *parent_objfile,
> -			    std::vector<std::string> *warnings_vector)
> +			    warnings_vec *warnings_vector)
>  {
>    unsigned long file_crc;
>    int file_crc_p;
> @@ -1342,8 +1357,13 @@ separate_debug_file_exists (const std::string &name, unsigned long crc,
>  			       " does not match \"%s\" (CRC mismatch).\n"),
>  			     name.c_str (), objfile_name (parent_objfile));
>  	  if (separate_debug_file_debug)
> -	    gdb_printf (gdb_stdlog, "%s", msg.c_str ());
> -	  warnings_vector->emplace_back (std::move (msg));
> +	    warning (_("the debug information found in \"%ps\""
> +		       " does not match \"%ps\" (CRC mismatch).\n"),
> +		     styled_string (file_name_style.style (),
> +				    name.c_str()),
> +		     styled_string (file_name_style.style (),
> +				    objfile_name (parent_objfile)));
> +	  add_warning(warnings_vector, name.c_str (), objfile_name (parent_objfile));

Again here, you should use gdb_printf to print the debug output, which
should NOT include styling.  And I (personally) would not bother with
the add_warning function.

If you do keep the 'add_warning' then remember - use a space between the
function name and the argument list.

>  	}
>  
>        return 0;
> @@ -1386,7 +1406,7 @@ find_separate_debug_file (const char *dir,
>  			  const char *canon_dir,
>  			  const char *debuglink,
>  			  unsigned long crc32, struct objfile *objfile,
> -			  std::vector<std::string> *warnings_vector)
> +			  warnings_vec *warnings_vector)

The comment above this function needs updating.

>  {
>    if (separate_debug_file_debug).
>      gdb_printf (gdb_stdlog,
> @@ -1534,7 +1554,7 @@ terminate_after_last_dir_separator (char *path)
>  
>  std::string
>  find_separate_debug_file_by_debuglink
> -  (struct objfile *objfile, std::vector<std::string> *warnings_vector)
> +  (struct objfile *objfile, warnings_vec *warnings_vector)
>  {
>    unsigned long crc32;
>  
> diff --git a/gdb/symfile.h b/gdb/symfile.h
> index b433e2be31a..0cb12176152 100644
> --- a/gdb/symfile.h
> +++ b/gdb/symfile.h
> @@ -241,6 +241,9 @@ 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 *);
>  
> +using warning_cb = std::function<void (void)>;
> +using warnings_vec = std::vector<warning_cb>;

You should add a comment above these types describing what they are for.

> +
>  /* Find separate debuginfo for OBJFILE (using .gnu_debuglink section).
>     Returns pathname, or an empty string.
>  
> @@ -248,7 +251,7 @@ extern void symbol_file_add_separate (const gdb_bfd_ref_ptr &, const char *,
>     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);
> +  (struct objfile *objfile, warnings_vec *warnings_vector);

The comment above this declaration needs updating, it still talks about
the warnings as being strings.

Thanks,
Andrew
>  
>  /* Build (allocate and populate) a section_addr_info struct from an
>     existing section table.  */
> -- 
> 2.39.1

Re: [PATCH v2] Use styled_string when defering warnings when loading separate debug files

[Tom Tromey]

>>>>> "Alexandra" == Alexandra Hájková via Gdb-patches <gdb-patches@sourceware.org> writes:

Alexandra> This improves commit 6647f05df023b63bbe056e9167e9e234172fa2ca, the
Alexandra> filenames used in the warnings are styled properly now.
 
Alexandra> +/* Use callbacks to produce the warnings.  */
Alexandra> +
Alexandra> +static void
Alexandra> +add_warning (warnings_vec *vec, std::string filename)
Alexandra> +{
Alexandra> +  vec->emplace_back ([filename] () {
Alexandra> +		     warning (_("\"%ps\": separate debug info file has no debug info"),
Alexandra> +			      styled_string (file_name_style.style (),
Alexandra> +					     filename.c_str()));
Alexandra> +		     });

This works but it seems like it would be a lot simpler to just use a
ui_file rather than introduce a vector of callback functions that emit
warnings.

Tom