[binutils-gdb] gdb: change subfile::name and buildsym_compunit::m_comp_dir to strings

[binutils-gdb] gdb: change subfile::name and buildsym_compunit::m_comp_dir to strings

[Simon Marchi]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ebd4e6d017f972cbba5d889310a09506bec4dbf4

commit ebd4e6d017f972cbba5d889310a09506bec4dbf4
Author: Simon Marchi <simon.marchi@efficios.com>
Date:   Fri Apr 8 11:04:24 2022 -0400

    gdb: change subfile::name and buildsym_compunit::m_comp_dir to strings
    
    Change subfile::name to be a string, for easier memory management.
    Change buildsym_compunit::m_comp_dir as well, since we move one in to
    the other at some point in patch_subfile_names, so it's easier to do
    both at the same time.  There are various NULL checks for both fields
    currently, replace them with empty checks, I think it ends up
    equivalent.
    
    I can't test the change in xcoffread.c, it's best-effort.
    
    Change-Id: I62b5fb08b2089e096768a090627ac7617e90a016

Diff:
---
 gdb/buildsym.c    | 64 +++++++++++++++++++++++++------------------------------
 gdb/buildsym.h    |  8 +++----
 gdb/dwarf2/read.c | 10 ++++-----
 gdb/xcoffread.c   |  3 +--
 4 files changed, 38 insertions(+), 47 deletions(-)

diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index aedc3c67b9a..034db598735 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -63,7 +63,7 @@ buildsym_compunit::buildsym_compunit (struct objfile *objfile_,
 				      CORE_ADDR last_addr)
   : m_objfile (objfile_),
     m_last_source_file (name == nullptr ? nullptr : xstrdup (name)),
-    m_comp_dir (comp_dir_ == nullptr ? nullptr : xstrdup (comp_dir_)),
+    m_comp_dir (comp_dir_ == nullptr ? "" : comp_dir_),
     m_language (language_),
     m_last_source_start_addr (last_addr)
 {
@@ -95,7 +95,6 @@ buildsym_compunit::~buildsym_compunit ()
        subfile = nextsub)
     {
       nextsub = subfile->next;
-      xfree (subfile->name);
       xfree (subfile->line_vector);
       delete subfile;
     }
@@ -503,45 +502,40 @@ buildsym_compunit::make_blockvector ()
 void
 buildsym_compunit::start_subfile (const char *name)
 {
-  const char *subfile_dirname;
-
-  subfile_dirname = m_comp_dir.get ();
-
   /* See if this subfile is already registered.  */
 
   for (subfile *subfile = m_subfiles; subfile; subfile = subfile->next)
     {
-      char *subfile_name;
+      std::string subfile_name_holder;
+      const char *subfile_name;
 
       /* If NAME is an absolute path, and this subfile is not, then
 	 attempt to create an absolute path to compare.  */
       if (IS_ABSOLUTE_PATH (name)
 	  && !IS_ABSOLUTE_PATH (subfile->name)
-	  && subfile_dirname != NULL)
-	subfile_name = concat (subfile_dirname, SLASH_STRING,
-			       subfile->name, (char *) NULL);
+	  && !m_comp_dir.empty ())
+	{
+	  subfile_name_holder = string_printf ("%s/%s", m_comp_dir.c_str (),
+					       subfile->name.c_str ());
+	  subfile_name = subfile_name_holder.c_str ();
+	}
       else
-	subfile_name = subfile->name;
+	subfile_name = subfile->name.c_str ();
 
       if (FILENAME_CMP (subfile_name, name) == 0)
 	{
 	  m_current_subfile = subfile;
-	  if (subfile_name != subfile->name)
-	    xfree (subfile_name);
 	  return;
 	}
-      if (subfile_name != subfile->name)
-	xfree (subfile_name);
     }
 
   /* This subfile is not known.  Add an entry for it.  */
 
   subfile_up subfile (new struct subfile);
+  subfile->name = name;
 
   m_current_subfile = subfile.get ();
 
-  subfile->name = xstrdup (name);
-
   /* Initialize line-number recording for this subfile.  */
   subfile->line_vector = NULL;
 
@@ -556,7 +550,7 @@ buildsym_compunit::start_subfile (const char *name)
      until after all the symbols have been processed for a given
      source file.  */
 
-  subfile->language = deduce_language_from_filename (subfile->name);
+  subfile->language = deduce_language_from_filename (subfile->name.c_str ());
   if (subfile->language == language_unknown && m_subfiles != nullptr)
     subfile->language = m_subfiles->language;
 
@@ -565,10 +559,10 @@ buildsym_compunit::start_subfile (const char *name)
      any other C++ suffixes accepted by deduce_language_from_filename.  */
   /* Likewise for f2c.  */
 
-  if (subfile->name)
+  if (!subfile->name.empty ())
     {
       struct subfile *s;
-      enum language sublang = deduce_language_from_filename (subfile->name);
+      language sublang = deduce_language_from_filename (subfile->name.c_str ());
 
       if (sublang == language_cplus || sublang == language_fortran)
 	for (s = m_subfiles; s != NULL; s = s->next)
@@ -605,12 +599,12 @@ buildsym_compunit::patch_subfile_names (struct subfile *subfile,
 					const char *name)
 {
   if (subfile != NULL
-      && m_comp_dir == NULL
-      && subfile->name != NULL
-      && IS_DIR_SEPARATOR (subfile->name[strlen (subfile->name) - 1]))
+      && m_comp_dir.empty ()
+      && !subfile->name.empty ()
+      && IS_DIR_SEPARATOR (subfile->name.back ()))
     {
-      m_comp_dir.reset (subfile->name);
-      subfile->name = xstrdup (name);
+      m_comp_dir = std::move (subfile->name);
+      subfile->name = name;
       set_last_source_file (name);
 
       /* Default the source language to whatever can be deduced from
@@ -624,7 +618,8 @@ buildsym_compunit::patch_subfile_names (struct subfile *subfile,
 	 symbols, since symtabs aren't allocated until after all the
 	 symbols have been processed for a given source file.  */
 
-      subfile->language = deduce_language_from_filename (subfile->name);
+      subfile->language
+	= deduce_language_from_filename (subfile->name.c_str ());
       if (subfile->language == language_unknown
 	  && subfile->next != NULL)
 	{
@@ -642,8 +637,8 @@ void
 buildsym_compunit::push_subfile ()
 {
   gdb_assert (m_current_subfile != NULL);
-  gdb_assert (m_current_subfile->name != NULL);
-  m_subfile_stack.push_back (m_current_subfile->name);
+  gdb_assert (!m_current_subfile->name.empty ());
+  m_subfile_stack.push_back (m_current_subfile->name.c_str ());
 }
 
 const char *
@@ -746,7 +741,7 @@ buildsym_compunit::watch_main_source_file_lossage ()
   if (mainsub->line_vector == NULL
       && mainsub->symtab == NULL)
     {
-      const char *mainbase = lbasename (mainsub->name);
+      const char *mainbase = lbasename (mainsub->name.c_str ());
       int nr_matches = 0;
       struct subfile *prevsub;
       struct subfile *mainsub_alias = NULL;
@@ -759,7 +754,7 @@ buildsym_compunit::watch_main_source_file_lossage ()
 	{
 	  if (subfile == mainsub)
 	    continue;
-	  if (filename_cmp (lbasename (subfile->name), mainbase) == 0)
+	  if (filename_cmp (lbasename (subfile->name.c_str ()), mainbase) == 0)
 	    {
 	      ++nr_matches;
 	      mainsub_alias = subfile;
@@ -784,7 +779,6 @@ buildsym_compunit::watch_main_source_file_lossage ()
 	    m_subfiles = mainsub_alias->next;
 	  else
 	    prev_mainsub_alias->next = mainsub_alias->next;
-	  xfree (mainsub_alias->name);
 
 	  delete mainsub_alias;
 	}
@@ -967,7 +961,8 @@ buildsym_compunit::end_compunit_symtab_with_blockvector
 
       /* Allocate a symbol table if necessary.  */
       if (subfile->symtab == NULL)
-	subfile->symtab = allocate_symtab (cu, subfile->name);
+	subfile->symtab = allocate_symtab (cu, subfile->name.c_str ());
+
       struct symtab *symtab = subfile->symtab;
 
       /* Fill in its components.  */
@@ -997,12 +992,11 @@ buildsym_compunit::end_compunit_symtab_with_blockvector
 
   /* Fill out the compunit symtab.  */
 
-  if (m_comp_dir != NULL)
+  if (!m_comp_dir.empty ())
     {
       /* Reallocate the dirname on the symbol obstack.  */
-      const char *comp_dir = m_comp_dir.get ();
       cu->set_dirname (obstack_strdup (&m_objfile->objfile_obstack,
-				       comp_dir));
+				       m_comp_dir.c_str ()));
     }
 
   /* Save the debug format string (if any) in the symtab.  */
diff --git a/gdb/buildsym.h b/gdb/buildsym.h
index 50ecd33d544..6284aafc878 100644
--- a/gdb/buildsym.h
+++ b/gdb/buildsym.h
@@ -52,9 +52,7 @@ struct subfile
   DISABLE_COPY_AND_ASSIGN (subfile);
 
   struct subfile *next = nullptr;
-
-  /* Space for this is malloc'd.  */
-  char *name = nullptr;
+  std::string name;
 
   /* Space for this is malloc'd.  */
   struct linetable *line_vector = nullptr;
@@ -154,7 +152,7 @@ struct buildsym_compunit
 		     CORE_ADDR last_addr, struct compunit_symtab *cust)
     : m_objfile (objfile_),
       m_last_source_file (name == nullptr ? nullptr : xstrdup (name)),
-      m_comp_dir (comp_dir_ == nullptr ? nullptr : xstrdup (comp_dir_)),
+      m_comp_dir (comp_dir_ == nullptr ? "" : comp_dir_),
       m_compunit_symtab (cust),
       m_language (language_),
       m_last_source_start_addr (last_addr)
@@ -342,7 +340,7 @@ private:
   gdb::unique_xmalloc_ptr<char> m_last_source_file;
 
   /* E.g., DW_AT_comp_dir if DWARF.  Space for this is malloc'd.  */
-  gdb::unique_xmalloc_ptr<char> m_comp_dir;
+  std::string m_comp_dir;
 
   /* Space for this is not malloc'd, and is assumed to have at least
      the same lifetime as objfile.  */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 7c148aecdf5..698720276a9 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -9740,8 +9740,8 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
 		 assume there's a simple mapping from
 		 cu->line_header->file_names to subfiles, plus
 		 cu->line_header->file_names may contain dups.  */
-	      b->get_current_subfile ()->symtab
-		= allocate_symtab (cust, b->get_current_subfile ()->name);
+	      const char *name = b->get_current_subfile ()->name.c_str ();
+	      b->get_current_subfile ()->symtab = allocate_symtab (cust, name);
 	    }
 
 	  fe.symtab = b->get_current_subfile ()->symtab;
@@ -20083,7 +20083,7 @@ dwarf_record_line_1 (struct gdbarch *gdbarch, struct subfile *subfile,
     {
       gdb_printf (gdb_stdlog,
 		  "Recording line %u, file %s, address %s\n",
-		  line, lbasename (subfile->name),
+		  line, lbasename (subfile->name.c_str ()),
 		  paddress (gdbarch, address));
     }
 
@@ -20107,7 +20107,7 @@ dwarf_finish_line (struct gdbarch *gdbarch, struct subfile *subfile,
     {
       gdb_printf (gdb_stdlog,
 		  "Finishing current line, file %s, address %s\n",
-		  lbasename (subfile->name),
+		  lbasename (subfile->name.c_str ()),
 		  paddress (gdbarch, address));
     }
 
@@ -20500,7 +20500,7 @@ dwarf_decode_lines (struct line_header *lh, struct dwarf2_cu *cu,
 	{
 	  builder->get_current_subfile ()->symtab
 	    = allocate_symtab (cust,
-			       builder->get_current_subfile ()->name);
+			       builder->get_current_subfile ()->name.c_str ());
 	}
       fe.symtab = builder->get_current_subfile ()->symtab;
     }
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 296e71356dd..894a12ea865 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -766,10 +766,9 @@ process_linenos (CORE_ADDR start, CORE_ADDR end)
 	    if (fakename == NULL)
 	      fakename = " ?";
 	    start_subfile (fakename);
-	    xfree (get_current_subfile ()->name);
 	  }
 	  struct subfile *current_subfile = get_current_subfile ();
-	  current_subfile->name = xstrdup (inclTable[ii].name);
+	  current_subfile->name = inclTable[ii].name;
 #endif
 
 	  if (lv == lineTb)